diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 4c840628..4ba587cf 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -17,6 +17,7 @@ type AccountIdOf = ::AccountId; type TokenIdOf = as Inspect<::AccountId>>::AssetId; type TokenIdParameterOf = >>::AssetIdParameter; type AssetsOf = pallet_assets::Pallet>; +type AssetsErrorOf = pallet_assets::Error>; type AssetsInstanceOf = ::AssetsInstance; type AssetsWeightInfoOf = >>::WeightInfo; type BalanceOf = as Inspect<::AccountId>>::Balance; @@ -33,7 +34,7 @@ pub mod pallet { }; use frame_system::pallet_prelude::*; use sp_runtime::{ - traits::{StaticLookup, Zero}, + traits::{CheckedSub, StaticLookup, Zero}, Saturating, }; use sp_std::vec::Vec; @@ -338,13 +339,14 @@ pub mod pallet { let token_param: TokenIdParameterOf = token.clone().into(); // Cancel the approval and approve `new_allowance` if difference is more than zero. + let new_allowance = + current_allowance.checked_sub(&value).ok_or(AssetsErrorOf::::Unapproved)?; AssetsOf::::cancel_approval( origin.clone(), token_param.clone(), spender_source.clone(), ) .map_err(|e| e.with_weight(WeightOf::::approve(0, 1)))?; - let new_allowance = current_allowance.saturating_sub(value); let weight = if new_allowance.is_zero() { WeightOf::::approve(0, 1) } else { @@ -460,13 +462,18 @@ pub mod pallet { /// - `account` - The account from which the tokens will be destroyed. /// - `value` - The number of tokens to destroy. #[pallet::call_index(20)] - #[pallet::weight(AssetsWeightInfoOf::::burn())] + #[pallet::weight(::WeightInfo::balance_of() + AssetsWeightInfoOf::::burn())] pub fn burn( origin: OriginFor, token: TokenIdOf, account: AccountIdOf, value: BalanceOf, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { + let current_balance = AssetsOf::::balance(token.clone(), &account); + if current_balance < value { + return Err(AssetsErrorOf::::BalanceLow + .with_weight(::WeightInfo::balance_of())); + } AssetsOf::::burn( origin, token.clone().into(), @@ -474,7 +481,7 @@ pub mod pallet { value, )?; Self::deposit_event(Event::Transfer { token, from: Some(account), to: None, value }); - Ok(()) + Ok(().into()) } } diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 2e430ca8..051eb06d 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -272,20 +272,28 @@ fn decrease_allowance_works() { BadOrigin.with_weight(WeightInfo::approve(0, 0)) ); } + assets::create_mint_and_approve(owner, token, owner, value, spender, value); + assert_eq!(Assets::allowance(token, &owner, &spender), value); // Check error works for `Assets::cancel_approval()`. No error test for `approve_transfer` // because it is not possible. + assert_ok!(Assets::freeze_asset(signed(owner), token)); assert_noop!( Fungibles::decrease_allowance(signed(owner), token, spender, value / 2), - AssetsError::Unknown.with_weight(WeightInfo::approve(0, 1)) + AssetsError::AssetNotLive.with_weight(WeightInfo::approve(0, 1)) ); - assets::create_mint_and_approve(owner, token, owner, value, spender, value); - assert_eq!(Assets::allowance(token, &owner, &spender), value); + assert_ok!(Assets::thaw_asset(signed(owner), token)); // Owner balance is not changed if decreased by zero. assert_eq!( Fungibles::decrease_allowance(signed(owner), token, spender, 0), Ok(Some(WeightInfo::approve(0, 0)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), value); + // "Unapproved" error is returned if the current allowance is less than amount to decrease + // with. + assert_noop!( + Fungibles::decrease_allowance(signed(owner), token, spender, value * 2), + AssetsError::Unapproved + ); // Decrease allowance successfully. assert_eq!( Fungibles::decrease_allowance(signed(owner), token, spender, value / 2), @@ -295,13 +303,6 @@ fn decrease_allowance_works() { System::assert_last_event( Event::Approval { token, owner, spender, value: value / 2 }.into(), ); - // Saturating if current allowance is decreased more than the owner balance. - assert_eq!( - Fungibles::decrease_allowance(signed(owner), token, spender, value), - Ok(Some(WeightInfo::approve(0, 1)).into()) - ); - assert_eq!(Assets::allowance(token, &owner, &spender), 0); - System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); }); } @@ -416,10 +417,22 @@ fn burn_works() { let from = BOB; let total_supply = value * 2; - // Check error works for `Assets::burn()`. - assert_noop!(Fungibles::burn(signed(owner), token, from, value), AssetsError::Unknown); + // "BalanceLow" error is returned if token is not created. + assert_noop!( + Fungibles::burn(signed(owner), token, from, value), + AssetsError::BalanceLow.with_weight(WeightInfo::balance_of()) + ); assets::create_and_mint_to(owner, token, from, total_supply); assert_eq!(Assets::total_supply(TOKEN), total_supply); + // Check error works for `Assets::burn()`. + assert_ok!(Assets::freeze_asset(signed(owner), token)); + assert_noop!(Fungibles::burn(signed(owner), token, from, value), AssetsError::AssetNotLive); + assert_ok!(Assets::thaw_asset(signed(owner), token)); + // "BalanceLow" error is returned if the balance is less than amount to burn. + assert_noop!( + Fungibles::burn(signed(owner), token, from, total_supply * 2), + AssetsError::BalanceLow.with_weight(WeightInfo::balance_of()) + ); let balance_before_burn = Assets::balance(token, &from); assert_ok!(Fungibles::burn(signed(owner), token, from, value)); assert_eq!(Assets::total_supply(TOKEN), total_supply - value); @@ -677,8 +690,8 @@ mod read_weights { } mod ensure_codec_indexes { - use super::{Encode, RuntimeCall, *}; - use crate::{fungibles, fungibles::Call::*, mock::RuntimeCall::Fungibles}; + use super::{Encode, *}; + use crate::{fungibles, mock::RuntimeCall::Fungibles}; #[test] fn ensure_read_variant_indexes() { diff --git a/pallets/api/src/fungibles/weights.rs b/pallets/api/src/fungibles/weights.rs index 28c5a6b5..0957b1e0 100644 --- a/pallets/api/src/fungibles/weights.rs +++ b/pallets/api/src/fungibles/weights.rs @@ -2,7 +2,7 @@ //! Autogenerated weights for `fungibles` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 40.0.0 -//! DATE: 2024-09-13, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-10-01, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `R0GUE`, CPU: `` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` @@ -57,12 +57,12 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `413 + c * (102 ±0)` // Estimated: `3675` - // Minimum execution time: 20_000_000 picoseconds. - Weight::from_parts(1_473_469, 3675) - // Standard Error: 89_329 - .saturating_add(Weight::from_parts(19_606_122, 0).saturating_mul(a.into())) - // Standard Error: 89_329 - .saturating_add(Weight::from_parts(30_920_408, 0).saturating_mul(c.into())) + // Minimum execution time: 26_000_000 picoseconds. + Weight::from_parts(2_877_551, 3675) + // Standard Error: 100_168 + .saturating_add(Weight::from_parts(24_846_938, 0).saturating_mul(a.into())) + // Standard Error: 100_168 + .saturating_add(Weight::from_parts(38_375_510, 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)) @@ -74,7 +74,7 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3675` - // Minimum execution time: 1_000_000 picoseconds. + // Minimum execution time: 2_000_000 picoseconds. Weight::from_parts(2_000_000, 3675) .saturating_add(T::DbWeight::get().reads(1_u64)) } @@ -84,7 +84,7 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3599` - // Minimum execution time: 2_000_000 picoseconds. + // Minimum execution time: 3_000_000 picoseconds. Weight::from_parts(3_000_000, 3599) .saturating_add(T::DbWeight::get().reads(1_u64)) } @@ -94,8 +94,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3613` - // Minimum execution time: 4_000_000 picoseconds. - Weight::from_parts(4_000_000, 3613) + // Minimum execution time: 5_000_000 picoseconds. + Weight::from_parts(5_000_000, 3613) .saturating_add(T::DbWeight::get().reads(1_u64)) } /// Storage: `Assets::Metadata` (r:1 w:0) @@ -104,7 +104,7 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3605` - // Minimum execution time: 1_000_000 picoseconds. + // Minimum execution time: 2_000_000 picoseconds. Weight::from_parts(2_000_000, 3605) .saturating_add(T::DbWeight::get().reads(1_u64)) } @@ -124,7 +124,7 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3605` - // Minimum execution time: 1_000_000 picoseconds. + // Minimum execution time: 2_000_000 picoseconds. Weight::from_parts(2_000_000, 3605) .saturating_add(T::DbWeight::get().reads(1_u64)) } @@ -134,7 +134,7 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3675` - // Minimum execution time: 1_000_000 picoseconds. + // Minimum execution time: 2_000_000 picoseconds. Weight::from_parts(2_000_000, 3675) .saturating_add(T::DbWeight::get().reads(1_u64)) } @@ -154,12 +154,12 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `413 + c * (102 ±0)` // Estimated: `3675` - // Minimum execution time: 20_000_000 picoseconds. - Weight::from_parts(1_473_469, 3675) - // Standard Error: 89_329 - .saturating_add(Weight::from_parts(19_606_122, 0).saturating_mul(a.into())) - // Standard Error: 89_329 - .saturating_add(Weight::from_parts(30_920_408, 0).saturating_mul(c.into())) + // Minimum execution time: 26_000_000 picoseconds. + Weight::from_parts(2_877_551, 3675) + // Standard Error: 100_168 + .saturating_add(Weight::from_parts(24_846_938, 0).saturating_mul(a.into())) + // Standard Error: 100_168 + .saturating_add(Weight::from_parts(38_375_510, 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)) @@ -171,7 +171,7 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3675` - // Minimum execution time: 1_000_000 picoseconds. + // Minimum execution time: 2_000_000 picoseconds. Weight::from_parts(2_000_000, 3675) .saturating_add(RocksDbWeight::get().reads(1_u64)) } @@ -181,7 +181,7 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3599` - // Minimum execution time: 2_000_000 picoseconds. + // Minimum execution time: 3_000_000 picoseconds. Weight::from_parts(3_000_000, 3599) .saturating_add(RocksDbWeight::get().reads(1_u64)) } @@ -191,8 +191,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3613` - // Minimum execution time: 4_000_000 picoseconds. - Weight::from_parts(4_000_000, 3613) + // Minimum execution time: 5_000_000 picoseconds. + Weight::from_parts(5_000_000, 3613) .saturating_add(RocksDbWeight::get().reads(1_u64)) } /// Storage: `Assets::Metadata` (r:1 w:0) @@ -201,7 +201,7 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3605` - // Minimum execution time: 1_000_000 picoseconds. + // Minimum execution time: 2_000_000 picoseconds. Weight::from_parts(2_000_000, 3605) .saturating_add(RocksDbWeight::get().reads(1_u64)) } @@ -221,7 +221,7 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3605` - // Minimum execution time: 1_000_000 picoseconds. + // Minimum execution time: 2_000_000 picoseconds. Weight::from_parts(2_000_000, 3605) .saturating_add(RocksDbWeight::get().reads(1_u64)) } @@ -231,9 +231,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `6` // Estimated: `3675` - // Minimum execution time: 1_000_000 picoseconds. + // Minimum execution time: 2_000_000 picoseconds. Weight::from_parts(2_000_000, 3675) .saturating_add(RocksDbWeight::get().reads(1_u64)) } } - diff --git a/pop-api/integration-tests/src/fungibles/mod.rs b/pop-api/integration-tests/src/fungibles/mod.rs index d2e06285..5f7ed04b 100644 --- a/pop-api/integration-tests/src/fungibles/mod.rs +++ b/pop-api/integration-tests/src/fungibles/mod.rs @@ -253,11 +253,6 @@ fn decrease_allowance_works() { let addr = instantiate(CONTRACT, INIT_VALUE, vec![]); let amount: Balance = 100 * UNIT; - // Token does not exist. - assert_eq!( - decrease_allowance(&addr, 0, &BOB, amount), - Err(Module { index: 52, error: [3, 0] }), - ); // Create token and mint `amount` to contract address, then approve Bob to spend `amount`. let token = assets::create_mint_and_approve(&addr, 0, &addr, amount, &BOB, amount); // Token is not live, i.e. frozen or being destroyed. @@ -267,15 +262,20 @@ fn decrease_allowance_works() { Err(Module { index: 52, error: [16, 0] }), ); assets::thaw(&addr, token); + // "Unapproved" error is returned if the current allowance is less than `value`. + assert_eq!( + decrease_allowance(&addr, token, &BOB, amount * 2), + Err(Module { index: 52, error: [10, 0] }), + ); // Successfully decrease allowance. let allowance_before = Assets::allowance(token, &addr, &BOB); - assert_ok!(decrease_allowance(&addr, 0, &BOB, amount / 2 - 1 * UNIT)); + assert_ok!(decrease_allowance(&addr, token, &BOB, amount / 2 - 1 * UNIT)); let allowance_after = Assets::allowance(token, &addr, &BOB); assert_eq!(allowance_before - allowance_after, amount / 2 - 1 * UNIT); // Token is not live, i.e. frozen or being destroyed. assets::start_destroy(&addr, token); assert_eq!( - decrease_allowance(&addr, token, &BOB, amount), + decrease_allowance(&addr, token, &BOB, 1 * UNIT), Err(Module { index: 52, error: [16, 0] }), ); }); @@ -530,14 +530,14 @@ fn burn_works() { let amount: Balance = 100 * UNIT; // Token does not exist. - assert_eq!(burn(&addr, 1, &BOB, amount), Err(Module { index: 52, error: [3, 0] })); + assert_eq!(burn(&addr, 1, &BOB, 0), Err(Module { index: 52, error: [3, 0] })); let token = assets::create(&ALICE, 1, 1); - // Bob has no tokens and therefore doesn't exist. - assert_eq!(burn(&addr, token, &BOB, 1), Err(Module { index: 52, error: [1, 0] })); + // Bob has no tokens. + assert_eq!(burn(&addr, token, &BOB, amount), Err(Module { index: 52, error: [0, 0] })); // Burning can only be done by the manager. assets::mint(&ALICE, token, &BOB, amount); assert_eq!(burn(&addr, token, &BOB, 1), Err(Module { index: 52, error: [2, 0] })); - let token = assets::create_and_mint_to(&addr, 2, &BOB, amount); + let token = assets::create_and_mint_to(&addr, 2, &BOB, amount * 2); // Token is not live, i.e. frozen or being destroyed. assets::freeze(&addr, token); assert_eq!(burn(&addr, token, &BOB, amount), Err(Module { index: 52, error: [16, 0] }));