Add XCM Precompile to Pallet Revive by sphamjoli · Pull Request #8471 · paritytech/polkadot-sdk

sphamjoli · 2025-05-08T20:17:44Z

Create a unified precompile address that handles xcmExecute and xcmSend operations for devs to call via solidity using abi encodings.

See tracking parent issue

substrate/frame/revive/src/precompiles/custom/xcm.rs

athei · 2025-05-12T08:52:14Z

substrate/frame/revive/src/precompiles.rs

 //! Use `alloy` through our re-export in this module to implement Eth ABI.

 mod builtin;
+pub mod custom;


Should be either exist inside bultin or outside of pallet-revive. I would prefer the latter (for example implement Precompile on pallet_xcm. Then we can remove all the dependencies to xcm from pallet_revive.

I think given the cutoff date for stable2506, it might be fine to leave it where it is and then move it

substrate/frame/revive/src/precompiles/custom/xcm.rs

acatangiu

high-level everything looks good, I would also like to see it integrated with asset-hub-westend-runtime too not just mock runtime from pallet-revive

substrate/frame/revive/mock-network/src/parachain/contracts_config.rs

substrate/frame/revive/src/lib.rs

substrate/frame/revive/src/precompiles/builtin/xcm.rs

franciscoaguirre · 2025-05-13T21:00:13Z

substrate/frame/revive/src/precompiles/builtin/xcm.rs

+
+		match input {
+			IXcmCalls::xcmSend(IXcm::xcmSendCall { destination, message }) => {
+				let final_destination = xcm::VersionedLocation::decode_all(&mut &destination[..])


This precompile will be updated to only support the latest versions, since Versioned* types only support the latest 3 XCM versions. With this, XCMs crafted in SCs should always be upgradable. Probably the pattern here (for mad people) is to put XCMs in a storage item so you can update them when needed. We need another higher level precompile to make everyone's lives easier

athei · 2025-05-14T12:01:53Z

You should get your editor under control. Or run fmt before pushing.

athei · 2025-05-14T12:32:16Z

/cmd fmt

substrate/frame/revive/src/precompiles.rs

substrate/frame/revive/src/precompiles/builtin.rs

athei · 2025-05-14T12:28:32Z

substrate/frame/revive/src/lib.rs

+		/// The weigher used to calculate XCM execution costs
+		#[pallet::no_default_bounds]
+		type XcmWeigher: xcm_executor::traits::WeightBounds<<Self as frame_system::Config>::RuntimeCall>;


Can't we add this bound to the existing Xcm associated type? I think its all implemented by pallet_xcm anyways.

Since we moving the precompile anyways would it make sense to make changes in pallet_xcm to achieve this?

The right thing to do would be to define the pre-compile in pallet_xcm.

athei · 2025-05-14T12:29:47Z

substrate/frame/revive/src/precompiles/builtin/xcm.rs

This pre-compile does not charge any weight whatsoever. Please look into the already existing xcm_execute and xcm_send functions in this pallet. It is just migrating existing logic to a pre-compile.

substrate/frame/revive/Cargo.toml

substrate/frame/revive/mock-network/src/lib.rs

athei · 2025-05-14T12:49:51Z

substrate/frame/revive/src/lib.rs

+		/// The weigher used to calculate XCM execution costs
+		#[pallet::no_default_bounds]
+		type XcmWeigher: xcm_executor::traits::WeightBounds<
+			<Self as frame_system::Config>::RuntimeCall,
+		>;


Can't we just add the bound to the Xcm associated type? It is all implemented by pallet_xcm anyways AFAIK.

No, this is currently a runtime configuration object, also being set to the xcm-executor, not pallet-xcm. We should consolidate it somehow to not have to set it twice (pallet-revive and xcm-executor), but this is not trivial and “nice-to-have” at this point IMO

athei · 2025-05-14T14:16:56Z

substrate/frame/revive/src/precompiles/builtin.rs

When suggesting to move it to builtin I assumed you would add it to the Production type alias. As this is the way to expose Builtin pre-compiles. However, this won't be possible since builtin pre-compiles are forced to use a different address range.

And since we are planning to move it out of the pallet eventually we need to use an external address range. Otherwise we won't be able to migrate it.

Sorry for the confusion. Just move the xcm.rs as direct sub module of precompiles.rs.

acatangiu · 2025-05-14T18:16:15Z

@Brianspha @franciscoaguirre I agree with @athei that the XCM precompiles should be implemented by pallet-xcm and not pallet-revive.
If we can achieve that by deadline (tomorrow), that would be great. But if not, I propose we go ahead with the current implementation as we can change it during qa period.
The important bit we need to agree on is the SC interface as that we don’t want to change.
The underlying rust code we can commit to move/improve quickly after deadline as fix PR.

@athei wdyt?

tiagobndr · 2025-05-14T19:35:16Z

@Brianspha @franciscoaguirre I agree with @athei that the XCM precompiles should be implemented by pallet-xcm and not pallet-revive. If we can achieve that by deadline (tomorrow), that would be great. But if not, I propose we go ahead with the current implementation as we can change it during qa period. The important bit we need to agree on is the SC interface as that we don’t want to change. The underlying rust code we can commit to move/improve quickly after deadline as fix PR.

@athei wdyt?

Regarding the interface, I believe it is mature enough. There is some query-related functionality we might want to add to this precompile later but that's not part of this PR. As for tomorrow's deadline, I’d be cautious about migrating the precompile to pallet-xcm (even though I recognise its advantages) due to the limited time only. It's not just the implementation, it'd also require additional testing and review.

…olkadot-sdk into spha/xcm_precompile

paritytech-workflow-stopper · 2025-05-14T22:06:48Z

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/15031907799
Failed job name: test-linux-stable

athei · 2025-05-15T09:40:02Z

@Brianspha @franciscoaguirre I agree with @athei that the XCM precompiles should be implemented by pallet-xcm and not pallet-revive. If we can achieve that by deadline (tomorrow), that would be great. But if not, I propose we go ahead with the current implementation as we can change it during qa period. The important bit we need to agree on is the SC interface as that we don’t want to change. The underlying rust code we can commit to move/improve quickly after deadline as fix PR.

@athei wdyt?

I don't think it is realistic to have this pre-compile in a state that I feel comfortable to expose it until today anyways. And its value without query support is kinda non existent anyways. I don't get the rush.

franciscoaguirre · 2025-05-15T09:55:56Z

@athei I agree this belongs in pallet-xcm (or some other XCM crate). We shouldn't rush it and do things properly. Especially to avoid breaking changes in the future

sphamjoli · 2025-05-15T20:12:32Z

Closing the PR since we need to move all xcm related dependencies to pallet-xcm

This PR adds the XCM precompile (with `xcmSend`, `xcmExecute` and `weightMessage` functionalities) to `pallet-xcm`. This follows the discussion we had on the now closed [PR](#8471), which attempted to add the precompile to `pallet-revive`, but that approach would have introduced unwanted cyclic dependencies. That's why we decided to migrate the precompile to `pallet-xcm`, avoiding adding unnecessary dependencies to `pallet-revive`. This PR should also encapsulate unit tests in `precompiles.rs` as well as integration tests under `cumulus/parachains/integration-tests/emulated/tests`. See tracking parent [issue](#6718) --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Siphamandla Mjoli <brianm445@yahoo.com> Co-authored-by: Siphamandla Mjoli <siphamandla@parity.io> Co-authored-by: Alexander Theißen <alex.theissen@me.com> Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com> Co-authored-by: PG Herveou <pgherveou@gmail.com>

sphamjoli and others added 16 commits April 9, 2025 01:26

update: initial commit

6c702f3

update: revert file changes

7da970b

update: fix function

77c90ad

update: add documentation and fixed proof_size issue

2cd3f77

update: fix unverified commit

cfddbbd

update: test commit

5da9f96

update: revert changes to pallet xcm for weights

8a4b9ab

update: fix tests

63a7939

Merge branch 'master' into spha/xcm_precompile

f81110f

update: implement precompile using framework

7dfc036

update: remove alloy type crate

12a6b45

Merge branch 'master' into spha/xcm_precompile

5637a44

update: remove unnecessary import

d1e9fc4

update: add the XCM precompile to WAH's runtime

df3f49f

update: remove trace logs, revert changes to AH

8c61877

update: fix tests

884dc92

sphamjoli requested review from a team and athei May 8, 2025 20:17

Merge branch 'master' into spha/xcm_precompile

80647e1

bkontur reviewed May 9, 2025

View reviewed changes

substrate/frame/revive/src/precompiles/custom/xcm.rs Outdated Show resolved Hide resolved

fix: PR comments

a1f0dc3

sphamjoli requested a review from bkontur May 9, 2025 12:42

franciscoaguirre added T6-XCM This PR/Issue is related to XCM. T7-smart_contracts This PR/Issue is related to smart contracts. labels May 9, 2025

sphamjoli marked this pull request as ready for review May 9, 2025 15:19

Merge branch 'master' into spha/xcm_precompile

ee39c21

franciscoaguirre reviewed May 9, 2025

View reviewed changes

fix: PR comments

66bfe96

franciscoaguirre reviewed May 11, 2025

View reviewed changes

substrate/frame/revive/src/precompiles/custom/xcm.rs Outdated Show resolved Hide resolved

athei reviewed May 12, 2025

View reviewed changes

sphamjoli added 2 commits May 13, 2025 14:01

fix: PR comments

60abf42

fix: PR comments

24c0d9a

acatangiu reviewed May 13, 2025

View reviewed changes

substrate/frame/revive/mock-network/src/parachain/contracts_config.rs Show resolved Hide resolved

sphamjoli added 2 commits May 13, 2025 15:35

fix: param name in interface

dcf7375

update: use correct abi encoded results for messageWeight

49628d0

franciscoaguirre reviewed May 13, 2025

View reviewed changes

sphamjoli added 2 commits May 14, 2025 10:09

Merge branch 'master' into spha/xcm_precompile

a814da7

Merge branch 'master' into spha/xcm_precompile

fe317bb

Update from github-actions[bot] running command 'fmt'

eb6d839

athei reviewed May 14, 2025

View reviewed changes

Merge branch 'master' into spha/xcm_precompile

63b163c

athei reviewed May 14, 2025

View reviewed changes

sphamjoli and others added 5 commits May 14, 2025 21:45

Merge branch 'master' into spha/xcm_precompile

98874b4

fix: PR comments

231457a

Merge branch 'spha/xcm_precompile' of https://github.com/paritytech/p…

6856f6d

…olkadot-sdk into spha/xcm_precompile

fix: change CallRuntime to CallXcmExecute for xcm execute

9da7cef

Merge branch 'master' into spha/xcm_precompile

3bfec03

sphamjoli closed this May 15, 2025

tiagobndr mentioned this pull request May 28, 2025

Add XCM Precompile to pallet-xcm #8693

Merged

franciscoaguirre deleted the spha/xcm_precompile branch October 3, 2025 15:59

Conversation

sphamjoli commented May 8, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

acatangiu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

athei commented May 14, 2025

Uh oh!

athei commented May 14, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acatangiu commented May 14, 2025

Uh oh!

tiagobndr commented May 14, 2025

Uh oh!

paritytech-workflow-stopper bot commented May 14, 2025

Uh oh!

athei commented May 15, 2025

Uh oh!

franciscoaguirre commented May 15, 2025

Uh oh!

sphamjoli commented May 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants