Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

XCM: Tools for uniquely referencing messages #7234

Merged
merged 37 commits into from
May 25, 2023
Merged

Conversation

gavofyork
Copy link
Member

@gavofyork gavofyork commented May 15, 2023

Cumulus Companion: paritytech/cumulus#2601

This introduces several tools to ensure XCM messages are uniquely referenceable, using the topic identifiers introduced in XCMv3.

  • The SendXcm implementation WithUniqueTopic is introduced as a wrapping router and appends all XCMs sent through it with a SetTopic(t) where t is a universally unique XcmHash value which will be used to identify the message. This is returned by the fn deliver implementation for recording in events.
  • The barrier TrailingSetTopicAsId is introduced which is a wrapping barrier (i.e. accepts another barrier as a parameter) and which elides a trailing SetTopic on the message if one exists before passing the rest into the inner barrier. It is designed to wrap around all other barriers at the top level.

To use this functionality, the top-level XcmRouter type should be altered from:

type XcmRouter = (
    ...
);

To:

type XcmRouter = WithUniqueTopic<(
    ...
)>;

Also, the barrier condition should be altered from:

type Barrier = (
    ...
);

To:

type Barrier = TrailingSetTopicAsId<(
    ...
)>;

Messages coming from any chains instituting this will always have a SetTopic instruction as the last instruction, which allows chain explorers to track messages.

This is further improved by integrating the topic into the barriers API, allowing the Topic barriers to pass the topic statically into the dispatch system and thus have the event include it rather than the now-defunct message hash.

Also, topics are linkable when XCM is sent over a bridge. This is done through having the bridge primitives in universal_exports.rs append the derivative (bridged) messages with SetTopic containing a derivative topic value. For this to work, WithUniqueTopic does not append a SetTopic to the message if it already ends with one.

Additionally this adds the message ID to all events which send a message and refactors the Event enum in pallet XCM to have named fields. Also, added Weight to xcm::latest::prelude and by extension xcm::prelude.

Breaking Changes

This contains a breaking change into the XCM Barrier API, trait ShouldExecute: weight_credit: &mut Weight becomes properties: &mut Properties.

This won't affect you unless you implement the ShouldExecute trait; if you do, you'll need to alter the prototype accordingly. If you use or mutate the weight_credit argument, then you can find it in the new API as properties.weight_credit.

TODO

  • Repot the router into routing.rs
  • Barrier API should include the possibility to change the message hash/ID
  • *SetTopic should change message hash/ID accordingly
  • Changes of message hash/ID should be respected in any events deposited
    • UMP arrival/dispatch (Polkadot)
    • DMP arrival/dispatch (Cumulus)
    • XCMP arrival/dispatch (Cumulus)
    • Departure (send_xcm usage)
    • Consider adding the ID into existing events
  • WithUniqueTopic avoids setting topic again
  • Universal exports (bridges) support topic propagation
  • Integrate into relay-chains (Polkadot)
  • Integrate into parachains (Cumulus)
  • Tests (Polkadot)

@gavofyork gavofyork added A3-in_progress Pull request is in progress. No review needed at this stage. C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. T6-XCM This PR/Issue is related to XCM. B1-note_worthy Changes should be noted in the release notes labels May 15, 2023
@gavofyork gavofyork added D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. and removed D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels May 15, 2023
@gavofyork gavofyork marked this pull request as draft May 16, 2023 17:19
@gavofyork gavofyork marked this pull request as ready for review May 19, 2023 10:25
@paritytech-ci paritytech-ci requested review from a team May 19, 2023 10:25
@gavofyork gavofyork added T1-runtime This PR/Issue is related to the topic “runtime”. and removed T6-XCM This PR/Issue is related to XCM. labels May 19, 2023
@command-bot
Copy link

command-bot bot commented May 24, 2023

@gavofyork Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2889552 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2889552/artifacts/download.

@gavofyork
Copy link
Member Author

bot fmt

@command-bot
Copy link

command-bot bot commented May 24, 2023

@gavofyork https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2890345 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 17-9adc81a4-4725-4d45-990b-2ac3a07ba6a9 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented May 24, 2023

@gavofyork Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2890345 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2890345/artifacts/download.

@gavofyork gavofyork requested a review from KiChjang May 25, 2023 05:26
@@ -212,6 +212,52 @@ impl From<SendError> for Error {

pub type Result = result::Result<(), Error>;

/*
TODO: XCMv4
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing the code here should be migrated to v4 when we get to creating it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed.

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
@gavofyork gavofyork merged commit df3e3c7 into master May 25, 2023
@gavofyork gavofyork deleted the gav-unique-topics branch May 25, 2023 06:57
ordian added a commit that referenced this pull request May 25, 2023
* master:
  XCM: Tools for uniquely referencing messages (#7234)
  Companion: Substrate#13869 (#7119)
  Companion for Substrate#14214 (#7283)
  Fix flaky test and error reporting (#7282)
  impl guide: Update Collator Generation (#7250)
  Add staking-miner bin (#7273)
  metrics: tests: Fix flaky runtime_can_publish_metrics (#7279)
ordian added a commit that referenced this pull request May 25, 2023
…slashing-client

* ao-past-session-slashing-runtime:
  XCM: Tools for uniquely referencing messages (#7234)
  Companion: Substrate#13869 (#7119)
  Companion for Substrate#14214 (#7283)
  Fix flaky test and error reporting (#7282)
  impl guide: Update Collator Generation (#7250)
  Add staking-miner bin (#7273)
  metrics: tests: Fix flaky runtime_can_publish_metrics (#7279)
  [companion] Fix request-response protocols backpressure mechanism (#7276)
  PVF: instantiate runtime from bytes (#7270)
@muharem muharem self-requested a review May 30, 2023 10:10
@redzsina redzsina added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. labels Aug 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants