From 624f51b89f037e227f4cfed958e013e16a7ff5a8 Mon Sep 17 00:00:00 2001 From: Edwin Wang Date: Tue, 10 Oct 2023 15:41:17 +0800 Subject: [PATCH 01/10] Bump version to v0.9.84 --- Cargo.lock | 2 +- node/cli/Cargo.toml | 2 +- runtime/bifrost-kusama/src/lib.rs | 59 +---------------------------- runtime/bifrost-polkadot/src/lib.rs | 7 +--- 4 files changed, 6 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d5a3e0454..3d657ae08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6551,7 +6551,7 @@ dependencies = [ [[package]] name = "node-cli" -version = "0.9.82" +version = "0.9.84" dependencies = [ "clap", "cumulus-client-cli", diff --git a/node/cli/Cargo.toml b/node/cli/Cargo.toml index b0e4fa052..dd58c945a 100644 --- a/node/cli/Cargo.toml +++ b/node/cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "node-cli" -version = "0.9.82" +version = "0.9.84" authors = ["Liebi Technologies "] description = "Bifrost Parachain Node" build = "build.rs" diff --git a/runtime/bifrost-kusama/src/lib.rs b/runtime/bifrost-kusama/src/lib.rs index fc781312e..9e41efc41 100644 --- a/runtime/bifrost-kusama/src/lib.rs +++ b/runtime/bifrost-kusama/src/lib.rs @@ -26,10 +26,8 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); -pub mod migration; use bifrost_slp::{DerivativeAccountProvider, QueryResponseManager}; use core::convert::TryInto; -use frame_support::pallet_prelude::StorageVersion; // A few exports that help ease life for downstream crates. pub use frame_support::{ construct_runtime, match_types, parameter_types, @@ -132,7 +130,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("bifrost"), impl_name: create_runtime_str!("bifrost"), authoring_version: 1, - spec_version: 982, + spec_version: 984, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, @@ -1925,62 +1923,9 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, - (migration::XcmInterfaceMigration, BLPOnRuntimeUpgrade), + (), >; -use frame_support::traits::OnRuntimeUpgrade; -pub struct BLPOnRuntimeUpgrade(PhantomData); -impl OnRuntimeUpgrade for BLPOnRuntimeUpgrade { - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { - #[allow(unused_imports)] - use frame_support::{migration, Identity}; - log::info!("Bifrost `pre_upgrade`..."); - - let pool_count = nutsfinance_stable_asset::PoolCount::::get(); - for pool_id in 0..pool_count { - if let Some(old_metadata) = - bifrost_asset_registry::CurrencyMetadatas::::get(CurrencyId::BLP(pool_id)) - { - log::info!("Old currency_metadatas name is {:?}", old_metadata.name); - } - } - - Ok(vec![]) - } - - fn on_runtime_upgrade() -> Weight { - log::info!("Bifrost `on_runtime_upgrade`..."); - - let pool_count = nutsfinance_stable_asset::PoolCount::::get(); - let weight = bifrost_asset_registry::migration::update_blp_metadata::(pool_count); - - log::info!("Bifrost `on_runtime_upgrade finished`"); - - weight - } - - #[cfg(feature = "try-runtime")] - fn post_upgrade(_: sp_std::prelude::Vec) -> Result<(), &'static str> { - #[allow(unused_imports)] - use frame_support::{migration, Identity}; - log::info!("Bifrost `post_upgrade`..."); - - let pool_count = nutsfinance_stable_asset::PoolCount::::get(); - for pool_id in 0..pool_count { - if let Some(new_metadata) = - bifrost_asset_registry::CurrencyMetadatas::::get(CurrencyId::BLP(pool_id)) - { - let symbol = scale_info::prelude::format!("BLP{}", pool_id).as_bytes().to_vec(); - assert_eq!(new_metadata.symbol, symbol); - log::info!("New currency_metadatas name is {:?}", new_metadata.name); - } - } - - Ok(()) - } -} - #[cfg(feature = "runtime-benchmarks")] #[macro_use] extern crate frame_benchmarking; diff --git a/runtime/bifrost-polkadot/src/lib.rs b/runtime/bifrost-polkadot/src/lib.rs index 4a0e39371..082e2fe48 100644 --- a/runtime/bifrost-polkadot/src/lib.rs +++ b/runtime/bifrost-polkadot/src/lib.rs @@ -26,10 +26,8 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); -pub mod migration; use bifrost_slp::{DerivativeAccountProvider, QueryResponseManager}; use core::convert::TryInto; -use frame_support::pallet_prelude::StorageVersion; // A few exports that help ease life for downstream crates. use cumulus_pallet_parachain_system::{RelayNumberStrictlyIncreases, RelaychainDataProvider}; pub use frame_support::{ @@ -100,7 +98,6 @@ pub use node_primitives::{ GLMR_TOKEN_ID, }; // zenlink imports -use bifrost_salp::remove_storage::RemoveUnusedQueryIdContributionInfo; use zenlink_protocol::{ AssetBalance, AssetId as ZenlinkAssetId, LocalAssetHandler, MultiAssetsHandler, PairInfo, PairLpGenerate, ZenlinkMultiAssets, @@ -131,7 +128,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("bifrost_polkadot"), impl_name: create_runtime_str!("bifrost_polkadot"), authoring_version: 0, - spec_version: 982, + spec_version: 984, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, @@ -1731,7 +1728,7 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, - (RemoveUnusedQueryIdContributionInfo, migration::XcmInterfaceMigration), + (), >; #[cfg(feature = "runtime-benchmarks")] From e318bf8786e8a6e0f0ee9c8abb5427cbb3cba21b Mon Sep 17 00:00:00 2001 From: Herry Ho <45537372+herryho@users.noreply.github.com> Date: Wed, 11 Oct 2023 09:36:46 +0800 Subject: [PATCH 02/10] fix statemine fee (#1053) * fix statemine fee * fixes transfer_statemine_assets * add assert_eq to alice's balance of fees before and after tranfer_statemine_assets --- Cargo.lock | 1 + .../bifrost-kusama/src/statemine.rs | 18 ++++++ pallets/salp/src/mock.rs | 1 + pallets/xcm-interface/Cargo.toml | 1 + pallets/xcm-interface/src/lib.rs | 60 ++++++++++++------- runtime/bifrost-kusama/src/xcm_config.rs | 1 + runtime/bifrost-polkadot/src/xcm_config.rs | 1 + 7 files changed, 62 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3d657ae08..e73fd428c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15512,6 +15512,7 @@ dependencies = [ name = "xcm-interface" version = "0.8.0" dependencies = [ + "bifrost-asset-registry", "cumulus-primitives-core", "frame-benchmarking", "frame-support", diff --git a/integration-tests/bifrost-kusama/src/statemine.rs b/integration-tests/bifrost-kusama/src/statemine.rs index 594b8be2c..a08ed9559 100644 --- a/integration-tests/bifrost-kusama/src/statemine.rs +++ b/integration-tests/bifrost-kusama/src/statemine.rs @@ -138,6 +138,11 @@ fn cross_usdt() { Some((Weight::from_parts(10000000000, 1000000), 10_000_000_000)), )); + // get the fee balance of the alice before the transfer transaction + let alice_fee_balance_before = + Currencies::free_balance(RelayCurrencyId::get(), &AccountId::from(ALICE)); + + // Alice transfers 5 statemine asset to Bob assert_ok!(XcmInterface::transfer_statemine_assets( RuntimeOrigin::signed(ALICE.into()), 5 * USDT, @@ -145,6 +150,16 @@ fn cross_usdt() { Some(sp_runtime::AccountId32::from(BOB)) )); + // get the fee balance of the alice after the transfer transaction + let alice_fee_balance_after = + Currencies::free_balance(RelayCurrencyId::get(), &AccountId::from(ALICE)); + + // assert alice_fee_balance_before and alice_fee_balance_after are equal, since we + // didn't deduct any fee from alice in this test (integration test doesn't go through + // flexible fee) + assert_eq!(alice_fee_balance_before, alice_fee_balance_after); + + // assert Alice has 10-5 =5 statemine asset assert_eq!( Tokens::free_balance(CurrencyId::Token2(0), &AccountId::from(ALICE),), 5 * USDT @@ -153,7 +168,10 @@ fn cross_usdt() { Statemine::execute_with(|| { use statemine_runtime::*; println!("{:?}", System::events()); + + // assert Bob has 5 statemine asset assert_eq!(Assets::balance(1984, AccountId::from(BOB)), 5 * USDT); + assert!(System::events().iter().any(|r| matches!( r.event, RuntimeEvent::XcmpQueue(cumulus_pallet_xcmp_queue::Event::Success { diff --git a/pallets/salp/src/mock.rs b/pallets/salp/src/mock.rs index a0a2315c3..b59bcae5b 100644 --- a/pallets/salp/src/mock.rs +++ b/pallets/salp/src/mock.rs @@ -568,6 +568,7 @@ impl xcm_interface::Config for Test { type SalpHelper = Salp; type ParachainId = ParaInfo; type CallBackTimeOut = ConstU32<10>; + type CurrencyIdConvert = AssetIdMaps; } pub struct ParaInfo; diff --git a/pallets/xcm-interface/Cargo.toml b/pallets/xcm-interface/Cargo.toml index a4cac5ae3..b91ab8771 100644 --- a/pallets/xcm-interface/Cargo.toml +++ b/pallets/xcm-interface/Cargo.toml @@ -23,6 +23,7 @@ xcm = { git = "https://github.com/paritytech/polkadot", branch = "release-v0.9.4 cumulus-primitives-core = { git = "https://github.com/paritytech/cumulus", branch = "polkadot-v0.9.42", default-features = false } orml-traits = { version = "0.4.1-dev", default-features = false } node-primitives = { path = "../../node/primitives", default-features = false } +bifrost-asset-registry = { path = "../asset-registry", default-features = false } [dev-dependencies] sp-io = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.42" } diff --git a/pallets/xcm-interface/src/lib.rs b/pallets/xcm-interface/src/lib.rs index db2c780ac..905b11805 100644 --- a/pallets/xcm-interface/src/lib.rs +++ b/pallets/xcm-interface/src/lib.rs @@ -20,8 +20,9 @@ pub mod calls; pub mod traits; +use bifrost_asset_registry::AssetMetadata; pub use calls::*; -use node_primitives::{traits::XcmDestWeightAndFeeHandler, XcmOperationType}; +use node_primitives::{traits::XcmDestWeightAndFeeHandler, CurrencyIdMapping, XcmOperationType}; use orml_traits::MultiCurrency; pub use pallet::*; pub use traits::{ChainId, MessageId, Nonce, SalpHelper}; @@ -108,6 +109,13 @@ pub mod pallet { BalanceOf, >; + /// Convert MultiLocation to `T::CurrencyId`. + type CurrencyIdConvert: CurrencyIdMapping< + CurrencyIdOf, + MultiLocation, + AssetMetadata>, + >; + #[pallet::constant] type RelayNetwork: Get; @@ -124,6 +132,7 @@ pub mod pallet { XcmExecutionFailed, XcmSendFailed, OperationWeightAndFeeNotExist, + FailToConvert, } #[pallet::event] @@ -217,11 +226,27 @@ pub mod pallet { Some(account) => account, None => who.clone(), }; - let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); - let dst_location = T::AccountIdToMultiLocation::convert(dest.clone()); + let amount_u128 = TryInto::::try_into(amount).map_err(|_| Error::::FeeConvertFailed)?; + // get currency_id from asset_id + let asset_location = MultiLocation::new( + 1, + X3( + Parachain(parachains::Statemine::ID), + PalletInstance(parachains::Statemine::PALLET_ID), + GeneralIndex(asset_id.into()), + ), + ); + let currency_id = T::CurrencyIdConvert::get_currency_id(asset_location) + .ok_or(Error::::FailToConvert)?; + + // first, we need to withdraw the statemine asset from the user's account + T::MultiCurrency::withdraw(currency_id, &who, amount)?; + + let dst_location = T::AccountIdToMultiLocation::convert(dest.clone()); + let (dest_weight, xcm_fee) = Self::xcm_dest_weight_and_fee( T::RelaychainCurrencyId::get(), XcmOperationType::StatemineTransfer, @@ -234,9 +259,8 @@ pub mod pallet { let mut assets = MultiAssets::new(); let statemine_asset = MultiAsset { id: Concrete(MultiLocation::new( - 1, - X3( - Parachain(parachains::Statemine::ID), + 0, + X2( PalletInstance(parachains::Statemine::PALLET_ID), GeneralIndex(asset_id.into()), ), @@ -247,28 +271,22 @@ pub mod pallet { id: Concrete(MultiLocation::new(1, Junctions::Here)), fun: Fungible(xcm_fee_u128), }; - assets.push(statemine_asset); + assets.push(statemine_asset.clone()); assets.push(fee_asset.clone()); let msg = Xcm(vec![ WithdrawAsset(assets), - InitiateReserveWithdraw { - assets: All.into(), - reserve: MultiLocation::new(1, X1(Parachain(parachains::Statemine::ID))), - xcm: Xcm(vec![ - BuyExecution { fees: fee_asset, weight_limit: Unlimited }, - DepositAsset { assets: AllCounted(2).into(), beneficiary: dst_location }, - ]), + BuyExecution { + fees: fee_asset, + weight_limit: cumulus_primitives_core::Limited(dest_weight), }, + DepositAsset { assets: AllCounted(2).into(), beneficiary: dst_location }, ]); - let hash = msg.using_encoded(sp_io::hashing::blake2_256); - ::XcmExecutor::execute_xcm_in_credit( - origin_location, + + pallet_xcm::Pallet::::send_xcm( + Here, + MultiLocation::new(1, X1(Parachain(parachains::Statemine::ID))), msg, - hash, - dest_weight, - dest_weight, ) - .ensure_complete() .map_err(|_| Error::::XcmExecutionFailed)?; Self::deposit_event(Event::::TransferredStatemineMultiAsset(dest, amount)); diff --git a/runtime/bifrost-kusama/src/xcm_config.rs b/runtime/bifrost-kusama/src/xcm_config.rs index e351797d4..73f556869 100644 --- a/runtime/bifrost-kusama/src/xcm_config.rs +++ b/runtime/bifrost-kusama/src/xcm_config.rs @@ -918,4 +918,5 @@ impl xcm_interface::Config for Runtime { type SalpHelper = Salp; type ParachainId = SelfParaChainId; type CallBackTimeOut = ConstU32<10>; + type CurrencyIdConvert = AssetIdMaps; } diff --git a/runtime/bifrost-polkadot/src/xcm_config.rs b/runtime/bifrost-polkadot/src/xcm_config.rs index 29be0e6e2..50e4f6c87 100644 --- a/runtime/bifrost-polkadot/src/xcm_config.rs +++ b/runtime/bifrost-polkadot/src/xcm_config.rs @@ -728,4 +728,5 @@ impl xcm_interface::Config for Runtime { type SalpHelper = Salp; type ParachainId = SelfParaChainId; type CallBackTimeOut = ConstU32<10>; + type CurrencyIdConvert = AssetIdMaps; } From 6c115d5d94c2e7fc1cbd1193fa6e0bf781ca3757 Mon Sep 17 00:00:00 2001 From: NingBo Wang <2536935847@qq.com> Date: Thu, 12 Oct 2023 14:31:46 +0800 Subject: [PATCH 03/10] Slpx remove reserved account minimums (#1057) --- pallets/slpx/src/lib.rs | 10 ++--- pallets/slpx/src/mock.rs | 10 +++-- pallets/slpx/src/tests.rs | 84 +++++++++++++++++++++++++++++++++------ 3 files changed, 84 insertions(+), 20 deletions(-) diff --git a/pallets/slpx/src/lib.rs b/pallets/slpx/src/lib.rs index 25c491732..f8ead2785 100644 --- a/pallets/slpx/src/lib.rs +++ b/pallets/slpx/src/lib.rs @@ -38,7 +38,7 @@ use scale_info::TypeInfo; use sp_core::{Hasher, H160}; use sp_runtime::{ traits::{BlakeTwo256, CheckedSub}, - DispatchError, Saturating, + DispatchError, }; use sp_std::vec; use xcm::{latest::prelude::*, v3::MultiLocation}; @@ -655,16 +655,16 @@ impl Pallet { let free_balance = T::MultiCurrency::free_balance(currency_id, evm_caller_account_id); let execution_fee = Self::execution_fee(currency_id).unwrap_or_else(|| Self::get_default_fee(currency_id)); - let minimum_balance = T::MultiCurrency::minimum_balance(currency_id); + T::MultiCurrency::transfer( currency_id, evm_caller_account_id, &T::TreasuryAccount::get(), execution_fee, )?; - let balance_exclude_fee = free_balance - .checked_sub(&execution_fee.saturating_add(minimum_balance)) - .ok_or(Error::::FreeBalanceTooLow)?; + + let balance_exclude_fee = + free_balance.checked_sub(&execution_fee).ok_or(Error::::FreeBalanceTooLow)?; Ok(balance_exclude_fee) } diff --git a/pallets/slpx/src/mock.rs b/pallets/slpx/src/mock.rs index c29b65955..b2ceb667d 100644 --- a/pallets/slpx/src/mock.rs +++ b/pallets/slpx/src/mock.rs @@ -133,7 +133,7 @@ impl frame_system::Config for Test { // Pallet balances configuration parameter_types! { - pub const ExistentialDeposit: u128 = 1; + pub const ExistentialDeposit: u128 = 10_000_000_000; } impl pallet_balances::Config for Test { @@ -168,8 +168,12 @@ impl bifrost_currencies::Config for Test { // Pallet orml-tokens configuration parameter_type_with_key! { - pub ExistentialDeposits: |_currency_id: CurrencyId| -> u128 { - 0 + pub ExistentialDeposits: |currency_id: CurrencyId| -> u128 { + match currency_id { + &CurrencyId::Native(TokenSymbol::BNC) => 10 * 1_000_000_000, + &CurrencyId::Token(TokenSymbol::KSM) => 10 * 1_000_000_000, + _=> 10 * 1_000_000_000 + } }; } pub type ReserveIdentifier = [u8; 8]; diff --git a/pallets/slpx/src/tests.rs b/pallets/slpx/src/tests.rs index aaef5456e..0ce0c5e9b 100644 --- a/pallets/slpx/src/tests.rs +++ b/pallets/slpx/src/tests.rs @@ -102,25 +102,41 @@ fn test_whitelist_work() { #[test] fn test_execution_fee_work() { sp_io::TestExternalities::default().execute_with(|| { - assert_ok!(Currencies::deposit(CurrencyId::Token2(0), &ALICE, 50)); + assert_ok!(Currencies::deposit(CurrencyId::Token2(0), &ALICE, 50 * 1_000_000_000)); - assert_ok!(Slpx::set_execution_fee(RuntimeOrigin::root(), CurrencyId::Token2(0), 10)); - assert_eq!(Slpx::execution_fee(CurrencyId::Token2(0)), Some(10)); + assert_ok!(Slpx::set_execution_fee( + RuntimeOrigin::root(), + CurrencyId::Token2(0), + 10 * 1_000_000_000 + )); + assert_eq!(Slpx::execution_fee(CurrencyId::Token2(0)), Some(10 * 1_000_000_000)); let balance_exclude_fee = Slpx::charge_execution_fee(CurrencyId::Token2(0), &ALICE).unwrap(); - assert_eq!(balance_exclude_fee, 40); + assert_eq!(balance_exclude_fee, 40 * 1_000_000_000); - assert_ok!(Slpx::set_transfer_to_fee(RuntimeOrigin::root(), SupportChain::Moonbeam, 10)); - assert_eq!(Slpx::transfer_to_fee(SupportChain::Moonbeam), Some(10)); + assert_ok!(Slpx::set_transfer_to_fee( + RuntimeOrigin::root(), + SupportChain::Moonbeam, + 10 * 1_000_000_000 + )); + assert_eq!(Slpx::transfer_to_fee(SupportChain::Moonbeam), Some(10 * 1_000_000_000)); }); } #[test] fn test_zenlink() { sp_io::TestExternalities::default().execute_with(|| { - assert_ok!(Currencies::deposit(CurrencyId::Native(TokenSymbol::BNC), &ALICE, 50)); - assert_ok!(Currencies::deposit(CurrencyId::Token(TokenSymbol::KSM), &ALICE, 50)); + assert_ok!(Currencies::deposit( + CurrencyId::Native(TokenSymbol::BNC), + &ALICE, + 50 * 1_000_000_000 + )); + assert_ok!(Currencies::deposit( + CurrencyId::Token(TokenSymbol::KSM), + &ALICE, + 50 * 1_000_000_000 + )); let bnc_token: AssetId = AssetId::try_convert_from(CurrencyId::Native(TokenSymbol::BNC), 2001).unwrap(); @@ -132,14 +148,20 @@ fn test_zenlink() { RawOrigin::Signed(ALICE).into(), bnc_token, ksm_token, - 20u128, - 20u128, + 20u128 * 1_000_000_000, + 20u128 * 1_000_000_000, 0, 0, 100 )); - assert_eq!(Currencies::free_balance(CurrencyId::Native(TokenSymbol::BNC), &ALICE), 30u128); - assert_eq!(Currencies::free_balance(CurrencyId::Token(TokenSymbol::KSM), &ALICE), 30u128); + assert_eq!( + Currencies::free_balance(CurrencyId::Native(TokenSymbol::BNC), &ALICE), + 30u128 * 1_000_000_000 + ); + assert_eq!( + Currencies::free_balance(CurrencyId::Token(TokenSymbol::KSM), &ALICE), + 30u128 * 1_000_000_000 + ); let path = vec![bnc_token, ksm_token]; let balance = Currencies::free_balance(CurrencyId::Native(TokenSymbol::BNC), &ALICE); @@ -171,3 +193,41 @@ fn test_get_default_fee() { ); }); } + +#[test] +fn test_ed() { + sp_io::TestExternalities::default().execute_with(|| { + assert_ok!(Currencies::deposit( + CurrencyId::Native(TokenSymbol::BNC), + &ALICE, + 50 * 1_000_000_000 + )); + assert_ok!(Currencies::deposit( + CurrencyId::Token(TokenSymbol::KSM), + &ALICE, + 50 * 1_000_000_000 + )); + + assert_eq!( + Currencies::free_balance(CurrencyId::Native(TokenSymbol::BNC), &ALICE), + 50 * 1_000_000_000 + ); + assert_eq!( + Currencies::free_balance(CurrencyId::Token(TokenSymbol::KSM), &ALICE), + 50 * 1_000_000_000 + ); + + assert_ok!(Currencies::transfer( + RawOrigin::Signed(ALICE).into(), + BOB, + CurrencyId::Native(TokenSymbol::BNC), + 50 * 1_000_000_000 + )); + assert_ok!(Currencies::transfer( + RawOrigin::Signed(ALICE).into(), + BOB, + CurrencyId::Token(TokenSymbol::KSM), + 50 * 1_000_000_000 + )); + }); +} From d04834a8ad62523f65a14b99e553b9d910c908a5 Mon Sep 17 00:00:00 2001 From: yooml Date: Thu, 12 Oct 2023 17:51:52 +0800 Subject: [PATCH 04/10] Fix audit errors (#1058) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 🐛 audit * fix: 🐛 modify_fees * fix: 🐛 test --- pallets/stable-pool/src/lib.rs | 15 ++++++------ pallets/stable-pool/src/tests.rs | 42 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/pallets/stable-pool/src/lib.rs b/pallets/stable-pool/src/lib.rs index 2e23bd491..1c98eaf53 100644 --- a/pallets/stable-pool/src/lib.rs +++ b/pallets/stable-pool/src/lib.rs @@ -240,6 +240,13 @@ pub mod pallet { redeem_fee: Option, ) -> DispatchResult { T::ControlOrigin::ensure_origin(origin)?; + let fee_denominator: T::AtLeast64BitUnsigned = T::FeePrecision::get(); + ensure!( + mint_fee.map(|x| x < fee_denominator).unwrap_or(true) && + swap_fee.map(|x| x < fee_denominator).unwrap_or(true) && + redeem_fee.map(|x| x < fee_denominator).unwrap_or(true), + nutsfinance_stable_asset::Error::::ArgumentsError + ); Pools::::try_mutate_exists(pool_id, |maybe_pool_info| -> DispatchResult { let pool_info = maybe_pool_info .as_mut() @@ -614,14 +621,6 @@ impl Pallet { } T::MultiCurrency::transfer(pool_info.assets[i_usize], &pool_info.account_id, who, dy)?; T::MultiCurrency::withdraw(pool_info.pool_asset, who, redeem_amount)?; - let mut amounts: Vec = Vec::new(); - for idx in 0..pool_size { - if idx == i_usize { - amounts.push(dy); - } else { - amounts.push(Zero::zero()); - } - } pool_info.total_supply = total_supply; pool_info.balances = balances; diff --git a/pallets/stable-pool/src/tests.rs b/pallets/stable-pool/src/tests.rs index c6093cd41..a7ab1b8b7 100644 --- a/pallets/stable-pool/src/tests.rs +++ b/pallets/stable-pool/src/tests.rs @@ -529,6 +529,48 @@ fn redeem_single() { 2 )); assert_eq!(Tokens::free_balance(coin1, &6), 904_596_263_064); + assert_noop!( + StablePool::modify_fees( + RuntimeOrigin::root(), + 0, + Some(10_000_000_000), + Some(10_000_000_000), + Some(10_000_000_000) + ), + nutsfinance_stable_asset::Error::::ArgumentsError + ); + assert_ok!(StablePool::modify_fees( + RuntimeOrigin::root(), + 0, + Some(9_999_999_999), + Some(9_999_999_999), + Some(9_999_999_999), + )); + assert_ok!(StablePool::redeem_single( + RuntimeOrigin::signed(6).into(), + 0, + 5_000_000_000u128, + 1, + 0, + 2 + )); + assert_eq!(Tokens::free_balance(coin1, &6), 904_596_263_064); + assert_ok!(StablePool::modify_fees( + RuntimeOrigin::root(), + 0, + Some(9_999_999_999), + Some(9_999_999_999), + Some(999_999_999), + )); + assert_ok!(StablePool::redeem_single( + RuntimeOrigin::signed(6).into(), + 0, + 5_000_000_000u128, + 1, + 0, + 2 + )); + assert_eq!(Tokens::free_balance(coin1, &6), 908_716_032_298); }); } From c5ddc173a5194e5233a4ce5b6e1f65f6a5753a83 Mon Sep 17 00:00:00 2001 From: Edwin Date: Thu, 12 Oct 2023 05:55:05 -0500 Subject: [PATCH 05/10] Fix referendum status (#1059) --- pallets/vtoken-voting/src/benchmarking.rs | 2 +- pallets/vtoken-voting/src/lib.rs | 55 +++++++++++++---------- pallets/vtoken-voting/src/mock.rs | 4 +- pallets/vtoken-voting/src/tests.rs | 34 +++++++++----- 4 files changed, 59 insertions(+), 36 deletions(-) diff --git a/pallets/vtoken-voting/src/benchmarking.rs b/pallets/vtoken-voting/src/benchmarking.rs index 1a190e514..857b262ae 100644 --- a/pallets/vtoken-voting/src/benchmarking.rs +++ b/pallets/vtoken-voting/src/benchmarking.rs @@ -83,7 +83,7 @@ mod benchmarks { let r = T::MaxVotes::get() - 1; let response = Response::DispatchResult(MaybeErrorCode::Success); for (i, index) in (0..T::MaxVotes::get()).collect::>().iter().skip(1).enumerate() { - Pallet::::on_initialize(Zero::zero()); + Pallet::::on_idle(Zero::zero(), Weight::MAX); Pallet::::vote(RawOrigin::Signed(caller.clone()).into(), vtoken, *index, vote)?; Pallet::::notify_vote( control_origin.clone() as ::RuntimeOrigin, diff --git a/pallets/vtoken-voting/src/lib.rs b/pallets/vtoken-voting/src/lib.rs index ce045008d..84e456ad5 100644 --- a/pallets/vtoken-voting/src/lib.rs +++ b/pallets/vtoken-voting/src/lib.rs @@ -351,35 +351,42 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { - fn on_initialize(_: BlockNumberFor) -> Weight { - let mut weight = T::DbWeight::get().reads(1); + fn on_idle(_n: BlockNumberFor, remaining_weight: Weight) -> Weight { + let db_weight = T::DbWeight::get(); + let mut used_weight = db_weight.reads(3); + if remaining_weight.any_lt(used_weight) { + return Weight::zero(); + } let relay_current_block_number = T::RelaychainBlockNumberProvider::current_block_number(); - weight += T::DbWeight::get().reads(1); - let timeout = ReferendumTimeout::::get(relay_current_block_number); - if !timeout.is_empty() { - timeout.iter().for_each(|(vtoken, poll_index)| { - ReferendumInfoFor::::mutate( - vtoken, - poll_index, - |maybe_info| match maybe_info { - Some(info) => - if let ReferendumInfo::Ongoing(_) = info { - *info = ReferendumInfo::Completed( - relay_current_block_number.into(), - ); - }, - None => {}, - }, - ); - weight += T::DbWeight::get().reads_writes(1, 1); - }); - weight += T::DbWeight::get().reads_writes(1, 1); - ReferendumTimeout::::remove(relay_current_block_number); + for relay_block_number in ReferendumTimeout::::iter_keys() { + if relay_current_block_number >= relay_block_number { + let info_list = ReferendumTimeout::::get(relay_block_number); + let len = info_list.len() as u64; + let temp_weight = db_weight.reads_writes(len, len) + db_weight.writes(1); + if remaining_weight.any_lt(used_weight + temp_weight) { + return used_weight; + } + used_weight += temp_weight; + for (vtoken, poll_index) in info_list.iter() { + ReferendumInfoFor::::mutate(vtoken, poll_index, |maybe_info| { + match maybe_info { + Some(info) => + if let ReferendumInfo::Ongoing(_) = info { + *info = ReferendumInfo::Completed( + relay_current_block_number.into(), + ); + }, + None => {}, + } + }); + } + ReferendumTimeout::::remove(relay_block_number); + } } - weight + used_weight } } diff --git a/pallets/vtoken-voting/src/mock.rs b/pallets/vtoken-voting/src/mock.rs index cf527979e..65e639687 100644 --- a/pallets/vtoken-voting/src/mock.rs +++ b/pallets/vtoken-voting/src/mock.rs @@ -26,6 +26,7 @@ use frame_support::{ pallet_prelude::Weight, parameter_types, traits::{Everything, GenesisBuild, Get, Nothing}, + weights::RuntimeDbWeight, }; use frame_system::EnsureRoot; use node_primitives::{ @@ -73,6 +74,7 @@ frame_support::construct_runtime!( parameter_types! { pub const BlockHashCount: u64 = 250; + pub const DbWeight: RuntimeDbWeight = RuntimeDbWeight { read: 1, write: 2 }; } impl frame_system::Config for Runtime { type AccountData = pallet_balances::AccountData; @@ -83,7 +85,7 @@ impl frame_system::Config for Runtime { type BlockNumber = u64; type BlockWeights = (); type RuntimeCall = RuntimeCall; - type DbWeight = (); + type DbWeight = DbWeight; type RuntimeEvent = RuntimeEvent; type Hash = H256; type Hashing = BlakeTwo256; diff --git a/pallets/vtoken-voting/src/tests.rs b/pallets/vtoken-voting/src/tests.rs index e3690e48a..ca3a3b296 100644 --- a/pallets/vtoken-voting/src/tests.rs +++ b/pallets/vtoken-voting/src/tests.rs @@ -26,6 +26,7 @@ use frame_support::{ fungibles::Inspect, tokens::{Fortitude::Polite, Preservation::Expendable}, }, + weights::RuntimeDbWeight, }; use node_primitives::currency::{VBNC, VKSM}; use pallet_conviction_voting::Vote; @@ -657,7 +658,7 @@ fn notify_vote_success_max_works() { RelaychainDataProvider::set_block_number( 1 + UndecidingTimeout::::get(vtoken).unwrap(), ); - VtokenVoting::on_initialize(Zero::zero()); + VtokenVoting::on_idle(Zero::zero(), Weight::MAX); } }); } @@ -877,11 +878,13 @@ fn notify_remove_delegator_vote_with_no_data_works() { } #[test] -fn on_initialize_works() { +fn on_idle_works() { new_test_ext().execute_with(|| { let vtoken = VKSM; - RelaychainDataProvider::set_block_number(1); - for (query_id, poll_index) in (0..50).collect::>().iter().enumerate() { + for (index, poll_index) in (0..50).collect::>().iter().enumerate() { + let relay_block_number = index as BlockNumber; + let query_id = index as QueryId; + RelaychainDataProvider::set_block_number(relay_block_number); assert_ok!(VtokenVoting::vote( RuntimeOrigin::signed(ALICE), vtoken, @@ -895,16 +898,27 @@ fn on_initialize_works() { )); } + let count = 30; RelaychainDataProvider::set_block_number( - 1 + UndecidingTimeout::::get(vtoken).unwrap(), + count + UndecidingTimeout::::get(vtoken).unwrap(), ); - VtokenVoting::on_initialize(Zero::zero()); + let db_weight = RuntimeDbWeight { read: 1, write: 1 }; + let weight = + db_weight.reads(3) + db_weight.reads_writes(1, 2) * count + db_weight.writes(2) * count; + let used_weight = VtokenVoting::on_idle(Zero::zero(), weight); + assert_eq!(used_weight, Weight::from_parts(153, 0)); + let mut actual_count = 0; for poll_index in 0..50 { - assert_eq!( - ReferendumInfoFor::::get(vtoken, poll_index), - Some(ReferendumInfo::Completed(101)) - ); + let relay_block_number = poll_index as BlockNumber; + if ReferendumTimeout::::get( + relay_block_number + UndecidingTimeout::::get(vtoken).unwrap(), + ) + .is_empty() + { + actual_count += 1; + } } + assert_eq!(actual_count, count); }); } From aec4d975d28a39f14f2baa028b0ac2f2e8d05c2a Mon Sep 17 00:00:00 2001 From: yooml Date: Thu, 12 Oct 2023 19:21:37 +0800 Subject: [PATCH 06/10] =?UTF-8?q?fix:=20=F0=9F=90=9B=20gov=20(#1056)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- runtime/bifrost-kusama/src/governance/mod.rs | 14 ++++- runtime/bifrost-kusama/src/lib.rs | 52 ++++++------------- runtime/bifrost-kusama/src/xcm_config.rs | 2 +- .../bifrost-polkadot/src/governance/mod.rs | 14 ++++- runtime/bifrost-polkadot/src/lib.rs | 52 ++++++------------- runtime/bifrost-polkadot/src/xcm_config.rs | 2 +- 6 files changed, 60 insertions(+), 76 deletions(-) diff --git a/runtime/bifrost-kusama/src/governance/mod.rs b/runtime/bifrost-kusama/src/governance/mod.rs index 2d21427b5..019de14ae 100644 --- a/runtime/bifrost-kusama/src/governance/mod.rs +++ b/runtime/bifrost-kusama/src/governance/mod.rs @@ -23,9 +23,19 @@ pub use bifrost_runtime_common::dollar; mod fellowship; mod origins; pub use origins::{ - custom_origins, Fellows, FellowshipAdmin, FellowshipExperts, FellowshipInitiates, + custom_origins, CoreAdmin, Fellows, FellowshipAdmin, FellowshipExperts, FellowshipInitiates, FellowshipMasters, ReferendumCanceller, ReferendumKiller, SALPAdmin, SystemStakingAdmin, - ValidatorElection, WhitelistedCaller, *, + TechAdmin, ValidatorElection, WhitelistedCaller, *, }; mod tracks; pub use tracks::TracksInfo; + +pub type CoreAdminOrCouncil = EitherOfDiverse< + CoreAdmin, + EitherOfDiverse, +>; + +pub type TechAdminOrCouncil = EitherOfDiverse< + TechAdmin, + EitherOfDiverse, +>; diff --git a/runtime/bifrost-kusama/src/lib.rs b/runtime/bifrost-kusama/src/lib.rs index 9e41efc41..4a53cfe8c 100644 --- a/runtime/bifrost-kusama/src/lib.rs +++ b/runtime/bifrost-kusama/src/lib.rs @@ -105,7 +105,10 @@ use zenlink_stable_amm::traits::{StableAmmApi, StablePoolLpCurrencyIdGenerate, V // Governance configurations. pub mod governance; -use governance::{custom_origins, CoreAdmin, TechAdmin}; +use governance::{ + custom_origins, CoreAdmin, CoreAdminOrCouncil, SALPAdmin, SystemStakingAdmin, TechAdmin, + TechAdminOrCouncil, ValidatorElection, +}; // xcm config mod xcm_config; @@ -711,7 +714,7 @@ parameter_types! { pub VotingBondFactor: Balance = deposit::(0, 32); /// Daily council elections pub const TermDuration: BlockNumber = 24 * HOURS; - pub const DesiredMembers: u32 = 7; + pub const DesiredMembers: u32 = 3; pub const DesiredRunnersUp: u32 = 7; pub const PhragmenElectionPalletId: LockIdentifier = *b"phrelect"; pub const MaxVoters: u32 = 512; @@ -1137,7 +1140,7 @@ impl bifrost_flexible_fee::Config for Runtime { type WeightInfo = weights::bifrost_flexible_fee::BifrostWeight; type ExtraFeeMatcher = ExtraFeeMatcher; type ParachainId = ParachainInfo; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = TechAdminOrCouncil; type XcmWeightAndFeeHandler = XcmInterface; } @@ -1256,10 +1259,7 @@ impl bifrost_salp::Config for Runtime { type SlotLength = SlotLength; type VSBondValidPeriod = VSBondValidPeriod; type WeightInfo = weights::bifrost_salp::BifrostWeight; - type EnsureConfirmAsGovernance = EitherOfDiverse< - TechAdmin, - EitherOfDiverse, - >; + type EnsureConfirmAsGovernance = EitherOfDiverse; type XcmInterface = XcmInterface; type TreasuryAccount = BifrostTreasuryAccount; type BuybackPalletId = BuybackPalletId; @@ -1298,10 +1298,7 @@ impl bifrost_token_issuer::Config for Runtime { impl bifrost_call_switchgear::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type UpdateOrigin = EitherOfDiverse< - CoreAdmin, - EitherOfDiverse, - >; + type UpdateOrigin = CoreAdminOrCouncil; type WeightInfo = weights::bifrost_call_switchgear::BifrostWeight; } @@ -1367,10 +1364,7 @@ impl bifrost_slp::Config for Runtime { type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse< - TechAdmin, - EitherOfDiverse, - >; + type ControlOrigin = EitherOfDiverse; type WeightInfo = weights::bifrost_slp::BifrostWeight; type VtokenMinting = VtokenMinting; type BifrostSlpx = Slpx; @@ -1391,10 +1385,7 @@ impl bifrost_vstoken_conversion::Config for Runtime { type MultiCurrency = Currencies; type RelayCurrencyId = RelayCurrencyId; type TreasuryAccount = BifrostTreasuryAccount; - type ControlOrigin = EitherOfDiverse< - CoreAdmin, - EitherOfDiverse, - >; + type ControlOrigin = CoreAdminOrCouncil; type VsbondAccount = BifrostVsbondPalletId; type CurrencyIdConversion = AssetIdMaps; type WeightInfo = weights::bifrost_vstoken_conversion::BifrostWeight; @@ -1407,10 +1398,7 @@ parameter_types! { impl bifrost_farming::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse< - TechAdmin, - EitherOfDiverse, - >; + type ControlOrigin = TechAdminOrCouncil; type TreasuryAccount = BifrostTreasuryAccount; type Keeper = FarmingKeeperPalletId; type RewardIssuer = FarmingRewardIssuerPalletId; @@ -1430,10 +1418,7 @@ parameter_types! { impl bifrost_system_staking::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type EnsureConfirmAsGovernance = EitherOfDiverse< - CoreAdmin, - EitherOfDiverse, - >; + type EnsureConfirmAsGovernance = EitherOfDiverse; type WeightInfo = weights::bifrost_system_staking::BifrostWeight; type FarmingInfo = Farming; type VtokenMintingInterface = VtokenMinting; @@ -1461,7 +1446,7 @@ impl bifrost_system_maker::Config for Runtime { impl bifrost_fee_share::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = CoreAdminOrCouncil; type WeightInfo = weights::bifrost_fee_share::BifrostWeight; type FeeSharePalletId = FeeSharePalletId; } @@ -1469,7 +1454,7 @@ impl bifrost_fee_share::Config for Runtime { impl bifrost_cross_in_out::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = TechAdminOrCouncil; type EntrancePalletId = SlpEntrancePalletId; type WeightInfo = weights::bifrost_cross_in_out::BifrostWeight; type MaxLengthLimit = MaxLengthLimit; @@ -1613,10 +1598,7 @@ parameter_types! { impl bifrost_vtoken_minting::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse< - TechAdmin, - EitherOfDiverse, - >; + type ControlOrigin = TechAdminOrCouncil; type MaximumUnlockIdOfUser = MaximumUnlockIdOfUser; type MaximumUnlockIdOfTimeUnit = MaximumUnlockIdOfTimeUnit; type EntranceAccount = SlpEntrancePalletId; @@ -1637,7 +1619,7 @@ impl bifrost_vtoken_minting::Config for Runtime { impl bifrost_slpx::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = TechAdminOrCouncil; type MultiCurrency = Currencies; type DexOperator = ZenlinkProtocol; type VtokenMintingInterface = VtokenMinting; @@ -1678,7 +1660,7 @@ impl nutsfinance_stable_asset::Config for Runtime { impl bifrost_stable_pool::Config for Runtime { type WeightInfo = weights::bifrost_stable_pool::BifrostWeight; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = TechAdminOrCouncil; type CurrencyId = CurrencyId; type MultiCurrency = Currencies; type StableAsset = StableAsset; diff --git a/runtime/bifrost-kusama/src/xcm_config.rs b/runtime/bifrost-kusama/src/xcm_config.rs index 73f556869..1661bb3ee 100644 --- a/runtime/bifrost-kusama/src/xcm_config.rs +++ b/runtime/bifrost-kusama/src/xcm_config.rs @@ -905,7 +905,7 @@ parameter_types! { impl xcm_interface::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type UpdateOrigin = EitherOfDiverse; + type UpdateOrigin = TechAdminOrCouncil; type MultiCurrency = Currencies; type RelayNetwork = RelayNetwork; type RelaychainCurrencyId = RelayCurrencyId; diff --git a/runtime/bifrost-polkadot/src/governance/mod.rs b/runtime/bifrost-polkadot/src/governance/mod.rs index 2d21427b5..019de14ae 100644 --- a/runtime/bifrost-polkadot/src/governance/mod.rs +++ b/runtime/bifrost-polkadot/src/governance/mod.rs @@ -23,9 +23,19 @@ pub use bifrost_runtime_common::dollar; mod fellowship; mod origins; pub use origins::{ - custom_origins, Fellows, FellowshipAdmin, FellowshipExperts, FellowshipInitiates, + custom_origins, CoreAdmin, Fellows, FellowshipAdmin, FellowshipExperts, FellowshipInitiates, FellowshipMasters, ReferendumCanceller, ReferendumKiller, SALPAdmin, SystemStakingAdmin, - ValidatorElection, WhitelistedCaller, *, + TechAdmin, ValidatorElection, WhitelistedCaller, *, }; mod tracks; pub use tracks::TracksInfo; + +pub type CoreAdminOrCouncil = EitherOfDiverse< + CoreAdmin, + EitherOfDiverse, +>; + +pub type TechAdminOrCouncil = EitherOfDiverse< + TechAdmin, + EitherOfDiverse, +>; diff --git a/runtime/bifrost-polkadot/src/lib.rs b/runtime/bifrost-polkadot/src/lib.rs index 082e2fe48..6762c60dd 100644 --- a/runtime/bifrost-polkadot/src/lib.rs +++ b/runtime/bifrost-polkadot/src/lib.rs @@ -114,7 +114,10 @@ use xcm_config::{ use xcm_executor::XcmExecutor; pub mod governance; -use governance::{custom_origins, CoreAdmin, TechAdmin}; +use governance::{ + custom_origins, CoreAdminOrCouncil, SALPAdmin, SystemStakingAdmin, TechAdmin, + TechAdminOrCouncil, ValidatorElection, +}; impl_opaque_keys! { pub struct SessionKeys { @@ -681,7 +684,7 @@ parameter_types! { pub const VotingBondFactor: Balance = deposit(0, 32); /// Daily council elections pub const TermDuration: BlockNumber = 7 * DAYS; - pub const DesiredMembers: u32 = 13; + pub const DesiredMembers: u32 = 3; pub const DesiredRunnersUp: u32 = 20; pub const PhragmenElectionPalletId: LockIdentifier = *b"phrelect"; pub const MaxVoters: u32 = 512; @@ -993,7 +996,7 @@ impl bifrost_flexible_fee::Config for Runtime { type WeightInfo = weights::bifrost_flexible_fee::BifrostWeight; type ExtraFeeMatcher = ExtraFeeMatcher; type ParachainId = ParachainInfo; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = TechAdminOrCouncil; type XcmWeightAndFeeHandler = XcmInterface; } @@ -1111,10 +1114,7 @@ impl bifrost_salp::Config for Runtime { type SlotLength = SlotLength; type VSBondValidPeriod = VSBondValidPeriod; type WeightInfo = weights::bifrost_salp::BifrostWeight; - type EnsureConfirmAsGovernance = EitherOfDiverse< - TechAdmin, - EitherOfDiverse, - >; + type EnsureConfirmAsGovernance = EitherOfDiverse; type XcmInterface = XcmInterface; type TreasuryAccount = BifrostTreasuryAccount; type BuybackPalletId = BuybackPalletId; @@ -1128,10 +1128,7 @@ impl bifrost_salp::Config for Runtime { impl bifrost_call_switchgear::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type UpdateOrigin = EitherOfDiverse< - CoreAdmin, - EitherOfDiverse, - >; + type UpdateOrigin = CoreAdminOrCouncil; type WeightInfo = weights::bifrost_call_switchgear::BifrostWeight; } @@ -1195,10 +1192,7 @@ impl bifrost_slp::Config for Runtime { type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse< - TechAdmin, - EitherOfDiverse, - >; + type ControlOrigin = EitherOfDiverse; type WeightInfo = weights::bifrost_slp::BifrostWeight; type VtokenMinting = VtokenMinting; type BifrostSlpx = Slpx; @@ -1223,10 +1217,7 @@ impl bifrost_vstoken_conversion::Config for Runtime { type MultiCurrency = Currencies; type RelayCurrencyId = RelayCurrencyId; type TreasuryAccount = BifrostTreasuryAccount; - type ControlOrigin = EitherOfDiverse< - CoreAdmin, - EitherOfDiverse, - >; + type ControlOrigin = CoreAdminOrCouncil; type VsbondAccount = BifrostVsbondPalletId; type CurrencyIdConversion = AssetIdMaps; type WeightInfo = weights::bifrost_vstoken_conversion::BifrostWeight; @@ -1239,10 +1230,7 @@ parameter_types! { impl bifrost_farming::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse< - TechAdmin, - EitherOfDiverse, - >; + type ControlOrigin = TechAdminOrCouncil; type TreasuryAccount = BifrostTreasuryAccount; type Keeper = FarmingKeeperPalletId; type RewardIssuer = FarmingRewardIssuerPalletId; @@ -1262,10 +1250,7 @@ parameter_types! { impl bifrost_system_staking::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type EnsureConfirmAsGovernance = EitherOfDiverse< - CoreAdmin, - EitherOfDiverse, - >; + type EnsureConfirmAsGovernance = EitherOfDiverse; type WeightInfo = weights::bifrost_system_staking::BifrostWeight; type FarmingInfo = Farming; type VtokenMintingInterface = VtokenMinting; @@ -1293,7 +1278,7 @@ impl bifrost_system_maker::Config for Runtime { impl bifrost_fee_share::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = CoreAdminOrCouncil; type WeightInfo = weights::bifrost_fee_share::BifrostWeight; type FeeSharePalletId = FeeSharePalletId; } @@ -1301,7 +1286,7 @@ impl bifrost_fee_share::Config for Runtime { impl bifrost_cross_in_out::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = TechAdminOrCouncil; type EntrancePalletId = SlpEntrancePalletId; type WeightInfo = weights::bifrost_cross_in_out::BifrostWeight; type MaxLengthLimit = MaxLengthLimit; @@ -1309,7 +1294,7 @@ impl bifrost_cross_in_out::Config for Runtime { impl bifrost_slpx::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = TechAdminOrCouncil; type MultiCurrency = Currencies; type DexOperator = ZenlinkProtocol; type VtokenMintingInterface = VtokenMinting; @@ -1350,7 +1335,7 @@ impl nutsfinance_stable_asset::Config for Runtime { impl bifrost_stable_pool::Config for Runtime { type WeightInfo = weights::bifrost_stable_pool::BifrostWeight; - type ControlOrigin = EitherOfDiverse; + type ControlOrigin = TechAdminOrCouncil; type CurrencyId = CurrencyId; type MultiCurrency = Currencies; type StableAsset = StableAsset; @@ -1450,10 +1435,7 @@ parameter_types! { impl bifrost_vtoken_minting::Config for Runtime { type RuntimeEvent = RuntimeEvent; type MultiCurrency = Currencies; - type ControlOrigin = EitherOfDiverse< - TechAdmin, - EitherOfDiverse, - >; + type ControlOrigin = TechAdminOrCouncil; type MaximumUnlockIdOfUser = MaximumUnlockIdOfUser; type MaximumUnlockIdOfTimeUnit = MaximumUnlockIdOfTimeUnit; type EntranceAccount = SlpEntrancePalletId; diff --git a/runtime/bifrost-polkadot/src/xcm_config.rs b/runtime/bifrost-polkadot/src/xcm_config.rs index 50e4f6c87..809744930 100644 --- a/runtime/bifrost-polkadot/src/xcm_config.rs +++ b/runtime/bifrost-polkadot/src/xcm_config.rs @@ -718,7 +718,7 @@ parameter_types! { impl xcm_interface::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type UpdateOrigin = EitherOfDiverse; + type UpdateOrigin = TechAdminOrCouncil; type MultiCurrency = Currencies; type RelayNetwork = RelayNetwork; type RelaychainCurrencyId = RelayCurrencyId; From 8485a84f1e18b4fc860e6989e8213d2130425110 Mon Sep 17 00:00:00 2001 From: Edwin Date: Mon, 16 Oct 2023 05:42:49 -0500 Subject: [PATCH 07/10] Fix delegator vote (#1060) * Fix DelegatorVote * Revert on_initialize * Fix benchmarking * Fix benchmarking and test * Migrations for vtoken-voting --- .../bifrost-kusama/src/vtoken_voting.rs | 1 + pallets/flexible-fee/src/mock.rs | 5 + pallets/vtoken-voting/src/benchmarking.rs | 45 +++++-- pallets/vtoken-voting/src/lib.rs | 100 ++++++++------ pallets/vtoken-voting/src/migration.rs | 122 ++++++++++++++++++ pallets/vtoken-voting/src/mock.rs | 32 +++-- pallets/vtoken-voting/src/tests.rs | 47 +++++-- runtime/bifrost-kusama/src/lib.rs | 18 ++- runtime/bifrost-polkadot/src/lib.rs | 16 ++- 9 files changed, 309 insertions(+), 77 deletions(-) create mode 100644 pallets/vtoken-voting/src/migration.rs diff --git a/integration-tests/bifrost-kusama/src/vtoken_voting.rs b/integration-tests/bifrost-kusama/src/vtoken_voting.rs index 28e64c639..e9dfea6d8 100644 --- a/integration-tests/bifrost-kusama/src/vtoken_voting.rs +++ b/integration-tests/bifrost-kusama/src/vtoken_voting.rs @@ -130,6 +130,7 @@ fn vote_works() { assert_ok!(VtokenVoting::set_delegator_role( RuntimeOrigin::root(), vtoken, + poll_index, 5, VoteRole::Standard { aye: true, conviction: Conviction::Locked5x }, )); diff --git a/pallets/flexible-fee/src/mock.rs b/pallets/flexible-fee/src/mock.rs index 2c42a6fc0..372c08860 100644 --- a/pallets/flexible-fee/src/mock.rs +++ b/pallets/flexible-fee/src/mock.rs @@ -513,6 +513,10 @@ impl VTokenSupplyProvider for SimpleVTokenSupplyProvider { } } +parameter_types! { + pub const ReferendumCheckInterval: BlockNumber = 300; +} + impl bifrost_vtoken_voting::Config for Test { type RuntimeEvent = RuntimeEvent; type RuntimeOrigin = RuntimeOrigin; @@ -527,6 +531,7 @@ impl bifrost_vtoken_voting::Config for Test { type MaxVotes = ConstU32<256>; type ParachainId = ParaInfo; type QueryTimeout = QueryTimeout; + type ReferendumCheckInterval = ReferendumCheckInterval; type WeightInfo = (); } diff --git a/pallets/vtoken-voting/src/benchmarking.rs b/pallets/vtoken-voting/src/benchmarking.rs index 857b262ae..5d708bdea 100644 --- a/pallets/vtoken-voting/src/benchmarking.rs +++ b/pallets/vtoken-voting/src/benchmarking.rs @@ -43,7 +43,10 @@ fn account_vote(b: BalanceOf) -> AccountVote> { AccountVote::Standard { vote: v, balance: b } } -fn init_vote(vtoken: CurrencyIdOf) -> Result<(), BenchmarkError> { +fn init_vote( + vtoken: CurrencyIdOf, + poll_index: PollIndex, +) -> Result<(), BenchmarkError> { let derivative_index = 0; let token = CurrencyId::to_token(&vtoken).unwrap(); T::XcmDestWeightAndFee::set_xcm_dest_weight_and_fee( @@ -58,6 +61,7 @@ fn init_vote(vtoken: CurrencyIdOf) -> Result<(), BenchmarkError> { Pallet::::set_delegator_role( RawOrigin::Root.into(), vtoken, + poll_index, derivative_index, VoteRole::Standard { aye: true, conviction: Conviction::Locked1x }, )?; @@ -79,10 +83,18 @@ mod benchmarks { let control_origin = T::ControlOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - init_vote::(vtoken)?; + init_vote::(vtoken, poll_index)?; + let derivative_index = 0; let r = T::MaxVotes::get() - 1; let response = Response::DispatchResult(MaybeErrorCode::Success); for (i, index) in (0..T::MaxVotes::get()).collect::>().iter().skip(1).enumerate() { + Pallet::::set_delegator_role( + RawOrigin::Root.into(), + vtoken, + *index, + derivative_index, + VoteRole::Standard { aye: true, conviction: Conviction::Locked1x }, + )?; Pallet::::on_idle(Zero::zero(), Weight::MAX); Pallet::::vote(RawOrigin::Signed(caller.clone()).into(), vtoken, *index, vote)?; Pallet::::notify_vote( @@ -113,14 +125,25 @@ mod benchmarks { let caller = funded_account::("caller", 0); whitelist_account!(caller); let vtoken = VKSM; + let poll_index = 0u32; let old_vote = account_vote::(100u32.into()); let control_origin = T::ControlOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - init_vote::(vtoken)?; + init_vote::(vtoken, poll_index)?; + let derivative_index = 0; let r = 50; let response = Response::DispatchResult(MaybeErrorCode::Success); for index in (0..r).collect::>().iter() { + if *index != 0 { + Pallet::::set_delegator_role( + RawOrigin::Root.into(), + vtoken, + *index, + derivative_index, + VoteRole::Standard { aye: true, conviction: Conviction::Locked1x }, + )?; + } Pallet::::vote(RawOrigin::Signed(caller.clone()).into(), vtoken, *index, old_vote)?; Pallet::::notify_vote( control_origin.clone() as ::RuntimeOrigin, @@ -156,7 +179,7 @@ mod benchmarks { let poll_index = 0u32; let vote = account_vote::(100u32.into()); - init_vote::(vtoken)?; + init_vote::(vtoken, poll_index)?; Pallet::::vote(origin.clone().into(), vtoken, poll_index, vote)?; Pallet::::set_referendum_status( RawOrigin::Root.into(), @@ -182,7 +205,7 @@ mod benchmarks { let vote = account_vote::(100u32.into()); let derivative_index = 0u16; - init_vote::(vtoken)?; + init_vote::(vtoken, poll_index)?; Pallet::::vote(origin.clone().into(), vtoken, poll_index, vote)?; Pallet::::set_referendum_status( RawOrigin::Root.into(), @@ -212,7 +235,7 @@ mod benchmarks { let poll_index = 0u32; let vote = account_vote::(100u32.into()); - init_vote::(vtoken)?; + init_vote::(vtoken, poll_index)?; let caller = funded_account::("caller", 0); whitelist_account!(caller); let origin_caller = RawOrigin::Signed(caller); @@ -235,10 +258,11 @@ mod benchmarks { let origin = T::ControlOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let vtoken = VKSM; + let poll_index = 0u32; let derivative_index = 10; let vote_role = VoteRole::SplitAbstain; - init_vote::(vtoken)?; + init_vote::(vtoken, poll_index)?; T::DerivativeAccount::add_delegator( CurrencyId::to_token(&vtoken).unwrap(), derivative_index, @@ -249,6 +273,7 @@ mod benchmarks { _( origin as ::RuntimeOrigin, vtoken, + poll_index, derivative_index, vote_role, ); @@ -264,7 +289,7 @@ mod benchmarks { let poll_index = 0u32; let info = ReferendumInfo::Completed(>::block_number()); - init_vote::(vtoken)?; + init_vote::(vtoken, poll_index)?; let caller = funded_account::("caller", 0); whitelist_account!(caller); let origin_caller = RawOrigin::Signed(caller); @@ -312,7 +337,7 @@ mod benchmarks { let query_id = 1u64; let response = Response::DispatchResult(MaybeErrorCode::Success); - init_vote::(vtoken)?; + init_vote::(vtoken, poll_index)?; let caller = funded_account::("caller", 0); whitelist_account!(caller); let origin_caller = RawOrigin::Signed(caller); @@ -334,7 +359,7 @@ mod benchmarks { let query_id = 1u64; let response = Response::DispatchResult(MaybeErrorCode::Success); - init_vote::(vtoken)?; + init_vote::(vtoken, poll_index)?; let caller = funded_account::("caller", 0); whitelist_account!(caller); let origin_caller = RawOrigin::Signed(caller); diff --git a/pallets/vtoken-voting/src/lib.rs b/pallets/vtoken-voting/src/lib.rs index 84e456ad5..e2217783e 100644 --- a/pallets/vtoken-voting/src/lib.rs +++ b/pallets/vtoken-voting/src/lib.rs @@ -29,6 +29,8 @@ mod tests; mod call; mod vote; + +pub mod migration; pub mod weights; use crate::vote::{Casting, Tally, Voting}; @@ -81,7 +83,11 @@ pub type ReferendumInfoOf = ReferendumInfo, TallyOf>; pub mod pallet { use super::*; + /// The current storage version. + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); #[pallet::config] @@ -122,6 +128,9 @@ pub mod pallet { #[pallet::constant] type QueryTimeout: Get>; + #[pallet::constant] + type ReferendumCheckInterval: Get>; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -290,12 +299,13 @@ pub mod pallet { StorageMap<_, Twox64Concat, CurrencyIdOf, BlockNumberFor>; #[pallet::storage] - pub type DelegatorVote = StorageDoubleMap< + pub type DelegatorVote = StorageNMap< _, - Twox64Concat, - CurrencyIdOf, - Twox64Concat, - DerivativeIndex, + ( + NMapKey>, + NMapKey, + NMapKey, + ), AccountVote>, >; @@ -321,7 +331,7 @@ pub mod pallet { #[pallet::genesis_config] pub struct GenesisConfig { - pub delegator_votes: Vec<(CurrencyIdOf, u8, DerivativeIndex)>, + pub delegator_votes: Vec<(CurrencyIdOf, PollIndex, u8, DerivativeIndex)>, pub undeciding_timeouts: Vec<(CurrencyIdOf, BlockNumberFor)>, } @@ -335,14 +345,15 @@ pub mod pallet { #[pallet::genesis_build] impl GenesisBuild for GenesisConfig { fn build(&self) { - self.delegator_votes.iter().for_each(|(vtoken, role, derivative_index)| { - let vote_role = VoteRole::try_from(*role).unwrap(); - DelegatorVote::::insert( - vtoken, - derivative_index, - AccountVote::>::from(vote_role), - ); - }); + self.delegator_votes + .iter() + .for_each(|(vtoken, poll_index, role, derivative_index)| { + let vote_role = VoteRole::try_from(*role).unwrap(); + DelegatorVote::::insert( + (vtoken, poll_index, derivative_index), + AccountVote::>::from(vote_role), + ); + }); self.undeciding_timeouts.iter().for_each(|(vtoken, undeciding_timeout)| { UndecidingTimeout::::insert(vtoken, undeciding_timeout); }); @@ -351,10 +362,12 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { - fn on_idle(_n: BlockNumberFor, remaining_weight: Weight) -> Weight { + fn on_idle(n: BlockNumberFor, remaining_weight: Weight) -> Weight { let db_weight = T::DbWeight::get(); let mut used_weight = db_weight.reads(3); - if remaining_weight.any_lt(used_weight) { + if remaining_weight.any_lt(used_weight) || + n % T::ReferendumCheckInterval::get() != Zero::zero() + { return Weight::zero(); } let relay_current_block_number = @@ -409,7 +422,7 @@ pub mod pallet { Self::ensure_no_pending_vote(vtoken, poll_index)?; let new_vote = Self::compute_new_vote(vtoken, vote)?; - let derivative_index = Self::try_select_derivative_index(vtoken, new_vote)?; + let derivative_index = Self::try_select_derivative_index(vtoken, poll_index, new_vote)?; if let Some(d) = VoteDelegatorFor::::get((&who, vtoken, poll_index)) { ensure!(d == derivative_index, Error::::ChangeDelegator) } @@ -441,8 +454,8 @@ pub mod pallet { )?; // send XCM message - let delegator_vote = - DelegatorVote::::get(vtoken, derivative_index).ok_or(Error::::NoData)?; + let delegator_vote = DelegatorVote::::get((vtoken, poll_index, derivative_index)) + .ok_or(Error::::NoData)?; let vote_call = as ConvictionVotingCall>::vote(poll_index, delegator_vote); let notify_call = Call::::notify_vote { query_id: 0, response: Default::default() }; @@ -514,7 +527,10 @@ pub mod pallet { let who = ensure_signed(origin)?; Self::ensure_vtoken(&vtoken)?; Self::ensure_referendum_expired(vtoken, poll_index)?; - ensure!(DelegatorVote::::contains_key(vtoken, derivative_index), Error::::NoData); + ensure!( + DelegatorVote::::contains_key((vtoken, poll_index, derivative_index)), + Error::::NoData + ); let notify_call = Call::::notify_remove_delegator_vote { query_id: 0, @@ -573,6 +589,7 @@ pub mod pallet { pub fn set_delegator_role( origin: OriginFor, vtoken: CurrencyIdOf, + poll_index: PollIndex, #[pallet::compact] derivative_index: DerivativeIndex, vote_role: VoteRole, ) -> DispatchResult { @@ -584,13 +601,12 @@ pub mod pallet { Error::::NoData ); ensure!( - !DelegatorVote::::contains_key(vtoken, derivative_index), + !DelegatorVote::::contains_key((vtoken, poll_index, derivative_index)), Error::::DerivativeIndexOccupied ); DelegatorVote::::insert( - vtoken, - derivative_index, + (vtoken, poll_index, derivative_index), AccountVote::>::from(vote_role), ); @@ -745,8 +761,7 @@ pub mod pallet { let success = Response::DispatchResult(MaybeErrorCode::Success) == response; if success { DelegatorVote::::try_mutate_exists( - vtoken, - derivative_index, + (vtoken, poll_index, derivative_index), |maybe_vote| { if let Some(inner_vote) = maybe_vote { inner_vote @@ -782,7 +797,7 @@ pub mod pallet { vtoken_balance: BalanceOf, ) -> Result>, BalanceOf)>, DispatchError> { ensure!( - vote.balance() <= T::MultiCurrency::total_balance(vtoken, who), + vtoken_balance <= T::MultiCurrency::total_balance(vtoken, who), Error::::InsufficientFunds ); let mut old_vote = None; @@ -794,7 +809,9 @@ pub mod pallet { Ok(i) => { // Shouldn't be possible to fail, but we handle it gracefully. tally.remove(votes[i].1).ok_or(ArithmeticError::Underflow)?; - Self::try_sub_delegator_vote(vtoken, votes[i].2, votes[i].1)?; + Self::try_sub_delegator_vote( + vtoken, poll_index, votes[i].2, votes[i].1, + )?; old_vote = Some((votes[i].1, votes[i].3)); if let Some(approve) = votes[i].1.as_standard() { tally.reduce(approve, *delegations); @@ -814,7 +831,7 @@ pub mod pallet { } // Shouldn't be possible to fail, but we handle it gracefully. tally.add(vote).ok_or(ArithmeticError::Overflow)?; - Self::try_add_delegator_vote(vtoken, derivative_index, vote)?; + Self::try_add_delegator_vote(vtoken, poll_index, derivative_index, vote)?; if let Some(approve) = vote.as_standard() { tally.increase(approve, *delegations); } @@ -855,7 +872,7 @@ pub mod pallet { ensure!(matches!(scope, UnvoteScope::Any), Error::::NoPermission); // Shouldn't be possible to fail, but we handle it gracefully. tally.remove(v.1).ok_or(ArithmeticError::Underflow)?; - Self::try_sub_delegator_vote(vtoken, v.2, v.1)?; + Self::try_sub_delegator_vote(vtoken, poll_index, v.2, v.1)?; if let Some(approve) = v.1.as_standard() { tally.reduce(approve, *delegations); } @@ -1108,11 +1125,12 @@ pub mod pallet { fn try_select_derivative_index( vtoken: CurrencyIdOf, + poll_index: PollIndex, my_vote: AccountVote>, ) -> Result { let token = CurrencyId::to_token(&vtoken).map_err(|_| Error::::NoData)?; - let mut data = DelegatorVote::::iter_prefix(vtoken) + let mut data = DelegatorVote::::iter_prefix((vtoken, poll_index)) .map(|(index, vote)| { let (_, active) = T::DerivativeAccount::get_stake_info(token, index) .unwrap_or(Default::default()); @@ -1134,36 +1152,40 @@ pub mod pallet { Ok(*index) } - fn try_add_delegator_vote( + pub(crate) fn try_add_delegator_vote( vtoken: CurrencyIdOf, + poll_index: PollIndex, derivative_index: DerivativeIndex, vote: AccountVote>, ) -> Result>, DispatchError> { - DelegatorVote::::try_mutate_exists(vtoken, derivative_index, |maybe_vote| { - match maybe_vote { + DelegatorVote::::try_mutate_exists( + (vtoken, poll_index, derivative_index), + |maybe_vote| match maybe_vote { Some(inner_vote) => { inner_vote.checked_add(vote).map_err(|_| ArithmeticError::Overflow)?; Ok(*inner_vote) }, None => Err(Error::::NoData.into()), - } - }) + }, + ) } fn try_sub_delegator_vote( vtoken: CurrencyIdOf, + poll_index: PollIndex, derivative_index: DerivativeIndex, vote: AccountVote>, ) -> Result>, DispatchError> { - DelegatorVote::::try_mutate_exists(vtoken, derivative_index, |maybe_vote| { - match maybe_vote { + DelegatorVote::::try_mutate_exists( + (vtoken, poll_index, derivative_index), + |maybe_vote| match maybe_vote { Some(inner_vote) => { inner_vote.checked_sub(vote).map_err(|_| ArithmeticError::Underflow)?; Ok(*inner_vote) }, None => Err(Error::::NoData.into()), - } - }) + }, + ) } fn compute_new_vote( diff --git a/pallets/vtoken-voting/src/migration.rs b/pallets/vtoken-voting/src/migration.rs new file mode 100644 index 000000000..8be115336 --- /dev/null +++ b/pallets/vtoken-voting/src/migration.rs @@ -0,0 +1,122 @@ +// This file is part of Bifrost. + +// Copyright (C) 2019-2022 Liebi Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +use super::*; +use frame_support::{storage_alias, traits::OnRuntimeUpgrade, weights::Weight}; + +mod v0 { + use super::*; + + #[storage_alias] + pub(super) type DelegatorVote = StorageDoubleMap< + Pallet, + Twox64Concat, + CurrencyIdOf, + Twox64Concat, + DerivativeIndex, + AccountVote>, + >; +} + +pub mod v1 { + use super::*; + use frame_support::traits::StorageVersion; + + pub struct MigrateToV1(sp_std::marker::PhantomData); + impl OnRuntimeUpgrade for MigrateToV1 { + fn on_runtime_upgrade() -> Weight { + if StorageVersion::get::>() == 0 { + let weight_consumed = migrate_to_v1::(); + log::info!("Migrating vtoken-voting storage to v1"); + StorageVersion::new(1).put::>(); + weight_consumed + } else { + log::warn!("vtoken-voting migration should be removed."); + T::DbWeight::get().reads(1) + } + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + log::info!( + "vtoken-voting before migration: version: {:?}", + StorageVersion::get::>(), + ); + log::info!( + "vtoken-voting before migration: v0 count: {}", + v0::DelegatorVote::::iter().count(), + ); + ensure!( + v0::DelegatorVote::::iter().count() > 0, + "v0::DelegatorVote should not be empty before the migration" + ); + + Ok(Vec::new()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_: Vec) -> Result<(), &'static str> { + log::info!( + "vtoken-voting after migration: version: {:?}", + StorageVersion::get::>(), + ); + log::info!( + "vtoken-voting after migration: v1 count: {}", + DelegatorVote::::iter().count() + ); + ensure!( + DelegatorVote::::iter().count() > 0, + "DelegatorVote should not be empty after the migration" + ); + + Ok(()) + } + } +} + +pub fn migrate_to_v1() -> Weight { + let mut weight: Weight = Weight::zero(); + + let old_keys = v0::DelegatorVote::::iter_keys(); + let vtoken = VKSM; + + for (_, voting) in VotingFor::::iter() { + if let Voting::Casting(Casting { votes, .. }) = voting { + for (poll_index, vote, derivative_index, _) in votes.iter() { + if DelegatorVote::::contains_key((vtoken, poll_index, derivative_index)) { + let _ = Pallet::::try_add_delegator_vote( + vtoken, + *poll_index, + *derivative_index, + *vote, + ); + weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 2)); + } else { + DelegatorVote::::insert((vtoken, poll_index, derivative_index), vote); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + } + } + } + } + + for (vtoken, derivative_index) in old_keys { + v0::DelegatorVote::::remove(vtoken, derivative_index); + } + + weight +} diff --git a/pallets/vtoken-voting/src/mock.rs b/pallets/vtoken-voting/src/mock.rs index 65e639687..f3e8fd337 100644 --- a/pallets/vtoken-voting/src/mock.rs +++ b/pallets/vtoken-voting/src/mock.rs @@ -292,6 +292,7 @@ impl DerivativeAccountHandler for DerivativeAccount { parameter_types! { pub static RelaychainBlockNumber: BlockNumber = 1; + pub static ReferendumCheckInterval: BlockNumber = 1; } pub struct RelaychainDataProvider; @@ -336,6 +337,7 @@ impl vtoken_voting::Config for Runtime { type MaxVotes = ConstU32<256>; type ParachainId = ParachainId; type QueryTimeout = QueryTimeout; + type ReferendumCheckInterval = ReferendumCheckInterval; type WeightInfo = (); } @@ -353,20 +355,24 @@ pub fn new_test_ext() -> sp_io::TestExternalities { .assimilate_storage(&mut t) .unwrap(); + let mut delegator_votes = Vec::new(); + for poll_index in 0..=256 { + delegator_votes.extend(vec![ + (VKSM, poll_index, 0, 0), + (VKSM, poll_index, 1, 1), + (VKSM, poll_index, 2, 2), + (VKSM, poll_index, 3, 3), + (VKSM, poll_index, 4, 4), + (VKSM, poll_index, 5, 5), + (VKSM, poll_index, 10, 10), + (VKSM, poll_index, 11, 11), + (VKSM, poll_index, 15, 15), + (VKSM, poll_index, 20, 20), + (VKSM, poll_index, 21, 21), + ]); + } vtoken_voting::GenesisConfig:: { - delegator_votes: vec![ - (VKSM, 0, 0), - (VKSM, 1, 1), - (VKSM, 2, 2), - (VKSM, 3, 3), - (VKSM, 4, 4), - (VKSM, 5, 5), - (VKSM, 10, 10), - (VKSM, 11, 11), - (VKSM, 15, 15), - (VKSM, 20, 20), - (VKSM, 21, 21), - ], + delegator_votes, undeciding_timeouts: vec![(VKSM, 100)], } .assimilate_storage(&mut t) diff --git a/pallets/vtoken-voting/src/tests.rs b/pallets/vtoken-voting/src/tests.rs index ca3a3b296..a36ac22c8 100644 --- a/pallets/vtoken-voting/src/tests.rs +++ b/pallets/vtoken-voting/src/tests.rs @@ -427,12 +427,14 @@ fn kill_referendum_with_origin_signed_fails() { fn set_delegator_role_works() { new_test_ext().execute_with(|| { let vtoken = VKSM; + let poll_index = 3; let derivative_index: DerivativeIndex = 100; let role = aye(10, 3).into(); assert_ok!(VtokenVoting::set_delegator_role( RuntimeOrigin::root(), vtoken, + poll_index, derivative_index, role, )); @@ -595,7 +597,10 @@ fn notify_vote_success_works() { tally: TallyOf::::from_parts(10, 0, 2), })) ); - assert_eq!(DelegatorVote::::get(vtoken, derivative_index), Some(aye(2, 5))); + assert_eq!( + DelegatorVote::::get((vtoken, poll_index, derivative_index)), + Some(aye(2, 5)) + ); assert_eq!(tally(vtoken, poll_index), Tally::from_parts(10, 0, 2)); System::assert_last_event(RuntimeEvent::VtokenVoting(Event::Voted { who: ALICE, @@ -613,7 +618,10 @@ fn notify_vote_success_works() { tally: TallyOf::::from_parts(10, 0, 2), })) ); - assert_eq!(DelegatorVote::::get(vtoken, derivative_index), Some(aye(2, 5))); + assert_eq!( + DelegatorVote::::get((vtoken, poll_index, derivative_index)), + Some(aye(2, 5)) + ); System::assert_has_event(RuntimeEvent::VtokenVoting(Event::VoteNotified { vtoken, poll_index, @@ -707,7 +715,10 @@ fn notify_vote_fail_works() { tally: TallyOf::::from_parts(10, 0, 2), })) ); - assert_eq!(DelegatorVote::::get(vtoken, derivative_index), Some(aye(2, 5))); + assert_eq!( + DelegatorVote::::get((vtoken, poll_index, derivative_index)), + Some(aye(2, 5)) + ); assert_eq!(tally(vtoken, poll_index), Tally::from_parts(10, 0, 2)); System::assert_last_event(RuntimeEvent::VtokenVoting(Event::Voted { who: ALICE, @@ -719,7 +730,10 @@ fn notify_vote_fail_works() { assert_ok!(VtokenVoting::notify_vote(origin_response(), query_id, response.clone())); assert_eq!(ReferendumInfoFor::::get(vtoken, poll_index), None); - assert_eq!(DelegatorVote::::get(vtoken, derivative_index), Some(aye(0, 5))); + assert_eq!( + DelegatorVote::::get((vtoken, poll_index, derivative_index)), + Some(aye(0, 5)) + ); System::assert_has_event(RuntimeEvent::VtokenVoting(Event::VoteNotified { vtoken, poll_index, @@ -758,7 +772,10 @@ fn notify_remove_delegator_vote_success_works() { let response = response_success(); assert_ok!(VtokenVoting::vote(RuntimeOrigin::signed(ALICE), vtoken, poll_index, aye(2, 5))); - assert_eq!(DelegatorVote::::get(vtoken, derivative_index), Some(aye(2, 5))); + assert_eq!( + DelegatorVote::::get((vtoken, poll_index, derivative_index)), + Some(aye(2, 5)) + ); assert_eq!(tally(vtoken, poll_index), Tally::from_parts(10, 0, 2)); System::assert_last_event(RuntimeEvent::VtokenVoting(Event::Voted { who: ALICE, @@ -784,7 +801,10 @@ fn notify_remove_delegator_vote_success_works() { poll_index, derivative_index, )); - assert_eq!(DelegatorVote::::get(vtoken, derivative_index), Some(aye(2, 5))); + assert_eq!( + DelegatorVote::::get((vtoken, poll_index, derivative_index)), + Some(aye(2, 5)) + ); query_id = 1; assert_ok!(VtokenVoting::notify_remove_delegator_vote( @@ -792,7 +812,10 @@ fn notify_remove_delegator_vote_success_works() { query_id, response.clone() )); - assert_eq!(DelegatorVote::::get(vtoken, derivative_index), Some(aye(0, 5))); + assert_eq!( + DelegatorVote::::get((vtoken, poll_index, derivative_index)), + Some(aye(0, 5)) + ); System::assert_has_event(RuntimeEvent::VtokenVoting(Event::DelegatorVoteRemovedNotified { vtoken, poll_index, @@ -841,7 +864,10 @@ fn notify_remove_delegator_vote_fail_works() { poll_index, derivative_index, )); - assert_eq!(DelegatorVote::::get(vtoken, derivative_index), Some(aye(2, 5))); + assert_eq!( + DelegatorVote::::get((vtoken, poll_index, derivative_index)), + Some(aye(2, 5)) + ); query_id = 1; assert_ok!(VtokenVoting::notify_remove_delegator_vote( @@ -849,7 +875,10 @@ fn notify_remove_delegator_vote_fail_works() { query_id, response.clone() )); - assert_eq!(DelegatorVote::::get(vtoken, derivative_index), Some(aye(2, 5))); + assert_eq!( + DelegatorVote::::get((vtoken, poll_index, derivative_index)), + Some(aye(2, 5)) + ); System::assert_last_event(RuntimeEvent::VtokenVoting(Event::ResponseReceived { responder: Parent.into(), query_id, diff --git a/runtime/bifrost-kusama/src/lib.rs b/runtime/bifrost-kusama/src/lib.rs index 4a53cfe8c..6b0309880 100644 --- a/runtime/bifrost-kusama/src/lib.rs +++ b/runtime/bifrost-kusama/src/lib.rs @@ -1462,6 +1462,7 @@ impl bifrost_cross_in_out::Config for Runtime { parameter_types! { pub const QueryTimeout: BlockNumber = 100; + pub const ReferendumCheckInterval: BlockNumber = 300; } pub struct DerivativeAccountTokenFilter; @@ -1485,6 +1486,7 @@ impl bifrost_vtoken_voting::Config for Runtime { type ParachainId = SelfParaChainId; type MaxVotes = ConstU32<256>; type QueryTimeout = QueryTimeout; + type ReferendumCheckInterval = ReferendumCheckInterval; type WeightInfo = weights::bifrost_vtoken_voting::BifrostWeight; } @@ -1894,8 +1896,18 @@ pub type CheckedExtrinsic = generic::CheckedExtrinsic; -parameter_types! { - pub const XcmActionStr: &'static str = "XcmAction"; +/// All migrations that will run on the next runtime upgrade. +/// +/// This contains the combined migrations of the last 10 releases. It allows to skip runtime +/// upgrades in case governance decides to do so. THE ORDER IS IMPORTANT. +pub type Migrations = migrations::Unreleased; + +/// The runtime migrations per release. +pub mod migrations { + use super::*; + + /// Unreleased migrations. Add new ones here: + pub type Unreleased = (bifrost_vtoken_voting::migration::v1::MigrateToV1,); } /// Executive: handles dispatch to the various modules. @@ -1905,7 +1917,7 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, - (), + Migrations, >; #[cfg(feature = "runtime-benchmarks")] diff --git a/runtime/bifrost-polkadot/src/lib.rs b/runtime/bifrost-polkadot/src/lib.rs index 6762c60dd..d6a1e95c1 100644 --- a/runtime/bifrost-polkadot/src/lib.rs +++ b/runtime/bifrost-polkadot/src/lib.rs @@ -1346,6 +1346,7 @@ impl bifrost_stable_pool::Config for Runtime { parameter_types! { pub const QueryTimeout: BlockNumber = 100; + pub const ReferendumCheckInterval: BlockNumber = 300; } pub struct DerivativeAccountTokenFilter; @@ -1369,6 +1370,7 @@ impl bifrost_vtoken_voting::Config for Runtime { type ParachainId = SelfParaChainId; type MaxVotes = ConstU32<256>; type QueryTimeout = QueryTimeout; + type ReferendumCheckInterval = ReferendumCheckInterval; type WeightInfo = weights::bifrost_vtoken_voting::BifrostWeight; } @@ -1699,8 +1701,16 @@ pub type CheckedExtrinsic = generic::CheckedExtrinsic; -parameter_types! { - pub const XcmActionStr: &'static str = "XcmAction"; +/// All migrations that will run on the next runtime upgrade. +/// +/// This contains the combined migrations of the last 10 releases. It allows to skip runtime +/// upgrades in case governance decides to do so. THE ORDER IS IMPORTANT. +pub type Migrations = migrations::Unreleased; + +/// The runtime migrations per release. +pub mod migrations { + /// Unreleased migrations. Add new ones here: + pub type Unreleased = (); } /// Executive: handles dispatch to the various modules. @@ -1710,7 +1720,7 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, - (), + Migrations, >; #[cfg(feature = "runtime-benchmarks")] From 094a0512411c141148ba98d6e2e6252634b10b13 Mon Sep 17 00:00:00 2001 From: Edwin Date: Wed, 18 Oct 2023 07:09:18 -0500 Subject: [PATCH 08/10] Fix vtoken voting (#1061) * Fix unlock * Fix update_lock * Fix tests * Fix tests * Ensure no pending vote when unlock * Fix benchmarking * Fix set_delegator_role --- .../bifrost-kusama/src/vtoken_voting.rs | 1 - pallets/vtoken-voting/src/benchmarking.rs | 51 ++----- pallets/vtoken-voting/src/lib.rs | 69 ++++++--- pallets/vtoken-voting/src/mock.rs | 30 ++-- pallets/vtoken-voting/src/tests.rs | 141 ++++++++++++++++-- pallets/vtoken-voting/src/vote.rs | 11 +- 6 files changed, 211 insertions(+), 92 deletions(-) diff --git a/integration-tests/bifrost-kusama/src/vtoken_voting.rs b/integration-tests/bifrost-kusama/src/vtoken_voting.rs index e9dfea6d8..28e64c639 100644 --- a/integration-tests/bifrost-kusama/src/vtoken_voting.rs +++ b/integration-tests/bifrost-kusama/src/vtoken_voting.rs @@ -130,7 +130,6 @@ fn vote_works() { assert_ok!(VtokenVoting::set_delegator_role( RuntimeOrigin::root(), vtoken, - poll_index, 5, VoteRole::Standard { aye: true, conviction: Conviction::Locked5x }, )); diff --git a/pallets/vtoken-voting/src/benchmarking.rs b/pallets/vtoken-voting/src/benchmarking.rs index 5d708bdea..1c3f6c11f 100644 --- a/pallets/vtoken-voting/src/benchmarking.rs +++ b/pallets/vtoken-voting/src/benchmarking.rs @@ -43,10 +43,7 @@ fn account_vote(b: BalanceOf) -> AccountVote> { AccountVote::Standard { vote: v, balance: b } } -fn init_vote( - vtoken: CurrencyIdOf, - poll_index: PollIndex, -) -> Result<(), BenchmarkError> { +fn init_vote(vtoken: CurrencyIdOf) -> Result<(), BenchmarkError> { let derivative_index = 0; let token = CurrencyId::to_token(&vtoken).unwrap(); T::XcmDestWeightAndFee::set_xcm_dest_weight_and_fee( @@ -61,7 +58,6 @@ fn init_vote( Pallet::::set_delegator_role( RawOrigin::Root.into(), vtoken, - poll_index, derivative_index, VoteRole::Standard { aye: true, conviction: Conviction::Locked1x }, )?; @@ -83,18 +79,10 @@ mod benchmarks { let control_origin = T::ControlOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - init_vote::(vtoken, poll_index)?; - let derivative_index = 0; + init_vote::(vtoken)?; let r = T::MaxVotes::get() - 1; let response = Response::DispatchResult(MaybeErrorCode::Success); for (i, index) in (0..T::MaxVotes::get()).collect::>().iter().skip(1).enumerate() { - Pallet::::set_delegator_role( - RawOrigin::Root.into(), - vtoken, - *index, - derivative_index, - VoteRole::Standard { aye: true, conviction: Conviction::Locked1x }, - )?; Pallet::::on_idle(Zero::zero(), Weight::MAX); Pallet::::vote(RawOrigin::Signed(caller.clone()).into(), vtoken, *index, vote)?; Pallet::::notify_vote( @@ -125,25 +113,14 @@ mod benchmarks { let caller = funded_account::("caller", 0); whitelist_account!(caller); let vtoken = VKSM; - let poll_index = 0u32; let old_vote = account_vote::(100u32.into()); let control_origin = T::ControlOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - init_vote::(vtoken, poll_index)?; - let derivative_index = 0; + init_vote::(vtoken)?; let r = 50; let response = Response::DispatchResult(MaybeErrorCode::Success); for index in (0..r).collect::>().iter() { - if *index != 0 { - Pallet::::set_delegator_role( - RawOrigin::Root.into(), - vtoken, - *index, - derivative_index, - VoteRole::Standard { aye: true, conviction: Conviction::Locked1x }, - )?; - } Pallet::::vote(RawOrigin::Signed(caller.clone()).into(), vtoken, *index, old_vote)?; Pallet::::notify_vote( control_origin.clone() as ::RuntimeOrigin, @@ -179,7 +156,7 @@ mod benchmarks { let poll_index = 0u32; let vote = account_vote::(100u32.into()); - init_vote::(vtoken, poll_index)?; + init_vote::(vtoken)?; Pallet::::vote(origin.clone().into(), vtoken, poll_index, vote)?; Pallet::::set_referendum_status( RawOrigin::Root.into(), @@ -189,6 +166,12 @@ mod benchmarks { )?; Pallet::::set_vote_locking_period(RawOrigin::Root.into(), vtoken, 0u32.into())?; + let notify_origin = + T::ResponseOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let query_id = 0u64; + let response = Response::DispatchResult(MaybeErrorCode::Success); + Pallet::::notify_vote(notify_origin, query_id, response)?; + #[extrinsic_call] _(origin, vtoken, poll_index); @@ -205,7 +188,7 @@ mod benchmarks { let vote = account_vote::(100u32.into()); let derivative_index = 0u16; - init_vote::(vtoken, poll_index)?; + init_vote::(vtoken)?; Pallet::::vote(origin.clone().into(), vtoken, poll_index, vote)?; Pallet::::set_referendum_status( RawOrigin::Root.into(), @@ -235,7 +218,7 @@ mod benchmarks { let poll_index = 0u32; let vote = account_vote::(100u32.into()); - init_vote::(vtoken, poll_index)?; + init_vote::(vtoken)?; let caller = funded_account::("caller", 0); whitelist_account!(caller); let origin_caller = RawOrigin::Signed(caller); @@ -258,11 +241,10 @@ mod benchmarks { let origin = T::ControlOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let vtoken = VKSM; - let poll_index = 0u32; let derivative_index = 10; let vote_role = VoteRole::SplitAbstain; - init_vote::(vtoken, poll_index)?; + init_vote::(vtoken)?; T::DerivativeAccount::add_delegator( CurrencyId::to_token(&vtoken).unwrap(), derivative_index, @@ -273,7 +255,6 @@ mod benchmarks { _( origin as ::RuntimeOrigin, vtoken, - poll_index, derivative_index, vote_role, ); @@ -289,7 +270,7 @@ mod benchmarks { let poll_index = 0u32; let info = ReferendumInfo::Completed(>::block_number()); - init_vote::(vtoken, poll_index)?; + init_vote::(vtoken)?; let caller = funded_account::("caller", 0); whitelist_account!(caller); let origin_caller = RawOrigin::Signed(caller); @@ -337,7 +318,7 @@ mod benchmarks { let query_id = 1u64; let response = Response::DispatchResult(MaybeErrorCode::Success); - init_vote::(vtoken, poll_index)?; + init_vote::(vtoken)?; let caller = funded_account::("caller", 0); whitelist_account!(caller); let origin_caller = RawOrigin::Signed(caller); @@ -359,7 +340,7 @@ mod benchmarks { let query_id = 1u64; let response = Response::DispatchResult(MaybeErrorCode::Success); - init_vote::(vtoken, poll_index)?; + init_vote::(vtoken)?; let caller = funded_account::("caller", 0); whitelist_account!(caller); let origin_caller = RawOrigin::Signed(caller); diff --git a/pallets/vtoken-voting/src/lib.rs b/pallets/vtoken-voting/src/lib.rs index e2217783e..d97261c8a 100644 --- a/pallets/vtoken-voting/src/lib.rs +++ b/pallets/vtoken-voting/src/lib.rs @@ -298,6 +298,10 @@ pub mod pallet { pub type UndecidingTimeout = StorageMap<_, Twox64Concat, CurrencyIdOf, BlockNumberFor>; + #[pallet::storage] + pub type DelegatorVoteRole = + StorageDoubleMap<_, Twox64Concat, CurrencyIdOf, Twox64Concat, DerivativeIndex, VoteRole>; + #[pallet::storage] pub type DelegatorVote = StorageNMap< _, @@ -331,29 +335,24 @@ pub mod pallet { #[pallet::genesis_config] pub struct GenesisConfig { - pub delegator_votes: Vec<(CurrencyIdOf, PollIndex, u8, DerivativeIndex)>, + pub delegator_vote_roles: Vec<(CurrencyIdOf, u8, DerivativeIndex)>, pub undeciding_timeouts: Vec<(CurrencyIdOf, BlockNumberFor)>, } #[cfg(feature = "std")] impl Default for GenesisConfig { fn default() -> Self { - GenesisConfig { delegator_votes: vec![], undeciding_timeouts: vec![] } + GenesisConfig { delegator_vote_roles: vec![], undeciding_timeouts: vec![] } } } #[pallet::genesis_build] impl GenesisBuild for GenesisConfig { fn build(&self) { - self.delegator_votes - .iter() - .for_each(|(vtoken, poll_index, role, derivative_index)| { - let vote_role = VoteRole::try_from(*role).unwrap(); - DelegatorVote::::insert( - (vtoken, poll_index, derivative_index), - AccountVote::>::from(vote_role), - ); - }); + self.delegator_vote_roles.iter().for_each(|(vtoken, role, derivative_index)| { + let vote_role = VoteRole::try_from(*role).unwrap(); + DelegatorVoteRole::::insert(vtoken, derivative_index, vote_role); + }); self.undeciding_timeouts.iter().for_each(|(vtoken, undeciding_timeout)| { UndecidingTimeout::::insert(vtoken, undeciding_timeout); }); @@ -443,6 +442,11 @@ pub mod pallet { submitted = true; } + if !DelegatorVote::::contains_key((vtoken, poll_index, derivative_index)) { + let default_vote: AccountVote> = VoteRole::from(vote).into(); + DelegatorVote::::insert((vtoken, poll_index, derivative_index), default_vote); + } + // record vote info let maybe_old_vote = Self::try_vote( &who, @@ -501,9 +505,10 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; Self::ensure_vtoken(&vtoken)?; - Self::ensure_referendum_expired(vtoken, poll_index) + Self::ensure_referendum_completed(vtoken, poll_index) .or(Self::ensure_referendum_killed(vtoken, poll_index)) - .map_err(|_| Error::::NotExpired)?; + .map_err(|_| Error::::NoPermissionYet)?; + Self::ensure_no_pending_vote(vtoken, poll_index)?; Self::try_remove_vote(&who, vtoken, poll_index, UnvoteScope::Any)?; Self::update_lock(&who, vtoken, &poll_index)?; @@ -589,7 +594,6 @@ pub mod pallet { pub fn set_delegator_role( origin: OriginFor, vtoken: CurrencyIdOf, - poll_index: PollIndex, #[pallet::compact] derivative_index: DerivativeIndex, vote_role: VoteRole, ) -> DispatchResult { @@ -601,14 +605,11 @@ pub mod pallet { Error::::NoData ); ensure!( - !DelegatorVote::::contains_key((vtoken, poll_index, derivative_index)), + !DelegatorVoteRole::::contains_key(vtoken, derivative_index), Error::::DerivativeIndexOccupied ); - DelegatorVote::::insert( - (vtoken, poll_index, derivative_index), - AccountVote::>::from(vote_role), - ); + DelegatorVoteRole::::insert(vtoken, derivative_index, vote_role); Self::deposit_event(Event::::DelegatorRoleSet { vtoken, @@ -896,7 +897,8 @@ pub mod pallet { } Ok(()) }, - PollStatus::None => Ok(()), // Poll was cancelled. + PollStatus::Killed(_) => Ok(()), // Poll was killed. + PollStatus::None => Ok(()), // Poll was cancelled. }) } else { Ok(()) @@ -912,12 +914,13 @@ pub mod pallet { poll_index: &PollIndex, ) -> DispatchResult { let class_lock_needed = VotingFor::::mutate(who, |voting| { - voting.rejig(frame_system::Pallet::::block_number()); + voting.rejig(T::RelaychainBlockNumberProvider::current_block_number()); voting.locked_balance() }); let lock_needed = ClassLocksFor::::mutate(who, |locks| { locks.retain(|x| &x.0 != poll_index); - if !class_lock_needed.is_zero() { + if !class_lock_needed.is_zero() && !Self::is_referendum_killed(vtoken, *poll_index) + { let ok = locks.try_push((*poll_index, class_lock_needed)).is_ok(); debug_assert!( ok, @@ -1103,6 +1106,13 @@ pub mod pallet { Ok(responder) } + fn is_referendum_killed(vtoken: CurrencyIdOf, poll_index: PollIndex) -> bool { + match ReferendumInfoFor::::get(vtoken, poll_index) { + Some(ReferendumInfo::Killed(_)) => true, + _ => false, + } + } + fn try_access_poll( vtoken: CurrencyIdOf, poll_index: PollIndex, @@ -1119,6 +1129,7 @@ pub mod pallet { Ok(result) }, Some(ReferendumInfo::Completed(end)) => f(PollStatus::Completed(end, false)), + Some(ReferendumInfo::Killed(end)) => f(PollStatus::Killed(end)), _ => f(PollStatus::None), } } @@ -1130,7 +1141,19 @@ pub mod pallet { ) -> Result { let token = CurrencyId::to_token(&vtoken).map_err(|_| Error::::NoData)?; - let mut data = DelegatorVote::::iter_prefix((vtoken, poll_index)) + let vote_roles = DelegatorVoteRole::::iter_prefix(vtoken).collect::>(); + let mut delegator_votes = + DelegatorVote::::iter_prefix((vtoken, poll_index)).collect::>(); + let delegator_vote_keys = + delegator_votes.iter().map(|(index, _)| *index).collect::>(); + for vote_role in vote_roles { + if !delegator_vote_keys.contains(&vote_role.0) { + delegator_votes + .push((vote_role.0, AccountVote::>::from(vote_role.1))); + } + } + let mut data = delegator_votes + .into_iter() .map(|(index, vote)| { let (_, active) = T::DerivativeAccount::get_stake_info(token, index) .unwrap_or(Default::default()); diff --git a/pallets/vtoken-voting/src/mock.rs b/pallets/vtoken-voting/src/mock.rs index f3e8fd337..45871b118 100644 --- a/pallets/vtoken-voting/src/mock.rs +++ b/pallets/vtoken-voting/src/mock.rs @@ -355,24 +355,20 @@ pub fn new_test_ext() -> sp_io::TestExternalities { .assimilate_storage(&mut t) .unwrap(); - let mut delegator_votes = Vec::new(); - for poll_index in 0..=256 { - delegator_votes.extend(vec![ - (VKSM, poll_index, 0, 0), - (VKSM, poll_index, 1, 1), - (VKSM, poll_index, 2, 2), - (VKSM, poll_index, 3, 3), - (VKSM, poll_index, 4, 4), - (VKSM, poll_index, 5, 5), - (VKSM, poll_index, 10, 10), - (VKSM, poll_index, 11, 11), - (VKSM, poll_index, 15, 15), - (VKSM, poll_index, 20, 20), - (VKSM, poll_index, 21, 21), - ]); - } vtoken_voting::GenesisConfig:: { - delegator_votes, + delegator_vote_roles: vec![ + (VKSM, 0, 0), + (VKSM, 1, 1), + (VKSM, 2, 2), + (VKSM, 3, 3), + (VKSM, 4, 4), + (VKSM, 5, 5), + (VKSM, 10, 10), + (VKSM, 11, 11), + (VKSM, 15, 15), + (VKSM, 20, 20), + (VKSM, 21, 21), + ], undeciding_timeouts: vec![(VKSM, 100)], } .assimilate_storage(&mut t) diff --git a/pallets/vtoken-voting/src/tests.rs b/pallets/vtoken-voting/src/tests.rs index a36ac22c8..4c8b5b8e3 100644 --- a/pallets/vtoken-voting/src/tests.rs +++ b/pallets/vtoken-voting/src/tests.rs @@ -249,6 +249,12 @@ fn unsuccessful_conviction_vote_balance_can_be_unlocked() { new_test_ext().execute_with(|| { let poll_index = 3; let vtoken = VKSM; + let locking_period = 10; + assert_ok!(VtokenVoting::set_vote_locking_period( + RuntimeOrigin::root(), + vtoken, + locking_period, + )); assert_ok!(VtokenVoting::vote(RuntimeOrigin::signed(ALICE), vtoken, poll_index, aye(1, 1))); assert_ok!(VtokenVoting::notify_vote(origin_response(), 0, response_success())); @@ -261,6 +267,7 @@ fn unsuccessful_conviction_vote_balance_can_be_unlocked() { poll_index, ReferendumInfoOf::::Completed(3), )); + RelaychainDataProvider::set_block_number(13); assert_ok!(VtokenVoting::try_remove_vote(&ALICE, vtoken, poll_index, UnvoteScope::Any)); assert_ok!(VtokenVoting::update_lock(&ALICE, vtoken, &poll_index)); assert_eq!(usable_balance(vtoken, &ALICE), 10); @@ -272,7 +279,12 @@ fn successful_conviction_vote_balance_stays_locked_for_correct_time() { new_test_ext().execute_with(|| { let poll_index = 3; let vtoken = VKSM; - + let locking_period = 10; + assert_ok!(VtokenVoting::set_vote_locking_period( + RuntimeOrigin::root(), + vtoken, + locking_period, + )); for i in 1..=5 { assert_ok!(VtokenVoting::vote( RuntimeOrigin::signed(i), @@ -288,6 +300,7 @@ fn successful_conviction_vote_balance_stays_locked_for_correct_time() { poll_index, ReferendumInfoOf::::Completed(3), )); + RelaychainDataProvider::set_block_number(163); for i in 1..=5 { assert_ok!(VtokenVoting::try_remove_vote(&i, vtoken, poll_index, UnvoteScope::Any)); } @@ -302,12 +315,17 @@ fn successful_conviction_vote_balance_stays_locked_for_correct_time() { fn lock_amalgamation_valid_with_multiple_removed_votes() { new_test_ext().execute_with(|| { let vtoken = VKSM; + let response = response_success(); assert_ok!(VtokenVoting::vote(RuntimeOrigin::signed(ALICE), vtoken, 0, aye(5, 1))); assert_ok!(VtokenVoting::vote(RuntimeOrigin::signed(ALICE), vtoken, 1, aye(10, 1))); assert_ok!(VtokenVoting::vote(RuntimeOrigin::signed(ALICE), vtoken, 2, aye(5, 2))); assert_eq!(usable_balance(vtoken, &ALICE), 0); + assert_ok!(VtokenVoting::notify_vote(origin_response(), 0, response.clone())); + assert_ok!(VtokenVoting::notify_vote(origin_response(), 1, response.clone())); + assert_ok!(VtokenVoting::notify_vote(origin_response(), 2, response.clone())); + assert_ok!(VtokenVoting::set_referendum_status( RuntimeOrigin::root(), vtoken, @@ -326,30 +344,133 @@ fn lock_amalgamation_valid_with_multiple_removed_votes() { 2, ReferendumInfoOf::::Completed(1), )); - assert_ok!(VtokenVoting::kill_referendum(RuntimeOrigin::root(), vtoken, 0)); - assert_ok!(VtokenVoting::kill_referendum(RuntimeOrigin::root(), vtoken, 1)); - assert_ok!(VtokenVoting::kill_referendum(RuntimeOrigin::root(), vtoken, 2)); + + let locking_period = 10; + assert_ok!(VtokenVoting::set_vote_locking_period( + RuntimeOrigin::root(), + vtoken, + locking_period, + )); + + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(0, 5), (1, 10), (2, 5)]) + .unwrap() + ); assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 0)); assert_eq!(usable_balance(vtoken, &ALICE), 0); + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(1, 10), (2, 5), (0, 10)]) + .unwrap() + ); assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 1)); assert_eq!(usable_balance(vtoken, &ALICE), 0); + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(2, 5), (0, 10), (1, 10)]) + .unwrap() + ); assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 2)); assert_eq!(usable_balance(vtoken, &ALICE), 0); + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(0, 10), (1, 10), (2, 10)]) + .unwrap() + ); + + RelaychainDataProvider::set_block_number(21); - // run_to(3); assert_ok!(VtokenVoting::update_lock(&ALICE, vtoken, &0)); - assert_eq!(usable_balance(vtoken, &ALICE), 5); + assert_eq!(usable_balance(vtoken, &ALICE), 0); + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(1, 10), (2, 10)]).unwrap() + ); - // run_to(6); assert_ok!(VtokenVoting::update_lock(&ALICE, vtoken, &1)); - assert_eq!(usable_balance(vtoken, &ALICE), 10); + assert_eq!(usable_balance(vtoken, &ALICE), 0); + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(2, 10)]).unwrap() + ); - // run_to(7); assert_ok!(VtokenVoting::update_lock(&ALICE, vtoken, &2)); assert_eq!(usable_balance(vtoken, &ALICE), 10); + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![]).unwrap() + ); + }); +} + +#[test] +fn removed_votes_when_referendum_killed() { + new_test_ext().execute_with(|| { + let vtoken = VKSM; + let response = response_success(); + + assert_ok!(VtokenVoting::vote(RuntimeOrigin::signed(ALICE), vtoken, 0, aye(5, 1))); + assert_ok!(VtokenVoting::vote(RuntimeOrigin::signed(ALICE), vtoken, 1, aye(10, 1))); + assert_ok!(VtokenVoting::vote(RuntimeOrigin::signed(ALICE), vtoken, 2, aye(5, 2))); + assert_eq!(usable_balance(vtoken, &ALICE), 0); + + assert_ok!(VtokenVoting::notify_vote(origin_response(), 0, response.clone())); + assert_ok!(VtokenVoting::notify_vote(origin_response(), 1, response.clone())); + assert_ok!(VtokenVoting::notify_vote(origin_response(), 2, response.clone())); + + assert_ok!(VtokenVoting::set_referendum_status( + RuntimeOrigin::root(), + vtoken, + 0, + ReferendumInfoOf::::Completed(1), + )); + assert_ok!(VtokenVoting::set_referendum_status( + RuntimeOrigin::root(), + vtoken, + 1, + ReferendumInfoOf::::Completed(1), + )); + assert_ok!(VtokenVoting::set_referendum_status( + RuntimeOrigin::root(), + vtoken, + 2, + ReferendumInfoOf::::Completed(1), + )); + + assert_ok!(VtokenVoting::kill_referendum(RuntimeOrigin::root(), vtoken, 0)); + assert_ok!(VtokenVoting::kill_referendum(RuntimeOrigin::root(), vtoken, 1)); + assert_ok!(VtokenVoting::kill_referendum(RuntimeOrigin::root(), vtoken, 2)); + + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(0, 5), (1, 10), (2, 5)]) + .unwrap() + ); + + assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 0)); + assert_eq!(usable_balance(vtoken, &ALICE), 0); + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(1, 10), (2, 5)]).unwrap() + ); + + assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 1)); + assert_eq!(usable_balance(vtoken, &ALICE), 5); + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(2, 5)]).unwrap() + ); + + assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 2)); + assert_eq!(usable_balance(vtoken, &ALICE), 10); + assert_eq!( + ClassLocksFor::::get(&ALICE), + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![]).unwrap() + ); }); } @@ -427,14 +548,12 @@ fn kill_referendum_with_origin_signed_fails() { fn set_delegator_role_works() { new_test_ext().execute_with(|| { let vtoken = VKSM; - let poll_index = 3; let derivative_index: DerivativeIndex = 100; let role = aye(10, 3).into(); assert_ok!(VtokenVoting::set_delegator_role( RuntimeOrigin::root(), vtoken, - poll_index, derivative_index, role, )); diff --git a/pallets/vtoken-voting/src/vote.rs b/pallets/vtoken-voting/src/vote.rs index 07ef6f504..d9f9e58db 100644 --- a/pallets/vtoken-voting/src/vote.rs +++ b/pallets/vtoken-voting/src/vote.rs @@ -100,21 +100,21 @@ impl TryFrom for VoteRole { type Error = (); fn try_from(i: u8) -> Result { Ok(match i { - 0 => VoteRole::Standard { aye: true, conviction: Conviction::None }, // TODO remove + 0 => VoteRole::Standard { aye: true, conviction: Conviction::None }, 1 => VoteRole::Standard { aye: true, conviction: Conviction::Locked1x }, 2 => VoteRole::Standard { aye: true, conviction: Conviction::Locked2x }, 3 => VoteRole::Standard { aye: true, conviction: Conviction::Locked3x }, 4 => VoteRole::Standard { aye: true, conviction: Conviction::Locked4x }, 5 => VoteRole::Standard { aye: true, conviction: Conviction::Locked5x }, 6 => VoteRole::Standard { aye: true, conviction: Conviction::Locked6x }, - 10 => VoteRole::Standard { aye: false, conviction: Conviction::None }, // TODO remove + 10 => VoteRole::Standard { aye: false, conviction: Conviction::None }, 11 => VoteRole::Standard { aye: false, conviction: Conviction::Locked1x }, 12 => VoteRole::Standard { aye: false, conviction: Conviction::Locked2x }, 13 => VoteRole::Standard { aye: false, conviction: Conviction::Locked3x }, 14 => VoteRole::Standard { aye: false, conviction: Conviction::Locked4x }, 15 => VoteRole::Standard { aye: false, conviction: Conviction::Locked5x }, 16 => VoteRole::Standard { aye: false, conviction: Conviction::Locked6x }, - 20 => VoteRole::Split, // TODO remove + 20 => VoteRole::Split, 21 => VoteRole::SplitAbstain, _ => return Err(()), }) @@ -125,6 +125,7 @@ pub enum PollStatus { None, Ongoing(Tally), Completed(Moment, bool), + Killed(Moment), } impl PollStatus { @@ -153,11 +154,11 @@ pub enum AccountVote { impl AccountVote { /// Returns `Some` of the lock periods that the account is locked for, assuming that the /// referendum passed iff `approved` is `true`. - pub fn locked_if(self, approved: bool) -> Option<(u32, Balance)> { + pub fn locked_if(self, _approved: bool) -> Option<(u32, Balance)> { // winning side: can only be removed after the lock period ends. match self { AccountVote::Standard { vote: Vote { conviction: Conviction::None, .. }, .. } => None, - AccountVote::Standard { vote, balance } if vote.aye == approved => + AccountVote::Standard { vote, balance } /* if vote.aye == _approved */ => Some((vote.conviction.lock_periods(), balance)), _ => None, } From 7c3119d8bbfb2c2d7bd82d888bf3ac13f55cb704 Mon Sep 17 00:00:00 2001 From: Edwin Date: Wed, 18 Oct 2023 22:31:47 -0500 Subject: [PATCH 09/10] Fix vote (#1062) * Fix vote * Fix unlock --- pallets/vtoken-voting/src/lib.rs | 53 ++++++++++++------------------ pallets/vtoken-voting/src/tests.rs | 41 +++++++---------------- 2 files changed, 33 insertions(+), 61 deletions(-) diff --git a/pallets/vtoken-voting/src/lib.rs b/pallets/vtoken-voting/src/lib.rs index d97261c8a..aa3b5ed13 100644 --- a/pallets/vtoken-voting/src/lib.rs +++ b/pallets/vtoken-voting/src/lib.rs @@ -237,6 +237,8 @@ pub mod pallet { TooMany, /// Change delegator is not allowed. ChangeDelegator, + /// DelegatorVoteRole mismatch. + DelegatorVoteRoleMismatch, } /// Information concerning any given referendum. @@ -443,7 +445,11 @@ pub mod pallet { } if !DelegatorVote::::contains_key((vtoken, poll_index, derivative_index)) { - let default_vote: AccountVote> = VoteRole::from(vote).into(); + let role = DelegatorVoteRole::::get(vtoken, derivative_index) + .ok_or(Error::::NoData)?; + let target_role = VoteRole::from(vote); + ensure!(role == target_role, Error::::DelegatorVoteRoleMismatch); + let default_vote: AccountVote> = target_role.into(); DelegatorVote::::insert((vtoken, poll_index, derivative_index), default_vote); } @@ -510,7 +516,7 @@ pub mod pallet { .map_err(|_| Error::::NoPermissionYet)?; Self::ensure_no_pending_vote(vtoken, poll_index)?; - Self::try_remove_vote(&who, vtoken, poll_index, UnvoteScope::Any)?; + Self::try_remove_vote(&who, vtoken, poll_index, UnvoteScope::OnlyExpired)?; Self::update_lock(&who, vtoken, &poll_index)?; Self::deposit_event(Event::::Unlocked { who, vtoken, poll_index }); @@ -880,7 +886,7 @@ pub mod pallet { Ok(()) }, PollStatus::Completed(end, approved) => { - if let Some((lock_periods, balance)) = v.1.locked_if(approved) { + if let Some((lock_periods, _)) = v.1.locked_if(approved) { let unlock_at = end.saturating_add( VoteLockingPeriod::::get(vtoken) .ok_or(Error::::NoData)? @@ -892,7 +898,8 @@ pub mod pallet { matches!(scope, UnvoteScope::Any), Error::::NoPermissionYet ); - prior.accumulate(unlock_at, balance) + // v.3 is the actual locked vtoken balance + prior.accumulate(unlock_at, v.3) } } Ok(()) @@ -913,22 +920,11 @@ pub mod pallet { vtoken: CurrencyIdOf, poll_index: &PollIndex, ) -> DispatchResult { - let class_lock_needed = VotingFor::::mutate(who, |voting| { + VotingFor::::mutate(who, |voting| { voting.rejig(T::RelaychainBlockNumberProvider::current_block_number()); - voting.locked_balance() }); let lock_needed = ClassLocksFor::::mutate(who, |locks| { locks.retain(|x| &x.0 != poll_index); - if !class_lock_needed.is_zero() && !Self::is_referendum_killed(vtoken, *poll_index) - { - let ok = locks.try_push((*poll_index, class_lock_needed)).is_ok(); - debug_assert!( - ok, - "Vec bounded by number of classes; \ - all items in Vec associated with a unique class; \ - qed" - ); - } locks.iter().map(|x| x.1).max().unwrap_or(Zero::zero()) }); if lock_needed.is_zero() { @@ -1106,13 +1102,6 @@ pub mod pallet { Ok(responder) } - fn is_referendum_killed(vtoken: CurrencyIdOf, poll_index: PollIndex) -> bool { - match ReferendumInfoFor::::get(vtoken, poll_index) { - Some(ReferendumInfo::Killed(_)) => true, - _ => false, - } - } - fn try_access_poll( vtoken: CurrencyIdOf, poll_index: PollIndex, @@ -1137,7 +1126,7 @@ pub mod pallet { fn try_select_derivative_index( vtoken: CurrencyIdOf, poll_index: PollIndex, - my_vote: AccountVote>, + new_vote: AccountVote>, ) -> Result { let token = CurrencyId::to_token(&vtoken).map_err(|_| Error::::NoData)?; @@ -1154,22 +1143,22 @@ pub mod pallet { } let mut data = delegator_votes .into_iter() - .map(|(index, vote)| { - let (_, active) = T::DerivativeAccount::get_stake_info(token, index) + .map(|(index, voted)| { + let (_, available_vote) = T::DerivativeAccount::get_stake_info(token, index) .unwrap_or(Default::default()); - (active, vote, index) + (available_vote, voted, index) }) - .filter(|(_, vote, _)| VoteRole::from(*vote) == VoteRole::from(my_vote)) + .filter(|(_, voted, _)| VoteRole::from(*voted) == VoteRole::from(new_vote)) .collect::>(); data.sort_by(|a, b| { (b.0.saturating_sub(b.1.balance())).cmp(&(a.0.saturating_sub(a.1.balance()))) }); - let (active, vote, index) = data.first().ok_or(Error::::NoData)?; - active - .checked_sub(&vote.balance()) + let (available_vote, voted, index) = data.first().ok_or(Error::::NoData)?; + available_vote + .checked_sub(&voted.balance()) .ok_or(ArithmeticError::Underflow)? - .checked_sub(&my_vote.balance()) + .checked_sub(&new_vote.balance()) .ok_or(ArithmeticError::Underflow)?; Ok(*index) diff --git a/pallets/vtoken-voting/src/tests.rs b/pallets/vtoken-voting/src/tests.rs index 4c8b5b8e3..dd7fd457a 100644 --- a/pallets/vtoken-voting/src/tests.rs +++ b/pallets/vtoken-voting/src/tests.rs @@ -358,47 +358,30 @@ fn lock_amalgamation_valid_with_multiple_removed_votes() { .unwrap() ); - assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 0)); - assert_eq!(usable_balance(vtoken, &ALICE), 0); - assert_eq!( - ClassLocksFor::::get(&ALICE), - BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(1, 10), (2, 5), (0, 10)]) - .unwrap() + RelaychainDataProvider::set_block_number(10); + assert_noop!( + VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 0), + Error::::NoPermissionYet ); - assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 1)); + RelaychainDataProvider::set_block_number(11); + assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 0)); assert_eq!(usable_balance(vtoken, &ALICE), 0); assert_eq!( ClassLocksFor::::get(&ALICE), - BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(2, 5), (0, 10), (1, 10)]) - .unwrap() + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(1, 10), (2, 5)]).unwrap() ); - assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 2)); - assert_eq!(usable_balance(vtoken, &ALICE), 0); + RelaychainDataProvider::set_block_number(11); + assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 1)); + assert_eq!(usable_balance(vtoken, &ALICE), 5); assert_eq!( ClassLocksFor::::get(&ALICE), - BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(0, 10), (1, 10), (2, 10)]) - .unwrap() + BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(2, 5)]).unwrap() ); RelaychainDataProvider::set_block_number(21); - - assert_ok!(VtokenVoting::update_lock(&ALICE, vtoken, &0)); - assert_eq!(usable_balance(vtoken, &ALICE), 0); - assert_eq!( - ClassLocksFor::::get(&ALICE), - BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(1, 10), (2, 10)]).unwrap() - ); - - assert_ok!(VtokenVoting::update_lock(&ALICE, vtoken, &1)); - assert_eq!(usable_balance(vtoken, &ALICE), 0); - assert_eq!( - ClassLocksFor::::get(&ALICE), - BoundedVec::<(u32, u128), ConstU32<256>>::try_from(vec![(2, 10)]).unwrap() - ); - - assert_ok!(VtokenVoting::update_lock(&ALICE, vtoken, &2)); + assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 2)); assert_eq!(usable_balance(vtoken, &ALICE), 10); assert_eq!( ClassLocksFor::::get(&ALICE), From bba770c1d8f226bd497d3cb670eeb56f64f2d634 Mon Sep 17 00:00:00 2001 From: yooml Date: Fri, 20 Oct 2023 14:33:32 +0800 Subject: [PATCH 10/10] =?UTF-8?q?fix:=20=F0=9F=90=9B=20overflow=20(#1064)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 🐛 overflow * fix: 🐛 rm as_u128 --- pallets/stable-asset/src/lib.rs | 147 +++++++++++++++++++++---------- pallets/stable-pool/src/mock.rs | 17 +++- pallets/stable-pool/src/tests.rs | 39 ++++++++ 3 files changed, 155 insertions(+), 48 deletions(-) diff --git a/pallets/stable-asset/src/lib.rs b/pallets/stable-asset/src/lib.rs index e94516df1..e6c6acb12 100644 --- a/pallets/stable-asset/src/lib.rs +++ b/pallets/stable-asset/src/lib.rs @@ -41,7 +41,7 @@ use frame_support::{ }; use orml_traits::MultiCurrency; use scale_info::TypeInfo; -use sp_core::U512; +use sp_core::{U256, U512}; use sp_runtime::{ traits::{AccountIdConversion, CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, One, Zero}, SaturatedConversion, @@ -798,11 +798,23 @@ impl Pallet { let time_diff_div: T::AtLeast64BitUnsigned = t1.checked_sub(&t0)?.into(); if a1 > a0 { let diff = a1.checked_sub(&a0)?; - let amount = diff.checked_mul(&time_diff)?.checked_div(&time_diff_div)?; + let amount = u128::try_from( + U256::from(diff.saturated_into::()) + .checked_mul(U256::from(time_diff.saturated_into::()))? + .checked_div(U256::from(time_diff_div.saturated_into::()))?, + ) + .ok()? + .into(); Some(a0.checked_add(&amount)?) } else { let diff = a0.checked_sub(&a1)?; - let amount = diff.checked_mul(&time_diff)?.checked_div(&time_diff_div)?; + let amount = u128::try_from( + U256::from(diff.saturated_into::()) + .checked_mul(U256::from(time_diff.saturated_into::()))? + .checked_div(U256::from(time_diff_div.saturated_into::()))?, + ) + .ok()? + .into(); Some(a0.checked_sub(&amount)?) } } else { @@ -962,11 +974,16 @@ impl Pallet { let mint_fee: T::AtLeast64BitUnsigned = pool_info.mint_fee; if pool_info.mint_fee > zero { - fee_amount = mint_amount - .checked_mul(&mint_fee) - .ok_or(Error::::Math)? - .checked_div(&fee_denominator) - .ok_or(Error::::Math)?; + fee_amount = u128::try_from( + U256::from(mint_amount.saturated_into::()) + .checked_mul(U256::from(mint_fee.saturated_into::())) + .ok_or(Error::::Math)? + .checked_div(U256::from(fee_denominator.saturated_into::())) + .ok_or(Error::::Math)?, + ) + .map_err(|_| Error::::Math)? + .into(); + mint_amount = mint_amount.checked_sub(&fee_amount).ok_or(Error::::Math)?; } @@ -1033,11 +1050,15 @@ impl Pallet { .checked_div(&pool_info.precisions[output_index_usize]) .ok_or(Error::::Math)?; if pool_info.swap_fee > zero { - let fee_amount: T::AtLeast64BitUnsigned = dy - .checked_mul(&pool_info.swap_fee) - .ok_or(Error::::Math)? - .checked_div(&fee_denominator) - .ok_or(Error::::Math)?; + let fee_amount = u128::try_from( + U256::from(dy.saturated_into::()) + .checked_mul(U256::from(pool_info.swap_fee.saturated_into::())) + .ok_or(Error::::Math)? + .checked_div(U256::from(fee_denominator.saturated_into::())) + .ok_or(Error::::Math)?, + ) + .map_err(|_| Error::::Math)? + .into(); dy = dy.checked_sub(&fee_amount).ok_or(Error::::Math)?; } Ok(SwapResult { @@ -1080,7 +1101,13 @@ impl Pallet { let swap_exact_over_amount = T::SwapExactOverAmount::get(); if pool_info.swap_fee > zero { let diff = fee_denominator.checked_sub(&pool_info.swap_fee)?; - dy = dy.checked_mul(&fee_denominator)?.checked_div(&diff)?; + dy = u128::try_from( + U256::from(dy.saturated_into::()) + .checked_mul(U256::from(fee_denominator.saturated_into::()))? + .checked_div(U256::from(diff.saturated_into::()))?, + ) + .ok()? + .into(); } let a: T::AtLeast64BitUnsigned = Self::get_a( @@ -1132,22 +1159,30 @@ impl Pallet { let mut fee_amount: T::AtLeast64BitUnsigned = zero; if pool_info.redeem_fee > zero { - fee_amount = amount - .checked_mul(&pool_info.redeem_fee) - .ok_or(Error::::Math)? - .checked_div(&fee_denominator) - .ok_or(Error::::Math)?; + fee_amount = u128::try_from( + U256::from(amount.saturated_into::()) + .checked_mul(U256::from(pool_info.redeem_fee.saturated_into::())) + .ok_or(Error::::Math)? + .checked_div(U256::from(fee_denominator.saturated_into::())) + .ok_or(Error::::Math)?, + ) + .map_err(|_| Error::::Math)? + .into(); // Redemption fee is charged with pool token before redemption. amount = amount.checked_sub(&fee_amount).ok_or(Error::::Math)?; } for i in 0..pool_info.balances.len() { let balance_i: T::AtLeast64BitUnsigned = balances[i]; - let diff_i: T::AtLeast64BitUnsigned = balance_i - .checked_mul(&amount) - .ok_or(Error::::Math)? - .checked_div(&d) - .ok_or(Error::::Math)?; + let diff_i: T::AtLeast64BitUnsigned = u128::try_from( + U256::from(balance_i.saturated_into::()) + .checked_mul(U256::from(amount.saturated_into::())) + .ok_or(Error::::Math)? + .checked_div(U256::from(d.saturated_into::())) + .ok_or(Error::::Math)?, + ) + .map_err(|_| Error::::Math)? + .into(); balances[i] = balance_i.checked_sub(&diff_i).ok_or(Error::::Math)?; let amounts_i: T::AtLeast64BitUnsigned = diff_i.checked_div(&pool_info.precisions[i]).ok_or(Error::::Math)?; @@ -1196,11 +1231,15 @@ impl Pallet { let mut fee_amount: T::AtLeast64BitUnsigned = zero; if pool_info.redeem_fee > zero { - fee_amount = amount - .checked_mul(&pool_info.redeem_fee) - .ok_or(Error::::Math)? - .checked_div(&fee_denominator) - .ok_or(Error::::Math)?; + fee_amount = u128::try_from( + U256::from(amount.saturated_into::()) + .checked_mul(U256::from(pool_info.redeem_fee.saturated_into::())) + .ok_or(Error::::Math)? + .checked_div(U256::from(fee_denominator.saturated_into::())) + .ok_or(Error::::Math)?, + ) + .map_err(|_| Error::::Math)? + .into(); // Redemption fee is charged with pool token before redemption. amount = amount.checked_sub(&fee_amount).ok_or(Error::::Math)?; } @@ -1270,11 +1309,15 @@ impl Pallet { let div_amount: T::AtLeast64BitUnsigned = fee_denominator .checked_sub(&pool_info.redeem_fee) .ok_or(Error::::Math)?; - redeem_amount = redeem_amount - .checked_mul(&fee_denominator) - .ok_or(Error::::Math)? - .checked_div(&div_amount) - .ok_or(Error::::Math)?; + redeem_amount = u128::try_from( + U256::from(redeem_amount.saturated_into::()) + .checked_mul(U256::from(fee_denominator.saturated_into::())) + .ok_or(Error::::Math)? + .checked_div(U256::from(div_amount.saturated_into::())) + .ok_or(Error::::Math)?, + ) + .map_err(|_| Error::::Math)? + .into(); let sub_amount: T::AtLeast64BitUnsigned = old_d.checked_sub(&new_d).ok_or(Error::::Math)?; fee_amount = redeem_amount.checked_sub(&sub_amount).ok_or(Error::::Math)?; } @@ -1306,11 +1349,15 @@ impl Pallet { let mut balance_of: T::AtLeast64BitUnsigned = T::Assets::free_balance(pool_info.assets[i], &pool_info.account_id).into(); if let Some((denominator, numerator)) = Self::get_token_rate(pool_info.pool_id, pool_info.assets[i]) { - balance_of = balance_of - .checked_mul(&numerator) - .ok_or(Error::::Math)? - .checked_div(&denominator) - .ok_or(Error::::Math)?; + balance_of = u128::try_from( + U256::from(balance_of.saturated_into::()) + .checked_mul(U256::from(numerator.saturated_into::())) + .ok_or(Error::::Math)? + .checked_div(U256::from(denominator.saturated_into::())) + .ok_or(Error::::Math)?, + ) + .map_err(|_| Error::::Math)? + .into(); } *balance = balance_of .checked_mul(&pool_info.precisions[i]) @@ -1370,11 +1417,15 @@ impl Pallet { let mut balance_of: T::AtLeast64BitUnsigned = T::Assets::free_balance(pool_info.assets[i], &pool_info.account_id).into(); if let Some((denominator, numerator)) = Self::get_token_rate(pool_info.pool_id, pool_info.assets[i]) { - balance_of = balance_of - .checked_mul(&numerator) - .ok_or(Error::::Math)? - .checked_div(&denominator) - .ok_or(Error::::Math)?; + balance_of = u128::try_from( + U256::from(balance_of.saturated_into::()) + .checked_mul(U256::from(numerator.saturated_into::())) + .ok_or(Error::::Math)? + .checked_div(U256::from(denominator.saturated_into::())) + .ok_or(Error::::Math)?, + ) + .map_err(|_| Error::::Math)? + .into(); } *balance = balance_of .checked_mul(&pool_info.precisions[i]) @@ -2094,7 +2145,13 @@ impl StableAsset for Pallet { let mut balance_of: T::AtLeast64BitUnsigned = T::Assets::free_balance(output_asset, &pool_info.account_id).into(); if let Some((denominator, numerator)) = Self::get_token_rate(pool_info.pool_id, output_asset) { - balance_of = balance_of.checked_mul(&numerator)?.checked_div(&denominator)?; + balance_of = u128::try_from( + U256::from(balance_of.saturated_into::()) + .checked_mul(U256::from(numerator.saturated_into::()))? + .checked_div(U256::from(denominator.saturated_into::()))?, + ) + .ok()? + .into(); } // make sure pool can affort the output amount if swap_result.dy <= balance_of.into() { diff --git a/pallets/stable-pool/src/mock.rs b/pallets/stable-pool/src/mock.rs index 9a8ead303..d0b82188f 100644 --- a/pallets/stable-pool/src/mock.rs +++ b/pallets/stable-pool/src/mock.rs @@ -25,6 +25,7 @@ use frame_support::{ }; use frame_system::{EnsureRoot, EnsureSignedBy}; pub use node_primitives::{ + currency::{MOVR, VMOVR}, AccountId, Balance, CurrencyId, CurrencyIdMapping, SlpOperator, SlpxOperator, TokenSymbol, ASTR, BNC, DOT, DOT_TOKEN_ID, GLMR, VBNC, VDOT, }; @@ -368,6 +369,14 @@ impl Default for ExtBuilder { } } +pub fn million_unit(d: u128) -> u128 { + d.saturating_mul(10_u128.pow(18)) +} + +pub fn unit(d: u128) -> u128 { + d.saturating_mul(10_u128.pow(12)) +} + impl ExtBuilder { pub fn balances(mut self, endowed_accounts: Vec<(u128, CurrencyId, Balance)>) -> Self { self.endowed_accounts = endowed_accounts; @@ -376,10 +385,11 @@ impl ExtBuilder { pub fn new_test_ext(self) -> Self { self.balances(vec![ + (0, BNC, unit(1000)), + (0, MOVR, million_unit(1_000_000)), + (0, VMOVR, million_unit(1_000_000)), (1, BNC, 1_000_000_000_000), - // (1, VDOT, 100_000_000), (1, DOT, 100_000_000_000_000), - // (2, VDOT, 100_000_000_000_000), (3, DOT, 200_000_000), (4, DOT, 100_000_000), (6, BNC, 100_000_000_000_000), @@ -398,8 +408,9 @@ impl ExtBuilder { (DOT, 1_000_000, None), (ASTR, 10_000_000, None), (GLMR, 10_000_000, None), + (MOVR, 10_000_000, None), ], - vcurrency: vec![VDOT], + vcurrency: vec![VDOT, VMOVR], vsbond: vec![], phantom: Default::default(), } diff --git a/pallets/stable-pool/src/tests.rs b/pallets/stable-pool/src/tests.rs index a7ab1b8b7..92063177d 100644 --- a/pallets/stable-pool/src/tests.rs +++ b/pallets/stable-pool/src/tests.rs @@ -63,6 +63,26 @@ fn create_pool2() -> (CurrencyId, CurrencyId, CurrencyId, u128) { (coin0, coin1, pool_asset, 30160825295207673652903702381u128) } +fn create_movr_pool() -> (CurrencyId, CurrencyId, CurrencyId, u128) { + let coin0 = MOVR; + let coin1 = VMOVR; + let pool_asset: CurrencyId = CurrencyId::BLP(0); + + assert_ok!(StablePool::create_pool( + RuntimeOrigin::root(), + vec![coin0, coin1], + vec![1u128, 1u128], + 10000000u128, + 20000000u128, + 50000000u128, + 10000u128, + 2, + 1, + million_unit(1), + )); + (coin0, coin1, pool_asset, 30160825295207673652903702381u128) +} + #[test] fn modify_a_argument_error_failed() { env_logger::try_init().unwrap_or(()); @@ -767,3 +787,22 @@ fn edit_token_rate() { ); }); } + +#[test] +fn redeem_movr() { + ExtBuilder::default().new_test_ext().build().execute_with(|| { + let (coin0, coin1, pool_asset, _swap_id) = create_movr_pool(); + assert_ok!(StablePool::edit_token_rate( + RuntimeOrigin::root(), + 0, + vec![(coin0, (1, 1)), (coin1, (90_000_000, 100_000_000))] + )); + let amounts = vec![million_unit(100_000), million_unit(200_000)]; + assert_ok!(StablePool::mint_inner(&0, 0, amounts, 0)); + assert_eq!(Tokens::free_balance(pool_asset, &0), 321765598211330627258732); + assert_ok!(StablePool::redeem_proportion_inner(&0, 0, million_unit(300_000), vec![0, 0])); + assert_eq!(Tokens::free_balance(pool_asset, &0), 21765598211330627258732); + assert_eq!(Tokens::free_balance(coin0, &0), 992676625984156921892393); + assert_eq!(Tokens::free_balance(coin1, &0), 985353251968313843784786); + }); +}