Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chain specific message lane apis #503

Merged
merged 5 commits into from
Nov 23, 2020
Merged

Conversation

svyatonik
Copy link
Contributor

closes #457
on top of #496 => draft

I've tried all possible options, including:

  1. runtime API that has additional InstanceId argument;
  2. replacing runtime API with custom RPC methods (that are receiving additional InstanceId argument);
  3. adding macro like declare_message_lane_runtime_apis!(Rialto) (failed, because I have found no way to call proc macro from decl macro);
  4. bonus: I've tried to add our own wrapper of decl_runtime_apis that will generate these string constants, but apparently there's no way to call proc macro from other proc macro. Probably good idea to have this in original substrate macro in the future

@HCastano HCastano changed the base branch from master to limit-messages-weight-in-batch November 17, 2020 19:18
Copy link
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a little unfortunate that there's no nice way to do this right now. Although I imagine it wouldn't be too difficult or controversial to add an Into<String> implementation (or something similar) to the runtime API macros in Substrate.

As far as having separate runtime APIs goes, I also think it would be nice to see the macros accept a generic argument and that was we can reuse the same API with different types (so in our case different instances of the chains). That might be a bit harder to do though.

relays/substrate/src/messages_lane.rs Outdated Show resolved Hide resolved
Base automatically changed from limit-messages-weight-in-batch to master November 17, 2020 20:27
@svyatonik svyatonik force-pushed the chain-specific-message-lane-apis branch from 78aeb01 to c099b83 Compare November 18, 2020 08:34
@svyatonik svyatonik marked this pull request as ready for review November 18, 2020 08:46
@HCastano HCastano requested a review from tomusdrw November 18, 2020 20:27
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@svyatonik svyatonik merged commit 367d847 into master Nov 23, 2020
@svyatonik svyatonik deleted the chain-specific-message-lane-apis branch November 23, 2020 07:55
svyatonik pushed a commit that referenced this pull request Jul 17, 2023
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
* replace generic message lane APIs with chain-specific

* moved SubstrateHeadersSyncPipeline to headers_pipeline.rs

* substrate-specific message lane trait

* Update relays/substrate/src/messages_lane.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
* replace generic message lane APIs with chain-specific

* moved SubstrateHeadersSyncPipeline to headers_pipeline.rs

* substrate-specific message lane trait

* Update relays/substrate/src/messages_lane.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chain-specific runtime message-delivery API
3 participants