Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
fadf4a6
first commit
ndkazu Feb 24, 2025
d1a9344
corrected tests
ndkazu Feb 25, 2025
3dcb4a3
updated benchmarking
ndkazu Feb 25, 2025
f8b7181
Merge branch 'paritytech:master' into voting_hook
ndkazu Feb 25, 2025
f18593a
prdoc
ndkazu Feb 25, 2025
524b264
Merge branch 'voting_hook' of github.com:ndkazu/polkadot-sdk into vot…
ndkazu Feb 25, 2025
0a8ad67
cargo fmt
ndkazu Feb 25, 2025
ce3c5e5
Merge branch 'master' into voting_hook
ndkazu Feb 25, 2025
ec983ca
Merge branch 'paritytech:master' into voting_hook
ndkazu Feb 27, 2025
70b4811
Merge branch 'paritytech:master' into voting_hook
ndkazu Feb 28, 2025
ec732ce
Merge branch 'paritytech:master' into voting_hook
ndkazu Feb 28, 2025
4546609
Merge branch 'paritytech:master' into voting_hook
ndkazu Mar 1, 2025
406215b
Merge branch 'master' into voting_hook
ndkazu Mar 3, 2025
c0bd834
Merge branch 'master' into voting_hook
ndkazu Mar 4, 2025
1f1667e
Merge branch 'master' into voting_hook
ndkazu Mar 5, 2025
ea48a53
Merge branch 'master' into voting_hook
ndkazu Mar 7, 2025
0940e8b
Merge branch 'master' into voting_hook
ndkazu Mar 8, 2025
e38231c
Merge branch 'master' into voting_hook
ndkazu Mar 10, 2025
caddeda
Merge branch 'master' into voting_hook
ndkazu Mar 11, 2025
8191677
Merge branch 'master' into voting_hook
ndkazu Mar 14, 2025
84865b0
Some corrections implemented
ndkazu Mar 15, 2025
a5a91b5
Merge branch 'voting_hook' of github.com:ndkazu/polkadot-sdk into vot…
ndkazu Mar 15, 2025
43f7fd1
Merge branch 'master' into voting_hook
ndkazu Mar 15, 2025
63b61be
Merge branch 'master' into voting_hook
ndkazu Mar 17, 2025
9eb281b
Merge branch 'master' into voting_hook
ndkazu Mar 17, 2025
2059dc9
Documentation
ndkazu Mar 17, 2025
0ef16e5
cargo fmt
ndkazu Mar 17, 2025
5632f80
clippy
ndkazu Mar 17, 2025
bfaad91
Runtimes update
ndkazu Mar 17, 2025
e3f7bee
clippy fix
ndkazu Mar 17, 2025
6ba6a01
Update pr_7703.prdoc
ndkazu Mar 17, 2025
9807ca9
Merge branch 'master' into voting_hook
ndkazu Mar 17, 2025
f696d51
Merge branch 'master' into voting_hook
ndkazu Mar 18, 2025
890b383
Merge branch 'master' into voting_hook
ndkazu Mar 21, 2025
4a6b2ab
Merge branch 'master' into voting_hook
ndkazu Mar 21, 2025
200dc86
Merge branch 'master' into voting_hook
ndkazu Mar 22, 2025
d507252
Merge branch 'master' into voting_hook
ndkazu Mar 25, 2025
d7579d7
Some corrections applied
ndkazu Mar 26, 2025
b1985f5
Added more doc for voting-hook
ndkazu Mar 26, 2025
d937f24
Merge branch 'master' into voting_hook
ndkazu Mar 26, 2025
a27d12a
Cargo fmt
ndkazu Mar 26, 2025
f300715
Merge branch 'voting_hook' of github.com:ndkazu/polkadot-sdk into vot…
ndkazu Mar 26, 2025
f89ce2d
Merge branch 'master' into voting_hook
ndkazu Mar 26, 2025
2441cac
Update substrate/frame/conviction-voting/src/lib.rs
bkchr Mar 26, 2025
7e99503
Merge branch 'master' into voting_hook
ndkazu Mar 26, 2025
f145819
Updated prdoc
ndkazu Mar 28, 2025
d8271cb
Merge branch 'master' into voting_hook
ndkazu Mar 28, 2025
ef7f6f5
Merge branch 'master' into voting_hook
ndkazu Mar 28, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions polkadot/runtime/rococo/src/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ impl pallet_conviction_voting::Config for Runtime {
frame_support::traits::tokens::currency::ActiveIssuanceOf<Balances, Self::AccountId>;
type Polls = Referenda;
type BlockNumberProvider = System;
type VotingHooks = ();
}

parameter_types! {
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/westend/src/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ impl pallet_conviction_voting::Config for Runtime {
frame_support::traits::tokens::currency::ActiveIssuanceOf<Balances, Self::AccountId>;
type Polls = Referenda;
type BlockNumberProvider = System;
type VotingHooks = ();
}

parameter_types! {
Expand Down
37 changes: 37 additions & 0 deletions prdoc/pr_7703.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
title: 'Add voting hooks to Conviction_Voting'
doc:
- audience: Runtime Dev
description: |
This change introduces voting hooks to the conviction-voting pallet, enabling developers to
customize behavior during various stages of the voting process. These hooks provide a mechanism
to execute specific logic before a vote is recorded, before a vote is removed, or when a vote
fails to be recorded, while maintaining compatibility with the existing conviction-voting pallet.

The key hooks include:
- `on_vote`: Called before a vote is recorded. This hook allows developers to validate or
perform actions based on the vote. If it returns an error, the voting operation is reverted.
However, any storage modifications made by this hook will persist even if the vote fails later.
- `on_remove_vote`: Called before a vote is removed. This hook cannot fail and is useful for
cleanup or additional logic when a vote is removed.
- `lock_balance_on_unsuccessful_vote`: Called when a vote fails to be recorded, such as due to
insufficient balance. It allows locking a specific balance amount as part of the failure handling.

Advantages of using voting hooks:
- Flexibility: Developers can implement custom logic to extend or modify the behavior of the
conviction-voting pallet.
- Control: Hooks provide fine-grained control over different stages of the voting process.
- Error Handling: The `on_vote` hook enables early validation, preventing (to some extent) invalid votes from
being recorded.

How to use:
- Implement the `VotingHooks` trait in your runtime or custom module.
- Define the desired behavior for each hook method, such as validation logic in `on_vote` or
cleanup actions in `on_remove_vote`.
- Integrate the implementation with the conviction-voting pallet to enable the hooks.
crates:
- name: pallet-conviction-voting
bump: major
- name: rococo-runtime
bump: minor
- name: westend-runtime
bump: minor
1 change: 1 addition & 0 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,7 @@ impl pallet_conviction_voting::Config for Runtime {
type MaxTurnout = frame_support::traits::TotalIssuanceOf<Balances, Self::AccountId>;
type Polls = Referenda;
type BlockNumberProvider = System;
type VotingHooks = ();
}

parameter_types! {
Expand Down
8 changes: 8 additions & 0 deletions substrate/frame/conviction-voting/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ benchmarks_instance_pallet! {
whitelist_account!(caller);
let account_vote = account_vote::<T, I>(100u32.into());

T::VotingHooks::on_vote_worst_case(&caller);

let (class, all_polls) = fill_voting::<T, I>();
let polls = &all_polls[&class];
let r = polls.len() - 1;
Expand Down Expand Up @@ -100,6 +102,8 @@ benchmarks_instance_pallet! {
whitelist_account!(caller);
let old_account_vote = account_vote::<T, I>(100u32.into());

T::VotingHooks::on_vote_worst_case(&caller);

let (class, all_polls) = fill_voting::<T, I>();
let polls = &all_polls[&class];
let r = polls.len();
Expand Down Expand Up @@ -128,6 +132,8 @@ benchmarks_instance_pallet! {
whitelist_account!(caller);
let old_account_vote = account_vote::<T, I>(100u32.into());

T::VotingHooks::on_vote_worst_case(&caller);

let (class, all_polls) = fill_voting::<T, I>();
let polls = &all_polls[&class];
let r = polls.len();
Expand Down Expand Up @@ -157,6 +163,8 @@ benchmarks_instance_pallet! {
whitelist_account!(caller);
let old_account_vote = account_vote::<T, I>(100u32.into());

T::VotingHooks::on_vote_worst_case(&caller);

let (class, all_polls) = fill_voting::<T, I>();
let polls = &all_polls[&class];
let r = polls.len();
Expand Down
53 changes: 50 additions & 3 deletions substrate/frame/conviction-voting/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,15 @@ use sp_runtime::{
};

mod conviction;
mod traits;
mod types;
mod vote;
pub mod weights;

pub use self::{
conviction::Conviction,
pallet::*,
traits::{Status, VotingHooks},
types::{Delegations, Tally, UnvoteScope},
vote::{AccountVote, Casting, Delegating, Vote, Voting},
weights::WeightInfo,
Expand Down Expand Up @@ -142,6 +144,18 @@ pub mod pallet {
type VoteLockingPeriod: Get<BlockNumberFor<Self, I>>;
/// Provider for the block number. Normally this is the `frame_system` pallet.
type BlockNumberProvider: BlockNumberProvider;
/// Hooks are called when a new vote is registered or an existing vote is removed.
///
/// The trait does not expose weight information.
/// The weight of each hook is assumed to be benchmarked as part of the function that calls
/// it. Hooks should never recursively call into functions that called,
/// directly or indirectly, the function that called them.
/// This could lead to infinite recursion and stack overflow.
/// Note that this also means to not call into other generic functionality like batch or
/// similar. Also, anything that a hook did will be subject to the transactional semantics
/// of the calling function. This means that if the calling function fails, the hook will
/// be rolled back without further notice.
type VotingHooks: VotingHooks<Self::AccountId, PollIndexOf<Self, I>, BalanceOf<Self, I>>;
}

/// All voting for a particular voter in a particular voting class. We store the balance for the
Expand Down Expand Up @@ -410,6 +424,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
vote.balance() <= T::Currency::total_balance(who),
Error::<T, I>::InsufficientFunds
);
// Call on_vote hook
T::VotingHooks::on_before_vote(who, poll_index, vote)?;

T::Polls::try_access_poll(poll_index, |poll_status| {
let (tally, class) = poll_status.ensure_ongoing().ok_or(Error::<T, I>::NotOngoing)?;
VotingFor::<T, I>::try_mutate(who, &class, |voting| {
Expand All @@ -435,7 +452,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
tally.increase(approve, *delegations);
}
} else {
return Err(Error::<T, I>::AlreadyDelegating.into())
return Err(Error::<T, I>::AlreadyDelegating.into());
}
// Extend the lock to `balance` (rather than setting it) since we don't know what
// other votes are in place.
Expand Down Expand Up @@ -477,10 +494,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
tally.reduce(approve, *delegations);
}
Self::deposit_event(Event::VoteRemoved { who: who.clone(), vote: v.1 });
T::VotingHooks::on_remove_vote(who, poll_index, Status::Ongoing);
Ok(())
},
PollStatus::Completed(end, approved) => {
if let Some((lock_periods, balance)) = v.1.locked_if(approved) {
if let Some((lock_periods, balance)) =
v.1.locked_if(vote::LockedIf::Status(approved))
{
let unlock_at = end.saturating_add(
T::VoteLockingPeriod::get().saturating_mul(lock_periods.into()),
);
Expand All @@ -492,10 +512,37 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
);
prior.accumulate(unlock_at, balance)
}
} else if v.1.as_standard().is_some_and(|vote| vote != approved) {
// Unsuccessful vote, use special hook to lock the funds too in case of
// conviction.
if let Some(to_lock) =
T::VotingHooks::lock_balance_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 = T::BlockNumberProvider::current_block_number();
if now < unlock_at {
ensure!(
matches!(scope, UnvoteScope::Any),
Error::<T, I>::NoPermissionYet
);
prior.accumulate(unlock_at, to_lock)
}
}
}
}
// Call on_remove_vote hook
T::VotingHooks::on_remove_vote(who, poll_index, Status::Completed);
Ok(())
},
PollStatus::None => {
// Poll was cancelled.
T::VotingHooks::on_remove_vote(who, poll_index, Status::None);
Ok(())
},
PollStatus::None => Ok(()), // Poll was cancelled.
})
} else {
Ok(())
Expand Down
140 changes: 139 additions & 1 deletion substrate/frame/conviction-voting/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

//! The crate's tests.

use std::collections::BTreeMap;
use std::{cell::RefCell, collections::BTreeMap};

use frame_support::{
assert_noop, assert_ok, derive_impl, parameter_types,
Expand Down Expand Up @@ -155,6 +155,7 @@ impl Config for Test {
type MaxTurnout = frame_support::traits::TotalIssuanceOf<Balances, Self::AccountId>;
type Polls = TestPolls;
type BlockNumberProvider = System;
type VotingHooks = HooksHandler;
}

pub fn new_test_ext() -> sp_io::TestExternalities {
Expand Down Expand Up @@ -913,3 +914,140 @@ fn errors_with_remove_vote_work() {
);
});
}

thread_local! {
static LAST_ON_VOTE_DATA: RefCell<Option<(u64, u8, AccountVote<u64>)>> = RefCell::new(None);
static LAST_ON_REMOVE_VOTE_DATA: RefCell<Option<(u64, u8, Status)>> = RefCell::new(None);
static LAST_LOCKED_IF_UNSUCCESSFUL_VOTE_DATA: RefCell<Option<(u64, u8)>> = RefCell::new(None);
static REMOVE_VOTE_LOCKED_AMOUNT: RefCell<Option<u64>> = RefCell::new(None);
}

pub struct HooksHandler;

impl HooksHandler {
fn last_on_vote_data() -> Option<(u64, u8, AccountVote<u64>)> {
LAST_ON_VOTE_DATA.with(|data| *data.borrow())
}

fn last_on_remove_vote_data() -> Option<(u64, u8, Status)> {
LAST_ON_REMOVE_VOTE_DATA.with(|data| *data.borrow())
}

fn last_locked_if_unsuccessful_vote_data() -> Option<(u64, u8)> {
LAST_LOCKED_IF_UNSUCCESSFUL_VOTE_DATA.with(|data| *data.borrow())
}

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<u64, u8, u64> for HooksHandler {
fn on_before_vote(who: &u64, ref_index: u8, vote: AccountVote<u64>) -> 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: Status) {
LAST_ON_REMOVE_VOTE_DATA.with(|data| {
*data.borrow_mut() = Some((*who, ref_index, ongoing));
});
}

fn lock_balance_on_unsuccessful_vote(who: &u64, ref_index: u8) -> Option<u64> {
LAST_LOCKED_IF_UNSUCCESSFUL_VOTE_DATA.with(|data| {
*data.borrow_mut() = Some((*who, ref_index));

REMOVE_VOTE_LOCKED_AMOUNT.with(|data| *data.borrow())
})
}

#[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, Status::Ongoing)));
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, Status::Completed)));
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, Status::Completed)));
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);
});
}
Loading
Loading