Skip to content

Commit

Permalink
Add more events and prevent duplicative submission (#1078)
Browse files Browse the repository at this point in the history 
* Add more events and prevent duplicative submission

* Fix type

* Bench
  • Loading branch information
@AurevoirXavier
AurevoirXavier authored Mar 30, 2023
1 parent 64a89d3 commit 2694c09
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 87 deletions.
21 changes: 17 additions & 4 deletions pallet/account-migration/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,24 +160,36 @@ mod benchmarks {
_(RawOrigin::None, from, to, [0; 64]);
}

fn other_multisig_members(count: u32) -> Vec<AccountId32> {
(0..count).map(|i| [i as u8; 32].into()).collect()
fn other_multisig_members<const N: usize, A>(count: u32) -> Vec<A>
where
A: From<[u8; N]>,
{
(0..count).map(|i| [i as u8; N].into()).collect()
}

#[benchmark]
fn migrate_multisig(x: Linear<0, 99>, y: Linear<0, 99>) {
fn migrate_multisig(x: Linear<0, 99>, y: Linear<0, 99>, z: Linear<0, 99>) {
let from = AccountId32::from([0; 32]);
// Worst-case scenario:
//
// 100 size multisig.
let other_members = other_multisig_members(x);
let (_, multisig) = multisig_of(from.clone(), other_members.clone(), y as _);
let to = [0; 20].into();
let new_multisig_params = if z == 0 {
None
} else {
Some(MultisigParams {
address: [0; 20].into(),
members: other_multisig_members(z),
threshold: Default::default(),
})
};

preset_data::<T>(&multisig);

#[extrinsic_call]
_(RawOrigin::None, from, other_members, y as _, to, [0; 64]);
_(RawOrigin::None, from, other_members, y as _, to, [0; 64], new_multisig_params);
}

#[benchmark]
Expand All @@ -197,6 +209,7 @@ mod benchmarks {
100,
to,
[0; 64],
None,
)
.unwrap();

Expand Down
38 changes: 33 additions & 5 deletions pallet/account-migration/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ pub mod pallet {
pub enum Event {
/// An account has been migrated.
Migrated { from: AccountId32, to: AccountId20 },
/// An multisig account has been migrated.
/// A new multisig account params was noted/recorded on-chain.
NewMultisigParamsNoted { from: AccountId32, to: MultisigParams },
/// A multisig account has been migrated.
MultisigMigrated { from: AccountId32, detail: MultisigMigrationDetail },
}

Expand Down Expand Up @@ -220,14 +222,19 @@ pub mod pallet {
///
/// The `_signature` should be provided by `who`.
#[pallet::call_index(1)]
#[pallet::weight(<T as Config>::WeightInfo::migrate_multisig(others.len() as _, *threshold as _))]
#[pallet::weight(<T as Config>::WeightInfo::migrate_multisig(
others.len() as _,
*threshold as _,
new_multisig_params.as_ref().map(|p| p.members.len()).unwrap_or_default() as _
))]
pub fn migrate_multisig(
origin: OriginFor<T>,
submitter: AccountId32,
others: Vec<AccountId32>,
threshold: u16,
to: AccountId20,
_signature: Signature,
new_multisig_params: Option<MultisigParams>,
) -> DispatchResult {
ensure_none(origin)?;

Expand All @@ -248,9 +255,13 @@ pub mod pallet {
if threshold < 2 {
Self::migrate_inner(&from, &to)?;

Self::deposit_event(Event::MultisigMigrated { from, detail });
Self::deposit_event(Event::MultisigMigrated { from: from.clone(), detail });
} else {
<Multisigs<T>>::insert(from, detail);
<Multisigs<T>>::insert(&from, detail);
}

if let Some(to) = new_multisig_params {
Self::deposit_event(Event::NewMultisigParamsNoted { from, to });
}

Ok(())
Expand Down Expand Up @@ -314,11 +325,12 @@ pub mod pallet {

Self::pre_check_signature(from, to, signature)
},
Call::migrate_multisig { submitter, others, threshold, to, signature } => {
Call::migrate_multisig { submitter, others, threshold, to, signature, .. } => {
let (_, multisig) =
multisig_of(submitter.to_owned(), others.to_owned(), *threshold);

Self::pre_check_existing(&multisig, to)?;
Self::pre_check_duplicative(&multisig)?;

Self::pre_check_signature(submitter, to, signature)
},
Expand Down Expand Up @@ -369,6 +381,14 @@ pub mod pallet {
Ok(())
}

fn pre_check_duplicative(multisig: &AccountId32) -> Result<(), TransactionValidityError> {
if <Multisigs<T>>::contains_key(multisig) {
Err(InvalidTransaction::Custom(E_DUPLICATIVE_SUBMISSION))?
} else {
Ok(())
}
}

fn pre_check_signature(
from: &AccountId32,
to: &AccountId20,
Expand Down Expand Up @@ -576,6 +596,14 @@ pub(crate) enum AssetStatus {
Destroying,
}

#[allow(missing_docs)]
#[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug, TypeInfo)]
pub struct MultisigParams {
address: AccountId20,
members: Vec<AccountId20>,
threshold: u16,
}

#[allow(missing_docs)]
#[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug, TypeInfo)]
pub struct MultisigMigrationDetail {
Expand Down
15 changes: 14 additions & 1 deletion pallet/account-migration/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ fn migrate_multisig_should_work() {
others: vec![b.public().0.into(), c.public().0.into()],
threshold: 2,
to,
signature: signature.0
signature: signature.0,
new_multisig_params: None
}));
assert_ok!(AccountMigration::migrate_multisig(
RuntimeOrigin::none(),
Expand All @@ -127,7 +128,19 @@ fn migrate_multisig_should_work() {
2,
to,
signature.0,
None
));
assert_noop!(
AccountMigration::pre_dispatch(&Call::migrate_multisig {
submitter: a.public().0.into(),
others: vec![b.public().0.into(), c.public().0.into()],
threshold: 2,
to,
signature: signature.0,
new_multisig_params: None
}),
TransactionValidityError::Invalid(InvalidTransaction::Custom(E_DUPLICATIVE_SUBMISSION))
);

assert!(<Multisigs<Runtime>>::get(&multisig).is_some());
assert_eq!(<frame_system::Account<Runtime>>::get(to).consumers, 0);
Expand Down
58 changes: 33 additions & 25 deletions pallet/account-migration/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
//! Autogenerated weights for darwinia_account_migration
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2023-03-17, STEPS: `2`, REPEAT: `1`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2023-03-29, STEPS: `2`, REPEAT: `1`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! WORST CASE MAP SIZE: `1000000`
//! HOSTNAME: `inv.cafe`, CPU: `13th Gen Intel(R) Core(TM) i9-13900K`
//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("pangolin-local"), DB CACHE: 1024

// Executed Command:
// target/x86_64-unknown-linux-gnu/release/darwinia
// target/release/darwinia
// benchmark
// pallet
// --header
Expand Down Expand Up @@ -56,7 +56,7 @@ use sp_std::marker::PhantomData;
/// Weight functions needed for darwinia_account_migration.
pub trait WeightInfo {
fn migrate() -> Weight;
fn migrate_multisig(x: u32, y: u32, ) -> Weight;
fn migrate_multisig(x: u32, y: u32, z: u32, ) -> Weight;
fn complete_multisig_migration() -> Weight;
}

Expand Down Expand Up @@ -97,8 +97,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Proof Size summary in bytes:
// Measured: `5667`
// Estimated: `52015`
// Minimum execution time: 202_426 nanoseconds.
Weight::from_ref_time(202_426_000)
// Minimum execution time: 169_486 nanoseconds.
Weight::from_ref_time(169_486_000)
.saturating_add(Weight::from_proof_size(52015))
.saturating_add(T::DbWeight::get().reads(12_u64))
.saturating_add(T::DbWeight::get().writes(18_u64))
Expand Down Expand Up @@ -137,18 +137,22 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
/// Proof: Deposit Deposits (max_values: None, max_size: Some(853), added: 3328, mode: MaxEncodedLen)
/// The range of component `x` is `[0, 99]`.
/// The range of component `y` is `[0, 99]`.
fn migrate_multisig(x: u32, y: u32, ) -> Weight {
/// The range of component `z` is `[0, 99]`.
fn migrate_multisig(x: u32, y: u32, z: u32, ) -> Weight {
// Proof Size summary in bytes:
// Measured: `5667`
// Estimated: `52015 + y * (7 ±0)`
// Minimum execution time: 12_545 nanoseconds.
Weight::from_ref_time(165_632_000)
// Measured: `5328 + z * (3 ±0)`
// Estimated: `52015 + y * (7 ±0) + z * (3 ±0)`
// Minimum execution time: 24_233 nanoseconds.
Weight::from_ref_time(153_169_000)
.saturating_add(Weight::from_proof_size(52015))
// Standard Error: 78_799
.saturating_add(Weight::from_ref_time(95_121).saturating_mul(x.into()))
// Standard Error: 7_204
.saturating_add(Weight::from_ref_time(13_888).saturating_mul(x.into()))
// Standard Error: 7_204
.saturating_add(Weight::from_ref_time(138_484).saturating_mul(z.into()))
.saturating_add(T::DbWeight::get().reads(12_u64))
.saturating_add(T::DbWeight::get().writes(18_u64))
.saturating_add(Weight::from_proof_size(7).saturating_mul(y.into()))
.saturating_add(Weight::from_proof_size(3).saturating_mul(z.into()))
}
/// Storage: AccountMigration Multisigs (r:1 w:1)
/// Proof Skipped: AccountMigration Multisigs (max_values: None, max_size: None, mode: Measured)
Expand All @@ -158,8 +162,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Proof Size summary in bytes:
// Measured: `4848`
// Estimated: `9914`
// Minimum execution time: 23_172 nanoseconds.
Weight::from_ref_time(23_172_000)
// Minimum execution time: 20_927 nanoseconds.
Weight::from_ref_time(20_927_000)
.saturating_add(Weight::from_proof_size(9914))
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().writes(1_u64))
Expand Down Expand Up @@ -202,8 +206,8 @@ impl WeightInfo for () {
// Proof Size summary in bytes:
// Measured: `5667`
// Estimated: `52015`
// Minimum execution time: 202_426 nanoseconds.
Weight::from_ref_time(202_426_000)
// Minimum execution time: 169_486 nanoseconds.
Weight::from_ref_time(169_486_000)
.saturating_add(Weight::from_proof_size(52015))
.saturating_add(RocksDbWeight::get().reads(12_u64))
.saturating_add(RocksDbWeight::get().writes(18_u64))
Expand Down Expand Up @@ -242,18 +246,22 @@ impl WeightInfo for () {
/// Proof: Deposit Deposits (max_values: None, max_size: Some(853), added: 3328, mode: MaxEncodedLen)
/// The range of component `x` is `[0, 99]`.
/// The range of component `y` is `[0, 99]`.
fn migrate_multisig(x: u32, y: u32, ) -> Weight {
/// The range of component `z` is `[0, 99]`.
fn migrate_multisig(x: u32, y: u32, z: u32, ) -> Weight {
// Proof Size summary in bytes:
// Measured: `5667`
// Estimated: `52015 + y * (7 ±0)`
// Minimum execution time: 12_545 nanoseconds.
Weight::from_ref_time(165_632_000)
// Measured: `5328 + z * (3 ±0)`
// Estimated: `52015 + y * (7 ±0) + z * (3 ±0)`
// Minimum execution time: 24_233 nanoseconds.
Weight::from_ref_time(153_169_000)
.saturating_add(Weight::from_proof_size(52015))
// Standard Error: 78_799
.saturating_add(Weight::from_ref_time(95_121).saturating_mul(x.into()))
// Standard Error: 7_204
.saturating_add(Weight::from_ref_time(13_888).saturating_mul(x.into()))
// Standard Error: 7_204
.saturating_add(Weight::from_ref_time(138_484).saturating_mul(z.into()))
.saturating_add(RocksDbWeight::get().reads(12_u64))
.saturating_add(RocksDbWeight::get().writes(18_u64))
.saturating_add(Weight::from_proof_size(7).saturating_mul(y.into()))
.saturating_add(Weight::from_proof_size(3).saturating_mul(z.into()))
}
/// Storage: AccountMigration Multisigs (r:1 w:1)
/// Proof Skipped: AccountMigration Multisigs (max_values: None, max_size: None, mode: Measured)
Expand All @@ -263,8 +271,8 @@ impl WeightInfo for () {
// Proof Size summary in bytes:
// Measured: `4848`
// Estimated: `9914`
// Minimum execution time: 23_172 nanoseconds.
Weight::from_ref_time(23_172_000)
// Minimum execution time: 20_927 nanoseconds.
Weight::from_ref_time(20_927_000)
.saturating_add(Weight::from_proof_size(9914))
.saturating_add(RocksDbWeight::get().reads(2_u64))
.saturating_add(RocksDbWeight::get().writes(1_u64))
Expand Down
30 changes: 17 additions & 13 deletions runtime/crab/src/weights/darwinia_account_migration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
//! Autogenerated weights for `darwinia_account_migration`
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2023-03-17, STEPS: `2`, REPEAT: `1`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2023-03-29, STEPS: `2`, REPEAT: `1`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! WORST CASE MAP SIZE: `1000000`
//! HOSTNAME: `inv.cafe`, CPU: `13th Gen Intel(R) Core(TM) i9-13900K`
//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("crab-local"), DB CACHE: 1024

// Executed Command:
// target/x86_64-unknown-linux-gnu/release/darwinia
// target/release/darwinia
// benchmark
// pallet
// --header
Expand Down Expand Up @@ -87,8 +87,8 @@ impl<T: frame_system::Config> darwinia_account_migration::WeightInfo for WeightI
// Proof Size summary in bytes:
// Measured: `5626`
// Estimated: `51933`
// Minimum execution time: 168_311 nanoseconds.
Weight::from_parts(168_311_000, 51933)
// Minimum execution time: 282_114 nanoseconds.
Weight::from_parts(282_114_000, 51933)
.saturating_add(T::DbWeight::get().reads(12))
.saturating_add(T::DbWeight::get().writes(18))
}
Expand Down Expand Up @@ -126,17 +126,21 @@ impl<T: frame_system::Config> darwinia_account_migration::WeightInfo for WeightI
/// Proof: Deposit Deposits (max_values: None, max_size: Some(853), added: 3328, mode: MaxEncodedLen)
/// The range of component `x` is `[0, 99]`.
/// The range of component `y` is `[0, 99]`.
fn migrate_multisig(x: u32, y: u32, ) -> Weight {
/// The range of component `z` is `[0, 99]`.
fn migrate_multisig(x: u32, y: u32, z: u32, ) -> Weight {
// Proof Size summary in bytes:
// Measured: `5626`
// Estimated: `51933 + y * (7 ±0)`
// Minimum execution time: 22_259 nanoseconds.
Weight::from_parts(169_703_500, 51933)
// Standard Error: 11_923
.saturating_add(Weight::from_ref_time(41_671).saturating_mul(x.into()))
// Measured: `5291 + z * (3 ±0)`
// Estimated: `51933 + y * (7 ±0) + z * (3 ±0)`
// Minimum execution time: 25_521 nanoseconds.
Weight::from_parts(124_506_666, 51933)
// Standard Error: 281_540
.saturating_add(Weight::from_ref_time(226_299).saturating_mul(x.into()))
// Standard Error: 281_540
.saturating_add(Weight::from_ref_time(295_461).saturating_mul(z.into()))
.saturating_add(T::DbWeight::get().reads(12))
.saturating_add(T::DbWeight::get().writes(18))
.saturating_add(Weight::from_proof_size(7).saturating_mul(y.into()))
.saturating_add(Weight::from_proof_size(3).saturating_mul(z.into()))
}
/// Storage: AccountMigration Multisigs (r:1 w:1)
/// Proof Skipped: AccountMigration Multisigs (max_values: None, max_size: None, mode: Measured)
Expand All @@ -146,8 +150,8 @@ impl<T: frame_system::Config> darwinia_account_migration::WeightInfo for WeightI
// Proof Size summary in bytes:
// Measured: `4877`
// Estimated: `9943`
// Minimum execution time: 21_026 nanoseconds.
Weight::from_parts(21_026_000, 9943)
// Minimum execution time: 21_466 nanoseconds.
Weight::from_parts(21_466_000, 9943)
.saturating_add(T::DbWeight::get().reads(2))
.saturating_add(T::DbWeight::get().writes(1))
}
Expand Down
Loading

0 comments on commit 2694c09

Please sign in to comment.