From 458c8580e19ea545557e35bd92dd7a27346812de Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Mon, 11 Jul 2022 00:44:29 +0800 Subject: [PATCH 1/8] Native asset should never be minted Signed-off-by: Dengjianping --- pallets/manta-pay/src/lib.rs | 2 +- primitives/src/assets.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pallets/manta-pay/src/lib.rs b/pallets/manta-pay/src/lib.rs index d6640da03..c6bcb3d81 100644 --- a/pallets/manta-pay/src/lib.rs +++ b/pallets/manta-pay/src/lib.rs @@ -830,7 +830,7 @@ where // pass the data forward. sinks .map(move |(account_id, deposit)| { - FungibleLedger::::can_deposit(asset_id.0, &account_id, deposit.0, true) + FungibleLedger::::can_deposit(asset_id.0, &account_id, deposit.0, false) .map(|_| WrapPair(account_id.clone(), deposit)) .map_err(|_| InvalidSinkAccount { account_id, diff --git a/primitives/src/assets.rs b/primitives/src/assets.rs index 9f8a10757..53e6eea45 100644 --- a/primitives/src/assets.rs +++ b/primitives/src/assets.rs @@ -419,7 +419,7 @@ where ) -> Result<(), FungibleLedgerError> { Self::ensure_valid(asset_id)?; FungibleLedgerError::from_deposit(if asset_id == A::NativeAssetId::get() { - >::can_deposit(account, amount, mint) + >::can_deposit(account, amount, false) } else { >::can_deposit( asset_id, account, amount, mint, @@ -448,10 +448,11 @@ where amount: Balance, ) -> Result<(), FungibleLedgerError> { Self::ensure_valid(asset_id)?; - Self::can_deposit(asset_id, beneficiary, amount, true)?; if asset_id == A::NativeAssetId::get() { + Self::can_deposit(asset_id, beneficiary, amount, false)?; >::deposit_creating(beneficiary, amount); } else { + Self::can_deposit(asset_id, beneficiary, amount, true)?; >::mint_into(asset_id, beneficiary, amount) .map_err(FungibleLedgerError::InvalidMint)?; } From 7f4e6f951dc4ca8f41a10977c9caee0574694e91 Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Mon, 11 Jul 2022 10:56:33 +0800 Subject: [PATCH 2/8] Fix test and remove unused crate Signed-off-by: Dengjianping --- Cargo.lock | 1 - node/Cargo.toml | 1 - node/src/service.rs | 16 ++++++++-------- primitives/src/assets.rs | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3bd124b5a..4ef51fee5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4993,7 +4993,6 @@ dependencies = [ "frame-benchmarking", "frame-benchmarking-cli", "futures 0.3.21", - "hex-literal", "jsonrpsee", "log", "manta-primitives", diff --git a/node/Cargo.toml b/node/Cargo.toml index 221e17973..0bbda9559 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -20,7 +20,6 @@ cfg-if = "1.0.0" clap = { version = "3.1", features = ["derive"] } codec = { package = 'parity-scale-codec', version = '3.1.2' } futures = "0.3.21" -hex-literal = "0.3.4" log = "0.4.16" serde = { version = "1.0.137", features = ["derive"] } diff --git a/node/src/service.rs b/node/src/service.rs index ea195eea2..46b2320fd 100644 --- a/node/src/service.rs +++ b/node/src/service.rs @@ -37,7 +37,7 @@ use cumulus_relay_chain_inprocess_interface::build_inprocess_relay_chain; use cumulus_relay_chain_interface::{RelayChainError, RelayChainInterface, RelayChainResult}; use cumulus_relay_chain_rpc_interface::RelayChainRPCInterface; use jsonrpsee::RpcModule; -use polkadot_service::{CollatorPair, NativeExecutionDispatch}; +use polkadot_service::CollatorPair; use futures::lock::Mutex; pub use manta_primitives::types::{AccountId, Balance, Block, Hash, Header, Index as Nonce}; @@ -46,7 +46,7 @@ use sc_consensus::{ import_queue::{BasicQueue, Verifier as VerifierT}, BlockImportParams, }; -use sc_executor::NativeElseWasmExecutor; +use sc_executor::{NativeElseWasmExecutor, NativeExecutionDispatch}; use sc_network::NetworkService; pub use sc_rpc::{DenyUnsafe, SubscriptionTaskExecutor}; use sc_service::{Configuration, Error, Role, TFullBackend, TFullClient, TaskManager}; @@ -69,7 +69,7 @@ use substrate_prometheus_endpoint::Registry; /// Native Manta Parachain executor instance. pub struct MantaRuntimeExecutor; -impl sc_executor::NativeExecutionDispatch for MantaRuntimeExecutor { +impl NativeExecutionDispatch for MantaRuntimeExecutor { type ExtendHostFunctions = frame_benchmarking::benchmarking::HostFunctions; fn dispatch(method: &str, data: &[u8]) -> Option> { @@ -83,7 +83,7 @@ impl sc_executor::NativeExecutionDispatch for MantaRuntimeExecutor { /// Native Calamari Parachain executor instance. pub struct CalamariRuntimeExecutor; -impl sc_executor::NativeExecutionDispatch for CalamariRuntimeExecutor { +impl NativeExecutionDispatch for CalamariRuntimeExecutor { type ExtendHostFunctions = frame_benchmarking::benchmarking::HostFunctions; fn dispatch(method: &str, data: &[u8]) -> Option> { @@ -97,7 +97,7 @@ impl sc_executor::NativeExecutionDispatch for CalamariRuntimeExecutor { /// Native Dolphin Parachain executor instance. pub struct DolphinRuntimeExecutor; -impl sc_executor::NativeExecutionDispatch for DolphinRuntimeExecutor { +impl NativeExecutionDispatch for DolphinRuntimeExecutor { type ExtendHostFunctions = frame_benchmarking::benchmarking::HostFunctions; fn dispatch(method: &str, data: &[u8]) -> Option> { @@ -267,7 +267,7 @@ where + pallet_transaction_payment_rpc::TransactionPaymentRuntimeApi + frame_rpc_system::AccountNonceApi, StateBackend: sp_api::StateBackend, - Executor: sc_executor::NativeExecutionDispatch + 'static, + Executor: NativeExecutionDispatch + 'static, FullRpc: Fn( rpc::FullDeps, TransactionPool>, ) -> Result, Error> @@ -545,7 +545,7 @@ where + sp_block_builder::BlockBuilder + sp_consensus_aura::AuraApi::Pair as Pair>::Public>, StateBackend: sp_api::StateBackend, - Executor: sc_executor::NativeExecutionDispatch + 'static, + Executor: NativeExecutionDispatch + 'static, <::Pair as Pair>::Signature: TryFrom> + std::hash::Hash + sp_runtime::traits::Member + Codec, { @@ -628,7 +628,7 @@ where + pallet_transaction_payment_rpc::TransactionPaymentRuntimeApi + frame_rpc_system::AccountNonceApi, StateBackend: sp_api::StateBackend, - Executor: sc_executor::NativeExecutionDispatch + 'static, + Executor: NativeExecutionDispatch + 'static, <::Pair as Pair>::Signature: TryFrom> + std::hash::Hash + sp_runtime::traits::Member + Codec, FullRpc: Fn( diff --git a/primitives/src/assets.rs b/primitives/src/assets.rs index 53e6eea45..353f1b58d 100644 --- a/primitives/src/assets.rs +++ b/primitives/src/assets.rs @@ -419,7 +419,7 @@ where ) -> Result<(), FungibleLedgerError> { Self::ensure_valid(asset_id)?; FungibleLedgerError::from_deposit(if asset_id == A::NativeAssetId::get() { - >::can_deposit(account, amount, false) + >::can_deposit(account, amount, mint) } else { >::can_deposit( asset_id, account, amount, mint, From 9992b64a755d002c9663fa5ab2b09dfc26501644 Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Mon, 11 Jul 2022 15:50:48 +0800 Subject: [PATCH 3/8] Remove minting test case for native asset Signed-off-by: Dengjianping --- primitives/src/assets.rs | 2 +- runtime/calamari/tests/integration_tests.rs | 20 -------------------- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/primitives/src/assets.rs b/primitives/src/assets.rs index 353f1b58d..53e6eea45 100644 --- a/primitives/src/assets.rs +++ b/primitives/src/assets.rs @@ -419,7 +419,7 @@ where ) -> Result<(), FungibleLedgerError> { Self::ensure_valid(asset_id)?; FungibleLedgerError::from_deposit(if asset_id == A::NativeAssetId::get() { - >::can_deposit(account, amount, mint) + >::can_deposit(account, amount, false) } else { >::can_deposit( asset_id, account, amount, mint, diff --git a/runtime/calamari/tests/integration_tests.rs b/runtime/calamari/tests/integration_tests.rs index f8515e260..03316235e 100644 --- a/runtime/calamari/tests/integration_tests.rs +++ b/runtime/calamari/tests/integration_tests.rs @@ -1254,26 +1254,6 @@ fn concrete_fungible_ledger_can_deposit_and_mint_works() { FungibleLedgerError::BelowMinimum ); - let remaining_to_max = u128::MAX - Balances::total_issuance(); - assert_ok!(CalamariConcreteFungibleLedger::mint( - >::NativeAssetId::get(), - &new_account, - remaining_to_max, - ),); - assert_eq!( - Balances::free_balance(new_account.clone()), - remaining_to_max - ); - assert_err!( - CalamariConcreteFungibleLedger::can_deposit( - >::NativeAssetId::get(), - &new_account, - 1, - true, - ), - FungibleLedgerError::Overflow - ); - // Non-native asset tests: let min_balance = 10u128; From 8ab34fcb1022d4891d519bdcbb6ae22d97852ed4 Mon Sep 17 00:00:00 2001 From: Georgi Zlatarev <45011053+ghzlatarev@users.noreply.github.com> Date: Mon, 11 Jul 2022 11:04:43 +0300 Subject: [PATCH 4/8] `existence_requirement` argument for `FungibleLedger` trait functions (#638) * Inital refactor, does not compile Signed-off-by: Georgi Zlatarev * Inital refactor, does not compile Signed-off-by: Georgi Zlatarev * Inital refactor, does not compile Signed-off-by: Georgi Zlatarev * Compiles Signed-off-by: Georgi Zlatarev * Should work Signed-off-by: Georgi Zlatarev * AssetId and u32 thing Signed-off-by: Georgi Zlatarev * Works with concrete Balance and AssetId Signed-off-by: Georgi Zlatarev * Works with concrete Balance and AssetId Signed-off-by: Georgi Zlatarev * Simplify and error handling Signed-off-by: Georgi Zlatarev * Names Signed-off-by: Georgi Zlatarev * Names Signed-off-by: Georgi Zlatarev * Use new struct for AssetTransactor Signed-off-by: Georgi Zlatarev * Clean up Signed-off-by: Georgi Zlatarev * Fix runtime/common/Cargo.toml Signed-off-by: Georgi Zlatarev * Fix tests Signed-off-by: Georgi Zlatarev * Clean up Signed-off-by: Georgi Zlatarev * Comments Signed-off-by: Georgi Zlatarev * Fix integration test Signed-off-by: Georgi Zlatarev * Fix manta-pay test, changelog, more clean up Signed-off-by: Georgi Zlatarev * Add trace logs in TransactAsset impl Signed-off-by: Georgi Zlatarev * Equalize Barrier impl between production and xcm-mock Signed-off-by: Georgi Zlatarev * Rever the Barrier change to not explode the scope of the PR Signed-off-by: Georgi Zlatarev * Add comments Signed-off-by: Georgi Zlatarev * Taplo fmt Signed-off-by: Georgi Zlatarev * Fix clippy Signed-off-by: Georgi Zlatarev * PR comments Signed-off-by: Georgi Zlatarev * Revert how assets are registered Signed-off-by: Georgi Zlatarev * Fix clippy Signed-off-by: Georgi Zlatarev * Initial impl of keep_alive args, using same values as before Signed-off-by: Georgi Zlatarev * Use keep_alive = true for all manta-pay calls Signed-off-by: Georgi Zlatarev * Rename can_withdraw to can_reduce_by_amount and comments Signed-off-by: Georgi Zlatarev * fix changelog Signed-off-by: Georgi Zlatarev * Cover allow death case Signed-off-by: Georgi Zlatarev * Cover allow death case Signed-off-by: Georgi Zlatarev * More tests Signed-off-by: Georgi Zlatarev * changelog Signed-off-by: Georgi Zlatarev Co-authored-by: Brandon H. Gomes --- CHANGELOG.md | 1 + pallets/manta-pay/src/lib.rs | 59 ++++++---- primitives/src/assets.rs | 80 +++++++------ primitives/src/xcm.rs | 4 +- runtime/calamari/tests/integration_tests.rs | 122 +++++++++++++++++--- 5 files changed, 179 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index daa831ff1..49004dc8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - [\#614](https://github.com/Manta-Network/Manta/pull/614) Remove `OnRuntimeUpgrade` from calamari-runtime. - [\#619](https://github.com/Manta-Network/Manta/pull/619) Add CI runtime upgrade test for Dolphin and improve test scenario. - [\#622](https://github.com/Manta-Network/Manta/pull/622) Update parameter path from `sdk` to `manta-parameters`. +- [\#638](https://github.com/Manta-Network/Manta/pull/638) `existence_requirement` argument for `FungibleLedger` trait functions. - [\#652](https://github.com/Manta-Network/Manta/pull/652) Reduce CI failure rate by switching AWS CI runners from AMD to Intel - [\#653](https://github.com/Manta-Network/Manta/pull/653) Add concurrency groups for pull request CI builds to reduce CI costs - [\#657](https://github.com/Manta-Network/Manta/pull/657) retire manta-pc-launch with polkadot-launch. diff --git a/pallets/manta-pay/src/lib.rs b/pallets/manta-pay/src/lib.rs index 022e06dec..aa104640d 100644 --- a/pallets/manta-pay/src/lib.rs +++ b/pallets/manta-pay/src/lib.rs @@ -57,7 +57,8 @@ extern crate alloc; use alloc::{vec, vec::Vec}; use core::marker::PhantomData; -use frame_support::{transactional, PalletId}; +use frame_support::{traits::tokens::ExistenceRequirement, transactional, PalletId}; + use manta_accounting::{ asset, transfer::{ @@ -218,8 +219,14 @@ pub mod pallet { sink: T::AccountId, ) -> DispatchResultWithPostInfo { let origin = ensure_signed(origin)?; - FungibleLedger::::transfer(asset.id, &origin, &sink, asset.value) - .map_err(Error::::from)?; + FungibleLedger::::transfer( + asset.id, + &origin, + &sink, + asset.value, + ExistenceRequirement::KeepAlive, + ) + .map_err(Error::::from)?; Self::deposit_event(Event::Transfer { asset, source: origin, @@ -353,17 +360,14 @@ pub mod pallet { /// [`Overflow`](FungibleLedgerError::Overflow) from [`FungibleLedgerError`] PublicUpdateOverflow, - /// [`Underflow`](FungibleLedgerError::Underflow) from [`FungibleLedgerError`] - PublicUpdateUnderflow, - - /// [`Frozen`](FungibleLedgerError::Frozen) from [`FungibleLedgerError`] - PublicUpdateFrozen, + /// [`CannotWithdraw`](FungibleLedgerError::CannotWithdrawMoreThan(Balance)) from [`FungibleLedgerError`] + PublicUpdateCannotWithdraw, - /// [`NoFunds`](FungibleLedgerError::NoFunds) from [`FungibleLedgerError`] - PublicUpdateNoFunds, + /// [`InvalidMint`](FungibleLedgerError::InvalidMint) from [`FungibleLedgerError`] + PublicUpdateInvalidMint, - /// [`WouldDie`](FungibleLedgerError::WouldDie) from [`FungibleLedgerError`] - PublicUpdateWouldDie, + /// [`InvalidBurn`](FungibleLedgerError::InvalidBurn) from [`FungibleLedgerError`] + PublicUpdateInvalidBurn, /// [`InvalidTransfer`](FungibleLedgerError::InvalidTransfer) from [`FungibleLedgerError`] PublicUpdateInvalidTransfer, @@ -425,12 +429,10 @@ pub mod pallet { FungibleLedgerError::CannotCreate => Self::PublicUpdateCannotCreate, FungibleLedgerError::UnknownAsset => Self::PublicUpdateUnknownAsset, FungibleLedgerError::Overflow => Self::PublicUpdateOverflow, - FungibleLedgerError::Underflow => Self::PublicUpdateUnderflow, - FungibleLedgerError::Frozen => Self::PublicUpdateFrozen, - FungibleLedgerError::NoFunds => Self::PublicUpdateNoFunds, - FungibleLedgerError::WouldDie => Self::PublicUpdateWouldDie, - FungibleLedgerError::InvalidTransfer(_e) => Self::PublicUpdateInvalidTransfer, - _ => Self::InternalLedgerError, + FungibleLedgerError::CannotWithdrawMoreThan(_) => Self::PublicUpdateCannotWithdraw, + FungibleLedgerError::InvalidMint(_) => Self::PublicUpdateInvalidMint, + FungibleLedgerError::InvalidBurn(_) => Self::PublicUpdateInvalidBurn, + FungibleLedgerError::InvalidTransfer(_) => Self::PublicUpdateInvalidTransfer, } } } @@ -806,13 +808,18 @@ where { sources .map(move |(account_id, withdraw)| { - FungibleLedger::::can_withdraw(asset_id.0, &account_id, withdraw.0) - .map(|_| WrapPair(account_id.clone(), withdraw)) - .map_err(|_| InvalidSourceAccount { - account_id, - asset_id, - withdraw, - }) + FungibleLedger::::can_reduce_by_amount( + asset_id.0, + &account_id, + withdraw.0, + ExistenceRequirement::KeepAlive, + ) + .map(|_| WrapPair(account_id.clone(), withdraw)) + .map_err(|_| InvalidSourceAccount { + account_id, + asset_id, + withdraw, + }) }) .collect() } @@ -906,6 +913,7 @@ where &account_id, &Pallet::::account_id(), withdraw.0, + ExistenceRequirement::KeepAlive, )?; } for WrapPair(account_id, deposit) in sinks { @@ -914,6 +922,7 @@ where &Pallet::::account_id(), &account_id, deposit.0, + ExistenceRequirement::KeepAlive, )?; } Ok(()) diff --git a/primitives/src/assets.rs b/primitives/src/assets.rs index 3ac9ec508..1378fe933 100644 --- a/primitives/src/assets.rs +++ b/primitives/src/assets.rs @@ -26,7 +26,7 @@ use frame_support::{ currency::Currency, fungible::Inspect as FungibleInspect, fungibles::{Inspect as FungiblesInspect, Mutate, Transfer}, - DepositConsequence, ExistenceRequirement, WithdrawConsequence, WithdrawReasons, + DepositConsequence, ExistenceRequirement, WithdrawReasons, }, Parameter, }; @@ -278,21 +278,8 @@ pub enum FungibleLedgerError { /// type. Overflow, - /// There has been an underflow in the system. This is indicative of a corrupt state and - /// likely unrecoverable. - Underflow, - - /// Account continued in existence. - /// Not enough of the funds in the account are unavailable for withdrawal. - Frozen, - - /// Withdraw could not happen since the amount to be withdrawn is less than the total funds in - /// the account. - NoFunds, - - /// The withdraw would mean the account dying when it needs to exist (usually because it is a - /// provider and there are consumer references on it). - WouldDie, + /// Cannot withdraw more than the specified amount + CannotWithdrawMoreThan(Balance), /// Unable to Mint an Asset InvalidMint(DispatchError), @@ -317,21 +304,6 @@ impl FungibleLedgerError { DepositConsequence::Success => return Ok(()), }) } - - /// Converts a withdraw `consequence` into a [`FungibleLedgerError`] or into `Ok(())` when the - /// value of `consequence` is [`Success`](WithdrawConsequence::Success). - #[inline] - pub fn from_withdraw(consequence: WithdrawConsequence) -> Result<(), Self> { - Err(match consequence { - WithdrawConsequence::Frozen => Self::Frozen, - WithdrawConsequence::NoFunds => Self::NoFunds, - WithdrawConsequence::Overflow => Self::Overflow, - WithdrawConsequence::Underflow => Self::Underflow, - WithdrawConsequence::UnknownAsset => Self::UnknownAsset, - WithdrawConsequence::WouldDie => Self::WouldDie, - WithdrawConsequence::Success | WithdrawConsequence::ReducedToZero(_) => return Ok(()), - }) - } } /// Unified Interface for Fungible Assets @@ -355,10 +327,11 @@ where ) -> Result<(), FungibleLedgerError>; /// Check whether `account` can decrease its balance by `amount` in the given `asset_id`. - fn can_withdraw( + fn can_reduce_by_amount( asset_id: AssetId, account: &C::AccountId, amount: Balance, + existence_requirement: ExistenceRequirement, ) -> Result<(), FungibleLedgerError>; /// Mints `amount` of an asset with the given `asset_id` to `beneficiary`. @@ -374,6 +347,7 @@ where source: &C::AccountId, destination: &C::AccountId, amount: Balance, + existence_requirement: ExistenceRequirement, ) -> Result<(), FungibleLedgerError>; /// Performs a burn from `who` for `amount` of `asset_id` @@ -381,6 +355,7 @@ where asset_id: AssetId, who: &C::AccountId, amount: Balance, + existence_requirement: ExistenceRequirement, ) -> Result<(), FungibleLedgerError>; } @@ -424,17 +399,32 @@ where } #[inline] - fn can_withdraw( + fn can_reduce_by_amount( asset_id: AssetId, account: &C::AccountId, amount: Balance, + existence_requirement: ExistenceRequirement, ) -> Result<(), FungibleLedgerError> { Self::ensure_valid(asset_id)?; - FungibleLedgerError::from_withdraw(if asset_id == A::NativeAssetId::get() { - >::can_withdraw(account, amount) + + let keep_alive = match existence_requirement { + ExistenceRequirement::KeepAlive => true, + ExistenceRequirement::AllowDeath => false, + }; + let reducible_amount = if asset_id == A::NativeAssetId::get() { + >::reducible_balance(account, keep_alive) } else { - >::can_withdraw(asset_id, account, amount) - }) + >::reducible_balance( + asset_id, account, keep_alive, + ) + }; + + if reducible_amount >= amount { + return Ok(()); + } + Err(FungibleLedgerError::CannotWithdrawMoreThan( + reducible_amount, + )) } #[inline] @@ -460,6 +450,7 @@ where source: &C::AccountId, destination: &C::AccountId, amount: Balance, + existence_requirement: ExistenceRequirement, ) -> Result<(), FungibleLedgerError> { Self::ensure_valid(asset_id)?; if asset_id == A::NativeAssetId::get() { @@ -467,15 +458,19 @@ where source, destination, amount, - ExistenceRequirement::KeepAlive, + existence_requirement, ) } else { + let keep_alive = match existence_requirement { + ExistenceRequirement::KeepAlive => true, + ExistenceRequirement::AllowDeath => false, + }; >::transfer( asset_id, source, destination, amount, - true, + keep_alive, ) .map(|_| ()) } @@ -487,18 +482,21 @@ where asset_id: AssetId, who: &C::AccountId, amount: Balance, + existence_requirement: ExistenceRequirement, ) -> Result<(), FungibleLedgerError> { Self::ensure_valid(asset_id)?; - Self::can_withdraw(asset_id, who, amount)?; + Self::can_reduce_by_amount(asset_id, who, amount, existence_requirement)?; if asset_id == A::NativeAssetId::get() { >::withdraw( who, amount, WithdrawReasons::TRANSFER, - ExistenceRequirement::AllowDeath, + existence_requirement, ) .map_err(FungibleLedgerError::InvalidBurn)?; } else { + // `existence_requirement` is used in the `can_reduce_by_amount` checks + // so it doesn't matter that `burn_from` uses `allow_death` by default in own chosen implementation >::burn_from(asset_id, who, amount) .map_err(FungibleLedgerError::InvalidBurn)?; } diff --git a/primitives/src/xcm.rs b/primitives/src/xcm.rs index cbcef6ec1..f72169d38 100644 --- a/primitives/src/xcm.rs +++ b/primitives/src/xcm.rs @@ -24,7 +24,7 @@ use sp_std::{marker::PhantomData, result}; use frame_support::{ pallet_prelude::Get, - traits::fungibles::Mutate, + traits::{fungibles::Mutate, tokens::ExistenceRequirement}, weights::{constants::WEIGHT_PER_SECOND, Weight}, }; @@ -383,7 +383,7 @@ impl< let (asset_id, amount, who) = Self::match_asset_and_location(asset, location)?; - MultiAdapterFungibleLedger::burn(asset_id, &who, amount) + MultiAdapterFungibleLedger::burn(asset_id, &who, amount, ExistenceRequirement::AllowDeath) .map_err(|_| XcmError::FailedToTransactAsset("Failed Burn"))?; Ok(asset.clone().into()) diff --git a/runtime/calamari/tests/integration_tests.rs b/runtime/calamari/tests/integration_tests.rs index 4ec96204c..32b9c5319 100644 --- a/runtime/calamari/tests/integration_tests.rs +++ b/runtime/calamari/tests/integration_tests.rs @@ -38,7 +38,9 @@ use frame_support::{ assert_err, assert_ok, codec::Encode, dispatch::Dispatchable, - traits::{PalletInfo, StorageInfo, StorageInfoTrait, ValidatorSet}, + traits::{ + tokens::ExistenceRequirement, PalletInfo, StorageInfo, StorageInfoTrait, ValidatorSet, + }, weights::constants::*, StorageHasher, Twox128, }; @@ -994,6 +996,7 @@ fn concrete_fungible_ledger_transfers_work() { &alice.clone(), &charlie.clone(), INITIAL_BALANCE + 1, + ExistenceRequirement::KeepAlive ), FungibleLedgerError::InvalidTransfer(DispatchError::Module(ModuleError { index: ::PalletInfo::index::< @@ -1017,6 +1020,7 @@ fn concrete_fungible_ledger_transfers_work() { &alice.clone(), &charlie.clone(), INITIAL_BALANCE, + ExistenceRequirement::KeepAlive ), FungibleLedgerError::InvalidTransfer(DispatchError::Module(ModuleError { index: ::PalletInfo::index::< @@ -1039,6 +1043,7 @@ fn concrete_fungible_ledger_transfers_work() { &alice.clone(), &charlie.clone(), transfer_amount, + ExistenceRequirement::KeepAlive )); current_balance_alice -= transfer_amount; current_balance_charlie += transfer_amount; @@ -1056,6 +1061,7 @@ fn concrete_fungible_ledger_transfers_work() { &alice.clone(), &new_account, NativeTokenExistentialDeposit::get() - 1, + ExistenceRequirement::KeepAlive ), FungibleLedgerError::InvalidTransfer(DispatchError::Module(ModuleError { index: ::PalletInfo::index::< @@ -1073,6 +1079,7 @@ fn concrete_fungible_ledger_transfers_work() { &alice.clone(), &new_account, NativeTokenExistentialDeposit::get(), + ExistenceRequirement::KeepAlive )); current_balance_alice -= NativeTokenExistentialDeposit::get(); assert_eq!(Balances::free_balance(alice.clone()), current_balance_alice); @@ -1087,6 +1094,7 @@ fn concrete_fungible_ledger_transfers_work() { &bob.clone(), &alice.clone(), INITIAL_BALANCE - NativeTokenExistentialDeposit::get(), + ExistenceRequirement::KeepAlive )); current_balance_alice += INITIAL_BALANCE - NativeTokenExistentialDeposit::get(); assert_eq!(Balances::free_balance(alice.clone()), current_balance_alice); @@ -1095,6 +1103,19 @@ fn concrete_fungible_ledger_transfers_work() { NativeTokenExistentialDeposit::get() ); + // Transfer the ED should work if AllowDeath is selected + assert_ok!(CalamariConcreteFungibleLedger::transfer( + >::NativeAssetId::get(), + &bob.clone(), + &alice.clone(), + NativeTokenExistentialDeposit::get(), + ExistenceRequirement::AllowDeath + )); + current_balance_alice += NativeTokenExistentialDeposit::get(); + assert_eq!(Balances::free_balance(alice.clone()), current_balance_alice); + assert_eq!(Balances::free_balance(bob.clone()), 0); + assert!(!frame_system::Account::::contains_key(bob.clone())); + // Transfer tests for non-native assets: let min_balance = 10u128; @@ -1131,13 +1152,14 @@ fn concrete_fungible_ledger_transfers_work() { amount ); - // Transferring and falling below ED of the asset should not work. + // Transferring and falling below ED of the asset should not work with KeepAlive. assert_err!( CalamariConcreteFungibleLedger::transfer( >::StartNonNativeAssetId::get(), &alice.clone(), &bob.clone(), amount, + ExistenceRequirement::KeepAlive ), FungibleLedgerError::InvalidTransfer(DispatchError::Module(ModuleError { index: ::PalletInfo::index::< @@ -1162,6 +1184,7 @@ fn concrete_fungible_ledger_transfers_work() { &alice.clone(), &bob.clone(), min_balance - 1, + ExistenceRequirement::KeepAlive ), FungibleLedgerError::InvalidTransfer(DispatchError::Token( sp_runtime::TokenError::BelowMinimum @@ -1181,6 +1204,7 @@ fn concrete_fungible_ledger_transfers_work() { &alice.clone(), &bob.clone(), transfer_amount, + ExistenceRequirement::KeepAlive ),); assert_eq!( Assets::balance( @@ -1197,6 +1221,22 @@ fn concrete_fungible_ledger_transfers_work() { transfer_amount ); + // Transferring all of the balance of an account should work. + assert_ok!(CalamariConcreteFungibleLedger::transfer( + >::StartNonNativeAssetId::get(), + &bob.clone(), + &alice.clone(), + transfer_amount, + ExistenceRequirement::AllowDeath + ),); + assert_eq!( + Assets::balance( + >::StartNonNativeAssetId::get(), + bob.clone() + ), + 0 + ); + // Transferring invalid asset ID should not work. assert_err!( CalamariConcreteFungibleLedger::transfer( @@ -1204,6 +1244,7 @@ fn concrete_fungible_ledger_transfers_work() { &alice.clone(), &charlie.clone(), transfer_amount, + ExistenceRequirement::KeepAlive ), FungibleLedgerError::InvalidAssetId ); @@ -1220,6 +1261,7 @@ fn concrete_fungible_ledger_transfers_work() { &alice.clone(), &charlie.clone(), transfer_amount, + ExistenceRequirement::KeepAlive ), FungibleLedgerError::InvalidTransfer(DispatchError::Module(ModuleError { index: ::PalletInfo::index::< @@ -1359,42 +1401,62 @@ fn concrete_fungible_ledger_can_deposit_and_mint_works() { }); } +// `can_reduce_by_amount` uses `reducible_amount` implementation in order to use the `keep_alive` argument. +// Unfortunately that function does not return the reason for failure cases like `can_withdraw`. +// The errors that would've been returned if `can_withdraw` was used instead of `reducible_amount` +// are included as comments on top of each case for more clarity. #[test] -fn concrete_fungible_ledger_can_withdraw_works() { +fn concrete_fungible_ledger_can_reduce_by_amount_works() { let alice = get_account_id_from_seed::("Alice"); let bob = get_account_id_from_seed::("Bob"); + let charlie = get_account_id_from_seed::("Charlie"); ExtBuilder::default() - .with_balances(vec![(alice.clone(), INITIAL_BALANCE)]) + .with_balances(vec![(charlie.clone(), INITIAL_BALANCE)]) .build() .execute_with(|| { + let existential_deposit = NativeTokenExistentialDeposit::get(); + // Native asset tests: assert_err!( - CalamariConcreteFungibleLedger::can_withdraw( + CalamariConcreteFungibleLedger::can_reduce_by_amount( >::NativeAssetId::get(), - &alice.clone(), + &charlie.clone(), INITIAL_BALANCE + 1, + ExistenceRequirement::KeepAlive ), - FungibleLedgerError::Underflow + // Underflow + FungibleLedgerError::CannotWithdrawMoreThan(INITIAL_BALANCE - existential_deposit) ); assert_err!( - CalamariConcreteFungibleLedger::can_withdraw( + CalamariConcreteFungibleLedger::can_reduce_by_amount( >::NativeAssetId::get(), - &alice.clone(), + &charlie.clone(), INITIAL_BALANCE, + ExistenceRequirement::KeepAlive ), - FungibleLedgerError::WouldDie + // WouldDie + FungibleLedgerError::CannotWithdrawMoreThan(INITIAL_BALANCE - existential_deposit) ); + assert_ok!(CalamariConcreteFungibleLedger::can_reduce_by_amount( + >::NativeAssetId::get(), + &charlie.clone(), + INITIAL_BALANCE, + ExistenceRequirement::AllowDeath + ),); + assert_err!( - CalamariConcreteFungibleLedger::can_withdraw( + CalamariConcreteFungibleLedger::can_reduce_by_amount( >::NativeAssetId::get(), &bob.clone(), INITIAL_BALANCE, + ExistenceRequirement::KeepAlive ), - FungibleLedgerError::NoFunds + // NoFunds + FungibleLedgerError::CannotWithdrawMoreThan(0) ); // Non-native asset tests: @@ -1431,27 +1493,47 @@ fn concrete_fungible_ledger_can_withdraw_works() { ); assert_err!( - CalamariConcreteFungibleLedger::can_withdraw( + CalamariConcreteFungibleLedger::can_reduce_by_amount( >::StartNonNativeAssetId::get(), &alice.clone(), INITIAL_BALANCE + 1, + ExistenceRequirement::AllowDeath ), - FungibleLedgerError::Underflow + // Underflow + FungibleLedgerError::CannotWithdrawMoreThan(INITIAL_BALANCE) ); - assert_ok!(CalamariConcreteFungibleLedger::can_withdraw( + assert_ok!(CalamariConcreteFungibleLedger::can_reduce_by_amount( >::StartNonNativeAssetId::get(), &alice.clone(), INITIAL_BALANCE, + ExistenceRequirement::AllowDeath ),); assert_err!( - CalamariConcreteFungibleLedger::can_withdraw( + CalamariConcreteFungibleLedger::can_reduce_by_amount( >::StartNonNativeAssetId::get(), &bob.clone(), 10, + ExistenceRequirement::AllowDeath + ), + // NoFunds + FungibleLedgerError::CannotWithdrawMoreThan(0) + ); + + assert_ok!(CalamariConcreteFungibleLedger::mint( + >::StartNonNativeAssetId::get(), + &bob.clone(), + INITIAL_BALANCE, + ),); + assert_err!( + CalamariConcreteFungibleLedger::can_reduce_by_amount( + >::StartNonNativeAssetId::get(), + &alice.clone(), + INITIAL_BALANCE, + ExistenceRequirement::KeepAlive ), - FungibleLedgerError::NoFunds + FungibleLedgerError::CannotWithdrawMoreThan(INITIAL_BALANCE - min_balance) ); assert_ok!(Assets::freeze( @@ -1460,12 +1542,14 @@ fn concrete_fungible_ledger_can_withdraw_works() { sp_runtime::MultiAddress::Id(alice.clone()), )); assert_err!( - CalamariConcreteFungibleLedger::can_withdraw( + CalamariConcreteFungibleLedger::can_reduce_by_amount( >::StartNonNativeAssetId::get(), &alice.clone(), 10, + ExistenceRequirement::AllowDeath ), - FungibleLedgerError::Frozen + // Frozen + FungibleLedgerError::CannotWithdrawMoreThan(0) ); }); } From adf68f88b2097472a7ae3e98452bd3d8d1cf7b9b Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Tue, 12 Jul 2022 00:34:03 +0800 Subject: [PATCH 5/8] Rename mint param Signed-off-by: Dengjianping --- primitives/src/assets.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/primitives/src/assets.rs b/primitives/src/assets.rs index 63b3ee608..25055af1e 100644 --- a/primitives/src/assets.rs +++ b/primitives/src/assets.rs @@ -324,7 +324,7 @@ where asset_id: AssetId, account: &C::AccountId, amount: Balance, - mint: bool, + will_increase_total_supply: bool, ) -> Result<(), FungibleLedgerError>; /// Check whether `account` can decrease its balance by `amount` in the given `asset_id`. @@ -390,14 +390,17 @@ where asset_id: AssetId, account: &C::AccountId, amount: Balance, - mint: bool, + will_increase_total_supply: bool, ) -> Result<(), FungibleLedgerError> { Self::ensure_valid(asset_id)?; FungibleLedgerError::from_deposit(if asset_id == A::NativeAssetId::get() { >::can_deposit(account, amount, false) } else { >::can_deposit( - asset_id, account, amount, mint, + asset_id, + account, + amount, + will_increase_total_supply, ) }) } From 571bee66d3fcf651c8f5c86ea7a94b51e7738b3c Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Tue, 12 Jul 2022 09:17:51 +0800 Subject: [PATCH 6/8] [no ci]Fix typo Signed-off-by: Dengjianping --- node/src/command.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/src/command.rs b/node/src/command.rs index 41b453bfd..d3ee1c5b8 100644 --- a/node/src/command.rs +++ b/node/src/command.rs @@ -385,7 +385,7 @@ pub fn run_with(cli: Cli) -> Result { Some(Subcommand::Benchmark(cmd)) => { let runner = cli.create_runner(cmd)?; - // Switch on the concrete benchmark sub-command- + // Switch on the concrete benchmark sub-command match cmd { BenchmarkCmd::Pallet(cmd) => { if cfg!(feature = "runtime-benchmarks") { From 9c9ebce4d2448e67cf599502968fc462ca0a782a Mon Sep 17 00:00:00 2001 From: rob thijssen Date: Tue, 12 Jul 2022 05:51:06 +0300 Subject: [PATCH 7/8] include calamari bootnodes in genesis (#666) Signed-off-by: Rob Thijssen --- genesis/calamari-genesis.json | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/genesis/calamari-genesis.json b/genesis/calamari-genesis.json index 58996bb33..09c81038b 100644 --- a/genesis/calamari-genesis.json +++ b/genesis/calamari-genesis.json @@ -2,7 +2,13 @@ "name": "Calamari Parachain", "id": "calamari", "chainType": "Live", - "bootNodes": [], + "bootNodes": [ + "/dns/crispy.calamari.systems/tcp/30333/p2p/12D3KooWNE4LBfkYB2B7D4r9vL54YMMGsfAsXdkhWfBw8VHJSEQc", + "/dns/crunchy.calamari.systems/tcp/30333/p2p/12D3KooWL3ELxcoMGA6han3wPQoym5DKbYHqkWkCuqyjaCXpyJTt", + "/dns/hotdog.calamari.systems/tcp/30333/p2p/12D3KooWMHdpUCCS9j8hvNLTV8PeqJ16KaVEjb5PVdYgAQUFUcCG", + "/dns/tasty.calamari.systems/tcp/30333/p2p/12D3KooWGs2hfnRQ3Y2eAoUyWKUL3g7Jmcsf8FpyhVYeNpXeBMSu", + "/dns/tender.calamari.systems/tcp/30333/p2p/12D3KooWNXZeUSEKRPsp1yiDH99qSVawQSWHqG4umPjgHsn1joci" + ], "telemetryEndpoints": [ [ "/dns/api.telemetry.manta.systems/tcp/443/x-parity-wss/%2Fsubmit%2F", From 986f26fb48e4304e7bc91349aae3110b558a4a9c Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Tue, 12 Jul 2022 11:09:47 +0800 Subject: [PATCH 8/8] Rename will_increase_total_supply param Signed-off-by: Dengjianping --- primitives/src/assets.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/primitives/src/assets.rs b/primitives/src/assets.rs index 25055af1e..ccf28e2e9 100644 --- a/primitives/src/assets.rs +++ b/primitives/src/assets.rs @@ -324,7 +324,7 @@ where asset_id: AssetId, account: &C::AccountId, amount: Balance, - will_increase_total_supply: bool, + can_increase_total_supply: bool, ) -> Result<(), FungibleLedgerError>; /// Check whether `account` can decrease its balance by `amount` in the given `asset_id`. @@ -390,7 +390,7 @@ where asset_id: AssetId, account: &C::AccountId, amount: Balance, - will_increase_total_supply: bool, + can_increase_total_supply: bool, ) -> Result<(), FungibleLedgerError> { Self::ensure_valid(asset_id)?; FungibleLedgerError::from_deposit(if asset_id == A::NativeAssetId::get() { @@ -400,7 +400,7 @@ where asset_id, account, amount, - will_increase_total_supply, + can_increase_total_supply, ) }) }