From 427ec1a746a46a42f68dbee81247fb8907308e81 Mon Sep 17 00:00:00 2001 From: SunTiebing <1045060705@qq.com> Date: Tue, 3 Dec 2024 23:04:07 +0800 Subject: [PATCH 1/3] Separate the logic for retrieving the current block for different tokens. --- .../src/agents/bifrost_agent/agent.rs | 4 + .../src/agents/relaychain_agent/agent.rs | 4 + pallets/vtoken-voting/src/lib.rs | 116 +++++++++++------- pallets/vtoken-voting/src/mock.rs | 1 + pallets/vtoken-voting/src/traits.rs | 3 + runtime/bifrost-kusama/src/lib.rs | 1 + runtime/bifrost-polkadot/src/lib.rs | 1 + 7 files changed, 85 insertions(+), 45 deletions(-) diff --git a/pallets/vtoken-voting/src/agents/bifrost_agent/agent.rs b/pallets/vtoken-voting/src/agents/bifrost_agent/agent.rs index 24c250a24..53652e515 100644 --- a/pallets/vtoken-voting/src/agents/bifrost_agent/agent.rs +++ b/pallets/vtoken-voting/src/agents/bifrost_agent/agent.rs @@ -147,4 +147,8 @@ impl VotingAgent for BifrostAgent { Ok( as ConvictionVotingCall>::remove_vote(Some(class), poll_index) .encode()) } + + fn block_number(&self) -> BlockNumberFor { + T::LocalBlockNumberProvider::current_block_number() + } } diff --git a/pallets/vtoken-voting/src/agents/relaychain_agent/agent.rs b/pallets/vtoken-voting/src/agents/relaychain_agent/agent.rs index 73e2a9b2e..e9cc8d81e 100644 --- a/pallets/vtoken-voting/src/agents/relaychain_agent/agent.rs +++ b/pallets/vtoken-voting/src/agents/relaychain_agent/agent.rs @@ -117,4 +117,8 @@ impl VotingAgent for RelaychainAgent { ) .encode()) } + + fn block_number(&self) -> BlockNumberFor { + BlockNumberFor::::from(T::RelaychainBlockNumberProvider::current_block_number()) + } } diff --git a/pallets/vtoken-voting/src/lib.rs b/pallets/vtoken-voting/src/lib.rs index 95d4bd80b..54e864a82 100644 --- a/pallets/vtoken-voting/src/lib.rs +++ b/pallets/vtoken-voting/src/lib.rs @@ -71,6 +71,7 @@ pub use weights::WeightInfo; use xcm::v4::{prelude::*, Location, Weight as XcmWeight}; const CONVICTION_VOTING_ID: LockIdentifier = *b"vtvoting"; +const SUPPORTED_VTOKENS: &[CurrencyId] = &[VKSM, VDOT, VBNC]; type PollIndex = u32; type PollClass = u16; @@ -156,6 +157,8 @@ pub mod pallet { type WeightInfo: WeightInfo; type PalletsOrigin: CallerTrait; + + type LocalBlockNumberProvider: BlockNumberProvider>; } #[pallet::event] @@ -459,20 +462,25 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { - fn on_idle(n: BlockNumberFor, remaining_weight: Weight) -> Weight { + fn on_idle( + bifrost_current_block_number: 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) || - n % T::ReferendumCheckInterval::get() != Zero::zero() + bifrost_current_block_number % T::ReferendumCheckInterval::get() != Zero::zero() { return Weight::zero(); } let relay_current_block_number = T::RelaychainBlockNumberProvider::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); + for block_number in ReferendumTimeout::::iter_keys() { + if relay_current_block_number >= block_number || + bifrost_current_block_number >= block_number + { + let info_list = ReferendumTimeout::::get(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) { @@ -480,18 +488,30 @@ pub mod pallet { } used_weight += temp_weight; for (vtoken, poll_index) in info_list.iter() { + let current_block_number = match Self::get_agent_block_number(&vtoken) { + Ok(block_number) => block_number, + Err(e) => { + log::error!( + "Failed to get {:?} current block number: {:?}", + vtoken, + e + ); + // exit the current loop + continue; + }, + }; + ReferendumInfoFor::::mutate(vtoken, poll_index, |maybe_info| { match maybe_info { Some(info) => if let ReferendumInfo::Ongoing(_) = info { - *info = - ReferendumInfo::Completed(relay_current_block_number); + *info = ReferendumInfo::Completed(current_block_number); }, None => {}, } }); } - ReferendumTimeout::::remove(relay_block_number); + ReferendumTimeout::::remove(block_number); } } @@ -637,10 +657,12 @@ pub mod pallet { Self::ensure_vtoken(&vtoken)?; Self::ensure_referendum_completed(vtoken, poll_index)?; + let current_block_number = Self::get_agent_block_number(&vtoken)?; + ReferendumInfoFor::::insert( vtoken, poll_index, - ReferendumInfo::Killed(T::RelaychainBlockNumberProvider::current_block_number()), + ReferendumInfo::Killed(current_block_number), ); Self::deposit_event(Event::::ReferendumKilled { vtoken, poll_index }); @@ -752,41 +774,7 @@ pub mod pallet { Self::deposit_event(Event::::VoteNotified { vtoken, poll_index, success }); } - if let Some((vtoken, poll_index)) = PendingReferendumInfo::::get(query_id) { - if success { - ReferendumInfoFor::::try_mutate_exists( - vtoken, - poll_index, - |maybe_info| -> DispatchResult { - if let Some(info) = maybe_info { - if let ReferendumInfo::Ongoing(status) = info { - let relay_current_block_number = - T::RelaychainBlockNumberProvider::current_block_number(); - status.submitted = Some(relay_current_block_number); - ReferendumTimeout::::mutate( - relay_current_block_number.saturating_add( - UndecidingTimeout::::get(vtoken) - .ok_or(Error::::NoData)?, - ), - |ref_vec| { - ref_vec - .try_push((vtoken, poll_index)) - .map_err(|_| Error::::TooMany) - }, - )?; - Self::deposit_event(Event::::ReferendumInfoCreated { - vtoken, - poll_index, - info: info.clone(), - }); - } - } - Ok(()) - }, - )?; - } else { - ReferendumInfoFor::::remove(vtoken, poll_index); - } + if let Some((_, _)) = PendingReferendumInfo::::get(query_id) { PendingReferendumInfo::::remove(query_id); } @@ -862,6 +850,7 @@ pub mod pallet { if let Some((old_vote, vtoken_balance)) = maybe_old_vote { Self::try_vote(&who, vtoken, poll_index, old_vote, vtoken_balance)?; } + ReferendumInfoFor::::remove(vtoken, poll_index); } else { if !VoteDelegatorFor::::contains_key((&who, vtoken, poll_index)) { VoteDelegatorFor::::insert((&who, vtoken, poll_index), derivative_index); @@ -876,6 +865,36 @@ pub mod pallet { } Ok(()) })?; + + ReferendumInfoFor::::try_mutate_exists( + vtoken, + poll_index, + |maybe_info| -> DispatchResult { + if let Some(info) = maybe_info { + if let ReferendumInfo::Ongoing(status) = info { + let current_block_number = Self::get_agent_block_number(&vtoken)?; + status.submitted = Some(current_block_number); + ReferendumTimeout::::mutate( + current_block_number.saturating_add( + UndecidingTimeout::::get(vtoken) + .ok_or(Error::::NoData)?, + ), + |ref_vec| { + ref_vec + .try_push((vtoken, poll_index)) + .map_err(|_| Error::::TooMany) + }, + )?; + Self::deposit_event(Event::::ReferendumInfoCreated { + vtoken, + poll_index, + info: info.clone(), + }); + } + } + Ok(()) + }, + )?; } Ok(()) @@ -1211,7 +1230,7 @@ pub mod pallet { } fn ensure_vtoken(vtoken: &CurrencyIdOf) -> Result<(), DispatchError> { - ensure!([VKSM, VDOT, VBNC].contains(vtoken), Error::::VTokenNotSupport); + ensure!(SUPPORTED_VTOKENS.contains(vtoken), Error::::VTokenNotSupport); Ok(()) } @@ -1439,6 +1458,13 @@ pub mod pallet { } } + pub(crate) fn get_agent_block_number( + currency_id: &CurrencyIdOf, + ) -> Result, Error> { + let voting_agent = Self::get_voting_agent(¤cy_id)?; + Ok(voting_agent.block_number()) + } + pub(crate) fn convert_vtoken_to_dest_location( vtoken: CurrencyId, ) -> Result> { diff --git a/pallets/vtoken-voting/src/mock.rs b/pallets/vtoken-voting/src/mock.rs index 397c7f9a9..bab4adefe 100644 --- a/pallets/vtoken-voting/src/mock.rs +++ b/pallets/vtoken-voting/src/mock.rs @@ -442,6 +442,7 @@ impl vtoken_voting::Config for Runtime { type ReferendumCheckInterval = ReferendumCheckInterval; type WeightInfo = (); type PalletsOrigin = OriginCaller; + type LocalBlockNumberProvider = System; } pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/pallets/vtoken-voting/src/traits.rs b/pallets/vtoken-voting/src/traits.rs index 30fe18202..ecddfa3df 100644 --- a/pallets/vtoken-voting/src/traits.rs +++ b/pallets/vtoken-voting/src/traits.rs @@ -108,6 +108,9 @@ pub trait VotingAgent { poll_index: PollIndex, derivative_index: DerivativeIndex, ) -> Result, Error>; + + /// Get current agent block number. + fn block_number(&self) -> BlockNumberFor; } /// Trait defining calls related to conviction voting mechanisms. diff --git a/runtime/bifrost-kusama/src/lib.rs b/runtime/bifrost-kusama/src/lib.rs index 1e680eee3..36327a330 100644 --- a/runtime/bifrost-kusama/src/lib.rs +++ b/runtime/bifrost-kusama/src/lib.rs @@ -1434,6 +1434,7 @@ impl bifrost_vtoken_voting::Config for Runtime { type ReferendumCheckInterval = ReferendumCheckInterval; type WeightInfo = weights::bifrost_vtoken_voting::BifrostWeight; type PalletsOrigin = OriginCaller; + type LocalBlockNumberProvider = System; } // Bifrost modules end diff --git a/runtime/bifrost-polkadot/src/lib.rs b/runtime/bifrost-polkadot/src/lib.rs index 3a287316f..9bac6aad7 100644 --- a/runtime/bifrost-polkadot/src/lib.rs +++ b/runtime/bifrost-polkadot/src/lib.rs @@ -1334,6 +1334,7 @@ impl bifrost_vtoken_voting::Config for Runtime { type ReferendumCheckInterval = ReferendumCheckInterval; type WeightInfo = weights::bifrost_vtoken_voting::BifrostWeight; type PalletsOrigin = OriginCaller; + type LocalBlockNumberProvider = System; } // Bifrost modules end From 3e386b65917c041746917bac6ec9b2e92843bfb1 Mon Sep 17 00:00:00 2001 From: SunTiebing <1045060705@qq.com> Date: Wed, 4 Dec 2024 10:06:35 +0800 Subject: [PATCH 2/3] change ReferendumTimeout max number to 100 --- pallets/vtoken-voting/src/lib.rs | 2 +- pallets/vtoken-voting/src/tests/common_test.rs | 4 ++-- pallets/vtoken-voting/src/tests/vbnc_test.rs | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pallets/vtoken-voting/src/lib.rs b/pallets/vtoken-voting/src/lib.rs index 54e864a82..550bb46c7 100644 --- a/pallets/vtoken-voting/src/lib.rs +++ b/pallets/vtoken-voting/src/lib.rs @@ -420,7 +420,7 @@ pub mod pallet { _, Twox64Concat, BlockNumberFor, - BoundedVec<(CurrencyIdOf, PollIndex), ConstU32<50>>, + BoundedVec<(CurrencyIdOf, PollIndex), ConstU32<100>>, ValueQuery, >; diff --git a/pallets/vtoken-voting/src/tests/common_test.rs b/pallets/vtoken-voting/src/tests/common_test.rs index 721d4aebd..c53e9af9d 100644 --- a/pallets/vtoken-voting/src/tests/common_test.rs +++ b/pallets/vtoken-voting/src/tests/common_test.rs @@ -896,7 +896,7 @@ fn notify_vote_success_max_works() { fn notify_vote_success_exceed_max_fail() { for &vtoken in TOKENS { new_test_ext().execute_with(|| { - for poll_index in 0..50 { + for poll_index in 0..100 { assert_ok!(VtokenVoting::vote( RuntimeOrigin::signed(ALICE), vtoken, @@ -909,7 +909,7 @@ fn notify_vote_success_exceed_max_fail() { response_success() )); } - let poll_index = 50; + let poll_index = 100; assert_ok!(VtokenVoting::vote( RuntimeOrigin::signed(ALICE), vtoken, diff --git a/pallets/vtoken-voting/src/tests/vbnc_test.rs b/pallets/vtoken-voting/src/tests/vbnc_test.rs index ebf45665e..99a324c4c 100644 --- a/pallets/vtoken-voting/src/tests/vbnc_test.rs +++ b/pallets/vtoken-voting/src/tests/vbnc_test.rs @@ -507,7 +507,7 @@ fn errors_with_vote_works() { Error::::InsufficientFunds ); - for poll_index in 0..256 { + for poll_index in 0..100 { assert_ok!(VtokenVoting::vote( RuntimeOrigin::signed(1), vtoken, @@ -516,8 +516,8 @@ fn errors_with_vote_works() { )); } assert_noop!( - VtokenVoting::vote(RuntimeOrigin::signed(1), vtoken, 256, aye(10, 0)), - Error::::MaxVotesReached + VtokenVoting::vote(RuntimeOrigin::signed(1), vtoken, 100, aye(10, 0)), + Error::::TooMany ); }); } From 7256a77e813bd819a5debb25afacd8ede96aded2 Mon Sep 17 00:00:00 2001 From: SunTiebing <1045060705@qq.com> Date: Wed, 4 Dec 2024 16:34:30 +0800 Subject: [PATCH 3/3] change RelaychainBlockNumber to agent block number --- pallets/vtoken-voting/src/lib.rs | 16 +++++++++------- pallets/vtoken-voting/src/tests/vbnc_test.rs | 18 +++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/pallets/vtoken-voting/src/lib.rs b/pallets/vtoken-voting/src/lib.rs index 550bb46c7..b13a02511 100644 --- a/pallets/vtoken-voting/src/lib.rs +++ b/pallets/vtoken-voting/src/lib.rs @@ -1150,7 +1150,8 @@ pub mod pallet { .ok_or(Error::::NoData)? .saturating_mul(lock_periods.into()), ); - let now = T::RelaychainBlockNumberProvider::current_block_number(); + + let now = Self::get_agent_block_number(&vtoken)?; if now < unlock_at { ensure!( matches!(scope, UnvoteScope::Any), @@ -1174,8 +1175,9 @@ pub mod pallet { /// Rejig the lock on an account. It will never get more stringent (since that would /// indicate a security hole) but may be reduced from what they are currently. pub(crate) fn update_lock(who: &AccountIdOf, vtoken: CurrencyIdOf) -> DispatchResult { + let current_block = Self::get_agent_block_number(&vtoken)?; let lock_needed = VotingFor::::mutate(who, |voting| { - voting.rejig(T::RelaychainBlockNumberProvider::current_block_number()); + voting.rejig(current_block); voting.locked_balance() }); @@ -1278,8 +1280,10 @@ pub mod pallet { (Some(ReferendumInfo::Completed(moment)), Some((lock_periods, _balance))) => { let locking_period = VoteLockingPeriod::::get(vtoken).ok_or(Error::::NoData)?; + + let current_block = Self::get_agent_block_number(&vtoken)?; ensure!( - T::RelaychainBlockNumberProvider::current_block_number() >= + current_block >= moment.saturating_add( locking_period.saturating_mul(lock_periods.into()) ), @@ -1288,10 +1292,8 @@ pub mod pallet { Ok(()) }, (Some(ReferendumInfo::Completed(moment)), None) => { - ensure!( - T::RelaychainBlockNumberProvider::current_block_number() >= moment, - Error::::NotExpired - ); + let current_block = Self::get_agent_block_number(&vtoken)?; + ensure!(current_block >= moment, Error::::NotExpired); Ok(()) }, _ => Err(Error::::NotExpired.into()), diff --git a/pallets/vtoken-voting/src/tests/vbnc_test.rs b/pallets/vtoken-voting/src/tests/vbnc_test.rs index 99a324c4c..e57647ebf 100644 --- a/pallets/vtoken-voting/src/tests/vbnc_test.rs +++ b/pallets/vtoken-voting/src/tests/vbnc_test.rs @@ -143,7 +143,7 @@ fn successful_but_zero_conviction_vote_balance_can_be_unlocked() { assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(BOB), vtoken, poll_index)); assert_eq!(usable_balance(vtoken, &BOB), 20); - RelaychainDataProvider::set_block_number(13); + System::set_block_number(13); assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, poll_index)); assert_eq!(usable_balance(vtoken, &ALICE), 10); }); @@ -181,7 +181,7 @@ fn unsuccessful_conviction_vote_balance_can_be_unlocked() { poll_index, ReferendumInfoOf::::Completed(3), )); - RelaychainDataProvider::set_block_number(13); + System::set_block_number(13); assert_ok!(VtokenVoting::try_remove_vote(&ALICE, vtoken, poll_index, UnvoteScope::Any)); assert_ok!(VtokenVoting::update_lock(&ALICE, vtoken)); assert_eq!(usable_balance(vtoken, &ALICE), 10); @@ -221,7 +221,7 @@ fn ensure_balance_after_unlock() { poll_index, ReferendumInfoOf::::Completed(3), )); - RelaychainDataProvider::set_block_number(13); + System::set_block_number(13); assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, poll_index)); assert_eq!(usable_balance(vtoken, &ALICE), 0); assert_eq!(Tokens::accounts(&ALICE, vtoken).frozen, 10); @@ -269,7 +269,7 @@ fn ensure_comprehensive_balance_after_unlock() { poll_index, ReferendumInfoOf::::Completed(3), )); - RelaychainDataProvider::set_block_number(13); + System::set_block_number(13); assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, poll_index)); assert_eq!(usable_balance(vtoken, &ALICE), 8); assert_eq!(Tokens::accounts(&ALICE, vtoken).frozen, 2); @@ -314,7 +314,7 @@ fn successful_conviction_vote_balance_stays_locked_for_correct_time() { poll_index, ReferendumInfoOf::::Completed(3), )); - RelaychainDataProvider::set_block_number(163); + System::set_block_number(163); for i in 1..=5 { assert_ok!(VtokenVoting::try_remove_vote(&i, vtoken, poll_index, UnvoteScope::Any)); } @@ -392,7 +392,7 @@ fn lock_amalgamation_valid_with_multiple_removed_votes() { .unwrap() ); - RelaychainDataProvider::set_block_number(10); + System::set_block_number(10); assert_noop!( VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 0), Error::::NoPermissionYet @@ -400,7 +400,7 @@ fn lock_amalgamation_valid_with_multiple_removed_votes() { assert_eq!(VotingFor::::get(&ALICE).locked_balance(), 10); assert_eq!(usable_balance(vtoken, &ALICE), 0); - RelaychainDataProvider::set_block_number(11); + System::set_block_number(11); assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 0)); assert_eq!(VotingFor::::get(&ALICE).locked_balance(), 10); assert_eq!(usable_balance(vtoken, &ALICE), 0); @@ -410,7 +410,7 @@ fn lock_amalgamation_valid_with_multiple_removed_votes() { .unwrap() ); - RelaychainDataProvider::set_block_number(11); + System::set_block_number(11); assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 1)); assert_eq!(usable_balance(vtoken, &ALICE), 0); assert_eq!( @@ -419,7 +419,7 @@ fn lock_amalgamation_valid_with_multiple_removed_votes() { .unwrap() ); - RelaychainDataProvider::set_block_number(21); + System::set_block_number(21); assert_ok!(VtokenVoting::unlock(RuntimeOrigin::signed(ALICE), vtoken, 2)); assert_eq!(usable_balance(vtoken, &ALICE), 10); assert_eq!(