-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add XCM Precompile to pallet-xcm
#8693
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
Changes from 10 commits
659256b
d21b766
3126166
8f7fdd4
ddacec4
b341837
9306406
b963c4f
240b8ea
43c77ce
a57e581
26b3f6e
2bd9c17
39f124b
e2c632b
a040a1b
10a0a83
7b7acdc
3b1ad85
fd8f350
6442869
ea60b5e
10e27f9
2a10514
51c4683
79d8a5f
56bd6fc
6969f17
2b732f4
f1ab023
cb26365
b7738d3
bf0cfae
eea0712
d46a1e8
6c9d83c
8c57620
fa2dd49
859193c
e1694a9
7a6c735
6ec2d74
bf67b1e
f5ae802
56872ec
09bdb41
742c423
723d3ea
06627c7
ab76d38
5368ddf
7bcb6cf
3024678
f674007
87f59d3
d7dca77
f299795
0885f63
c708434
a29e51b
5665007
5333e1f
c87d86e
97f58bf
c141c83
9a1ec3a
fd8fccb
8a435b8
047d8b5
43c3a7d
ea843df
ac19567
3b9ac22
a0cbd43
faceac5
115ac42
84f40f7
d5a5f66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |
| pub mod benchmarking; | ||
| #[cfg(test)] | ||
| mod mock; | ||
| pub mod precompiles; | ||
| #[cfg(test)] | ||
| mod tests; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| // This file is part of Substrate. | ||
|
|
||
| // Copyright (C) Parity Technologies (UK) Ltd. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| use crate::{Call, Config, VersionedLocation, VersionedXcm, Weight, WeightInfo}; | ||
| use alloc::vec::Vec; | ||
| use alloy::sol_types::SolValue; | ||
| use codec::{DecodeAll, Encode}; | ||
| use core::{marker::PhantomData, num::NonZero}; | ||
| use pallet_revive::{precompiles::*, Origin}; | ||
| use tracing::log::error; | ||
| use xcm_executor::traits::WeightBounds; | ||
|
|
||
| alloy::sol!("src/precompiles/IXcm.sol"); | ||
| use IXcm::*; | ||
|
franciscoaguirre marked this conversation as resolved.
Outdated
|
||
|
|
||
| pub struct XcmPrecompile<T>(PhantomData<T>); | ||
|
|
||
| impl<Runtime> Precompile for XcmPrecompile<Runtime> | ||
|
tiagobndr marked this conversation as resolved.
|
||
| where | ||
| Runtime: crate::Config + pallet_revive::Config, | ||
| Call<Runtime>: Into<<Runtime as pallet_revive::Config>::RuntimeCall>, | ||
| { | ||
| type T = Runtime; | ||
| const MATCHER: AddressMatcher = AddressMatcher::Fixed(NonZero::new(10).unwrap()); | ||
| const HAS_CONTRACT_INFO: bool = false; | ||
|
acatangiu marked this conversation as resolved.
|
||
| type Interface = IXcm::IXcmCalls; | ||
|
|
||
| fn call( | ||
| _address: &[u8; 20], | ||
| input: &Self::Interface, | ||
| env: &mut impl Ext<T = Self::T>, | ||
| ) -> Result<Vec<u8>, Error> { | ||
| let origin = env.caller(); | ||
| let frame_origin = match origin { | ||
| Origin::Root => frame_system::RawOrigin::Root.into(), | ||
| Origin::Signed(account_id) => | ||
| frame_system::RawOrigin::Signed(account_id.clone()).into(), | ||
| }; | ||
|
|
||
| match input { | ||
| IXcmCalls::xcmSend(IXcm::xcmSendCall { destination, message }) => { | ||
| let _weight = <Runtime as Config>::WeightInfo::send(); | ||
| // TODO: Charge gas for the weight | ||
|
|
||
| let final_destination = VersionedLocation::decode_all(&mut &destination[..]) | ||
| .map_err(|e| { | ||
| error!("XCM send failed: Invalid destination format. Error: {e:?}"); | ||
|
raymondkfcheung marked this conversation as resolved.
Outdated
|
||
| Error::Revert("Invalid destination format".into()) | ||
| })?; | ||
|
|
||
| let final_message = | ||
| VersionedXcm::<()>::decode_all(&mut &message[..]).map_err(|e| { | ||
| error!("XCM send failed: Invalid message format. Error: {e:?}"); | ||
| Error::Revert("Invalid message format".into()) | ||
|
raymondkfcheung marked this conversation as resolved.
Outdated
|
||
| })?; | ||
|
|
||
| crate::Pallet::<Runtime>::send( | ||
| frame_origin, | ||
| final_destination.into(), | ||
| final_message.into(), | ||
| ) | ||
| .map(|message_id| message_id.encode()) | ||
| .map_err(|e| { | ||
| error!( | ||
| "XCM send failed: destination or message format may be incompatible. \ | ||
| Error: {e:?}" | ||
| ); | ||
| Error::Revert( | ||
| "XCM send failed: destination or message format may be incompatible".into(), | ||
| ) | ||
| }) | ||
| }, | ||
| IXcmCalls::xcmExecute(IXcm::xcmExecuteCall { message, weight }) => { | ||
| let final_message = VersionedXcm::decode_all(&mut &message[..]).map_err(|e| { | ||
| error!("XCM execute failed: Invalid message format. Error: {e:?}"); | ||
| Error::Revert("Invalid message format".into()) | ||
|
raymondkfcheung marked this conversation as resolved.
Outdated
|
||
| })?; | ||
|
|
||
| let weight = Weight::from_parts(weight.refTime, weight.proofSize); | ||
| // env.gas_meter_mut().charge(RuntimeCosts::CallXcmExecute(weight.clone()))?; // | ||
| // TODO: Charge gas | ||
|
|
||
| crate::Pallet::<Runtime>::execute(frame_origin, final_message.into(), weight) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are missing a weight refund here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please check if the solution is good
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't we want to env.charge(<Runtime as Config>::WeightInfo::execute());as well?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to handle it twice. Use this function: https://paritytech.github.io/polkadot-sdk/master/frame_support/dispatch/fn.extract_actual_weight.html Then you don't need to clone the value either.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @athei can you please review my impl? |
||
| .map(|results| results.encode()) | ||
| .map_err(|e| { | ||
| error!( | ||
| "XCM execute failed: message may be invalid or execution \ | ||
| constraints not satisfied. Error: {e:?}" | ||
| ); | ||
| Error::Revert( | ||
| "XCM execute failed: message may be invalid or execution \ | ||
| constraints not satisfied" | ||
| .into(), | ||
| ) | ||
| }) | ||
| }, | ||
| IXcmCalls::weighMessage(IXcm::weighMessageCall { message }) => { | ||
| let converted_message = | ||
| VersionedXcm::decode_all(&mut &message[..]).map_err(|error| { | ||
| error!("XCM weightMessage: Invalid message format. Error: {error:?}"); | ||
| Error::Revert("XCM weightMessage: Invalid message format".into()) | ||
| })?; | ||
|
|
||
| let mut final_message = converted_message.try_into().map_err(|e| { | ||
| error!("XCM weightMessage: Conversion to Xcm failed with Error: {e:?}"); | ||
| Error::Revert("XCM weightMessage: Conversion to Xcm failed".into()) | ||
| })?; | ||
|
|
||
| let weight = <<Runtime>::Weigher>::weight(&mut final_message).map_err(|e| { | ||
| error!("XCM weightMessage: Failed to calculate weight. Error: {e:?}"); | ||
| Error::Revert("XCM weightMessage: Failed to calculate weight".into()) | ||
| })?; | ||
|
|
||
| let final_weight = | ||
| IXcm::Weight { proofSize: weight.proof_size(), refTime: weight.ref_time() }; | ||
|
|
||
| Ok(final_weight.abi_encode()) | ||
| }, | ||
| } | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @franciscoaguirre we don't want to put that in the ethereum-standards crate ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not an ethereum standard
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought the crate would hold ever sol files
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would only hold ethereum standards, else we should change the name of the crate
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Depends on whether the XCM.sol file needs to be shared with other pallets or not. Right now it doesn't . So it should be fine to leave it there for now. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| // SPDX-License-Identifier: MIT | ||
|
tiagobndr marked this conversation as resolved.
|
||
| pragma solidity ^0.8.20; | ||
|
|
||
| /// @title Defines all functions that can be used to interact with XCM | ||
| /// @author Tiago Bandeira | ||
|
tiagobndr marked this conversation as resolved.
Outdated
|
||
| /// @dev Parameters MUST use SCALE codec serialisation | ||
| interface IXcm { | ||
| /// Weight v2 | ||
| struct Weight { | ||
| /// The computational time used to execute some logic based on reference hardware | ||
| uint64 refTime; | ||
| /// The size of the proof needed to execute some logic | ||
| uint64 proofSize; | ||
| } | ||
|
tiagobndr marked this conversation as resolved.
|
||
|
|
||
| /// @notice Execute a Versioned XCM message locally with the caller's origin | ||
| /// @param message The Versioned XCM message to send | ||
| /// @param weight The maximum amount of weight to be used to execute the message | ||
| function xcmExecute(bytes calldata message, Weight calldata weight) external; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't return anything from either
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about emitting an event @franciscoaguirre
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We get that out of the box from calling
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
From the call to the precompile, we get back whatever gets returned from
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like this interface. In practice, as @franciscoaguirre pointed out, how would you even know if it succeeded or not? I suggest to version the interface in one way or the other, such that it will remain stable (then return values are possible - even if they are just untyped bytes[] the caller at least get's a chance to decode them, can be added later).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW if the precompile reverts on errors, callers can at least check the call success variable in Solidity. But versioning it explicitly still seems like a good idea to me. Unless to whatever this is calling into is absolutely stable forever? I can't really imagine that.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And those things should be documented in the interface as well.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And pleases also consider this: Given the precompile reverts on error, any change in the conditions when reverts happen is a breaking change. Unless I am missing things I don't yet see how this will work out unless we get a mapping of exactly one (versioned) interface to one precompile contract address. |
||
|
|
||
| /// @notice Send an Versioned XCM message to a destination chain | ||
| /// @param destination The destination location, encoded according to the XCM format | ||
| /// @param message The Versioned XCM message to send | ||
| function xcmSend(bytes calldata destination, bytes calldata message) external; | ||
|
|
||
| /// @notice Given a message estimate the weight cost | ||
| /// @param message The XCM message to send | ||
| /// @returns weight estimated for sending the message | ||
| function weighMessage(bytes calldata message) external view returns (Weight weight); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.