From a18522efa1e660bd5c1c0301d576378f8f3e6414 Mon Sep 17 00:00:00 2001 From: Martin Hloska Date: Sat, 26 Oct 2024 09:23:47 +0200 Subject: [PATCH] add voting hooks to conviction-voting pallet - patch2 --- polkadot/runtime/rococo/src/governance/mod.rs | 1 + substrate/frame/conviction-voting/src/lib.rs | 37 ++++- .../frame/conviction-voting/src/tests.rs | 139 ++++++++++++++++++ .../frame/conviction-voting/src/traits.rs | 40 +++++ 4 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 substrate/frame/conviction-voting/src/traits.rs diff --git a/polkadot/runtime/rococo/src/governance/mod.rs b/polkadot/runtime/rococo/src/governance/mod.rs index ef2adf60753d..47aac9f05630 100644 --- a/polkadot/runtime/rococo/src/governance/mod.rs +++ b/polkadot/runtime/rococo/src/governance/mod.rs @@ -47,6 +47,7 @@ impl pallet_conviction_voting::Config for Runtime { type MaxTurnout = frame_support::traits::tokens::currency::ActiveIssuanceOf; type Polls = Referenda; + type VotingHooks = (); } parameter_types! { diff --git a/substrate/frame/conviction-voting/src/lib.rs b/substrate/frame/conviction-voting/src/lib.rs index 466fc70a619b..9ff86f328c5c 100644 --- a/substrate/frame/conviction-voting/src/lib.rs +++ b/substrate/frame/conviction-voting/src/lib.rs @@ -43,6 +43,7 @@ use sp_runtime::{ use sp_std::prelude::*; mod conviction; +pub mod traits; mod types; mod vote; pub mod weights; @@ -50,6 +51,7 @@ pub mod weights; pub use self::{ conviction::Conviction, pallet::*, + traits::VotingHooks, types::{Delegations, Tally, UnvoteScope}, vote::{AccountVote, Casting, Delegating, Vote, Voting}, weights::WeightInfo, @@ -136,6 +138,9 @@ pub mod pallet { /// those successful voters are locked into the consequences that their votes entail. #[pallet::constant] type VoteLockingPeriod: Get>; + + /// Hooks are called when a new vote is registered or an existing vote is removed. + type VotingHooks: VotingHooks, BalanceOf>; } /// All voting for a particular voter in a particular voting class. We store the balance for the @@ -427,6 +432,9 @@ impl, I: 'static> Pallet { // Extend the lock to `balance` (rather than setting it) since we don't know what // other votes are in place. Self::extend_lock(who, &class, vote.balance()); + + // Call on_vote hook + T::VotingHooks::on_vote(who, poll_index, vote)?; Ok(()) }) }) @@ -462,6 +470,7 @@ impl, I: 'static> Pallet { if let Some(approve) = v.1.as_standard() { tally.reduce(approve, *delegations); } + T::VotingHooks::on_remove_vote(who, poll_index, Some(true)); Ok(()) }, PollStatus::Completed(end, approved) => { @@ -477,10 +486,36 @@ impl, I: 'static> Pallet { ); prior.accumulate(unlock_at, balance) } + } else if v.1.as_standard() == Some(!approved) { + // Unsuccessful vote, use special hook to lock the funds too in case of conviction. + if let Some(to_lock) = + T::VotingHooks::balance_locked_on_unsuccessful_vote(who, poll_index) + { + if let AccountVote::Standard { vote, .. } = v.1 { + let unlock_at = end.saturating_add( + T::VoteLockingPeriod::get() + .saturating_mul(vote.conviction.lock_periods().into()), + ); + let now = frame_system::Pallet::::block_number(); + if now < unlock_at { + ensure!( + matches!(scope, UnvoteScope::Any), + Error::::NoPermissionYet + ); + prior.accumulate(unlock_at, to_lock) + } + } + } } + // Call on_remove_vote hook + T::VotingHooks::on_remove_vote(who, poll_index, Some(false)); + Ok(()) + }, + PollStatus::None => { + // Poll was cancelled. + T::VotingHooks::on_remove_vote(who, poll_index, None); Ok(()) }, - PollStatus::None => Ok(()), // Poll was cancelled. }) } else { Ok(()) diff --git a/substrate/frame/conviction-voting/src/tests.rs b/substrate/frame/conviction-voting/src/tests.rs index 74baeace898b..0020e2222400 100644 --- a/substrate/frame/conviction-voting/src/tests.rs +++ b/substrate/frame/conviction-voting/src/tests.rs @@ -17,6 +17,7 @@ //! The crate's tests. +use std::cell::RefCell; use std::collections::BTreeMap; use frame_support::{ @@ -164,6 +165,7 @@ impl Config for Test { type WeightInfo = (); type MaxTurnout = frame_support::traits::TotalIssuanceOf; type Polls = TestPolls; + type VotingHooks = HooksHandler; } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -856,3 +858,140 @@ fn errors_with_remove_vote_work() { ); }); } + +thread_local! { + static LAST_ON_VOTE_DATA: RefCell)>> = RefCell::new(None); + static LAST_ON_REMOVE_VOTE_DATA: RefCell)>> = RefCell::new(None); + static LAST_LOCKED_IF_UNSUCCESSFUL_VOTE_DATA: RefCell> = RefCell::new(None); + static REMOVE_VOTE_LOCKED_AMOUNT: RefCell> = RefCell::new(None); +} + +pub struct HooksHandler; + +impl HooksHandler { + fn last_on_vote_data() -> Option<(u64, u8, AccountVote)> { + LAST_ON_VOTE_DATA.with(|data| data.borrow().clone()) + } + + fn last_on_remove_vote_data() -> Option<(u64, u8, Option)> { + LAST_ON_REMOVE_VOTE_DATA.with(|data| data.borrow().clone()) + } + + fn last_locked_if_unsuccessful_vote_data() -> Option<(u64, u8)> { + LAST_LOCKED_IF_UNSUCCESSFUL_VOTE_DATA.with(|data| data.borrow().clone()) + } + + fn reset() { + LAST_ON_VOTE_DATA.with(|data| *data.borrow_mut() = None); + LAST_ON_REMOVE_VOTE_DATA.with(|data| *data.borrow_mut() = None); + LAST_LOCKED_IF_UNSUCCESSFUL_VOTE_DATA.with(|data| *data.borrow_mut() = None); + REMOVE_VOTE_LOCKED_AMOUNT.with(|data| *data.borrow_mut() = None); + } + + fn with_remove_locked_amount(v: u64) { + REMOVE_VOTE_LOCKED_AMOUNT.with(|data| *data.borrow_mut() = Some(v)); + } +} + +impl VotingHooks for HooksHandler { + fn on_vote(who: &u64, ref_index: u8, vote: AccountVote) -> DispatchResult { + LAST_ON_VOTE_DATA.with(|data| { + *data.borrow_mut() = Some((*who, ref_index, vote)); + }); + Ok(()) + } + + fn on_remove_vote(who: &u64, ref_index: u8, ongoing: Option) { + LAST_ON_REMOVE_VOTE_DATA.with(|data| { + *data.borrow_mut() = Some((*who, ref_index, ongoing)); + }); + } + + fn balance_locked_on_unsuccessful_vote(who: &u64, ref_index: u8) -> Option { + LAST_LOCKED_IF_UNSUCCESSFUL_VOTE_DATA.with(|data| { + *data.borrow_mut() = Some((*who, ref_index)); + + REMOVE_VOTE_LOCKED_AMOUNT.with(|data| data.borrow().clone()) + }) + } + + #[cfg(feature = "runtime-benchmarks")] + fn on_vote_worst_case(_who: &u64) {} + + #[cfg(feature = "runtime-benchmarks")] + fn on_remove_vote_worst_case(_who: &u64) {} +} + +#[test] +fn voting_hooks_are_called_correctly() { + new_test_ext().execute_with(|| { + let c = class(3); + + let usable_balance_1 = Balances::usable_balance(1); + dbg!(usable_balance_1); + + // Voting + assert_ok!(Voting::vote(RuntimeOrigin::signed(1), 3, aye(1, 1))); + assert_eq!( + HooksHandler::last_on_vote_data(), + Some(( + 1, + 3, + AccountVote::Standard { + vote: Vote { aye: true, conviction: Conviction::Locked1x }, + balance: 1 + } + )) + ); + assert_ok!(Voting::vote(RuntimeOrigin::signed(2), 3, nay(20, 2))); + assert_eq!( + HooksHandler::last_on_vote_data(), + Some(( + 2, + 3, + AccountVote::Standard { + vote: Vote { aye: false, conviction: Conviction::Locked2x }, + balance: 20 + } + )) + ); + HooksHandler::reset(); + + // removing vote while ongoing + assert_ok!(Voting::vote(RuntimeOrigin::signed(3), 3, nay(20, 0))); + assert_eq!( + HooksHandler::last_on_vote_data(), + Some(( + 3, + 3, + AccountVote::Standard { + vote: Vote { aye: false, conviction: Conviction::None }, + balance: 20 + } + )) + ); + assert_ok!(Voting::remove_vote(RuntimeOrigin::signed(3), Some(c), 3)); + assert_eq!(HooksHandler::last_on_remove_vote_data(), Some((3, 3, Some(true)))); + HooksHandler::reset(); + + Polls::set(vec![(3, Completed(3, false))].into_iter().collect()); + + // removing successful vote while completed + assert_ok!(Voting::remove_vote(RuntimeOrigin::signed(2), Some(c), 3)); + assert_eq!(HooksHandler::last_on_remove_vote_data(), Some((2, 3, Some(false)))); + assert_eq!(HooksHandler::last_locked_if_unsuccessful_vote_data(), None); + + HooksHandler::reset(); + + HooksHandler::with_remove_locked_amount(5); + + // removing unsuccessful vote when completed + assert_ok!(Voting::remove_vote(RuntimeOrigin::signed(1), Some(c), 3)); + assert_eq!(HooksHandler::last_on_remove_vote_data(), Some((1, 3, Some(false)))); + assert_eq!(HooksHandler::last_locked_if_unsuccessful_vote_data(), Some((1, 3))); + + // Removing unsuccessful vote when completed should lock if given amount from the hook + assert_ok!(Voting::unlock(RuntimeOrigin::signed(1), c, 1)); + assert_eq!(Balances::usable_balance(1), 5); + }); +} \ No newline at end of file diff --git a/substrate/frame/conviction-voting/src/traits.rs b/substrate/frame/conviction-voting/src/traits.rs new file mode 100644 index 000000000000..b841c0142048 --- /dev/null +++ b/substrate/frame/conviction-voting/src/traits.rs @@ -0,0 +1,40 @@ +use crate::AccountVote; +use frame_support::dispatch::DispatchResult; + +pub trait VotingHooks { + // Called when vote is executed. + fn on_vote(who: &AccountId, ref_index: Index, vote: AccountVote) -> DispatchResult; + + // Called when removed vote is executed. + // is_finished indicates the state of the referendum = None if referendum is cancelled, Some(true) if referendum is ongoing and Some(false) when finished. + fn on_remove_vote(who: &AccountId, ref_index: Index, ongoing: Option); + + // Called when removed vote is executed and voter lost the direction to possibly lock some balance. + // Can return an amount that should be locked for the conviction time. + fn balance_locked_on_unsuccessful_vote(who: &AccountId, ref_index: Index) -> Option; + + #[cfg(feature = "runtime-benchmarks")] + fn on_vote_worst_case(who: &AccountId); + + #[cfg(feature = "runtime-benchmarks")] + fn on_remove_vote_worst_case(who: &AccountId); +} + +// Default implementation for VotingHooks +impl VotingHooks for () { + fn on_vote(_who: &A, _ref_index: I, _vote: AccountVote) -> DispatchResult { + Ok(()) + } + + fn on_remove_vote(_who: &A, _ref_index: I, _ongoing: Option) {} + + fn balance_locked_on_unsuccessful_vote(_who: &A, _ref_index: I) -> Option { + None + } + + #[cfg(feature = "runtime-benchmarks")] + fn on_vote_worst_case(_who: &A) {} + + #[cfg(feature = "runtime-benchmarks")] + fn on_remove_vote_worst_case(_who: &A) {} +} \ No newline at end of file