diff --git a/authority/src/lib.rs b/authority/src/lib.rs index d5b9f42e0..3634907df 100644 --- a/authority/src/lib.rs +++ b/authority/src/lib.rs @@ -267,7 +267,7 @@ pub mod module { #[pallet::weight(T::WeightInfo::fast_track_scheduled_dispatch())] pub fn fast_track_scheduled_dispatch( origin: OriginFor, - initial_origin: T::PalletsOrigin, + initial_origin: Box, task_id: ScheduleTaskIndex, when: DispatchTime, ) -> DispatchResult { @@ -285,7 +285,7 @@ pub mod module { T::Scheduler::reschedule_named((&initial_origin, task_id).encode(), when) .map_err(|_| Error::::FailedToFastTrack)?; - Self::deposit_event(Event::FastTracked(initial_origin, task_id, dispatch_at)); + Self::deposit_event(Event::FastTracked(*initial_origin, task_id, dispatch_at)); Ok(()) } @@ -293,7 +293,7 @@ pub mod module { #[pallet::weight(T::WeightInfo::delay_scheduled_dispatch())] pub fn delay_scheduled_dispatch( origin: OriginFor, - initial_origin: T::PalletsOrigin, + initial_origin: Box, task_id: ScheduleTaskIndex, additional_delay: T::BlockNumber, ) -> DispatchResult { @@ -308,7 +308,7 @@ pub mod module { let now = frame_system::Pallet::::block_number(); let dispatch_at = now.saturating_add(additional_delay); - Self::deposit_event(Event::Delayed(initial_origin, task_id, dispatch_at)); + Self::deposit_event(Event::Delayed(*initial_origin, task_id, dispatch_at)); Ok(()) } @@ -316,13 +316,13 @@ pub mod module { #[pallet::weight(T::WeightInfo::cancel_scheduled_dispatch())] pub fn cancel_scheduled_dispatch( origin: OriginFor, - initial_origin: T::PalletsOrigin, + initial_origin: Box, task_id: ScheduleTaskIndex, ) -> DispatchResult { T::AuthorityConfig::check_cancel_schedule(origin, &initial_origin)?; T::Scheduler::cancel_named((&initial_origin, task_id).encode()).map_err(|_| Error::::FailedToCancel)?; - Self::deposit_event(Event::Cancelled(initial_origin, task_id)); + Self::deposit_event(Event::Cancelled(*initial_origin, task_id)); Ok(()) } } diff --git a/authority/src/tests.rs b/authority/src/tests.rs index eb23db55e..593919af9 100644 --- a/authority/src/tests.rs +++ b/authority/src/tests.rs @@ -207,7 +207,7 @@ fn fast_track_scheduled_dispatch_work() { let pallets_origin = schedule_origin.caller().clone(); assert_ok!(Authority::fast_track_scheduled_dispatch( Origin::root(), - pallets_origin, + Box::new(pallets_origin), 0, DispatchTime::At(4), )); @@ -234,7 +234,7 @@ fn fast_track_scheduled_dispatch_work() { assert_ok!(Authority::fast_track_scheduled_dispatch( Origin::root(), - frame_system::RawOrigin::Root.into(), + Box::new(frame_system::RawOrigin::Root.into()), 1, DispatchTime::At(4), )); @@ -284,7 +284,7 @@ fn delay_scheduled_dispatch_work() { let pallets_origin = schedule_origin.caller().clone(); assert_ok!(Authority::delay_scheduled_dispatch( Origin::root(), - pallets_origin, + Box::new(pallets_origin), 0, 4, )); @@ -311,7 +311,7 @@ fn delay_scheduled_dispatch_work() { assert_ok!(Authority::delay_scheduled_dispatch( Origin::root(), - frame_system::RawOrigin::Root.into(), + Box::new(frame_system::RawOrigin::Root.into()), 1, 4, )); @@ -358,7 +358,11 @@ fn cancel_scheduled_dispatch_work() { }; let pallets_origin = schedule_origin.caller().clone(); - assert_ok!(Authority::cancel_scheduled_dispatch(Origin::root(), pallets_origin, 0)); + assert_ok!(Authority::cancel_scheduled_dispatch( + Origin::root(), + Box::new(pallets_origin), + 0 + )); System::assert_last_event(mock::Event::Authority(Event::Cancelled( OriginCaller::Authority(DelayedOrigin { delay: 1, @@ -381,7 +385,7 @@ fn cancel_scheduled_dispatch_work() { assert_ok!(Authority::cancel_scheduled_dispatch( Origin::root(), - frame_system::RawOrigin::Root.into(), + Box::new(frame_system::RawOrigin::Root.into()), 1 )); System::assert_last_event(mock::Event::Authority(Event::Cancelled( @@ -390,3 +394,13 @@ fn cancel_scheduled_dispatch_work() { ))); }); } + +#[test] +fn call_size_limit() { + assert!( + core::mem::size_of::>() <= 200, + "size of Call is more than 200 bytes: some calls have too big arguments, use Box to \ + reduce the size of Call. + If the limit is too strong, maybe consider increasing the limit", + ); +} diff --git a/xcm/Cargo.toml b/xcm/Cargo.toml index 0ecffa30f..55b728c10 100644 --- a/xcm/Cargo.toml +++ b/xcm/Cargo.toml @@ -9,6 +9,7 @@ edition = "2018" [dependencies] codec = { package = "parity-scale-codec", version = "2.2.0", default-features = false } +sp-std = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.9", default-features = false } frame-support = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.9", default-features = false } frame-system = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.9", default-features = false } @@ -20,6 +21,7 @@ pallet-xcm = { git = "https://github.com/paritytech/polkadot", branch = "release default = ["std"] std = [ "codec/std", + "sp-std/std", "frame-support/std", "frame-system/std", "xcm/std", diff --git a/xcm/src/lib.rs b/xcm/src/lib.rs index 9ba8fef1e..72f80bd15 100644 --- a/xcm/src/lib.rs +++ b/xcm/src/lib.rs @@ -5,6 +5,7 @@ use frame_support::{pallet_prelude::*, traits::EnsureOrigin}; use frame_system::pallet_prelude::*; +use sp_std::boxed::Box; use xcm::v0::prelude::*; @@ -48,15 +49,19 @@ pub mod module { impl Pallet { /// Send an XCM message as parachain sovereign. #[pallet::weight(100_000_000)] - pub fn send_as_sovereign(origin: OriginFor, dest: MultiLocation, message: Xcm<()>) -> DispatchResult { + pub fn send_as_sovereign( + origin: OriginFor, + dest: Box, + message: Box>, + ) -> DispatchResult { let _ = T::SovereignOrigin::ensure_origin(origin)?; - pallet_xcm::Pallet::::send_xcm(MultiLocation::Null, dest.clone(), message.clone()).map_err( + pallet_xcm::Pallet::::send_xcm(MultiLocation::Null, *dest.clone(), *message.clone()).map_err( |e| match e { XcmError::CannotReachDestination(..) => Error::::Unreachable, _ => Error::::SendFailure, }, )?; - Self::deposit_event(Event::Sent(MultiLocation::Null, dest, message)); + Self::deposit_event(Event::Sent(MultiLocation::Null, *dest, *message)); Ok(()) } } diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 9b6308bfd..983fa31df 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -148,11 +148,11 @@ pub mod module { origin: OriginFor, currency_id: T::CurrencyId, amount: T::Balance, - dest: MultiLocation, + dest: Box, dest_weight: Weight, ) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_transfer(who, currency_id, amount, dest, dest_weight) + Self::do_transfer(who, currency_id, amount, *dest, dest_weight) } /// Transfer `MultiAsset`. @@ -171,12 +171,12 @@ pub mod module { #[transactional] pub fn transfer_multiasset( origin: OriginFor, - asset: MultiAsset, - dest: MultiLocation, + asset: Box, + dest: Box, dest_weight: Weight, ) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_transfer_multiasset(who, asset, dest, dest_weight, true) + Self::do_transfer_multiasset(who, *asset, *dest, dest_weight, true) } } diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index bd0b3caf9..693d35b09 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -47,14 +47,16 @@ fn send_relay_chain_asset_to_relay_chain() { Some(ALICE).into(), CurrencyId::R, 500, - ( - Parent, - Junction::AccountId32 { - network: NetworkId::Kusama, - id: BOB.into(), - }, - ) - .into(), + Box::new( + ( + Parent, + Junction::AccountId32 { + network: NetworkId::Kusama, + id: BOB.into(), + }, + ) + .into() + ), 30, )); assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), 500); @@ -77,15 +79,17 @@ fn cannot_lost_fund_on_send_failed() { Some(ALICE).into(), CurrencyId::A, 500, - ( - Parent, - Parachain(100), - Junction::AccountId32 { - network: NetworkId::Kusama, - id: BOB.into(), - }, - ) - .into(), + Box::new( + ( + Parent, + Parachain(100), + Junction::AccountId32 { + network: NetworkId::Kusama, + id: BOB.into(), + }, + ) + .into() + ), 30, ), Error::::XcmExecutionFailed @@ -108,15 +112,17 @@ fn send_relay_chain_asset_to_sibling() { Some(ALICE).into(), CurrencyId::R, 500, - ( - Parent, - Parachain(2), - Junction::AccountId32 { - network: NetworkId::Any, - id: BOB.into(), - }, - ) - .into(), + Box::new( + ( + Parent, + Parachain(2), + Junction::AccountId32 { + network: NetworkId::Any, + id: BOB.into(), + }, + ) + .into() + ), 30, )); assert_eq!(ParaTokens::free_balance(CurrencyId::R, &ALICE), 500); @@ -149,15 +155,17 @@ fn send_sibling_asset_to_reserve_sibling() { Some(ALICE).into(), CurrencyId::B, 500, - ( - Parent, - Parachain(2), - Junction::AccountId32 { - network: NetworkId::Any, - id: BOB.into(), - }, - ) - .into(), + Box::new( + ( + Parent, + Parachain(2), + Junction::AccountId32 { + network: NetworkId::Any, + id: BOB.into(), + }, + ) + .into() + ), 30, )); @@ -187,15 +195,17 @@ fn send_sibling_asset_to_non_reserve_sibling() { Some(ALICE).into(), CurrencyId::B, 500, - ( - Parent, - Parachain(3), - Junction::AccountId32 { - network: NetworkId::Any, - id: BOB.into(), - }, - ) - .into(), + Box::new( + ( + Parent, + Parachain(3), + Junction::AccountId32 { + network: NetworkId::Any, + id: BOB.into(), + }, + ) + .into() + ), 30 )); assert_eq!(ParaTokens::free_balance(CurrencyId::B, &ALICE), 500); @@ -223,15 +233,17 @@ fn send_self_parachain_asset_to_sibling() { Some(ALICE).into(), CurrencyId::A, 500, - ( - Parent, - Parachain(2), - Junction::AccountId32 { - network: NetworkId::Any, - id: BOB.into(), - }, - ) - .into(), + Box::new( + ( + Parent, + Parachain(2), + Junction::AccountId32 { + network: NetworkId::Any, + id: BOB.into(), + }, + ) + .into() + ), 30, )); @@ -252,19 +264,21 @@ fn transfer_no_reserve_assets_fails() { assert_noop!( ParaXTokens::transfer_multiasset( Some(ALICE).into(), - MultiAsset::ConcreteFungible { + Box::new(MultiAsset::ConcreteFungible { id: GeneralKey("B".into()).into(), amount: 100 - }, - ( - Parent, - Parachain(2), - Junction::AccountId32 { - network: NetworkId::Any, - id: BOB.into() - } - ) - .into(), + }), + Box::new( + ( + Parent, + Parachain(2), + Junction::AccountId32 { + network: NetworkId::Any, + id: BOB.into() + } + ) + .into() + ), 50, ), Error::::AssetHasNoReserve @@ -280,19 +294,21 @@ fn transfer_to_self_chain_fails() { assert_noop!( ParaXTokens::transfer_multiasset( Some(ALICE).into(), - MultiAsset::ConcreteFungible { + Box::new(MultiAsset::ConcreteFungible { id: (Parent, Parachain(1), GeneralKey("A".into())).into(), amount: 100 - }, - ( - Parent, - Parachain(1), - Junction::AccountId32 { - network: NetworkId::Any, - id: BOB.into() - } - ) - .into(), + }), + Box::new( + ( + Parent, + Parachain(1), + Junction::AccountId32 { + network: NetworkId::Any, + id: BOB.into() + } + ) + .into() + ), 50, ), Error::::NotCrossChainTransfer @@ -308,15 +324,17 @@ fn transfer_to_invalid_dest_fails() { assert_noop!( ParaXTokens::transfer_multiasset( Some(ALICE).into(), - MultiAsset::ConcreteFungible { + Box::new(MultiAsset::ConcreteFungible { id: (Parent, Parachain(1), GeneralKey("A".into())).into(), amount: 100, - }, - (Junction::AccountId32 { - network: NetworkId::Any, - id: BOB.into() - }) - .into(), + }), + Box::new( + (Junction::AccountId32 { + network: NetworkId::Any, + id: BOB.into() + }) + .into() + ), 50, ), Error::::InvalidDest @@ -338,8 +356,8 @@ fn send_as_sovereign() { let call = relay::Call::System(frame_system::Call::::remark_with_event(vec![1, 1, 1])); assert_ok!(para::OrmlXcm::send_as_sovereign( para::Origin::root(), - Junction::Parent.into(), - WithdrawAsset { + Box::new(Junction::Parent.into()), + Box::new(WithdrawAsset { assets: vec![MultiAsset::ConcreteFungible { id: MultiLocation::Null, amount: 1_000_000_000_000 @@ -355,7 +373,7 @@ fn send_as_sovereign() { call: call.encode().into(), }], }] - } + }) )); }); @@ -384,8 +402,8 @@ fn send_as_sovereign_fails_if_bad_origin() { assert_err!( para::OrmlXcm::send_as_sovereign( para::Origin::signed(ALICE), - Junction::Parent.into(), - WithdrawAsset { + Box::new(Junction::Parent.into()), + Box::new(WithdrawAsset { assets: vec![MultiAsset::ConcreteFungible { id: MultiLocation::Null, amount: 1_000_000_000_000 @@ -401,9 +419,27 @@ fn send_as_sovereign_fails_if_bad_origin() { call: call.encode().into(), }], }] - } + }) ), DispatchError::BadOrigin, ); }); } + +#[test] +fn call_size_limit() { + // Ensures Call enum doesn't allocate more than 200 bytes in runtime + assert!( + core::mem::size_of::>() <= 200, + "size of Call is more than 200 bytes: some calls have too big arguments, use Box to \ + reduce the size of Call. + If the limit is too strong, maybe consider increasing the limit", + ); + + assert!( + core::mem::size_of::>() <= 200, + "size of Call is more than 200 bytes: some calls have too big arguments, use Box to \ + reduce the size of Call. + If the limit is too strong, maybe consider increasing the limit", + ); +}