Skip to content

Limit Call size to 200 bytes #598

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

Merged
merged 13 commits into from
Sep 2, 2021
Merged

Limit Call size to 200 bytes #598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
07d5b93
xtokens reduce Call size
ferrell-code Sep 1, 2021
d388f59
fmt
ferrell-code Sep 1, 2021
819f313
limit to 200
ferrell-code Sep 1, 2021
6f5da03
xcm and authority
ferrell-code Sep 2, 2021
36248a0
fmt
ferrell-code Sep 2, 2021
ca45617
add sp_std Box
ferrell-code Sep 2, 2021
f981733
xtoken tests
ferrell-code Sep 2, 2021
4613a9a
add xcm size limit test
ferrell-code Sep 2, 2021
1832f55
authority
ferrell-code Sep 2, 2021
951cb9b
Revert "add xcm size limit test"
ferrell-code Sep 2, 2021
4e1d4ef
Revert "Revert "add xcm size limit test""
ferrell-code Sep 2, 2021
e555cd6
Revert "authority"
ferrell-code Sep 2, 2021
33bc599
authority lower call size
ferrell-code Sep 2, 2021
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
12 changes: 6 additions & 6 deletions authority/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ pub mod module {
#[pallet::weight(T::WeightInfo::fast_track_scheduled_dispatch())]
pub fn fast_track_scheduled_dispatch(
origin: OriginFor<T>,
initial_origin: T::PalletsOrigin,
initial_origin: Box<T::PalletsOrigin>,
Copy link
Contributor Author

@ferrell-code ferrell-code Sep 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

So boxing T::PalletsOrigin decreases the call size for Authority from 608 bytes to 24 bytes in karura. Not entirely sure why but numbers don't lie

task_id: ScheduleTaskIndex,
when: DispatchTime<T::BlockNumber>,
) -> DispatchResult {
Expand All @@ -285,15 +285,15 @@ pub mod module {
T::Scheduler::reschedule_named((&initial_origin, task_id).encode(), when)
.map_err(|_| Error::<T>::FailedToFastTrack)?;

Self::deposit_event(Event::FastTracked(initial_origin, task_id, dispatch_at));
Self::deposit_event(Event::FastTracked(*initial_origin, task_id, dispatch_at));
Ok(())
}

/// Delay a scheduled dispatchable.
#[pallet::weight(T::WeightInfo::delay_scheduled_dispatch())]
pub fn delay_scheduled_dispatch(
origin: OriginFor<T>,
initial_origin: T::PalletsOrigin,
initial_origin: Box<T::PalletsOrigin>,
task_id: ScheduleTaskIndex,
additional_delay: T::BlockNumber,
) -> DispatchResult {
Expand All @@ -308,21 +308,21 @@ pub mod module {
let now = frame_system::Pallet::<T>::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(())
}

/// Cancel a scheduled dispatchable.
#[pallet::weight(T::WeightInfo::cancel_scheduled_dispatch())]
pub fn cancel_scheduled_dispatch(
origin: OriginFor<T>,
initial_origin: T::PalletsOrigin,
initial_origin: Box<T::PalletsOrigin>,
task_id: ScheduleTaskIndex,
) -> DispatchResult {
T::AuthorityConfig::check_cancel_schedule(origin, &initial_origin)?;
T::Scheduler::cancel_named((&initial_origin, task_id).encode()).map_err(|_| Error::<T>::FailedToCancel)?;

Self::deposit_event(Event::Cancelled(initial_origin, task_id));
Self::deposit_event(Event::Cancelled(*initial_origin, task_id));
Ok(())
}
}
Expand Down
26 changes: 20 additions & 6 deletions authority/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
));
Expand All @@ -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),
));
Expand Down Expand Up @@ -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,
));
Expand All @@ -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,
));
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -390,3 +394,13 @@ fn cancel_scheduled_dispatch_work() {
)));
});
}

#[test]
fn call_size_limit() {
assert!(
core::mem::size_of::<authority::Call::<Runtime>>() <= 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",
);
Comment on lines +397 to +405
Copy link
Contributor Author

@ferrell-code ferrell-code Sep 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This tests passes, yet in karura runtime the Call size for orml_authority is printed as 608 bytes, this test when printed says call size of 56 bytes. That doesn't really make sense to me

Edit: Nvm, I think I figured it out it probably is because karura sets something different in the runtime than the mock 👍

}
2 changes: 2 additions & 0 deletions xcm/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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",
Expand Down
11 changes: 8 additions & 3 deletions xcm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;

Expand Down Expand Up @@ -48,15 +49,19 @@ pub mod module {
impl<T: Config> Pallet<T> {
/// Send an XCM message as parachain sovereign.
#[pallet::weight(100_000_000)]
pub fn send_as_sovereign(origin: OriginFor<T>, dest: MultiLocation, message: Xcm<()>) -> DispatchResult {
pub fn send_as_sovereign(
origin: OriginFor<T>,
dest: Box<MultiLocation>,
message: Box<Xcm<()>>,
) -> DispatchResult {
let _ = T::SovereignOrigin::ensure_origin(origin)?;
pallet_xcm::Pallet::<T>::send_xcm(MultiLocation::Null, dest.clone(), message.clone()).map_err(
pallet_xcm::Pallet::<T>::send_xcm(MultiLocation::Null, *dest.clone(), *message.clone()).map_err(
|e| match e {
XcmError::CannotReachDestination(..) => Error::<T>::Unreachable,
_ => Error::<T>::SendFailure,
},
)?;
Self::deposit_event(Event::Sent(MultiLocation::Null, dest, message));
Self::deposit_event(Event::Sent(MultiLocation::Null, *dest, *message));
Ok(())
}
}
Expand Down
10 changes: 5 additions & 5 deletions xtokens/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,11 @@ pub mod module {
origin: OriginFor<T>,
currency_id: T::CurrencyId,
amount: T::Balance,
dest: MultiLocation,
dest: Box<MultiLocation>,
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`.
Expand All @@ -171,12 +171,12 @@ pub mod module {
#[transactional]
pub fn transfer_multiasset(
origin: OriginFor<T>,
asset: MultiAsset,
dest: MultiLocation,
asset: Box<MultiAsset>,
dest: Box<MultiLocation>,
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)
}
}

Expand Down
Loading