From 4419bad85e4ba213040448a8325c8195049a4448 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 24 Jul 2024 13:59:00 +0200 Subject: [PATCH 01/11] refactor: pallet --- pallets/api/src/fungibles/benchmarking.rs | 8 +- pallets/api/src/fungibles/mod.rs | 145 ++++++++++------------ 2 files changed, 69 insertions(+), 84 deletions(-) diff --git a/pallets/api/src/fungibles/benchmarking.rs b/pallets/api/src/fungibles/benchmarking.rs index 7c0ad6ce..438aa9b6 100644 --- a/pallets/api/src/fungibles/benchmarking.rs +++ b/pallets/api/src/fungibles/benchmarking.rs @@ -1,6 +1,6 @@ //! Benchmarking setup for pallet-cards -use super::{AccountIdOf, AssetIdOf, Assets, AssetsInstanceOf, BalanceOf, Call, Config, Pallet}; +use super::{AccountIdOf, AssetIdOf, AssetsInstanceOf, AssetsOf, BalanceOf, Call, Config, Pallet}; use frame_benchmarking::{account, v2::*}; use frame_support::{ assert_ok, @@ -35,13 +35,13 @@ mod benchmarks { let spender: AccountIdOf = account("Bob", 0, SEED); let value = >::from(100u32); T::Currency::make_free_balance_be(&owner, 100u32.into()); - assert_ok!( as Create>>::create( + assert_ok!( as Create>>::create( asset.clone().into(), owner.clone(), true, min_balance )); - assert_ok!( as Mutate>>::approve( + assert_ok!( as Mutate>>::approve( asset.clone(), &owner, &spender, @@ -51,7 +51,7 @@ mod benchmarks { #[extrinsic_call] _(RawOrigin::Signed(owner.clone()), asset.clone(), spender.clone(), decreased_value); - assert_eq!(Assets::::allowance(asset, &owner, &spender), decreased_value); + assert_eq!(AssetsOf::::allowance(asset, &owner, &spender), decreased_value); Ok(()) } diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 3b096490..8e474303 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -1,61 +1,68 @@ /// The fungibles pallet serves as a wrapper around the pallet_assets, offering a streamlined /// interface for interacting with fungible assets. The goal is to provide a simplified, consistent /// API that adheres to standards in the smart contract space. -pub use pallet::*; - #[cfg(feature = "runtime-benchmarks")] mod benchmarking; #[cfg(test)] mod tests; +use frame_support::traits::fungibles::{metadata::Inspect as MetadataInspect, Inspect}; +pub use pallet::*; + +type AccountIdOf = ::AccountId; +type AssetIdOf = > as Inspect< + ::AccountId, +>>::AssetId; +type AssetIdParameterOf = >>::AssetIdParameter; +type AssetsOf = pallet_assets::Pallet>; +type AssetsInstanceOf = ::AssetsInstance; +type AssetsWeightInfoOf = >>::WeightInfo; +type BalanceOf = > as Inspect< + ::AccountId, +>>::Balance; + #[frame_support::pallet] pub mod pallet { + use super::*; use frame_support::{ dispatch::{DispatchResult, DispatchResultWithPostInfo, WithPostDispatchInfo}, pallet_prelude::*, - traits::fungibles::{ - approvals::Inspect as ApprovalInspect, metadata::Inspect as MetadataInspect, Inspect, - }, + traits::fungibles::approvals::Inspect as ApprovalInspect, }; use frame_system::pallet_prelude::*; use pallet_assets::WeightInfo; use sp_runtime::{traits::StaticLookup, Saturating}; use sp_std::vec::Vec; - pub(crate) type AccountIdOf = ::AccountId; - pub(crate) type AssetIdOf = > as Inspect< - ::AccountId, - >>::AssetId; - type AssetIdParameterOf = - >>::AssetIdParameter; - pub(crate) type Assets = pallet_assets::Pallet>; - pub(crate) type AssetsInstanceOf = ::AssetsInstance; - type AssetsWeightInfo = >>::WeightInfo; - pub(crate) type BalanceOf = > as Inspect< - ::AccountId, - >>::Balance; - - /// The required input for state queries through the fungibles api. + /// State reads for the fungibles api with required input. #[derive(Encode, Decode, Debug, MaxEncodedLen)] #[repr(u8)] #[allow(clippy::unnecessary_cast)] - pub enum FungiblesKey { + pub enum Read { + /// Total token supply for a given asset ID. #[codec(index = 0)] TotalSupply(AssetIdOf), + /// Account balance for a given asset ID. #[codec(index = 1)] BalanceOf(AssetIdOf, AccountIdOf), + /// Allowance for a spender approved by an owner, for a given asset ID. #[codec(index = 2)] Allowance(AssetIdOf, AccountIdOf, AccountIdOf), + /// Token name for a given asset ID. #[codec(index = 3)] TokenName(AssetIdOf), + /// Token symbol for a given asset ID. #[codec(index = 4)] TokenSymbol(AssetIdOf), + /// Token decimals for a given asset ID. #[codec(index = 5)] TokenDecimals(AssetIdOf), } + /// Configure the pallet by specifying the parameters and types on which it depends. #[pallet::config] pub trait Config: frame_system::Config + pallet_assets::Config { + /// The instance of pallet assets it is tightly coupled to. type AssetsInstance; } @@ -67,15 +74,12 @@ pub mod pallet { /// Transfers `value` amount of tokens from the caller's account to account `to`, with additional /// `data` in unspecified format. /// - /// # Arguments + /// # Parameters /// * `id` - The ID of the asset. /// * `to` - The recipient account. /// * `value` - The number of tokens to transfer. - /// - /// # Returns - /// Returns `Ok(())` if successful, or an error if the transfer fails. #[pallet::call_index(0)] - #[pallet::weight(AssetsWeightInfo::::transfer_keep_alive())] + #[pallet::weight(AssetsWeightInfoOf::::transfer_keep_alive())] pub fn transfer( origin: OriginFor, id: AssetIdOf, @@ -83,70 +87,69 @@ pub mod pallet { amount: BalanceOf, ) -> DispatchResult { let target = T::Lookup::unlookup(target); - Assets::::transfer_keep_alive(origin, id.into(), target, amount) + AssetsOf::::transfer_keep_alive(origin, id.into(), target, amount) } /// Approves an account to spend a specified number of tokens on behalf of the caller. /// - /// # Arguments + /// # Parameters /// * `id` - The ID of the asset. /// * `spender` - The account that is allowed to spend the tokens. /// * `value` - The number of tokens to approve. - /// - /// # Returns - /// Returns `Ok(())` if successful, or an error if the approval fails. #[pallet::call_index(2)] - #[pallet::weight(T::DbWeight::get().reads(1) + AssetsWeightInfo::::cancel_approval() + AssetsWeightInfo::::approve_transfer())] + #[pallet::weight(T::DbWeight::get().reads(1) + AssetsWeightInfoOf::::cancel_approval() + AssetsWeightInfoOf::::approve_transfer())] pub fn approve( origin: OriginFor, id: AssetIdOf, spender: AccountIdOf, - mut value: BalanceOf, + 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 current_allowance = Assets::::allowance(id.clone(), &who, &spender); + 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()); } - // If the new value is greater than the current allowance, approve the difference. + // If the new value is greater than the current allowance, approve the difference + // because `approve_transfer` works additively (see pallet-assets). if value > current_allowance { - value.saturating_reduce(current_allowance); - Assets::::approve_transfer(origin, id, spender, value).map_err(|e| { + AssetsOf::::approve_transfer( + origin, + id, + spender, + value.saturating_sub(current_allowance), + ) + .map_err(|e| { e.with_weight( - T::DbWeight::get().reads(1) + AssetsWeightInfo::::approve_transfer(), + T::DbWeight::get().reads(1) + AssetsWeightInfoOf::::approve_transfer(), ) })?; } else { // If the new value is less than the current allowance, cancel the approval and set the new value - Assets::::cancel_approval(origin.clone(), id.clone(), spender.clone()).map_err( - |e| { + AssetsOf::::cancel_approval(origin.clone(), id.clone(), spender.clone()) + .map_err(|e| { e.with_weight( - T::DbWeight::get().reads(1) + AssetsWeightInfo::::cancel_approval(), + T::DbWeight::get().reads(1) + + AssetsWeightInfoOf::::cancel_approval(), ) - }, - )?; - Assets::::approve_transfer(origin, id, spender, value)?; + })?; + AssetsOf::::approve_transfer(origin, id, spender, value)?; } - Ok(().into()) } /// Increases the allowance of a spender. /// - /// # Arguments + /// # Parameters /// * `id` - The ID of the asset. /// * `spender` - The account that is allowed to spend the tokens. /// * `value` - The number of tokens to increase the allowance by. - /// - /// # Returns - /// Returns `Ok(())` if successful, or an error if the operation fails. #[pallet::call_index(3)] - #[pallet::weight(AssetsWeightInfo::::approve_transfer())] + #[pallet::weight(AssetsWeightInfoOf::::approve_transfer())] pub fn increase_allowance( origin: OriginFor, id: AssetIdOf, @@ -154,84 +157,66 @@ pub mod pallet { value: BalanceOf, ) -> DispatchResult { let spender = T::Lookup::unlookup(spender); - Assets::::approve_transfer(origin, id.into(), spender, value) + AssetsOf::::approve_transfer(origin, id.into(), spender, value) } } impl Pallet { /// Returns the total token supply for a given asset ID. /// - /// # Arguments + /// # Parameters /// * `id` - The ID of the asset. - /// - /// # Returns - /// The total supply of the token, or an error if the operation fails. pub fn total_supply(id: AssetIdOf) -> BalanceOf { - Assets::::total_supply(id) + AssetsOf::::total_supply(id) } /// Returns the account balance for the specified `owner` for a given asset ID. Returns `0` if /// the account is non-existent. /// - /// # Arguments + /// # Parameters /// * `id` - The ID of the asset. /// * `owner` - The account whose balance is being queried. - /// - /// # Returns - /// The balance of the specified account, or an error if the operation fails. pub fn balance_of(id: AssetIdOf, owner: &AccountIdOf) -> BalanceOf { - Assets::::balance(id, owner) + AssetsOf::::balance(id, owner) } /// Returns the amount which `spender` is still allowed to withdraw from `owner` for a given /// asset ID. Returns `0` if no allowance has been set. /// - /// # Arguments + /// # Parameters /// * `id` - The ID of the asset. /// * `owner` - The account that owns the tokens. /// * `spender` - The account that is allowed to spend the tokens. - /// - /// # Returns - /// The remaining allowance, or an error if the operation fails. pub fn allowance( id: AssetIdOf, owner: &AccountIdOf, spender: &AccountIdOf, ) -> BalanceOf { - Assets::::allowance(id, owner, spender) + AssetsOf::::allowance(id, owner, spender) } /// Returns the token name for a given asset ID. /// - /// # Arguments + /// # Parameters /// * `id` - The ID of the asset. - /// - /// # Returns - /// The name of the token as a byte vector, or an error if the operation fails. pub fn token_name(id: AssetIdOf) -> Vec { - as MetadataInspect>>::name(id) + as MetadataInspect>>::name(id) } /// Returns the token symbol for a given asset ID. /// - /// # Arguments + /// # Parameters /// * `id` - The ID of the asset. - /// - /// # Returns - /// The symbol of the token as a byte vector, or an error if the operation fails. pub fn token_symbol(id: AssetIdOf) -> Vec { - as MetadataInspect>>::symbol(id) + as MetadataInspect>>::symbol(id) } /// Returns the token decimals for a given asset ID. /// - /// # Arguments + /// # Parameters /// * `id` - The ID of the asset. - /// - /// # Returns - /// The number of decimals of the token as a byte vector, or an error if the operation fails. pub fn token_decimals(id: AssetIdOf) -> u8 { - as MetadataInspect>>::decimals(id) + as MetadataInspect>>::decimals(id) } } } From 792fa0754b47d94c91bed3a32deb0a53f4a6ff58 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 24 Jul 2024 13:59:33 +0200 Subject: [PATCH 02/11] refactor: integration-tests --- pop-api/integration-tests/src/local_fungibles.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 3ff0bf52..a8511ffb 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -348,11 +348,11 @@ fn transfer_works() { Module { index: 52, error: 0 }, ); // Successful transfer. - let bob_balance_before_transfer = Assets::balance(asset, &BOB); + let balance_before_transfer = Assets::balance(asset, &BOB); let result = transfer(addr.clone(), asset, BOB, amount / 2); assert!(!result.did_revert(), "Contract reverted!"); - let bob_balance_after_transfer = Assets::balance(asset, &BOB); - assert_eq!(bob_balance_after_transfer, bob_balance_before_transfer + amount / 2); + let balance_after_transfer = Assets::balance(asset, &BOB); + assert_eq!(balance_after_transfer, balance_before_transfer + amount / 2); // Transfer asset to account that does not exist. assert_eq!( decoded::(transfer(addr.clone(), asset, FERDIE, amount / 4)), @@ -547,11 +547,11 @@ fn token_metadata_works() { // ); // thaw_asset(addr.clone(), asset); // // Successful mint. -// let bob_balance_before_mint = Assets::balance(asset, &BOB); +// let balance_before_mint = Assets::balance(asset, &BOB); // let result = transfer_from(addr.clone(), asset, None, Some(BOB), amount, &[0u8]); // assert!(!result.did_revert(), "Contract reverted!"); -// let bob_balance_after_mint = Assets::balance(asset, &BOB); -// assert_eq!(bob_balance_after_mint, bob_balance_before_mint + amount); +// let balance_after_mint = Assets::balance(asset, &BOB); +// assert_eq!(balance_after_mint, balance_before_mint + amount); // // Can not mint more tokens than Balance::MAX. // assert_eq!( // decoded::(transfer_from( From 96da2f0727bcf5018316bb7d7d6b51ae7b704960 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 24 Jul 2024 14:11:06 +0200 Subject: [PATCH 03/11] refactor: ink api library --- pallets/api/src/fungibles/mod.rs | 12 ++++++------ pop-api/Cargo.toml | 3 ++- pop-api/src/lib.rs | 2 ++ pop-api/src/v0/assets/fungibles.rs | 16 ++++++++-------- pop-api/src/v0/mod.rs | 1 + 5 files changed, 19 insertions(+), 15 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 8e474303..4ad2e2d4 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -49,13 +49,13 @@ pub mod pallet { #[codec(index = 2)] Allowance(AssetIdOf, AccountIdOf, AccountIdOf), /// Token name for a given asset ID. - #[codec(index = 3)] + #[codec(index = 8)] TokenName(AssetIdOf), /// Token symbol for a given asset ID. - #[codec(index = 4)] + #[codec(index = 9)] TokenSymbol(AssetIdOf), /// Token decimals for a given asset ID. - #[codec(index = 5)] + #[codec(index = 10)] TokenDecimals(AssetIdOf), } @@ -78,7 +78,7 @@ pub mod pallet { /// * `id` - The ID of the asset. /// * `to` - The recipient account. /// * `value` - The number of tokens to transfer. - #[pallet::call_index(0)] + #[pallet::call_index(3)] #[pallet::weight(AssetsWeightInfoOf::::transfer_keep_alive())] pub fn transfer( origin: OriginFor, @@ -96,7 +96,7 @@ pub mod pallet { /// * `id` - The ID of the asset. /// * `spender` - The account that is allowed to spend the tokens. /// * `value` - The number of tokens to approve. - #[pallet::call_index(2)] + #[pallet::call_index(5)] #[pallet::weight(T::DbWeight::get().reads(1) + AssetsWeightInfoOf::::cancel_approval() + AssetsWeightInfoOf::::approve_transfer())] pub fn approve( origin: OriginFor, @@ -148,7 +148,7 @@ pub mod pallet { /// * `id` - The ID of the asset. /// * `spender` - The account that is allowed to spend the tokens. /// * `value` - The number of tokens to increase the allowance by. - #[pallet::call_index(3)] + #[pallet::call_index(6)] #[pallet::weight(AssetsWeightInfoOf::::approve_transfer())] pub fn increase_allowance( origin: OriginFor, diff --git a/pop-api/Cargo.toml b/pop-api/Cargo.toml index ff9b003a..5638b9d2 100644 --- a/pop-api/Cargo.toml +++ b/pop-api/Cargo.toml @@ -25,4 +25,5 @@ std = [ "primitives/std", "sp-io/std", ] -fungibles = [] +assets = [] +fungibles = ["assets"] diff --git a/pop-api/src/lib.rs b/pop-api/src/lib.rs index a3f75f3f..d2470a92 100644 --- a/pop-api/src/lib.rs +++ b/pop-api/src/lib.rs @@ -13,6 +13,8 @@ pub type Result = core::result::Result; mod constants { // Errors: pub(crate) const DECODING_FAILED: u32 = 255; + // TODO: Not used but will be used in the future when the remaining fungibles features will be + // implemented. pub(crate) const _MODULE_ERROR: u8 = 3; // Function IDs: diff --git a/pop-api/src/v0/assets/fungibles.rs b/pop-api/src/v0/assets/fungibles.rs index 0805393f..42373cd7 100644 --- a/pop-api/src/v0/assets/fungibles.rs +++ b/pop-api/src/v0/assets/fungibles.rs @@ -23,23 +23,23 @@ mod constants { /// - allowance pub const ALLOWANCE: u8 = 2; /// - transfer - pub(super) const TRANSFER: u8 = 0; + pub(super) const TRANSFER: u8 = 3; /// - transfer_from - pub(super) const TRANSFER_FROM: u8 = 1; + pub(super) const TRANSFER_FROM: u8 = 4; /// - approve - pub(super) const APPROVE: u8 = 2; + pub(super) const APPROVE: u8 = 5; /// - increase_allowance - pub(super) const INCREASE_ALLOWANCE: u8 = 3; + pub(super) const INCREASE_ALLOWANCE: u8 = 6; /// - decrease_allowance - pub(super) const DECREASE_ALLOWANCE: u8 = 4; + pub(super) const DECREASE_ALLOWANCE: u8 = 7; /// 2. PSP-22 Metadata Interface: /// - token_name - pub const TOKEN_NAME: u8 = 3; + pub const TOKEN_NAME: u8 = 8; /// - token_symbol - pub const TOKEN_SYMBOL: u8 = 4; + pub const TOKEN_SYMBOL: u8 = 9; /// - token_decimals - pub const TOKEN_DECIMALS: u8 = 5; + pub const TOKEN_DECIMALS: u8 = 10; // 3. Asset Management: // - create diff --git a/pop-api/src/v0/mod.rs b/pop-api/src/v0/mod.rs index 4a0da363..1c3642e1 100644 --- a/pop-api/src/v0/mod.rs +++ b/pop-api/src/v0/mod.rs @@ -1,5 +1,6 @@ use crate::{primitives::error::Error, StatusCode}; +#[cfg(feature = "assets")] pub mod assets; pub(crate) const V0: u8 = 0; From cb54a57d9174f23e36fc9395dc81d1e57253b455 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 24 Jul 2024 16:33:46 +0200 Subject: [PATCH 04/11] refactor: devnet runtime --- runtime/devnet/Cargo.toml | 1 + runtime/devnet/src/config/api.rs | 31 ++++++++++++++++++++++++++++ runtime/devnet/src/config/assets.rs | 8 ++----- runtime/devnet/src/config/mod.rs | 1 + runtime/devnet/src/extensions/mod.rs | 18 +++++++++------- runtime/devnet/src/lib.rs | 28 ------------------------- 6 files changed, 46 insertions(+), 41 deletions(-) create mode 100644 runtime/devnet/src/config/api.rs diff --git a/runtime/devnet/Cargo.toml b/runtime/devnet/Cargo.toml index 4a31f6f6..113f1fe1 100644 --- a/runtime/devnet/Cargo.toml +++ b/runtime/devnet/Cargo.toml @@ -171,6 +171,7 @@ runtime-benchmarks = [ "frame-system-benchmarking/runtime-benchmarks", "frame-system/runtime-benchmarks", "pallet-assets/runtime-benchmarks", + "pallet-api/runtime-benchmarks", "pallet-balances/runtime-benchmarks", "pallet-collator-selection/runtime-benchmarks", "pallet-contracts/runtime-benchmarks", diff --git a/runtime/devnet/src/config/api.rs b/runtime/devnet/src/config/api.rs new file mode 100644 index 00000000..1a8925bf --- /dev/null +++ b/runtime/devnet/src/config/api.rs @@ -0,0 +1,31 @@ +use crate::{config::assets::TrustBackedAssetsInstance, fungibles, Runtime, RuntimeCall}; +use codec::{Decode, Encode, MaxEncodedLen}; +use frame_support::traits::Contains; + +#[derive(Encode, Decode, Debug, MaxEncodedLen)] +#[repr(u8)] +pub enum RuntimeStateKeys { + #[codec(index = 150)] + Fungibles(fungibles::Read), +} + +impl fungibles::Config for Runtime { + type AssetsInstance = TrustBackedAssetsInstance; +} + +/// A type to identify allowed calls to the Runtime from contracts. Used by Pop API +pub struct AllowedApiCalls; + +impl Contains for AllowedApiCalls { + fn contains(c: &RuntimeCall) -> bool { + use fungibles::Call as FungiblesCall; + matches!( + c, + RuntimeCall::Fungibles( + FungiblesCall::transfer { .. } + | FungiblesCall::approve { .. } + | FungiblesCall::increase_allowance { .. } + ) + ) + } +} diff --git a/runtime/devnet/src/config/assets.rs b/runtime/devnet/src/config/assets.rs index 26874710..78aed8b5 100644 --- a/runtime/devnet/src/config/assets.rs +++ b/runtime/devnet/src/config/assets.rs @@ -9,8 +9,8 @@ use parachains_common::{AssetIdForTrustBackedAssets, CollectionId, ItemId, Signa use sp_runtime::traits::Verify; use crate::{ - deposit, fungibles, AccountId, Assets, Balance, Balances, BlockNumber, Nfts, Runtime, - RuntimeEvent, RuntimeHoldReason, DAYS, EXISTENTIAL_DEPOSIT, UNIT, + deposit, AccountId, Assets, Balance, Balances, BlockNumber, Nfts, Runtime, RuntimeEvent, + RuntimeHoldReason, DAYS, EXISTENTIAL_DEPOSIT, UNIT, }; /// We allow root to execute privileged asset operations. @@ -120,7 +120,3 @@ impl pallet_assets::Config for Runtime { #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); } - -impl fungibles::Config for Runtime { - type AssetsInstance = TrustBackedAssetsInstance; -} diff --git a/runtime/devnet/src/config/mod.rs b/runtime/devnet/src/config/mod.rs index e0aaa3a1..f62ffa76 100644 --- a/runtime/devnet/src/config/mod.rs +++ b/runtime/devnet/src/config/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod api; pub mod assets; mod contracts; mod proxy; diff --git a/runtime/devnet/src/extensions/mod.rs b/runtime/devnet/src/extensions/mod.rs index 4496c458..27af328a 100644 --- a/runtime/devnet/src/extensions/mod.rs +++ b/runtime/devnet/src/extensions/mod.rs @@ -1,13 +1,15 @@ mod v0; use crate::{ - config::assets::TrustBackedAssetsInstance, + config::{ + api::{AllowedApiCalls, RuntimeStateKeys}, + assets::TrustBackedAssetsInstance, + }, fungibles::{ self, - FungiblesKey::{self, *}, + Read::{self, *}, }, - state_keys::RuntimeStateKeys, - AccountId, AllowedApiCalls, RuntimeCall, RuntimeOrigin, + AccountId, RuntimeCall, RuntimeOrigin, }; use codec::{Decode, Encode}; use frame_support::{ @@ -187,16 +189,18 @@ where env.write(&result, false, None) } -// Example wrapper to enable versioning of `RuntimeStateKeys`. +/// Wrapper to enable versioning of `RuntimeStateKeys`. #[derive(Decode, Debug)] enum VersionedStateRead { + /// Version zero of reading state from api. #[codec(index = 0)] V0(RuntimeStateKeys), } -// Wrapper to enable versioning of `RuntimeCall`. +/// Wrapper to enable versioning of `RuntimeCall`. #[derive(Decode, Debug)] enum VersionedDispatch { + /// Version zero of dispatch calls from api. #[codec(index = 0)] V0(RuntimeCall), } @@ -282,7 +286,7 @@ impl TryFrom for FuncId { } fn read_fungibles_state( - key: FungiblesKey, + key: Read, env: &mut Environment, ) -> Result, DispatchError> where diff --git a/runtime/devnet/src/lib.rs b/runtime/devnet/src/lib.rs index 4d4ffc7c..15971e74 100644 --- a/runtime/devnet/src/lib.rs +++ b/runtime/devnet/src/lib.rs @@ -252,22 +252,6 @@ impl Contains for FilteredCalls { } } -/// A type to identify allowed calls to the Runtime from contracts. Used by Pop API -pub struct AllowedApiCalls; -impl Contains for AllowedApiCalls { - fn contains(c: &RuntimeCall) -> bool { - use fungibles::Call as FungiblesCall; - matches!( - c, - RuntimeCall::Fungibles( - FungiblesCall::transfer { .. } - | FungiblesCall::approve { .. } - | FungiblesCall::increase_allowance { .. } - ) - ) - } -} - /// The default types are being injected by [`derive_impl`](`frame_support::derive_impl`) from /// [`ParaChainDefaultConfig`](`struct@frame_system::config_preludes::ParaChainDefaultConfig`), /// but overridden as needed. @@ -975,15 +959,3 @@ cumulus_pallet_parachain_system::register_validate_block! { Runtime = Runtime, BlockExecutor = cumulus_pallet_aura_ext::BlockExecutor::, } - -pub(crate) mod state_keys { - use super::fungibles; - use codec::{Decode, Encode, MaxEncodedLen}; - - #[derive(Encode, Decode, Debug, MaxEncodedLen)] - #[repr(u8)] - pub enum RuntimeStateKeys { - #[codec(index = 150)] - Fungibles(fungibles::FungiblesKey), - } -} From 9ec274490e62269547abd41571aafd08ba1fa9ed Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 24 Jul 2024 18:26:06 +0200 Subject: [PATCH 05/11] chore: benchmark fungibles approve --- node/src/command.rs | 13 +-- pallets/api/src/fungibles/benchmarking.rs | 40 +++++-- pallets/api/src/fungibles/mod.rs | 8 +- pallets/api/src/fungibles/weights.rs | 75 +++++++++++++ pallets/api/src/mock.rs | 1 + runtime/devnet/src/config/api.rs | 1 + runtime/devnet/src/lib.rs | 1 + scripts/pallet-weights-template.hbs | 122 ++++++++++++++++++++++ 8 files changed, 241 insertions(+), 20 deletions(-) create mode 100644 pallets/api/src/fungibles/weights.rs create mode 100644 scripts/pallet-weights-template.hbs diff --git a/node/src/command.rs b/node/src/command.rs index 98af5450..8ba956e1 100644 --- a/node/src/command.rs +++ b/node/src/command.rs @@ -64,18 +64,9 @@ impl RuntimeResolver for PathBuf { fn load_spec(id: &str) -> std::result::Result, String> { Ok(match id { - #[cfg(not(feature = "paseo"))] - "dev-rococo" => Box::new(chain_spec::development_config(Relay::RococoLocal)), - #[cfg(feature = "paseo")] - "dev-paseo" => Box::new(chain_spec::development_config(Relay::PaseoLocal)), - #[cfg(not(feature = "paseo"))] - "pop-rococo" => Box::new(chain_spec::testnet_config(Relay::Rococo)), - #[cfg(feature = "paseo")] - "pop-paseo" => Box::new(chain_spec::testnet_config(Relay::Paseo)), - #[cfg(feature = "paseo")] + "dev" | "dev-paseo" => Box::new(chain_spec::development_config(Relay::PaseoLocal)), + "test" | "pop-paseo" => Box::new(chain_spec::testnet_config(Relay::Paseo)), "" | "local" => Box::new(chain_spec::development_config(Relay::PaseoLocal)), - #[cfg(not(feature = "paseo"))] - "" | "local" => Box::new(chain_spec::development_config(Relay::RococoLocal)), path => { let path: PathBuf = path.into(); match path.runtime() { diff --git a/pallets/api/src/fungibles/benchmarking.rs b/pallets/api/src/fungibles/benchmarking.rs index 438aa9b6..6f182932 100644 --- a/pallets/api/src/fungibles/benchmarking.rs +++ b/pallets/api/src/fungibles/benchmarking.rs @@ -1,4 +1,4 @@ -//! Benchmarking setup for pallet-cards +//! Benchmarking setup for pallet-api::fungibles use super::{AccountIdOf, AssetIdOf, AssetsInstanceOf, AssetsOf, BalanceOf, Call, Config, Pallet}; use frame_benchmarking::{account, v2::*}; @@ -17,6 +17,13 @@ use sp_runtime::traits::Zero; const SEED: u32 = 1; +// See if `generic_event` has been emitted. +fn assert_has_event( + generic_event: >>::RuntimeEvent, +) { + frame_system::Pallet::::assert_has_event(generic_event.into()); +} + #[benchmarks( where > as Inspect<::AccountId>>::AssetId: Zero, @@ -28,30 +35,49 @@ mod benchmarks { // current allowance. #[benchmark] fn approve() -> Result<(), BenchmarkError> { - let asset = AssetIdOf::::zero(); + 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); - T::Currency::make_free_balance_be(&owner, 100u32.into()); + T::Currency::make_free_balance_be(&owner, u32::MAX.into()); assert_ok!( as Create>>::create( - asset.clone().into(), + asset_id.clone().into(), owner.clone(), true, min_balance )); assert_ok!( as Mutate>>::approve( - asset.clone(), + asset_id.clone(), &owner, &spender, value )); #[extrinsic_call] - _(RawOrigin::Signed(owner.clone()), asset.clone(), spender.clone(), decreased_value); + _(RawOrigin::Signed(owner.clone()), asset_id.clone(), spender.clone(), decreased_value); - assert_eq!(AssetsOf::::allowance(asset, &owner, &spender), 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(), + ); Ok(()) } diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 4ad2e2d4..d80fc310 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -5,9 +5,12 @@ mod benchmarking; #[cfg(test)] mod tests; +pub mod weights; use frame_support::traits::fungibles::{metadata::Inspect as MetadataInspect, Inspect}; pub use pallet::*; +use pallet_assets::WeightInfo as AssetsWeightInfoTrait; +use weights::WeightInfo; type AccountIdOf = ::AccountId; type AssetIdOf = > as Inspect< @@ -30,7 +33,6 @@ pub mod pallet { traits::fungibles::approvals::Inspect as ApprovalInspect, }; use frame_system::pallet_prelude::*; - use pallet_assets::WeightInfo; use sp_runtime::{traits::StaticLookup, Saturating}; use sp_std::vec::Vec; @@ -64,6 +66,8 @@ pub mod pallet { pub trait Config: frame_system::Config + pallet_assets::Config { /// The instance of pallet assets it is tightly coupled to. type AssetsInstance; + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; } #[pallet::pallet] @@ -97,7 +101,7 @@ 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(T::DbWeight::get().reads(1) + AssetsWeightInfoOf::::cancel_approval() + AssetsWeightInfoOf::::approve_transfer())] + #[pallet::weight(::WeightInfo::approve())] pub fn approve( origin: OriginFor, id: AssetIdOf, diff --git a/pallets/api/src/fungibles/weights.rs b/pallets/api/src/fungibles/weights.rs new file mode 100644 index 00000000..ed2b04fc --- /dev/null +++ b/pallets/api/src/fungibles/weights.rs @@ -0,0 +1,75 @@ +//! 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: `[]` +//! WORST CASE MAP SIZE: `1000000` +//! HOSTNAME: `R0GUE`, CPU: `` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` + +// Executed Command: +// ./target/release/pop-node +// benchmark +// pallet +// --chain=dev +// --wasm-execution=compiled +// --pallet=pallet_api::fungibles +// --steps=20 +// --repeat=5 +// --json +// --template +// ./scripts/pallet-weights-template.hbs +// --output=./pallets/api/src/fungibles/weights.rs +// --extrinsic= + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] +#![allow(missing_docs)] + +use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; +use core::marker::PhantomData; + +/// Weight functions needed for `pallet_api::fungibles`. +pub trait WeightInfo { + fn approve() -> Weight; +} + +/// Weights for `pallet_api::fungibles` using the Substrate node and recommended hardware. +pub struct SubstrateWeight(PhantomData); +impl WeightInfo for SubstrateWeight { + /// Storage: `Assets::Approvals` (r:1 w:1) + /// Proof: `Assets::Approvals` (`max_values`: None, `max_size`: Some(148), added: 2623, mode: `MaxEncodedLen`) + /// Storage: `Assets::Asset` (r:1 w:1) + /// 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 { + // 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)) + } +} + +// For backwards compatibility and tests. +impl WeightInfo for () { + /// Storage: `Assets::Approvals` (r:1 w:1) + /// Proof: `Assets::Approvals` (`max_values`: None, `max_size`: Some(148), added: 2623, mode: `MaxEncodedLen`) + /// Storage: `Assets::Asset` (r:1 w:1) + /// 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 { + // 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)) + } +} + diff --git a/pallets/api/src/mock.rs b/pallets/api/src/mock.rs index 5f0340a9..f5d155ef 100644 --- a/pallets/api/src/mock.rs +++ b/pallets/api/src/mock.rs @@ -98,6 +98,7 @@ impl pallet_assets::Config for Test { } impl crate::fungibles::Config for Test { type AssetsInstance = AssetsInstance; + type WeightInfo = (); } pub(crate) const ALICE: AccountId = 1; diff --git a/runtime/devnet/src/config/api.rs b/runtime/devnet/src/config/api.rs index 1a8925bf..cbf7401b 100644 --- a/runtime/devnet/src/config/api.rs +++ b/runtime/devnet/src/config/api.rs @@ -11,6 +11,7 @@ pub enum RuntimeStateKeys { impl fungibles::Config for Runtime { type AssetsInstance = TrustBackedAssetsInstance; + type WeightInfo = fungibles::weights::SubstrateWeight; } /// A type to identify allowed calls to the Runtime from contracts. Used by Pop API diff --git a/runtime/devnet/src/lib.rs b/runtime/devnet/src/lib.rs index 15971e74..e22201f1 100644 --- a/runtime/devnet/src/lib.rs +++ b/runtime/devnet/src/lib.rs @@ -599,6 +599,7 @@ construct_runtime!( mod benches { frame_benchmarking::define_benchmarks!( [frame_system, SystemBench::] + [pallet_api::fungibles, Fungibles] [pallet_balances, Balances] [pallet_session, SessionBench::] [pallet_timestamp, Timestamp] diff --git a/scripts/pallet-weights-template.hbs b/scripts/pallet-weights-template.hbs new file mode 100644 index 00000000..9e1e5a46 --- /dev/null +++ b/scripts/pallet-weights-template.hbs @@ -0,0 +1,122 @@ +{{header}} +//! Autogenerated weights for `{{pallet}}` +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}} +//! DATE: {{date}}, STEPS: `{{cmd.steps}}`, REPEAT: `{{cmd.repeat}}`, LOW RANGE: `{{cmd.lowest_range_values}}`, HIGH RANGE: `{{cmd.highest_range_values}}` +//! WORST CASE MAP SIZE: `{{cmd.worst_case_map_values}}` +//! HOSTNAME: `R0GUE`, CPU: `{{cpuname}}` +//! WASM-EXECUTION: `{{cmd.wasm_execution}}`, CHAIN: `{{cmd.chain}}`, DB CACHE: `{{cmd.db_cache}}` + +// Executed Command: +{{#each args as |arg|}} +// {{arg}} +{{/each}} + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] +#![allow(missing_docs)] + +use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; +use core::marker::PhantomData; + +/// Weight functions needed for `{{pallet}}`. +pub trait WeightInfo { + {{#each benchmarks as |benchmark|}} + fn {{benchmark.name~}} + ( + {{~#each benchmark.components as |c| ~}} + {{c.name}}: u32, {{/each~}} + ) -> Weight; + {{/each}} +} + +/// Weights for `{{pallet}}` using the Substrate node and recommended hardware. +pub struct SubstrateWeight(PhantomData); +{{#if (eq pallet "frame_system")}} +impl WeightInfo for SubstrateWeight { +{{else}} +impl WeightInfo for SubstrateWeight { +{{/if}} + {{#each benchmarks as |benchmark|}} + {{#each benchmark.comments as |comment|}} + /// {{comment}} + {{/each}} + {{#each benchmark.component_ranges as |range|}} + /// The range of component `{{range.name}}` is `[{{range.min}}, {{range.max}}]`. + {{/each}} + fn {{benchmark.name~}} + ( + {{~#each benchmark.components as |c| ~}} + {{~#if (not c.is_used)}}_{{/if}}{{c.name}}: u32, {{/each~}} + ) -> Weight { + // Proof Size summary in bytes: + // Measured: `{{benchmark.base_recorded_proof_size}}{{#each benchmark.component_recorded_proof_size as |cp|}} + {{cp.name}} * ({{cp.slope}} ±{{underscore cp.error}}){{/each}}` + // Estimated: `{{benchmark.base_calculated_proof_size}}{{#each benchmark.component_calculated_proof_size as |cp|}} + {{cp.name}} * ({{cp.slope}} ±{{underscore cp.error}}){{/each}}` + // Minimum execution time: {{underscore benchmark.min_execution_time}}_000 picoseconds. + Weight::from_parts({{underscore benchmark.base_weight}}, {{benchmark.base_calculated_proof_size}}) + {{#each benchmark.component_weight as |cw|}} + // Standard Error: {{underscore cw.error}} + .saturating_add(Weight::from_parts({{underscore cw.slope}}, 0).saturating_mul({{cw.name}}.into())) + {{/each}} + {{#if (ne benchmark.base_reads "0")}} + .saturating_add(T::DbWeight::get().reads({{benchmark.base_reads}}_u64)) + {{/if}} + {{#each benchmark.component_reads as |cr|}} + .saturating_add(T::DbWeight::get().reads(({{cr.slope}}_u64).saturating_mul({{cr.name}}.into()))) + {{/each}} + {{#if (ne benchmark.base_writes "0")}} + .saturating_add(T::DbWeight::get().writes({{benchmark.base_writes}}_u64)) + {{/if}} + {{#each benchmark.component_writes as |cw|}} + .saturating_add(T::DbWeight::get().writes(({{cw.slope}}_u64).saturating_mul({{cw.name}}.into()))) + {{/each}} + {{#each benchmark.component_calculated_proof_size as |cp|}} + .saturating_add(Weight::from_parts(0, {{cp.slope}}).saturating_mul({{cp.name}}.into())) + {{/each}} + } + {{/each}} +} + +// For backwards compatibility and tests. +impl WeightInfo for () { + {{#each benchmarks as |benchmark|}} + {{#each benchmark.comments as |comment|}} + /// {{comment}} + {{/each}} + {{#each benchmark.component_ranges as |range|}} + /// The range of component `{{range.name}}` is `[{{range.min}}, {{range.max}}]`. + {{/each}} + fn {{benchmark.name~}} + ( + {{~#each benchmark.components as |c| ~}} + {{~#if (not c.is_used)}}_{{/if}}{{c.name}}: u32, {{/each~}} + ) -> Weight { + // Proof Size summary in bytes: + // Measured: `{{benchmark.base_recorded_proof_size}}{{#each benchmark.component_recorded_proof_size as |cp|}} + {{cp.name}} * ({{cp.slope}} ±{{underscore cp.error}}){{/each}}` + // Estimated: `{{benchmark.base_calculated_proof_size}}{{#each benchmark.component_calculated_proof_size as |cp|}} + {{cp.name}} * ({{cp.slope}} ±{{underscore cp.error}}){{/each}}` + // Minimum execution time: {{underscore benchmark.min_execution_time}}_000 picoseconds. + Weight::from_parts({{underscore benchmark.base_weight}}, {{benchmark.base_calculated_proof_size}}) + {{#each benchmark.component_weight as |cw|}} + // Standard Error: {{underscore cw.error}} + .saturating_add(Weight::from_parts({{underscore cw.slope}}, 0).saturating_mul({{cw.name}}.into())) + {{/each}} + {{#if (ne benchmark.base_reads "0")}} + .saturating_add(RocksDbWeight::get().reads({{benchmark.base_reads}}_u64)) + {{/if}} + {{#each benchmark.component_reads as |cr|}} + .saturating_add(RocksDbWeight::get().reads(({{cr.slope}}_u64).saturating_mul({{cr.name}}.into()))) + {{/each}} + {{#if (ne benchmark.base_writes "0")}} + .saturating_add(RocksDbWeight::get().writes({{benchmark.base_writes}}_u64)) + {{/if}} + {{#each benchmark.component_writes as |cw|}} + .saturating_add(RocksDbWeight::get().writes(({{cw.slope}}_u64).saturating_mul({{cw.name}}.into()))) + {{/each}} + {{#each benchmark.component_calculated_proof_size as |cp|}} + .saturating_add(Weight::from_parts(0, {{cp.slope}}).saturating_mul({{cp.name}}.into())) + {{/each}} + } + {{/each}} +} + From f0fff9777950f32ac4fe2a77e526ddefecda565a Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 24 Jul 2024 18:26:39 +0200 Subject: [PATCH 06/11] test: ensure Lookup configuration --- runtime/devnet/src/lib.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/runtime/devnet/src/lib.rs b/runtime/devnet/src/lib.rs index e22201f1..51b42892 100644 --- a/runtime/devnet/src/lib.rs +++ b/runtime/devnet/src/lib.rs @@ -960,3 +960,20 @@ cumulus_pallet_parachain_system::register_validate_block! { Runtime = Runtime, BlockExecutor = cumulus_pallet_aura_ext::BlockExecutor::, } + +#[cfg(test)] +mod tests { + use crate::Runtime; + use std::any::TypeId; + + #[test] + fn test_lookup_config() { + type ExpectedLookup = sp_runtime::traits::AccountIdLookup; + type ConfigLookup = ::Lookup; + + let expected_type_id = TypeId::of::(); + let config_type_id = TypeId::of::(); + + assert_eq!(config_type_id, expected_type_id); + } +} From 4a476b01790f06c21d111fbc7fd697078c2cd49e Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 24 Jul 2024 18:26:06 +0200 Subject: [PATCH 07/11] fix: approve benchmark + weights --- pallets/api/src/fungibles/benchmarking.rs | 78 ++++++++++++++--------- pallets/api/src/fungibles/mod.rs | 35 +++++----- pallets/api/src/fungibles/weights.rs | 51 ++++++++++----- 3 files changed, 101 insertions(+), 63 deletions(-) diff --git a/pallets/api/src/fungibles/benchmarking.rs b/pallets/api/src/fungibles/benchmarking.rs index 6f182932..bb225f20 100644 --- a/pallets/api/src/fungibles/benchmarking.rs +++ b/pallets/api/src/fungibles/benchmarking.rs @@ -31,19 +31,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 +53,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())) } } From f3cc83a056ef5673c054c3a93552296762b335c7 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 25 Jul 2024 14:37:50 +0200 Subject: [PATCH 08/11] refactor: primitives --- Cargo.lock | 21 +++++++++---------- Cargo.toml | 2 +- pallets/api/Cargo.toml | 2 -- pop-api/Cargo.toml | 4 ++-- pop-api/integration-tests/Cargo.toml | 4 ++-- .../integration-tests/src/local_fungibles.rs | 2 +- pop-api/src/primitives.rs | 2 +- primitives/Cargo.toml | 3 ++- primitives/README.md | 1 + runtime/devnet/Cargo.toml | 4 ++-- runtime/devnet/src/extensions/mod.rs | 4 ++-- runtime/testnet/Cargo.toml | 4 ++-- 12 files changed, 26 insertions(+), 27 deletions(-) create mode 100644 primitives/README.md diff --git a/Cargo.lock b/Cargo.lock index c0dfbaac..c8f0a6c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6614,7 +6614,6 @@ dependencies = [ "pallet-assets", "pallet-balances", "parity-scale-codec", - "primitives", "scale-info", "sp-core", "sp-io", @@ -9699,6 +9698,14 @@ dependencies = [ "substrate-prometheus-endpoint", ] +[[package]] +name = "pop-primitives" +version = "0.0.0" +dependencies = [ + "parity-scale-codec", + "scale-info", +] + [[package]] name = "pop-runtime-common" version = "0.0.0" @@ -9762,8 +9769,8 @@ dependencies = [ "parity-scale-codec", "polkadot-parachain-primitives", "polkadot-runtime-common", + "pop-primitives", "pop-runtime-common", - "primitives", "rand", "scale-info", "smallvec", @@ -9836,8 +9843,8 @@ dependencies = [ "parity-scale-codec", "polkadot-parachain-primitives", "polkadot-runtime-common", + "pop-primitives", "pop-runtime-common", - "primitives", "scale-info", "smallvec", "sp-api", @@ -9943,14 +9950,6 @@ dependencies = [ "uint", ] -[[package]] -name = "primitives" -version = "0.0.0" -dependencies = [ - "parity-scale-codec", - "scale-info", -] - [[package]] name = "prioritized-metered-channel" version = "0.6.1" diff --git a/Cargo.toml b/Cargo.toml index 79bf5ebe..8b2d0a8f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,7 +56,7 @@ pallet-api = { path = "pallets/api", default-features = false } pop-runtime-devnet = { path = "runtime/devnet", default-features = true } # default-features=true required for `-p pop-node` builds pop-runtime-testnet = { path = "runtime/testnet", default-features = true } # default-features=true required for `-p pop-node` builds pop-runtime-common = { path = "runtime/common", default-features = false } -primitives = { path = "./primitives", default-features = false } +pop-primitives = { path = "./primitives", default-features = false } # Substrate sc-basic-authorship = "0.35.0" diff --git a/pallets/api/Cargo.toml b/pallets/api/Cargo.toml index 0965405d..a813a09c 100644 --- a/pallets/api/Cargo.toml +++ b/pallets/api/Cargo.toml @@ -18,7 +18,6 @@ frame-benchmarking.workspace = true frame-support.workspace = true frame-system.workspace = true pallet-assets.workspace = true -primitives.workspace = true sp-runtime.workspace = true sp-std.workspace = true @@ -43,7 +42,6 @@ std = [ "frame-system/std", "pallet-assets/std", "pallet-balances/std", - "primitives/std", "scale-info/std", "sp-core/std", "sp-io/std", diff --git a/pop-api/Cargo.toml b/pop-api/Cargo.toml index 5638b9d2..0f878b4b 100644 --- a/pop-api/Cargo.toml +++ b/pop-api/Cargo.toml @@ -10,7 +10,7 @@ enumflags2 = { version = "0.7.7" } ink = { version = "5.0.0", default-features = false } sp-io = { version = "31.0.0", default-features = false, features = ["disable_panic_handler", "disable_oom", "disable_allocator"] } -primitives = { path = "../primitives", default-features = false } +pop-primitives = { path = "../primitives", default-features = false } [lib] name = "pop_api" @@ -22,7 +22,7 @@ default = ["std"] std = [ "enumflags2/std", "ink/std", - "primitives/std", + "pop-primitives/std", "sp-io/std", ] assets = [] diff --git a/pop-api/integration-tests/Cargo.toml b/pop-api/integration-tests/Cargo.toml index 52b3b634..cc56630a 100644 --- a/pop-api/integration-tests/Cargo.toml +++ b/pop-api/integration-tests/Cargo.toml @@ -11,7 +11,7 @@ frame-system = { version = "29.0.0", default-features = false } pallet-balances = { version = "29.0.2", default-features = false } pallet-assets = { version = "30.0.0", default-features = false } pallet-contracts = { version = "28.0.0", default-features = false } -primitives = { path = "../../primitives", default-features = false } +pop-primitives = { path = "../../primitives", default-features = false } pop-runtime-devnet = { path = "../../runtime/devnet", default-features = false } sp-io = { version = "31.0.0", default-features = false } sp-runtime = { version = "32.0.0", default-features = false } @@ -25,7 +25,7 @@ std = [ "pallet-balances/std", "pallet-assets/std", "pallet-contracts/std", - "primitives/std", + "pop-primitives/std", "pop-runtime-devnet/std", "scale/std", "sp-io/std", diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index a8511ffb..7571a0ec 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -1,5 +1,5 @@ use super::*; -use primitives::error::{ +use pop_primitives::error::{ ArithmeticError::*, Error::{self, *}, TokenError::*, diff --git a/pop-api/src/primitives.rs b/pop-api/src/primitives.rs index 34d7c38d..a3d596a5 100644 --- a/pop-api/src/primitives.rs +++ b/pop-api/src/primitives.rs @@ -1,5 +1,5 @@ use ink::env::{DefaultEnvironment, Environment}; -pub use primitives::*; +pub use pop_primitives::*; pub(crate) type AccountId = ::AccountId; pub(crate) type Balance = ::Balance; diff --git a/primitives/Cargo.toml b/primitives/Cargo.toml index cb62ddc0..e7d55ffe 100644 --- a/primitives/Cargo.toml +++ b/primitives/Cargo.toml @@ -1,5 +1,6 @@ [package] -name = "primitives" +name = "pop-primitives" +description = "Primitives crate for Pop" license = "GPL-3.0-only" version = "0.0.0" edition = "2021" diff --git a/primitives/README.md b/primitives/README.md new file mode 100644 index 00000000..ded7918a --- /dev/null +++ b/primitives/README.md @@ -0,0 +1 @@ +Reserved crate for pop-primitives. \ No newline at end of file diff --git a/runtime/devnet/Cargo.toml b/runtime/devnet/Cargo.toml index 113f1fe1..46d23284 100644 --- a/runtime/devnet/Cargo.toml +++ b/runtime/devnet/Cargo.toml @@ -22,7 +22,7 @@ scale-info.workspace = true smallvec.workspace = true # Local -primitives.workspace = true +pop-primitives.workspace = true pop-runtime-common.workspace = true pallet-api.workspace = true @@ -140,7 +140,7 @@ std = [ "parachains-common/std", "polkadot-parachain-primitives/std", "polkadot-runtime-common/std", - "primitives/std", + "pop-primitives/std", "scale-info/std", "sp-api/std", "sp-io/std", diff --git a/runtime/devnet/src/extensions/mod.rs b/runtime/devnet/src/extensions/mod.rs index 27af328a..4b1c1064 100644 --- a/runtime/devnet/src/extensions/mod.rs +++ b/runtime/devnet/src/extensions/mod.rs @@ -20,7 +20,7 @@ use frame_support::{ use pallet_contracts::chain_extension::{ BufInBufOutState, ChainExtension, Environment, Ext, InitState, RetVal, }; -use primitives::AssetId; +use pop_primitives::AssetId; use sp_core::crypto::UncheckedFrom; use sp_runtime::{traits::Dispatchable, DispatchError}; use sp_std::vec::Vec; @@ -208,7 +208,7 @@ enum VersionedDispatch { // Converts a `DispatchError` to a `u32` status code based on the version of the API the contract uses. // The contract calling the chain extension can convert the status code to the descriptive `Error`. // -// For `Error` see `primitives::::error::Error`. +// For `Error` see `pop_primitives::::error::Error`. // // The error encoding can vary per version, allowing for flexible and backward-compatible error handling. // As a result, contracts maintain compatibility across different versions of the runtime. diff --git a/runtime/testnet/Cargo.toml b/runtime/testnet/Cargo.toml index f269bed8..79ea9cac 100644 --- a/runtime/testnet/Cargo.toml +++ b/runtime/testnet/Cargo.toml @@ -22,7 +22,7 @@ scale-info.workspace = true smallvec.workspace = true # Local -primitives.workspace = true +pop-primitives.workspace = true pop-runtime-common.workspace = true # Substrate @@ -137,7 +137,7 @@ std = [ "parachains-common/std", "polkadot-parachain-primitives/std", "polkadot-runtime-common/std", - "primitives/std", + "pop-primitives/std", "scale-info/std", "sp-api/std", "sp-io/std", From bd8548976a082b60fd1f500894159fcb218094cc Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 25 Jul 2024 14:58:51 +0200 Subject: [PATCH 09/11] refactor: bits and pieces --- pallets/api/src/fungibles/tests.rs | 6 ++-- .../integration-tests/src/local_fungibles.rs | 33 +++++++++---------- pop-api/src/lib.rs | 4 +-- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 3f464ee8..e42283d1 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -11,10 +11,10 @@ fn transfer_works() { new_test_ext().execute_with(|| { let amount: Balance = 100 * UNIT; create_asset_and_mint_to(ALICE, ASSET, ALICE, amount); - let bob_balance_before_transfer = Assets::balance(ASSET, &BOB); + let balance_before_transfer = Assets::balance(ASSET, &BOB); assert_ok!(Fungibles::transfer(signed(ALICE), ASSET, BOB, amount / 2)); - let bob_balance_after_transfer = Assets::balance(ASSET, &BOB); - assert_eq!(bob_balance_after_transfer, bob_balance_before_transfer + amount / 2); + let balance_after_transfer = Assets::balance(ASSET, &BOB); + assert_eq!(balance_after_transfer, balance_before_transfer + amount / 2); }); } diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 7571a0ec..c62f0713 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -6,6 +6,7 @@ use pop_primitives::error::{ }; const ASSET_ID: AssetId = 1; +const CONTRACT: &str = "contracts/fungibles/target/ink/fungibles.wasm"; fn decoded(result: ExecReturnValue) -> T { match ::decode(&mut &result.data[2..]) { @@ -263,7 +264,7 @@ fn token_decimals_asset(asset_id: AssetId) -> u8 { fn total_supply_works() { new_test_ext().execute_with(|| { let _ = env_logger::try_init(); - let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]); + let addr = instantiate(CONTRACT, INIT_VALUE, vec![]); // No tokens in circulation. assert_eq!(Assets::total_supply(ASSET_ID), total_supply(addr.clone(), ASSET_ID)); @@ -280,7 +281,7 @@ fn total_supply_works() { fn balance_of_works() { new_test_ext().execute_with(|| { let _ = env_logger::try_init(); - let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]); + let addr = instantiate(CONTRACT, INIT_VALUE, vec![]); // No tokens in circulation. assert_eq!(Assets::balance(ASSET_ID, BOB), balance_of(addr.clone(), ASSET_ID, BOB)); @@ -297,7 +298,7 @@ fn balance_of_works() { fn allowance_works() { new_test_ext().execute_with(|| { let _ = env_logger::try_init(); - let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]); + let addr = instantiate(CONTRACT, INIT_VALUE, vec![]); // No tokens in circulation. assert_eq!( @@ -320,7 +321,7 @@ fn allowance_works() { fn transfer_works() { new_test_ext().execute_with(|| { let _ = env_logger::try_init(); - let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]); + let addr = instantiate(CONTRACT, INIT_VALUE, vec![]); let amount: Balance = 100 * UNIT; // Asset does not exist. @@ -371,7 +372,7 @@ fn transfer_works() { fn approve_works() { new_test_ext().execute_with(|| { let _ = env_logger::try_init(); - let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", 0, vec![]); + let addr = instantiate(CONTRACT, 0, vec![]); let amount: Balance = 100 * UNIT; // Asset does not exist. assert_eq!( @@ -381,8 +382,7 @@ fn approve_works() { let asset = create_asset_and_mint_to(ALICE, 0, addr.clone(), amount); assert_eq!(decoded::(approve(addr.clone(), asset, BOB, amount)), ConsumerRemaining); - let addr = - instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![1]); + let addr = instantiate(CONTRACT, INIT_VALUE, vec![1]); // Create asset with Alice as owner and mint `amount` to contract address. let asset = create_asset_and_mint_to(ALICE, 1, addr.clone(), amount); // Asset is not live, i.e. frozen or being destroyed. @@ -412,7 +412,7 @@ fn approve_works() { fn increase_allowance_works() { new_test_ext().execute_with(|| { let _ = env_logger::try_init(); - let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", 0, vec![]); + let addr = instantiate(CONTRACT, 0, vec![]); let amount: Balance = 100 * UNIT; // Asset does not exist. assert_eq!( @@ -425,8 +425,7 @@ fn increase_allowance_works() { ConsumerRemaining ); - let addr = - instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![1]); + let addr = instantiate(CONTRACT, INIT_VALUE, vec![1]); // Create asset with Alice as owner and mint `amount` to contract address. let asset = create_asset_and_mint_to(ALICE, 1, addr.clone(), amount); // Asset is not live, i.e. frozen or being destroyed. @@ -467,7 +466,7 @@ fn increase_allowance_works() { fn token_metadata_works() { new_test_ext().execute_with(|| { let _ = env_logger::try_init(); - let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]); + let addr = instantiate(CONTRACT, INIT_VALUE, vec![]); let name: Vec = vec![11, 12, 13]; let symbol: Vec = vec![21, 22, 23]; @@ -502,7 +501,7 @@ fn token_metadata_works() { // new_test_ext().execute_with(|| { // let _ = env_logger::try_init(); // let addr = -// instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]); +// instantiate(CONTRACT, INIT_VALUE, vec![]); // // // No tokens in circulation. // assert_eq!(Assets::asset_exists(ASSET_ID), asset_exists(addr.clone(), ASSET_ID)); @@ -519,7 +518,7 @@ fn token_metadata_works() { // new_test_ext().execute_with(|| { // let _ = env_logger::try_init(); // let addr = -// instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]); +// instantiate(CONTRACT, INIT_VALUE, vec![]); // let amount: Balance = 100 * UNIT; // // // Asset does not exist. @@ -579,14 +578,14 @@ fn token_metadata_works() { // new_test_ext().execute_with(|| { // let _ = env_logger::try_init(); // // Instantiate a contract without balance (relay token). -// let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", 0, vec![0]); +// let addr = instantiate(CONTRACT, 0, vec![0]); // // No balance to pay for fees. // assert_eq!( // decoded::(create(addr.clone(), ASSET_ID, addr.clone(), 1)), // Module { index: 10, error: 2 }, // ); // // Instantiate a contract without balance (relay token). -// let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", 100, vec![2]); +// let addr = instantiate(CONTRACT, 100, vec![2]); // // No balance to pay the deposit. // assert_eq!( // decoded::(create(addr.clone(), ASSET_ID, addr.clone(), 1)), @@ -594,7 +593,7 @@ fn token_metadata_works() { // ); // // Instantiate a contract with balance. // let addr = -// instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![1]); +// instantiate(CONTRACT, INIT_VALUE, vec![1]); // assert_eq!( // decoded::(create(addr.clone(), ASSET_ID, BOB, 0)), // Module { index: 52, error: 7 }, @@ -618,7 +617,7 @@ fn token_metadata_works() { // new_test_ext().execute_with(|| { // let _ = env_logger::try_init(); // let addr = -// instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]); +// instantiate(CONTRACT, INIT_VALUE, vec![]); // // create_asset(addr.clone(), ASSET_ID, 1); // let result = set_metadata(addr.clone(), ASSET_ID, vec![12], vec![12], 12); diff --git a/pop-api/src/lib.rs b/pop-api/src/lib.rs index d2470a92..a984bb9e 100644 --- a/pop-api/src/lib.rs +++ b/pop-api/src/lib.rs @@ -2,6 +2,7 @@ use constants::DECODING_FAILED; use ink::env::chain_extension::FromStatusCode; +#[cfg(feature = "assets")] pub use v0::assets; pub mod primitives; @@ -13,8 +14,7 @@ pub type Result = core::result::Result; mod constants { // Errors: pub(crate) const DECODING_FAILED: u32 = 255; - // TODO: Not used but will be used in the future when the remaining fungibles features will be - // implemented. + // TODO: will be used in the future when the remaining fungibles features will be implemented. pub(crate) const _MODULE_ERROR: u8 = 3; // Function IDs: From 0e49f6309553ea596c7e576979166674483ed682 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 25 Jul 2024 15:43:53 +0200 Subject: [PATCH 10/11] fix: test devnet --- runtime/devnet/src/extensions/v0.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/devnet/src/extensions/v0.rs b/runtime/devnet/src/extensions/v0.rs index d9a40264..b26668f7 100644 --- a/runtime/devnet/src/extensions/v0.rs +++ b/runtime/devnet/src/extensions/v0.rs @@ -60,7 +60,7 @@ fn nested_errors(nested_error: &[u8], limit: Option) -> bool { #[cfg(test)] mod tests { use super::*; - use primitives::error::{ + use pop_primitives::error::{ ArithmeticError::*, Error::{self, *}, TokenError::*, From 8f6139c577a2a2916780cb66229eec6de3d8c1ed Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Fri, 26 Jul 2024 00:58:38 +0200 Subject: [PATCH 11/11] refactor: apply final comments --- Cargo.lock | 2 -- pallets/api/src/fungibles/benchmarking.rs | 4 +-- pallets/api/src/fungibles/mod.rs | 35 ++++++++++++++++------- pallets/api/src/fungibles/tests.rs | 3 ++ pop-api/Cargo.toml | 2 -- runtime/devnet/Cargo.toml | 1 - runtime/devnet/src/config/api.rs | 4 ++- runtime/devnet/src/extensions/mod.rs | 22 ++++++-------- runtime/devnet/src/lib.rs | 2 ++ runtime/testnet/Cargo.toml | 1 - 10 files changed, 44 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c8f0a6c9..d3ca53e7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9731,7 +9731,6 @@ dependencies = [ "cumulus-primitives-aura", "cumulus-primitives-core", "cumulus-primitives-utility", - "enumflags2", "env_logger 0.11.3", "frame-benchmarking", "frame-executive", @@ -9806,7 +9805,6 @@ dependencies = [ "cumulus-primitives-aura", "cumulus-primitives-core", "cumulus-primitives-utility", - "enumflags2", "env_logger 0.11.3", "frame-benchmarking", "frame-executive", diff --git a/pallets/api/src/fungibles/benchmarking.rs b/pallets/api/src/fungibles/benchmarking.rs index bb225f20..d3d65b97 100644 --- a/pallets/api/src/fungibles/benchmarking.rs +++ b/pallets/api/src/fungibles/benchmarking.rs @@ -32,8 +32,8 @@ mod benchmarks { use super::*; // Parameter: - // - 'a': whether `approve_transfer` has been called. - // - 'c': whether `cancel_approval` has been called. + // - 'a': whether `approve_transfer` is required. + // - 'c': whether `cancel_approval` is required. #[benchmark] fn approve(a: Linear<0, 1>, c: Linear<0, 1>) -> Result<(), BenchmarkError> { let asset_id = AssetIdOf::::zero(); diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 3067e1d3..cd34664a 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -1,6 +1,7 @@ /// The fungibles pallet serves as a wrapper around the pallet_assets, offering a streamlined /// interface for interacting with fungible assets. The goal is to provide a simplified, consistent /// API that adheres to standards in the smart contract space. + #[cfg(feature = "runtime-benchmarks")] mod benchmarking; #[cfg(test)] @@ -49,10 +50,22 @@ pub mod pallet { TotalSupply(AssetIdOf), /// Account balance for a given asset ID. #[codec(index = 1)] - BalanceOf(AssetIdOf, AccountIdOf), + BalanceOf { + /// The asset ID. + id: AssetIdOf, + /// The account ID of the owner. + owner: AccountIdOf, + }, /// Allowance for a spender approved by an owner, for a given asset ID. #[codec(index = 2)] - Allowance(AssetIdOf, AccountIdOf, AccountIdOf), + Allowance { + /// The asset ID. + id: AssetIdOf, + /// The account ID of the owner. + owner: AccountIdOf, + /// The account ID of the spender. + spender: AccountIdOf, + }, /// Token name for a given asset ID. #[codec(index = 8)] TokenName(AssetIdOf), @@ -69,7 +82,7 @@ pub mod pallet { pub trait Config: frame_system::Config + pallet_assets::Config { /// The instance of pallet assets it is tightly coupled to. type AssetsInstance; - /// Weight information for extrinsics in this pallet. + /// Weight information for dispatchables in this pallet. type WeightInfo: WeightInfo; } @@ -118,13 +131,14 @@ pub mod pallet { 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(Some(weight(0, 0)).into()); + let return_weight = if value == current_allowance { + weight(0, 0) } // If the new value is greater than the current allowance, approve the difference - // because `approve_transfer` works additively (see pallet-assets). - if value > current_allowance { + // because `approve_transfer` works additively (see `pallet-assets`). + else if value > current_allowance { AssetsOf::::approve_transfer( origin, id, @@ -132,7 +146,7 @@ pub mod pallet { value.saturating_sub(current_allowance), ) .map_err(|e| e.with_weight(weight(1, 0)))?; - Ok(Some(weight(1, 0)).into()) + weight(1, 0) } 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()) @@ -141,8 +155,9 @@ pub mod pallet { return Ok(Some(weight(0, 1)).into()); } AssetsOf::::approve_transfer(origin, id, spender, value)?; - Ok(().into()) - } + weight(1, 1) + }; + Ok(Some(return_weight).into()) } /// Increases the allowance of a spender. diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index e42283d1..dbfa0b34 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -33,6 +33,9 @@ fn approve_works() { // Approves an amount to spend that is higher than the current allowance. assert_ok!(Fungibles::approve(signed(ALICE), ASSET, BOB, amount * 2)); assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount * 2); + // Approves an amount to spend that is equal to the current allowance. + assert_ok!(Fungibles::approve(signed(ALICE), ASSET, BOB, amount * 2)); + assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount * 2); // Sets allowance to zero. assert_ok!(Fungibles::approve(signed(ALICE), ASSET, BOB, 0)); assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), 0); diff --git a/pop-api/Cargo.toml b/pop-api/Cargo.toml index 0f878b4b..9946abf7 100644 --- a/pop-api/Cargo.toml +++ b/pop-api/Cargo.toml @@ -6,7 +6,6 @@ version = "0.0.0" edition = "2021" [dependencies] -enumflags2 = { version = "0.7.7" } ink = { version = "5.0.0", default-features = false } sp-io = { version = "31.0.0", default-features = false, features = ["disable_panic_handler", "disable_oom", "disable_allocator"] } @@ -20,7 +19,6 @@ crate-type = ["rlib"] [features] default = ["std"] std = [ - "enumflags2/std", "ink/std", "pop-primitives/std", "sp-io/std", diff --git a/runtime/devnet/Cargo.toml b/runtime/devnet/Cargo.toml index 46d23284..455a86fd 100644 --- a/runtime/devnet/Cargo.toml +++ b/runtime/devnet/Cargo.toml @@ -90,7 +90,6 @@ parachain-info.workspace = true [dev-dependencies] env_logger = "0.11.2" -enumflags2 = "0.7.9" hex = "0.4.3" rand = "0.8.5" diff --git a/runtime/devnet/src/config/api.rs b/runtime/devnet/src/config/api.rs index cbf7401b..ae179e4a 100644 --- a/runtime/devnet/src/config/api.rs +++ b/runtime/devnet/src/config/api.rs @@ -2,9 +2,11 @@ use crate::{config::assets::TrustBackedAssetsInstance, fungibles, Runtime, Runti use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::traits::Contains; +/// A query of runtime state. #[derive(Encode, Decode, Debug, MaxEncodedLen)] #[repr(u8)] -pub enum RuntimeStateKeys { +pub enum RuntimeRead { + /// Fungible token queries. #[codec(index = 150)] Fungibles(fungibles::Read), } diff --git a/runtime/devnet/src/extensions/mod.rs b/runtime/devnet/src/extensions/mod.rs index 4b1c1064..3aed89df 100644 --- a/runtime/devnet/src/extensions/mod.rs +++ b/runtime/devnet/src/extensions/mod.rs @@ -2,7 +2,7 @@ mod v0; use crate::{ config::{ - api::{AllowedApiCalls, RuntimeStateKeys}, + api::{AllowedApiCalls, RuntimeRead}, assets::TrustBackedAssetsInstance, }, fungibles::{ @@ -178,7 +178,7 @@ where let result = match key { VersionedStateRead::V0(key) => match key { - RuntimeStateKeys::Fungibles(key) => read_fungibles_state::(key, env), + RuntimeRead::Fungibles(key) => read_fungibles_state::(key, env), }, }? .encode(); @@ -189,18 +189,18 @@ where env.write(&result, false, None) } -/// Wrapper to enable versioning of `RuntimeStateKeys`. +/// Wrapper to enable versioning of runtime state reads. #[derive(Decode, Debug)] enum VersionedStateRead { - /// Version zero of reading state from api. + /// Version zero of state reads. #[codec(index = 0)] - V0(RuntimeStateKeys), + V0(RuntimeRead), } -/// Wrapper to enable versioning of `RuntimeCall`. +/// Wrapper to enable versioning of runtime calls. #[derive(Decode, Debug)] enum VersionedDispatch { - /// Version zero of dispatch calls from api. + /// Version zero of dispatch calls. #[codec(index = 0)] V0(RuntimeCall), } @@ -299,17 +299,13 @@ where env.charge_weight(T::DbWeight::get().reads(1_u64))?; match key { TotalSupply(id) => Ok(fungibles::Pallet::::total_supply(id).encode()), - BalanceOf(id, owner) => Ok(fungibles::Pallet::::balance_of(id, &owner).encode()), - Allowance(id, owner, spender) => { + BalanceOf { id, owner } => Ok(fungibles::Pallet::::balance_of(id, &owner).encode()), + Allowance { id, owner, spender } => { Ok(fungibles::Pallet::::allowance(id, &owner, &spender).encode()) }, TokenName(id) => Ok(fungibles::Pallet::::token_name(id).encode()), TokenSymbol(id) => Ok(fungibles::Pallet::::token_symbol(id).encode()), TokenDecimals(id) => Ok(fungibles::Pallet::::token_decimals(id).encode()), - // AssetsKeys::AssetExists(id) => { - // env.charge_weight(T::DbWeight::get().reads(1_u64))?; - // Ok(pallet_assets::Pallet::::asset_exists(id).encode()) - // }, } } diff --git a/runtime/devnet/src/lib.rs b/runtime/devnet/src/lib.rs index 51b42892..23895310 100644 --- a/runtime/devnet/src/lib.rs +++ b/runtime/devnet/src/lib.rs @@ -966,6 +966,8 @@ mod tests { use crate::Runtime; use std::any::TypeId; + // Ensures that the account id lookup does not perform any state reads. When this changes, + // `pallet_api::fungibles` dispatchables need to be re-evaluated. #[test] fn test_lookup_config() { type ExpectedLookup = sp_runtime::traits::AccountIdLookup; diff --git a/runtime/testnet/Cargo.toml b/runtime/testnet/Cargo.toml index 79ea9cac..51cab6d6 100644 --- a/runtime/testnet/Cargo.toml +++ b/runtime/testnet/Cargo.toml @@ -90,7 +90,6 @@ parachain-info.workspace = true [dev-dependencies] env_logger = "0.11.2" hex = "0.4.3" -enumflags2 = "0.7.9" [features] default = ["std"]