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.

Sign up for GitHub

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

Automatic EVM revert code registration for XC20 #1001

Merged
merged 8 commits into from
Aug 11, 2023
Merged

Automatic EVM revert code registration for XC20 #1001

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
35050d4
Automatic EVM revert code registration for XC20
Dinonard Aug 10, 2023
fb1a5da
Add negative test
Dinonard Aug 10, 2023
1a55274
Fmt fix
Dinonard Aug 10, 2023
dbcf02b
Add EVM module
Dinonard Aug 11, 2023
17015a6
Asset benchmarks
Dinonard Aug 11, 2023
b9523a2
Cleanup xc asset config
Dinonard Aug 11, 2023
c98a8f9
Update asset benchmarks
Dinonard Aug 11, 2023
7df59c8
Fix tests features
Dinonard Aug 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pallets/xc-asset-config/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ pub mod pallet {
/// a AssetLocation
type AssetId: Member + Parameter + Default + Copy + HasCompact + MaxEncodedLen;

/// TODO: this has essentially been deprecated and can be removed.
Dinonard marked this conversation as resolved.
Show resolved Hide resolved
/// Callback handling for cross-chain asset registration or unregistration.
type XcAssetChanged: XcAssetChanged<Self>;
Dinonard marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
10 changes: 9 additions & 1 deletion primitives/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ fp-evm = { workspace = true }

# Substrate dependencies
frame-support = { workspace = true }
pallet-assets = { workspace = true }
sp-core = { workspace = true }
sp-io = { workspace = true }
sp-runtime = { workspace = true }
Expand All @@ -30,7 +31,11 @@ xcm = { workspace = true }
xcm-builder = { workspace = true }
xcm-executor = { workspace = true }

# Astar pallets
# Frontier dependencies
pallet-evm = { workspace = true }

# Astar pallets & dependencies
pallet-evm-precompile-assets-erc20 = { workspace = true }
pallet-xc-asset-config = { workspace = true }

[features]
Expand All @@ -52,5 +57,8 @@ std = [
"xcm-executor/std",
"pallet-xc-asset-config/std",
"fp-evm/std",
"pallet-assets/std",
"pallet-evm/std",
"pallet-evm-precompile-assets-erc20/std",
]
runtime-benchmarks = ["xcm-builder/runtime-benchmarks"]
38 changes: 36 additions & 2 deletions primitives/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,15 @@ pub mod ethereum_checked;
/// XVM primitives.
pub mod xvm;

use sp_runtime::traits::BlakeTwo256;
use frame_support::ensure;
use sp_runtime::{
generic,
traits::{IdentifyAccount, Verify},
traits::{BlakeTwo256, IdentifyAccount, Verify},
};
use sp_std::marker::PhantomData;

use pallet_assets::AssetsCallback;
use pallet_evm_precompile_assets_erc20::AddressToAssetId;

/// Alias to 512-bit hash when used in the context of a transaction signature on the chain.
pub type Signature = sp_runtime::MultiSignature;
Expand Down Expand Up @@ -65,3 +69,33 @@ pub type Index = u32;
pub type Block = sp_runtime::generic::Block<Header, sp_runtime::OpaqueExtrinsic>;
/// Index of a transaction in the chain.
pub type Nonce = u32;
/// Revert opt code. It's inserted at the precompile addresses, to make them functional in EVM.
Dinonard marked this conversation as resolved.
Show resolved Hide resolved
pub const EVM_REVERT_CODE: &[u8] = &[0x60, 0x00, 0x60, 0x00, 0xfd];

/// Handler for automatic revert code registration.
///
/// When an asset is created, it automatically becomes available to the EVM via an `ERC20-like` interface.
/// In order for the precompile to work, dedicated asset address needs to have the revert code registered, otherwise the call will fail.
///
/// It is important to note that if the dedicated asset EVM address is already taken, asset creation should fail.
/// After asset has been destroyed, it is also safe to remove the revert code and free the address for future usage.
pub struct EvmRevertCodeHandler<A, R>(PhantomData<(A, R)>);
impl<A, R> AssetsCallback<AssetId, AccountId> for EvmRevertCodeHandler<A, R>
where
A: AddressToAssetId<AssetId>,
R: pallet_evm::Config,
{
fn created(id: &AssetId, _: &AccountId) -> Result<(), ()> {
let address = A::asset_id_to_address(*id);
// In case of collision, we need to cancel the asset creation.
ensure!(!pallet_evm::AccountCodes::<R>::contains_key(&address), ());
pallet_evm::AccountCodes::<R>::insert(address, EVM_REVERT_CODE.to_vec());
Ok(())
}

fn destroyed(id: &AssetId) -> Result<(), ()> {
let address = A::asset_id_to_address(*id);
pallet_evm::AccountCodes::<R>::remove(address);
Ok(())
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how the callback weights are taken into account inside pallet_assets (I couldn't find anything in code), but since we are doing DB writes who will be paying for 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.

Good catch, and reminder!
I had written down to redo the benchmarks and just didn't 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

Solved.

51 changes: 2 additions & 49 deletions primitives/src/xcm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@
#![cfg_attr(not(feature = "std"), no_std)]

use frame_support::{
ensure,
traits::{tokens::fungibles, Contains, ContainsPair, Get, ProcessMessageError},
traits::{tokens::fungibles, ContainsPair, Get},
weights::constants::WEIGHT_REF_TIME_PER_SECOND,
};
use sp_runtime::traits::{Bounded, Zero};
Expand All @@ -43,7 +42,7 @@ use sp_std::{borrow::Borrow, marker::PhantomData, vec::Vec};
// Polkadot imports
use xcm::latest::{prelude::*, Weight};
use xcm_builder::TakeRevenue;
use xcm_executor::traits::{MatchesFungibles, ShouldExecute, WeightTrader};
use xcm_executor::traits::{MatchesFungibles, WeightTrader};

use pallet_xc_asset_config::{ExecutionPaymentRate, XcAssetLocation};

Expand Down Expand Up @@ -271,52 +270,6 @@ impl<
}
}

/// Allows execution from `origin` if it is contained in `T` (i.e. `T::Contains(origin)`) taking
/// payments into account.
///
/// Only allows for sequence `DescendOrigin` -> `WithdrawAsset` -> `BuyExecution`
pub struct AllowPaidExecWithDescendOriginFrom<T>(PhantomData<T>);
impl<T: Contains<MultiLocation>> ShouldExecute for AllowPaidExecWithDescendOriginFrom<T> {
fn should_execute<RuntimeCall>(
origin: &MultiLocation,
message: &mut [Instruction<RuntimeCall>],
max_weight: Weight,
_weight_credit: &mut Weight,
) -> Result<(), ProcessMessageError> {
log::trace!(
target: "xcm::barriers",
"AllowPaidExecWithDescendOriginFrom origin: {:?}, message: {:?}, max_weight: {:?}, weight_credit: {:?}",
origin, message, max_weight, _weight_credit,
);
ensure!(T::contains(origin), ProcessMessageError::Unsupported);

match message
.iter_mut()
.take(3)
.collect::<Vec<_>>()
.as_mut_slice()
{
[DescendOrigin(..), WithdrawAsset(..), BuyExecution {
weight_limit: Limited(ref mut limit),
..
}] if limit.all_gte(max_weight) => {
*limit = max_weight;
Ok(())
}

[DescendOrigin(..), WithdrawAsset(..), BuyExecution {
weight_limit: ref mut limit @ Unlimited,
..
}] => {
*limit = Limited(max_weight);
Ok(())
}

_ => return Err(ProcessMessageError::Unsupported),
}
}
}

// TODO: remove this after uplift to `polkadot-v0.9.44` or beyond, and replace it with code in XCM builder.

use parity_scale_codec::{Compact, Encode};
Expand Down
155 changes: 1 addition & 154 deletions primitives/src/xcm/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@
// along with Astar. If not, see <http://www.gnu.org/licenses/>.

use super::*;
use frame_support::{
assert_ok,
traits::{Everything, Nothing},
};
use frame_support::assert_ok;
use sp_runtime::traits::Zero;
use xcm_executor::traits::Convert;

Expand Down Expand Up @@ -411,156 +408,6 @@ fn reserve_asset_filter_for_unsupported_asset_multi_location() {
assert!(!ReserveAssetFilter::contains(&multi_asset, &origin));
}

/// Returns valid XCM sequence for bypassing `AllowPaidExecWithDescendOriginFrom`
fn desc_origin_barrier_valid_sequence() -> Vec<Instruction<()>> {
vec![
DescendOrigin(X1(Junction::Parachain(1234))),
WithdrawAsset((Here, 100).into()),
BuyExecution {
fees: (Here, 100).into(),
weight_limit: WeightLimit::Unlimited,
},
]
}

#[test]
fn allow_paid_exec_with_descend_origin_works() {
let mut valid_message = desc_origin_barrier_valid_sequence();

let res = AllowPaidExecWithDescendOriginFrom::<Everything>::should_execute(
&Here.into(),
&mut valid_message,
Weight::from_parts(150, 0),
&mut Weight::zero(),
);
assert_eq!(res, Ok(()));

// Still works even if there are follow-up instructions
valid_message = desc_origin_barrier_valid_sequence();
valid_message.push(SetErrorHandler(Default::default()));
let res = AllowPaidExecWithDescendOriginFrom::<Everything>::should_execute(
&Here.into(),
&mut valid_message,
Weight::from_parts(100, 0),
&mut Weight::zero(),
);
assert_eq!(res, Ok(()));
}

#[test]
fn allow_paid_exec_with_descend_origin_with_weight_correction_works() {
let mut valid_message = desc_origin_barrier_valid_sequence();

// Ensure that `Limited` gets adjusted to the provided enforced_weight_limit
let enforced_weight_limit = Weight::from_parts(3, 0);
let res = AllowPaidExecWithDescendOriginFrom::<Everything>::should_execute(
&Here.into(),
&mut valid_message,
enforced_weight_limit,
&mut Weight::zero(),
);
assert_eq!(res, Ok(()));

if let BuyExecution {
weight_limit,
fees: _,
} = valid_message[2].clone()
{
assert_eq!(weight_limit, WeightLimit::Limited(enforced_weight_limit))
} else {
panic!("3rd instruction should be BuyExecution!");
}

// Ensure that we use `BuyExecution` with `Unlimited` weight limit
let _ = std::mem::replace(
&mut valid_message[2],
BuyExecution {
fees: (Here, 100).into(),
weight_limit: WeightLimit::Limited(enforced_weight_limit.add_ref_time(7)),
},
);

// Ensure that `Unlimited` gets adjusted to the provided max weight limit
let res = AllowPaidExecWithDescendOriginFrom::<Everything>::should_execute(
&Here.into(),
&mut valid_message,
enforced_weight_limit,
&mut Weight::zero(),
);
assert_eq!(res, Ok(()));

if let BuyExecution {
weight_limit,
fees: _,
} = valid_message[2].clone()
{
assert_eq!(weight_limit, WeightLimit::Limited(enforced_weight_limit))
} else {
panic!("3rd instruction should be BuyExecution!");
}
}

#[test]
fn allow_paid_exec_with_descend_origin_with_unsupported_origin_fails() {
let mut valid_message = desc_origin_barrier_valid_sequence();

let res = AllowPaidExecWithDescendOriginFrom::<Nothing>::should_execute(
&Here.into(),
&mut valid_message,
Weight::from_parts(100, 0),
&mut Weight::zero(),
);
assert_eq!(res, Err(ProcessMessageError::Unsupported));
}

#[test]
fn allow_paid_exec_with_descend_origin_with_invalid_message_fails() {
let mut invalid_message = vec![WithdrawAsset((Here, 100).into())];

let res = AllowPaidExecWithDescendOriginFrom::<Everything>::should_execute(
&Here.into(),
&mut invalid_message,
Weight::from_parts(100, 0),
&mut Weight::zero(),
);
assert_eq!(res, Err(ProcessMessageError::Unsupported));

// Should still fail, even if correct sequence follows next
invalid_message.append(&mut desc_origin_barrier_valid_sequence());
let res = AllowPaidExecWithDescendOriginFrom::<Everything>::should_execute(
&Here.into(),
&mut invalid_message,
Weight::from_parts(100, 0),
&mut Weight::zero(),
);
assert_eq!(res, Err(ProcessMessageError::Unsupported));
}

#[test]
fn allow_paid_exec_with_descend_origin_too_small_weight_fails() {
let mut valid_message = desc_origin_barrier_valid_sequence();
let enforced_weight_limit = Weight::from_parts(29, 0);

// Ensure that we use `BuyExecution` with `Limited` weight but with insufficient weight.
// This means that not enough execution time (weight) is being bought compared to the
// weight of whole sequence.
let _ = std::mem::replace(
&mut valid_message[2],
BuyExecution {
fees: (Here, 100).into(),
weight_limit: WeightLimit::Limited(enforced_weight_limit.sub_ref_time(7)),
},
);

let res = AllowPaidExecWithDescendOriginFrom::<Everything>::should_execute(
&Here.into(),
&mut valid_message,
enforced_weight_limit,
&mut Weight::zero(),
);
assert_eq!(res, Err(ProcessMessageError::Unsupported));
}

// TODO: can be deleted after uplift to `polkadot-v0.9.44` or beyond.
#[test]
fn hashed_description_sanity_check() {
Expand Down
Loading