diff --git a/polkadot/runtime/common/src/claims.rs b/polkadot/runtime/common/src/claims.rs index 60d9a0aa9e..f2be5a954b 100644 --- a/polkadot/runtime/common/src/claims.rs +++ b/polkadot/runtime/common/src/claims.rs @@ -1044,7 +1044,7 @@ mod tests { } #[test] - fn invalid_attest_transactions_are_recognised() { + fn invalid_attest_transactions_are_recognized() { new_test_ext().execute_with(|| { let p = PrevalidateAttests::::new(); let c = Call::Claims(ClaimsCall::attest(StatementKind::Regular.to_text().to_vec())); diff --git a/polkadot/runtime/kusama/src/lib.rs b/polkadot/runtime/kusama/src/lib.rs index 3d7eac08e7..036b8ab392 100644 --- a/polkadot/runtime/kusama/src/lib.rs +++ b/polkadot/runtime/kusama/src/lib.rs @@ -1296,7 +1296,9 @@ impl xcm_executor::Config for XcmConfig { type Weigher = FixedWeightBounds; // The weight trader piggybacks on the existing transaction-fee conversion logic. type Trader = UsingComponents>; - type ResponseHandler = (); + type ResponseHandler = XcmPallet; + type AssetTrap = XcmPallet; + type AssetClaims = XcmPallet; } parameter_types! { diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 1ae220330b..c46bca5675 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -669,7 +669,9 @@ impl xcm_executor::Config for XcmConfig { type Barrier = Barrier; type Weigher = FixedWeightBounds; type Trader = UsingComponents>; - type ResponseHandler = (); + type ResponseHandler = XcmPallet; + type AssetTrap = XcmPallet; + type AssetClaims = XcmPallet; } parameter_types! { diff --git a/polkadot/runtime/test-runtime/src/xcm_config.rs b/polkadot/runtime/test-runtime/src/xcm_config.rs index 3af079e92f..3a25b8c9ff 100644 --- a/polkadot/runtime/test-runtime/src/xcm_config.rs +++ b/polkadot/runtime/test-runtime/src/xcm_config.rs @@ -86,4 +86,6 @@ impl xcm_executor::Config for XcmConfig { type Weigher = FixedWeightBounds; type Trader = DummyWeightTrader; type ResponseHandler = super::Xcm; + type AssetTrap = super::Xcm; + type AssetClaims = super::Xcm; } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 0be1568f0f..52c8a72c13 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -938,7 +938,9 @@ impl xcm_executor::Config for XcmConfig { type Barrier = Barrier; type Weigher = FixedWeightBounds; type Trader = UsingComponents>; - type ResponseHandler = (); + type ResponseHandler = XcmPallet; + type AssetTrap = XcmPallet; + type AssetClaims = XcmPallet; } /// Type to convert an `Origin` type value into a `MultiLocation` value which represents an interior location diff --git a/polkadot/xcm/pallet-xcm/Cargo.toml b/polkadot/xcm/pallet-xcm/Cargo.toml index 821608e62e..49f3de61b1 100644 --- a/polkadot/xcm/pallet-xcm/Cargo.toml +++ b/polkadot/xcm/pallet-xcm/Cargo.toml @@ -11,6 +11,7 @@ log = { version = "0.4.14", default-features = false } sp-std = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } sp-runtime = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } +sp-core = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } frame-support = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } frame-system = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } @@ -20,7 +21,6 @@ xcm-executor = { path = "../xcm-executor", default-features = false } [dev-dependencies] pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "master" } polkadot-runtime-parachains = { path = "../../runtime/parachains" } -sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-io = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } xcm-builder = { path = "../xcm-builder" } polkadot-parachain = { path = "../../parachain" } @@ -31,6 +31,7 @@ std = [ "codec/std", "serde", "sp-std/std", + "sp-core/std", "sp-runtime/std", "frame-support/std", "frame-system/std", diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index acc5a6269f..83f8b65524 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -50,8 +50,12 @@ pub mod pallet { pallet_prelude::*, }; use frame_system::{pallet_prelude::*, Config as SysConfig}; - use sp_runtime::traits::{AccountIdConversion, BlockNumberProvider}; - use xcm_executor::traits::{InvertLocation, OnResponse, WeightBounds}; + use sp_core::H256; + use sp_runtime::traits::{AccountIdConversion, BlakeTwo256, BlockNumberProvider, Hash}; + use xcm_executor::{ + traits::{ClaimAssets, DropAssets, InvertLocation, OnResponse, WeightBounds}, + Assets, + }; #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] @@ -170,6 +174,10 @@ pub mod pallet { /// /// \[ id \] ResponseTaken(QueryId), + /// Some assets have been placed in an asset trap. + /// + /// \[ hash, origin, assets \] + AssetsTrapped(H256, MultiLocation, VersionedMultiAssets), } #[pallet::origin] @@ -236,6 +244,14 @@ pub mod pallet { pub(super) type Queries = StorageMap<_, Blake2_128Concat, QueryId, QueryStatus, OptionQuery>; + /// The existing asset traps. + /// + /// Key is the blake2 256 hash of (origin, versioned `MultiAssets`) pair. Value is the number of + /// times this pair has been trapped (usually just 1 if it exists at all). + #[pallet::storage] + #[pallet::getter(fn asset_trap)] + pub(super) type AssetTraps = StorageMap<_, Identity, H256, u32, ValueQuery>; + #[pallet::hooks] impl Hooks> for Pallet {} @@ -553,8 +569,47 @@ pub mod pallet { } } + impl DropAssets for Pallet { + fn drop_assets(origin: &MultiLocation, assets: Assets) -> Weight { + if assets.is_empty() { + return 0 + } + let versioned = VersionedMultiAssets::from(MultiAssets::from(assets)); + let hash = BlakeTwo256::hash_of(&(&origin, &versioned)); + AssetTraps::::mutate(hash, |n| *n += 1); + Self::deposit_event(Event::AssetsTrapped(hash, origin.clone(), versioned)); + // TODO: Put the real weight in there. + 0 + } + } + + impl ClaimAssets for Pallet { + fn claim_assets( + origin: &MultiLocation, + ticket: &MultiLocation, + assets: &MultiAssets, + ) -> bool { + let mut versioned = VersionedMultiAssets::from(assets.clone()); + match (ticket.parents, &ticket.interior) { + (0, X1(GeneralIndex(i))) => + versioned = match versioned.into_version(*i as u32) { + Ok(v) => v, + Err(()) => return false, + }, + (0, Here) => (), + _ => return false, + }; + let hash = BlakeTwo256::hash_of(&(origin, versioned)); + match AssetTraps::::get(hash) { + 0 => return false, + 1 => AssetTraps::::remove(hash), + n => AssetTraps::::insert(hash, n - 1), + } + return true + } + } + impl OnResponse for Pallet { - /// Returns `true` if we are expecting a response from `origin` for query `query_id`. fn expecting_response(origin: &MultiLocation, query_id: QueryId) -> bool { if let Some(QueryStatus::Pending { responder, .. }) = Queries::::get(query_id) { return MultiLocation::try_from(responder).map_or(false, |r| origin == &r) @@ -562,7 +617,6 @@ pub mod pallet { false } - /// Handler for receiving a `response` from `origin` relating to `query_id`. fn on_response( origin: &MultiLocation, query_id: QueryId, diff --git a/polkadot/xcm/pallet-xcm/src/mock.rs b/polkadot/xcm/pallet-xcm/src/mock.rs index 66c427b9ff..804a39646a 100644 --- a/polkadot/xcm/pallet-xcm/src/mock.rs +++ b/polkadot/xcm/pallet-xcm/src/mock.rs @@ -251,6 +251,8 @@ impl xcm_executor::Config for XcmConfig { type Weigher = FixedWeightBounds; type Trader = FixedRateOfFungible; type ResponseHandler = XcmPallet; + type AssetTrap = XcmPallet; + type AssetClaims = XcmPallet; } pub type LocalOriginToLocation = SignedToAccountId32; diff --git a/polkadot/xcm/pallet-xcm/src/tests.rs b/polkadot/xcm/pallet-xcm/src/tests.rs index 540ee153a6..c58e042610 100644 --- a/polkadot/xcm/pallet-xcm/src/tests.rs +++ b/polkadot/xcm/pallet-xcm/src/tests.rs @@ -14,11 +14,12 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -use crate::{mock::*, QueryStatus}; +use crate::{mock::*, AssetTraps, QueryStatus}; use frame_support::{assert_noop, assert_ok, traits::Currency}; use polkadot_parachain::primitives::{AccountIdConversion, Id as ParaId}; +use sp_runtime::traits::{BlakeTwo256, Hash}; use std::convert::TryInto; -use xcm::{latest::prelude::*, VersionedXcm}; +use xcm::{latest::prelude::*, VersionedMultiAssets, VersionedXcm}; use xcm_executor::XcmExecutor; const ALICE: AccountId = AccountId::new([0u8; 32]); @@ -295,3 +296,81 @@ fn execute_withdraw_to_deposit_works() { ); }); } + +/// Test drop/claim assets. +#[test] +fn trapped_assets_can_be_claimed() { + let balances = vec![(ALICE, INITIAL_BALANCE), (BOB, INITIAL_BALANCE)]; + new_test_ext_with_balances(balances).execute_with(|| { + let weight = 6 * BaseXcmWeight::get(); + let dest: MultiLocation = + Junction::AccountId32 { network: NetworkId::Any, id: BOB.into() }.into(); + + assert_ok!(XcmPallet::execute( + Origin::signed(ALICE), + Box::new(VersionedXcm::from(Xcm(vec![ + WithdrawAsset((Here, SEND_AMOUNT).into()), + buy_execution((Here, SEND_AMOUNT)), + // Don't propagated the error into the result. + SetErrorHandler(Xcm(vec![ClearError])), + // This will make an error. + Trap(0), + // This would succeed, but we never get to it. + DepositAsset { assets: All.into(), max_assets: 1, beneficiary: dest.clone() }, + ]))), + weight + )); + let source: MultiLocation = + Junction::AccountId32 { network: NetworkId::Any, id: ALICE.into() }.into(); + let trapped = AssetTraps::::iter().collect::>(); + let vma = VersionedMultiAssets::from(MultiAssets::from((Here, SEND_AMOUNT))); + let hash = BlakeTwo256::hash_of(&(source.clone(), vma.clone())); + assert_eq!( + last_events(2), + vec![ + Event::XcmPallet(crate::Event::AssetsTrapped(hash.clone(), source, vma)), + Event::XcmPallet(crate::Event::Attempted(Outcome::Complete( + 5 * BaseXcmWeight::get() + ))) + ] + ); + assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE - SEND_AMOUNT); + assert_eq!(Balances::total_balance(&BOB), INITIAL_BALANCE); + + let expected = vec![(hash, 1u32)]; + assert_eq!(trapped, expected); + + let weight = 3 * BaseXcmWeight::get(); + assert_ok!(XcmPallet::execute( + Origin::signed(ALICE), + Box::new(VersionedXcm::from(Xcm(vec![ + ClaimAsset { assets: (Here, SEND_AMOUNT).into(), ticket: Here.into() }, + buy_execution((Here, SEND_AMOUNT)), + DepositAsset { assets: All.into(), max_assets: 1, beneficiary: dest.clone() }, + ]))), + weight + )); + + assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE - SEND_AMOUNT); + assert_eq!(Balances::total_balance(&BOB), INITIAL_BALANCE + SEND_AMOUNT); + assert_eq!(AssetTraps::::iter().collect::>(), vec![]); + + let weight = 3 * BaseXcmWeight::get(); + assert_ok!(XcmPallet::execute( + Origin::signed(ALICE), + Box::new(VersionedXcm::from(Xcm(vec![ + ClaimAsset { assets: (Here, SEND_AMOUNT).into(), ticket: Here.into() }, + buy_execution((Here, SEND_AMOUNT)), + DepositAsset { assets: All.into(), max_assets: 1, beneficiary: dest }, + ]))), + weight + )); + assert_eq!( + last_event(), + Event::XcmPallet(crate::Event::Attempted(Outcome::Incomplete( + BaseXcmWeight::get(), + XcmError::UnknownClaim + ))) + ); + }); +} diff --git a/polkadot/xcm/src/lib.rs b/polkadot/xcm/src/lib.rs index af53b7bddc..b9aad191ef 100644 --- a/polkadot/xcm/src/lib.rs +++ b/polkadot/xcm/src/lib.rs @@ -217,6 +217,16 @@ pub enum VersionedMultiAssets { V1(v1::MultiAssets), } +impl VersionedMultiAssets { + pub fn into_version(self, n: u32) -> Result { + Ok(match n { + 0 => Self::V0(self.try_into()?), + 1 | 2 => Self::V1(self.try_into()?), + _ => return Err(()), + }) + } +} + impl From> for VersionedMultiAssets { fn from(x: Vec) -> Self { VersionedMultiAssets::V0(x) diff --git a/polkadot/xcm/src/v2/mod.rs b/polkadot/xcm/src/v2/mod.rs index 42d0283652..c89c20df74 100644 --- a/polkadot/xcm/src/v2/mod.rs +++ b/polkadot/xcm/src/v2/mod.rs @@ -392,6 +392,8 @@ pub enum Instruction { /// prioritized under standard asset ordering. Any others will remain in holding. /// - `beneficiary`: The new owner for the assets. /// + /// Kind: *Instruction* + /// /// Errors: DepositAsset { assets: MultiAssetFilter, max_assets: u32, beneficiary: MultiLocation }, @@ -411,6 +413,8 @@ pub enum Instruction { /// - `xcm`: The orders that should follow the `ReserveAssetDeposited` instruction /// which is sent onwards to `dest`. /// + /// Kind: *Instruction* + /// /// Errors: DepositReserveAsset { assets: MultiAssetFilter, @@ -428,6 +432,8 @@ pub enum Instruction { /// - `give`: The asset(s) to remove from holding. /// - `receive`: The minimum amount of assets(s) which `give` should be exchanged for. /// + /// Kind: *Instruction* + /// /// Errors: ExchangeAsset { give: MultiAssetFilter, receive: MultiAssets }, @@ -442,6 +448,8 @@ pub enum Instruction { /// - `xcm`: The instructions to execute on the assets once withdrawn *on the reserve /// location*. /// + /// Kind: *Instruction* + /// /// Errors: InitiateReserveWithdraw { assets: MultiAssetFilter, reserve: MultiLocation, xcm: Xcm<()> }, @@ -456,6 +464,8 @@ pub enum Instruction { /// NOTE: The `dest` location *MUST* respect this origin as a valid teleportation origin for all /// `assets`. If it does not, then the assets may be lost. /// + /// Kind: *Instruction* + /// /// Errors: InitiateTeleport { assets: MultiAssetFilter, dest: MultiLocation, xcm: Xcm<()> }, @@ -472,6 +482,8 @@ pub enum Instruction { /// is sent as a reply may take to execute. NOTE: If this is unexpectedly large then the /// response may not execute at all. /// + /// Kind: *Instruction* + /// /// Errors: QueryHolding { #[codec(compact)] @@ -490,13 +502,20 @@ pub enum Instruction { /// expected maximum weight of the total XCM to be executed for the /// `AllowTopLevelPaidExecutionFrom` barrier to allow the XCM be executed. /// + /// Kind: *Instruction* + /// /// Errors: BuyExecution { fees: MultiAsset, weight_limit: WeightLimit }, /// Refund any surplus weight previously bought with `BuyExecution`. + /// + /// Kind: *Instruction* + /// + /// Errors: None. RefundSurplus, - /// Set code that should be called in the case of an error happening. + /// Set the Error Handler Register. This is code that should be called in the case of an error + /// happening. /// /// An error occurring within execution of this code will _NOT_ result in the error register /// being set, nor will an error handler be called due to it. The error handler and appendix @@ -505,10 +524,15 @@ pub enum Instruction { /// The apparent weight of this instruction is inclusive of the inner `Xcm`; the executing /// weight however includes only the difference between the previous handler and the new /// handler, which can reasonably be negative, which would result in a surplus. + /// + /// Kind: *Instruction* + /// + /// Errors: None. SetErrorHandler(Xcm), - /// Set code that should be called after code execution (including the error handler if any) - /// is finished. This will be called regardless of whether an error occurred. + /// Set the Appendix Register. This is code that should be called after code execution + /// (including the error handler if any) is finished. This will be called regardless of whether + /// an error occurred. /// /// Any error occurring due to execution of this code will result in the error register being /// set, and the error handler (if set) firing. @@ -516,10 +540,38 @@ pub enum Instruction { /// The apparent weight of this instruction is inclusive of the inner `Xcm`; the executing /// weight however includes only the difference between the previous appendix and the new /// appendix, which can reasonably be negative, which would result in a surplus. + /// + /// Kind: *Instruction* + /// + /// Errors: None. SetAppendix(Xcm), - /// Clear the error register. + /// Clear the Error Register. + /// + /// Kind: *Instruction* + /// + /// Errors: None. ClearError, + + /// Create some assets which are being held on behalf of the origin. + /// + /// - `assets`: The assets which are to be claimed. This must match exactly with the assets + /// claimable by the origin of the ticket. + /// - `ticket`: The ticket of the asset; this is an abstract identifier to help locate the + /// asset. + /// + /// Kind: *Instruction* + /// + /// Errors: + ClaimAsset { assets: MultiAssets, ticket: MultiLocation }, + + /// Always throws an error of type `Trap`. + /// + /// Kind: *Instruction* + /// + /// Errors: + /// - `Trap`: All circumstances, whose inner value is the same as this item's inner value. + Trap(u64), } impl Xcm { @@ -572,6 +624,8 @@ impl Instruction { SetErrorHandler(xcm) => SetErrorHandler(xcm.into()), SetAppendix(xcm) => SetAppendix(xcm.into()), ClearError => ClearError, + ClaimAsset { assets, ticket } => ClaimAsset { assets, ticket }, + Trap(code) => Trap(code), } } } diff --git a/polkadot/xcm/src/v2/traits.rs b/polkadot/xcm/src/v2/traits.rs index facbdd0892..d5d96a5d0c 100644 --- a/polkadot/xcm/src/v2/traits.rs +++ b/polkadot/xcm/src/v2/traits.rs @@ -94,6 +94,10 @@ pub enum Error { Unroutable, /// The weight required was not specified when it should have been. UnknownWeightRequired, + /// An error was intentionally forced. A code is included. + Trap(u64), + /// The given claim could not be recognized/found. + UnknownClaim, } impl From<()> for Error { diff --git a/polkadot/xcm/xcm-builder/src/barriers.rs b/polkadot/xcm/xcm-builder/src/barriers.rs index 4d82380a03..d0261d5d09 100644 --- a/polkadot/xcm/xcm-builder/src/barriers.rs +++ b/polkadot/xcm/xcm-builder/src/barriers.rs @@ -30,7 +30,7 @@ use xcm_executor::traits::{OnResponse, ShouldExecute}; pub struct TakeWeightCredit; impl ShouldExecute for TakeWeightCredit { fn should_execute( - _origin: &Option, + _origin: &MultiLocation, _top_level: bool, _message: &mut Xcm, max_weight: Weight, @@ -49,19 +49,21 @@ impl ShouldExecute for TakeWeightCredit { pub struct AllowTopLevelPaidExecutionFrom(PhantomData); impl> ShouldExecute for AllowTopLevelPaidExecutionFrom { fn should_execute( - origin: &Option, + origin: &MultiLocation, top_level: bool, message: &mut Xcm, max_weight: Weight, _weight_credit: &mut Weight, ) -> Result<(), ()> { - let origin = origin.as_ref().ok_or(())?; ensure!(T::contains(origin), ()); ensure!(top_level, ()); let mut iter = message.0.iter_mut(); let i = iter.next().ok_or(())?; match i { - ReceiveTeleportedAsset(..) | WithdrawAsset(..) | ReserveAssetDeposited(..) => (), + ReceiveTeleportedAsset(..) | + WithdrawAsset(..) | + ReserveAssetDeposited(..) | + ClaimAsset { .. } => (), _ => return Err(()), } let mut i = iter.next().ok_or(())?; @@ -87,13 +89,12 @@ impl> ShouldExecute for AllowTopLevelPaidExecutionFro pub struct AllowUnpaidExecutionFrom(PhantomData); impl> ShouldExecute for AllowUnpaidExecutionFrom { fn should_execute( - origin: &Option, + origin: &MultiLocation, _top_level: bool, _message: &mut Xcm, _max_weight: Weight, _weight_credit: &mut Weight, ) -> Result<(), ()> { - let origin = origin.as_ref().ok_or(())?; ensure!(T::contains(origin), ()); Ok(()) } @@ -115,13 +116,12 @@ impl> Contains for IsChildSystemPara pub struct AllowKnownQueryResponses(PhantomData); impl ShouldExecute for AllowKnownQueryResponses { fn should_execute( - origin: &Option, + origin: &MultiLocation, _top_level: bool, message: &mut Xcm, _max_weight: Weight, _weight_credit: &mut Weight, ) -> Result<(), ()> { - let origin = origin.as_ref().ok_or(())?; match message.0.first() { Some(QueryResponse { query_id, .. }) if ResponseHandler::expecting_response(origin, *query_id) => diff --git a/polkadot/xcm/xcm-builder/src/mock.rs b/polkadot/xcm/xcm-builder/src/mock.rs index 21cb0c8650..3f00b50195 100644 --- a/polkadot/xcm/xcm-builder/src/mock.rs +++ b/polkadot/xcm/xcm-builder/src/mock.rs @@ -35,6 +35,7 @@ pub use sp_std::{ marker::PhantomData, }; pub use xcm::latest::prelude::*; +use xcm_executor::traits::{ClaimAssets, DropAssets}; pub use xcm_executor::{ traits::{ConvertOrigin, FilterAssetLocation, InvertLocation, OnResponse, TransactAsset}, Assets, Config, @@ -269,6 +270,37 @@ pub type TestBarrier = ( AllowUnpaidExecutionFrom>, ); +parameter_types! { + pub static TrappedAssets: Vec<(MultiLocation, MultiAssets)> = vec![]; +} + +pub struct TestAssetTrap; + +impl DropAssets for TestAssetTrap { + fn drop_assets(origin: &MultiLocation, assets: Assets) -> Weight { + let mut t: Vec<(MultiLocation, MultiAssets)> = TrappedAssets::get(); + t.push((origin.clone(), assets.into())); + TrappedAssets::set(t); + 5 + } +} + +impl ClaimAssets for TestAssetTrap { + fn claim_assets(origin: &MultiLocation, ticket: &MultiLocation, what: &MultiAssets) -> bool { + let mut t: Vec<(MultiLocation, MultiAssets)> = TrappedAssets::get(); + if let (0, X1(GeneralIndex(i))) = (ticket.parents, &ticket.interior) { + if let Some((l, a)) = t.get(*i as usize) { + if l == origin && a == what { + t.swap_remove(*i as usize); + TrappedAssets::set(t); + return true + } + } + } + false + } +} + pub struct TestConfig; impl Config for TestConfig { type Call = TestCall; @@ -282,4 +314,6 @@ impl Config for TestConfig { type Weigher = FixedWeightBounds; type Trader = FixedRateOfFungible; type ResponseHandler = TestResponseHandler; + type AssetTrap = TestAssetTrap; + type AssetClaims = TestAssetTrap; } diff --git a/polkadot/xcm/xcm-builder/src/tests.rs b/polkadot/xcm/xcm-builder/src/tests.rs index 22f72041c3..c7913c1281 100644 --- a/polkadot/xcm/xcm-builder/src/tests.rs +++ b/polkadot/xcm/xcm-builder/src/tests.rs @@ -58,7 +58,7 @@ fn take_weight_credit_barrier_should_work() { Xcm::<()>(vec![TransferAsset { assets: (Parent, 100).into(), beneficiary: Here.into() }]); let mut weight_credit = 10; let r = TakeWeightCredit::should_execute( - &Some(Parent.into()), + &Parent.into(), true, &mut message, 10, @@ -68,7 +68,7 @@ fn take_weight_credit_barrier_should_work() { assert_eq!(weight_credit, 0); let r = TakeWeightCredit::should_execute( - &Some(Parent.into()), + &Parent.into(), true, &mut message, 10, @@ -86,7 +86,7 @@ fn allow_unpaid_should_work() { AllowUnpaidFrom::set(vec![Parent.into()]); let r = AllowUnpaidExecutionFrom::>::should_execute( - &Some(Parachain(1).into()), + &Parachain(1).into(), true, &mut message, 10, @@ -95,7 +95,7 @@ fn allow_unpaid_should_work() { assert_eq!(r, Err(())); let r = AllowUnpaidExecutionFrom::>::should_execute( - &Some(Parent.into()), + &Parent.into(), true, &mut message, 10, @@ -112,7 +112,7 @@ fn allow_paid_should_work() { Xcm::<()>(vec![TransferAsset { assets: (Parent, 100).into(), beneficiary: Here.into() }]); let r = AllowTopLevelPaidExecutionFrom::>::should_execute( - &Some(Parachain(1).into()), + &Parachain(1).into(), true, &mut message, 10, @@ -128,7 +128,7 @@ fn allow_paid_should_work() { ]); let r = AllowTopLevelPaidExecutionFrom::>::should_execute( - &Some(Parent.into()), + &Parent.into(), true, &mut underpaying_message, 30, @@ -144,7 +144,7 @@ fn allow_paid_should_work() { ]); let r = AllowTopLevelPaidExecutionFrom::>::should_execute( - &Some(Parachain(1).into()), + &Parachain(1).into(), true, &mut paying_message, 30, @@ -153,7 +153,7 @@ fn allow_paid_should_work() { assert_eq!(r, Err(())); let r = AllowTopLevelPaidExecutionFrom::>::should_execute( - &Some(Parent.into()), + &Parent.into(), true, &mut paying_message, 30, @@ -202,6 +202,119 @@ fn transfer_should_work() { assert_eq!(sent_xcm(), vec![]); } +#[test] +fn basic_asset_trap_should_work() { + // we'll let them have message execution for free. + AllowUnpaidFrom::set(vec![X1(Parachain(1)).into(), X1(Parachain(2)).into()]); + + // Child parachain #1 owns 1000 tokens held by us in reserve. + add_asset(1001, (Here, 1000)); + // They want to transfer 100 of them to their sibling parachain #2 but have a problem + let r = XcmExecutor::::execute_xcm( + Parachain(1).into(), + Xcm(vec![ + WithdrawAsset((Here, 100).into()), + DepositAsset { + assets: Wild(All), + max_assets: 0, //< Whoops! + beneficiary: AccountIndex64 { index: 3, network: Any }.into(), + }, + ]), + 20, + ); + assert_eq!(r, Outcome::Complete(25)); + assert_eq!(assets(1001), vec![(Here, 900).into()]); + assert_eq!(assets(3), vec![]); + + // Incorrect ticket doesn't work. + let old_trapped_assets = TrappedAssets::get(); + let r = XcmExecutor::::execute_xcm( + Parachain(1).into(), + Xcm(vec![ + ClaimAsset { assets: (Here, 100).into(), ticket: GeneralIndex(1).into() }, + DepositAsset { + assets: Wild(All), + max_assets: 1, + beneficiary: AccountIndex64 { index: 3, network: Any }.into(), + }, + ]), + 20, + ); + assert_eq!(r, Outcome::Incomplete(10, XcmError::UnknownClaim)); + assert_eq!(assets(1001), vec![(Here, 900).into()]); + assert_eq!(assets(3), vec![]); + assert_eq!(old_trapped_assets, TrappedAssets::get()); + + // Incorrect origin doesn't work. + let old_trapped_assets = TrappedAssets::get(); + let r = XcmExecutor::::execute_xcm( + Parachain(2).into(), + Xcm(vec![ + ClaimAsset { assets: (Here, 100).into(), ticket: GeneralIndex(0).into() }, + DepositAsset { + assets: Wild(All), + max_assets: 1, + beneficiary: AccountIndex64 { index: 3, network: Any }.into(), + }, + ]), + 20, + ); + assert_eq!(r, Outcome::Incomplete(10, XcmError::UnknownClaim)); + assert_eq!(assets(1001), vec![(Here, 900).into()]); + assert_eq!(assets(3), vec![]); + assert_eq!(old_trapped_assets, TrappedAssets::get()); + + // Incorrect assets doesn't work. + let old_trapped_assets = TrappedAssets::get(); + let r = XcmExecutor::::execute_xcm( + Parachain(1).into(), + Xcm(vec![ + ClaimAsset { assets: (Here, 101).into(), ticket: GeneralIndex(0).into() }, + DepositAsset { + assets: Wild(All), + max_assets: 1, + beneficiary: AccountIndex64 { index: 3, network: Any }.into(), + }, + ]), + 20, + ); + assert_eq!(r, Outcome::Incomplete(10, XcmError::UnknownClaim)); + assert_eq!(assets(1001), vec![(Here, 900).into()]); + assert_eq!(assets(3), vec![]); + assert_eq!(old_trapped_assets, TrappedAssets::get()); + + let r = XcmExecutor::::execute_xcm( + Parachain(1).into(), + Xcm(vec![ + ClaimAsset { assets: (Here, 100).into(), ticket: GeneralIndex(0).into() }, + DepositAsset { + assets: Wild(All), + max_assets: 1, + beneficiary: AccountIndex64 { index: 3, network: Any }.into(), + }, + ]), + 20, + ); + assert_eq!(r, Outcome::Complete(20)); + assert_eq!(assets(1001), vec![(Here, 900).into()]); + assert_eq!(assets(3), vec![(Here, 100).into()]); + + // Same again doesn't work :-) + let r = XcmExecutor::::execute_xcm( + Parachain(1).into(), + Xcm(vec![ + ClaimAsset { assets: (Here, 100).into(), ticket: GeneralIndex(0).into() }, + DepositAsset { + assets: Wild(All), + max_assets: 1, + beneficiary: AccountIndex64 { index: 3, network: Any }.into(), + }, + ]), + 20, + ); + assert_eq!(r, Outcome::Incomplete(10, XcmError::UnknownClaim)); +} + #[test] fn errors_should_return_unused_weight() { // we'll let them have message execution for free. diff --git a/polkadot/xcm/xcm-builder/tests/mock/mod.rs b/polkadot/xcm/xcm-builder/tests/mock/mod.rs index ed64ce9470..e28352485f 100644 --- a/polkadot/xcm/xcm-builder/tests/mock/mod.rs +++ b/polkadot/xcm/xcm-builder/tests/mock/mod.rs @@ -166,7 +166,9 @@ impl xcm_executor::Config for XcmConfig { type Barrier = Barrier; type Weigher = FixedWeightBounds; type Trader = FixedRateOfFungible; - type ResponseHandler = (); + type ResponseHandler = XcmPallet; + type AssetTrap = XcmPallet; + type AssetClaims = XcmPallet; } pub type LocalOriginToLocation = SignedToAccountId32; diff --git a/polkadot/xcm/xcm-executor/src/assets.rs b/polkadot/xcm/xcm-executor/src/assets.rs index 8b87cc7ca5..c7049a3a64 100644 --- a/polkadot/xcm/xcm-executor/src/assets.rs +++ b/polkadot/xcm/xcm-executor/src/assets.rs @@ -94,6 +94,11 @@ impl Assets { self.fungible.len() + self.non_fungible.len() } + /// Returns `true` if `self` contains no assets. + pub fn is_empty(&self) -> bool { + self.fungible.is_empty() && self.non_fungible.is_empty() + } + /// A borrowing iterator over the fungible assets. pub fn fungible_assets_iter<'a>(&'a self) -> impl Iterator + 'a { self.fungible diff --git a/polkadot/xcm/xcm-executor/src/config.rs b/polkadot/xcm/xcm-executor/src/config.rs index abb241c492..153a9de9a7 100644 --- a/polkadot/xcm/xcm-executor/src/config.rs +++ b/polkadot/xcm/xcm-executor/src/config.rs @@ -15,8 +15,8 @@ // along with Polkadot. If not, see . use crate::traits::{ - ConvertOrigin, FilterAssetLocation, InvertLocation, OnResponse, ShouldExecute, TransactAsset, - WeightBounds, WeightTrader, + ClaimAssets, ConvertOrigin, DropAssets, FilterAssetLocation, InvertLocation, OnResponse, + ShouldExecute, TransactAsset, WeightBounds, WeightTrader, }; use frame_support::{ dispatch::{Dispatchable, Parameter}, @@ -58,4 +58,11 @@ pub trait Config { /// What to do when a response of a query is found. type ResponseHandler: OnResponse; + + /// The general asset trap - handler for when assets are left in the Holding Register at the + /// end of execution. + type AssetTrap: DropAssets; + + /// The handler for when there is an instruction to claim assets. + type AssetClaims: ClaimAssets; } diff --git a/polkadot/xcm/xcm-executor/src/lib.rs b/polkadot/xcm/xcm-executor/src/lib.rs index 5484121d5e..01a8e96b51 100644 --- a/polkadot/xcm/xcm-executor/src/lib.rs +++ b/polkadot/xcm/xcm-executor/src/lib.rs @@ -31,8 +31,8 @@ use xcm::latest::{ pub mod traits; use traits::{ - ConvertOrigin, FilterAssetLocation, InvertLocation, OnResponse, ShouldExecute, TransactAsset, - WeightBounds, WeightTrader, + ClaimAssets, ConvertOrigin, DropAssets, FilterAssetLocation, InvertLocation, OnResponse, + ShouldExecute, TransactAsset, WeightBounds, WeightTrader, }; mod assets; @@ -86,7 +86,6 @@ impl ExecuteXcm for XcmExecutor { if xcm_weight > weight_limit { return Outcome::Error(XcmError::WeightLimitReached(xcm_weight)) } - let origin = Some(origin); if let Err(_) = Config::Barrier::should_execute( &origin, @@ -98,7 +97,7 @@ impl ExecuteXcm for XcmExecutor { return Outcome::Error(XcmError::Barrier) } - let mut vm = Self::new(origin); + let mut vm = Self::new(origin.clone()); while !message.0.is_empty() { let result = vm.execute(message); @@ -116,9 +115,12 @@ impl ExecuteXcm for XcmExecutor { vm.refund_surplus(); drop(vm.trader); - // TODO #2841: Do something with holding? (Fail-safe AssetTrap?) + let mut weight_used = xcm_weight.saturating_sub(vm.total_surplus); + + if !vm.holding.is_empty() { + weight_used.saturating_accrue(Config::AssetTrap::drop_assets(&origin, vm.holding)); + }; - let weight_used = xcm_weight.saturating_sub(vm.total_surplus); match vm.error { None => Outcome::Complete(weight_used), // TODO: #2841 #REALWEIGHT We should deduct the cost of any instructions following @@ -129,10 +131,10 @@ impl ExecuteXcm for XcmExecutor { } impl XcmExecutor { - fn new(origin: Option) -> Self { + fn new(origin: MultiLocation) -> Self { Self { holding: Assets::new(), - origin, + origin: Some(origin), trader: Config::Trader::new(), error: None, total_surplus: 0, @@ -183,7 +185,7 @@ impl XcmExecutor { /// Drop the registered error handler and refund its weight. fn drop_error_handler(&mut self) { self.error_handler = Xcm::(vec![]); - self.total_surplus = self.total_surplus.saturating_add(self.error_handler_weight); + self.total_surplus.saturating_accrue(self.error_handler_weight); self.error_handler_weight = 0; } @@ -199,7 +201,7 @@ impl XcmExecutor { fn refund_surplus(&mut self) { let current_surplus = self.total_surplus.saturating_sub(self.total_refunded); if current_surplus > 0 { - self.total_refunded = self.total_refunded.saturating_add(current_surplus); + self.total_refunded.saturating_accrue(current_surplus); if let Some(w) = self.trader.refund_weight(current_surplus) { self.holding.subsume(w); } @@ -300,7 +302,7 @@ impl XcmExecutor { // reported back to the caller and this ensures that they account for the total // weight consumed correctly (potentially allowing them to do more operations in a // block than they otherwise would). - self.total_surplus = self.total_surplus.saturating_add(surplus); + self.total_surplus.saturating_accrue(surplus); Ok(()) }, QueryResponse { query_id, response, max_weight } => { @@ -390,14 +392,14 @@ impl XcmExecutor { }, SetErrorHandler(mut handler) => { let handler_weight = Config::Weigher::weight(&mut handler)?; - self.total_surplus = self.total_surplus.saturating_add(self.error_handler_weight); + self.total_surplus.saturating_accrue(self.error_handler_weight); self.error_handler = handler; self.error_handler_weight = handler_weight; Ok(()) }, SetAppendix(mut appendix) => { let appendix_weight = Config::Weigher::weight(&mut appendix)?; - self.total_surplus = self.total_surplus.saturating_add(self.appendix_weight); + self.total_surplus.saturating_accrue(self.appendix_weight); self.appendix = appendix; self.appendix_weight = appendix_weight; Ok(()) @@ -406,6 +408,16 @@ impl XcmExecutor { self.error = None; Ok(()) }, + ClaimAsset { assets, ticket } => { + let origin = self.origin.as_ref().ok_or(XcmError::BadOrigin)?; + let ok = Config::AssetClaims::claim_assets(origin, &ticket, &assets); + ensure!(ok, XcmError::UnknownClaim); + for asset in assets.drain().into_iter() { + self.holding.subsume(asset); + } + Ok(()) + }, + Trap(code) => Err(XcmError::Trap(code)), ExchangeAsset { .. } => Err(XcmError::Unimplemented), HrmpNewChannelOpenRequest { .. } => Err(XcmError::Unimplemented), HrmpChannelAccepted { .. } => Err(XcmError::Unimplemented), diff --git a/polkadot/xcm/xcm-executor/src/traits/drop_assets.rs b/polkadot/xcm/xcm-executor/src/traits/drop_assets.rs new file mode 100644 index 0000000000..5f82c5feb7 --- /dev/null +++ b/polkadot/xcm/xcm-executor/src/traits/drop_assets.rs @@ -0,0 +1,79 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +use crate::Assets; +use core::marker::PhantomData; +use frame_support::{traits::Contains, weights::Weight}; +use xcm::latest::{MultiAssets, MultiLocation}; + +/// Define a handler for when some non-empty `Assets` value should be dropped. +pub trait DropAssets { + /// Handler for receiving dropped assets. Returns the weight consumed by this operation. + fn drop_assets(origin: &MultiLocation, assets: Assets) -> Weight; +} +impl DropAssets for () { + fn drop_assets(_origin: &MultiLocation, _assets: Assets) -> Weight { + 0 + } +} + +/// Morph a given `DropAssets` implementation into one which can filter based on assets. This can +/// be used to ensure that `Assets` values which hold no value are ignored. +pub struct FilterAssets(PhantomData<(D, A)>); + +impl> DropAssets for FilterAssets { + fn drop_assets(origin: &MultiLocation, assets: Assets) -> Weight { + if A::contains(&assets) { + D::drop_assets(origin, assets) + } else { + 0 + } + } +} + +/// Morph a given `DropAssets` implementation into one which can filter based on origin. This can +/// be used to ban origins which don't have proper protections/policies against misuse of the +/// asset trap facility don't get to use it. +pub struct FilterOrigin(PhantomData<(D, O)>); + +impl> DropAssets for FilterOrigin { + fn drop_assets(origin: &MultiLocation, assets: Assets) -> Weight { + if O::contains(origin) { + D::drop_assets(origin, assets) + } else { + 0 + } + } +} + +/// Define any handlers for the `AssetClaim` instruction. +pub trait ClaimAssets { + /// Claim any assets available to `origin` and return them in a single `Assets` value, together + /// with the weight used by this operation. + fn claim_assets(origin: &MultiLocation, ticket: &MultiLocation, what: &MultiAssets) -> bool; +} + +#[impl_trait_for_tuples::impl_for_tuples(30)] +impl ClaimAssets for Tuple { + fn claim_assets(origin: &MultiLocation, ticket: &MultiLocation, what: &MultiAssets) -> bool { + for_tuples!( #( + if Tuple::claim_assets(origin, ticket, what) { + return true; + } + )* ); + false + } +} diff --git a/polkadot/xcm/xcm-executor/src/traits/mod.rs b/polkadot/xcm/xcm-executor/src/traits/mod.rs index 0a304590b6..94ef8bd4bd 100644 --- a/polkadot/xcm/xcm-executor/src/traits/mod.rs +++ b/polkadot/xcm/xcm-executor/src/traits/mod.rs @@ -18,6 +18,8 @@ mod conversion; pub use conversion::{Convert, ConvertOrigin, Decoded, Encoded, Identity, InvertLocation, JustTry}; +mod drop_assets; +pub use drop_assets::{ClaimAssets, DropAssets}; mod filter_asset_location; pub use filter_asset_location::FilterAssetLocation; mod matches_fungible; diff --git a/polkadot/xcm/xcm-executor/src/traits/should_execute.rs b/polkadot/xcm/xcm-executor/src/traits/should_execute.rs index 53600779fc..08c7433451 100644 --- a/polkadot/xcm/xcm-executor/src/traits/should_execute.rs +++ b/polkadot/xcm/xcm-executor/src/traits/should_execute.rs @@ -34,7 +34,7 @@ pub trait ShouldExecute { /// message may utilize in its execution. Typically non-zero only because of prior fee /// payment, but could in principle be due to other factors. fn should_execute( - origin: &Option, + origin: &MultiLocation, top_level: bool, message: &mut Xcm, max_weight: Weight, @@ -45,7 +45,7 @@ pub trait ShouldExecute { #[impl_trait_for_tuples::impl_for_tuples(30)] impl ShouldExecute for Tuple { fn should_execute( - origin: &Option, + origin: &MultiLocation, top_level: bool, message: &mut Xcm, max_weight: Weight, diff --git a/polkadot/xcm/xcm-simulator/example/src/parachain.rs b/polkadot/xcm/xcm-simulator/example/src/parachain.rs index 8f714a30cb..faa2d8b3fc 100644 --- a/polkadot/xcm/xcm-simulator/example/src/parachain.rs +++ b/polkadot/xcm/xcm-simulator/example/src/parachain.rs @@ -143,6 +143,8 @@ impl Config for XcmConfig { type Weigher = FixedWeightBounds; type Trader = FixedRateOfFungible; type ResponseHandler = (); + type AssetTrap = (); + type AssetClaims = (); } #[frame_support::pallet] diff --git a/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs b/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs index fa2218786d..f3173b25e0 100644 --- a/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs +++ b/polkadot/xcm/xcm-simulator/example/src/relay_chain.rs @@ -133,6 +133,8 @@ impl Config for XcmConfig { type Weigher = FixedWeightBounds; type Trader = FixedRateOfFungible; type ResponseHandler = (); + type AssetTrap = (); + type AssetClaims = (); } pub type LocalOriginToLocation = SignedToAccountId32;