diff --git a/pallets/api/src/fungibles/benchmarking.rs b/pallets/api/src/fungibles/benchmarking.rs index 6f182932..1ead42c0 100644 --- a/pallets/api/src/fungibles/benchmarking.rs +++ b/pallets/api/src/fungibles/benchmarking.rs @@ -24,6 +24,13 @@ fn assert_has_event( frame_system::Pallet::::assert_has_event(generic_event.into()); } +// Compare `generic_event` to the last emitted event. +fn assert_last_event( + generic_event: >>::RuntimeEvent, +) { + frame_system::Pallet::::assert_last_event(generic_event.into()); +} + #[benchmarks( where > as Inspect<::AccountId>>::AssetId: Zero, @@ -31,19 +38,20 @@ fn assert_has_event( mod benchmarks { use super::*; - // The worst case scenario is when the allowance is set to a value which is lower than the - // current allowance. + // Parameter: + // - 'a': whether `approve_transfer` has been called. + // - 'c': whether `cancel_approval` has been called. #[benchmark] - fn approve() -> Result<(), BenchmarkError> { + fn approve(a: Linear<0, 1>, c: Linear<0, 1>) -> Result<(), BenchmarkError> { let asset_id = AssetIdOf::::zero(); - let decreased_value = >::from(50u32); let min_balance = >::from(1u32); let owner: AccountIdOf = account("Alice", 0, SEED); let spender: AccountIdOf = account("Bob", 0, SEED); - let value = >::from(100u32); + let current_allowance = >::from(u32::MAX / 2); T::Currency::make_free_balance_be(&owner, u32::MAX.into()); + // Set the `current_allowance`. assert_ok!( as Create>>::create( - asset_id.clone().into(), + asset_id.clone(), owner.clone(), true, min_balance @@ -52,33 +60,52 @@ mod benchmarks { asset_id.clone(), &owner, &spender, - value + current_allowance, )); + let approval_value = match (a, c) { + // Equal to the current allowance. + (0, 0) => current_allowance, + // Greater than the current allowance. + (1, 0) => >::from(u32::MAX), + // Zero. + (0, 1) => >::from(0u32), + // Smaller than the current allowance. + (1, 1) => >::from(u32::MAX / 4), + _ => unreachable!("values can only be 0 or 1"), + }; #[extrinsic_call] - _(RawOrigin::Signed(owner.clone()), asset_id.clone(), spender.clone(), decreased_value); - - assert_eq!(AssetsOf::::allowance(asset_id.clone(), &owner, &spender), decreased_value); - // To make sure both dispatchables have been successfully executed - i.e. worst case - // scenario. - assert_has_event::( - pallet_assets::Event::ApprovalCancelled { - asset_id: asset_id.clone(), - owner: owner.clone(), - delegate: spender.clone(), - } - .into(), - ); - assert_has_event::( - pallet_assets::Event::ApprovedTransfer { - asset_id, - source: owner, - delegate: spender, - amount: value, - } - .into(), - ); + _(RawOrigin::Signed(owner.clone()), asset_id.clone(), spender.clone(), approval_value); + assert_eq!(AssetsOf::::allowance(asset_id.clone(), &owner, &spender), approval_value); + if c == 1 { + assert_has_event::( + pallet_assets::Event::ApprovalCancelled { + asset_id: asset_id.clone(), + owner: owner.clone(), + delegate: spender.clone(), + } + .into(), + ); + } + if a == 1 { + let amount = match c { + // When the allowance was cancelled and then approved with the new value. + 1 => approval_value, + // When the allowance was increased. + 0 => approval_value - current_allowance, + _ => unreachable!("`c` can only be 0 or 1"), + }; + assert_has_event::( + pallet_assets::Event::ApprovedTransfer { + asset_id, + source: owner, + delegate: spender, + amount, + } + .into(), + ); + } Ok(()) } diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index d80fc310..3067e1d3 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -33,7 +33,10 @@ pub mod pallet { traits::fungibles::approvals::Inspect as ApprovalInspect, }; use frame_system::pallet_prelude::*; - use sp_runtime::{traits::StaticLookup, Saturating}; + use sp_runtime::{ + traits::{StaticLookup, Zero}, + Saturating, + }; use sp_std::vec::Vec; /// State reads for the fungibles api with required input. @@ -101,22 +104,23 @@ pub mod pallet { /// * `spender` - The account that is allowed to spend the tokens. /// * `value` - The number of tokens to approve. #[pallet::call_index(5)] - #[pallet::weight(::WeightInfo::approve())] + #[pallet::weight(::WeightInfo::approve(1, 1))] pub fn approve( origin: OriginFor, id: AssetIdOf, spender: AccountIdOf, value: BalanceOf, ) -> DispatchResultWithPostInfo { - let who = ensure_signed(origin.clone()) - // To have the caller pay some fees. - .map_err(|e| e.with_weight(T::DbWeight::get().reads(1)))?; + let weight = |approve: u32, cancel: u32| -> Weight { + ::WeightInfo::approve(cancel, approve) + }; + let who = ensure_signed(origin.clone()).map_err(|e| e.with_weight(weight(0, 0)))?; let current_allowance = AssetsOf::::allowance(id.clone(), &who, &spender); let spender = T::Lookup::unlookup(spender); let id: AssetIdParameterOf = id.into(); // If the new value is equal to the current allowance, do nothing. if value == current_allowance { - return Ok(().into()); + return Ok(Some(weight(0, 0)).into()); } // If the new value is greater than the current allowance, approve the difference // because `approve_transfer` works additively (see pallet-assets). @@ -127,23 +131,18 @@ pub mod pallet { spender, value.saturating_sub(current_allowance), ) - .map_err(|e| { - e.with_weight( - T::DbWeight::get().reads(1) + AssetsWeightInfoOf::::approve_transfer(), - ) - })?; + .map_err(|e| e.with_weight(weight(1, 0)))?; + Ok(Some(weight(1, 0)).into()) } else { // If the new value is less than the current allowance, cancel the approval and set the new value AssetsOf::::cancel_approval(origin.clone(), id.clone(), spender.clone()) - .map_err(|e| { - e.with_weight( - T::DbWeight::get().reads(1) - + AssetsWeightInfoOf::::cancel_approval(), - ) - })?; + .map_err(|e| e.with_weight(weight(0, 1)))?; + if value.is_zero() { + return Ok(Some(weight(0, 1)).into()); + } AssetsOf::::approve_transfer(origin, id, spender, value)?; + Ok(().into()) } - Ok(().into()) } /// Increases the allowance of a spender. diff --git a/pallets/api/src/fungibles/weights.rs b/pallets/api/src/fungibles/weights.rs index ed2b04fc..a6c31654 100644 --- a/pallets/api/src/fungibles/weights.rs +++ b/pallets/api/src/fungibles/weights.rs @@ -1,7 +1,8 @@ + //! Autogenerated weights for `pallet_api::fungibles` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 33.0.0 -//! DATE: 2024-07-24, STEPS: `20`, REPEAT: `5`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-07-25, STEPS: `20`, REPEAT: `5`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `R0GUE`, CPU: `` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` @@ -31,7 +32,7 @@ use core::marker::PhantomData; /// Weight functions needed for `pallet_api::fungibles`. pub trait WeightInfo { - fn approve() -> Weight; + fn approve(a: u32, c: u32, ) -> Weight; } /// Weights for `pallet_api::fungibles` using the Substrate node and recommended hardware. @@ -43,14 +44,23 @@ impl WeightInfo for SubstrateWeight { /// Proof: `Assets::Asset` (`max_values`: None, `max_size`: Some(210), added: 2685, mode: `MaxEncodedLen`) /// Storage: `System::Account` (r:1 w:1) /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) - fn approve() -> Weight { + /// The range of component `a` is `[0, 1]`. + /// The range of component `c` is `[0, 1]`. + fn approve(a: u32, c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `515` - // Estimated: `3675` - // Minimum execution time: 53_000_000 picoseconds. - Weight::from_parts(54_000_000, 3675) - .saturating_add(T::DbWeight::get().reads(3_u64)) - .saturating_add(T::DbWeight::get().writes(3_u64)) + // Measured: `413 + c * (102 ±0)` + // Estimated: `3675 + c * (1797 ±0)` + // Minimum execution time: 35_000_000 picoseconds. + Weight::from_parts(1_207_482, 3675) + // Standard Error: 948_955 + .saturating_add(Weight::from_parts(34_649_659, 0).saturating_mul(a.into())) + // Standard Error: 948_955 + .saturating_add(Weight::from_parts(56_976_190, 0).saturating_mul(c.into())) + .saturating_add(T::DbWeight::get().reads(2_u64)) + .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(c.into()))) + .saturating_add(T::DbWeight::get().writes(2_u64)) + .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(c.into()))) + .saturating_add(Weight::from_parts(0, 1797).saturating_mul(c.into())) } } @@ -62,14 +72,23 @@ impl WeightInfo for () { /// Proof: `Assets::Asset` (`max_values`: None, `max_size`: Some(210), added: 2685, mode: `MaxEncodedLen`) /// Storage: `System::Account` (r:1 w:1) /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) - fn approve() -> Weight { + /// The range of component `a` is `[0, 1]`. + /// The range of component `c` is `[0, 1]`. + fn approve(a: u32, c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `515` - // Estimated: `3675` - // Minimum execution time: 53_000_000 picoseconds. - Weight::from_parts(54_000_000, 3675) - .saturating_add(RocksDbWeight::get().reads(3_u64)) - .saturating_add(RocksDbWeight::get().writes(3_u64)) + // Measured: `413 + c * (102 ±0)` + // Estimated: `3675 + c * (1797 ±0)` + // Minimum execution time: 35_000_000 picoseconds. + Weight::from_parts(1_207_482, 3675) + // Standard Error: 948_955 + .saturating_add(Weight::from_parts(34_649_659, 0).saturating_mul(a.into())) + // Standard Error: 948_955 + .saturating_add(Weight::from_parts(56_976_190, 0).saturating_mul(c.into())) + .saturating_add(RocksDbWeight::get().reads(2_u64)) + .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(c.into()))) + .saturating_add(RocksDbWeight::get().writes(2_u64)) + .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(c.into()))) + .saturating_add(Weight::from_parts(0, 1797).saturating_mul(c.into())) } }