paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.com/
1.89k stars 696 forks source link

pallet-conviction-voting: add voting hooks #5735

Open enthusiastmartin opened 2 months ago

enthusiastmartin commented 2 months ago

Description

We, at Hydration, need to perform additional actions when a new vote is registered and existing vote is removed. Our use case is tied to staking, where we need to reward an account for voting.

Therefore, we need to know when a new vote is registered. We propose this PR to add voting hooks to the conviction-voting pallet.

This change does not change the original behavior ( except one special case described below if desired).

In the pallet config, there is a new config parameter:

    type VotingHooks: VotingHooks<Self::AccountId, PollIndexOf<Self, I>, BalanceOf<Self, I>>;

Anything that integrates conviction-voting pallet, can decide to implement the voting hooks and perform desirable action on events such as on_vote and on_remove_vote.

The VotingHooks traits looks like:

pub trait VotingHooks<AccountId, Index, Balance> {
    // Called when vote is executed.
    fn on_vote(who: &AccountId, ref_index: Index, vote: AccountVote<Balance>) -> 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<bool>);

    // 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<Balance>;

    #[cfg(feature = "runtime-benchmarks")]
    fn on_vote_worst_case(who: &AccountId);

    #[cfg(feature = "runtime-benchmarks")]
    fn on_remove_vote_worst_case(who: &AccountId);
}

The on_voteand on_remove_vote are pretty self-explanatory and are called when a vote is added/removed.

There is a hook called balance_locked_on_unsuccessful_vote. This is a special hook that is called when a vote that is being removed was in opposition to the winning vote. The result of this function can return a balance and that amount will be locked for the conviction time. This is the only change that modifies original behavior of the pallet where there was no locking in such case.

It is a special case, where a chain can decide that it needs to lock some balance even in the case of not-winning vote. We, at Hydration, use these hooks with staking functionality where we require to lock too in such cases. Obviously, returning None by defaut, completely disables this feature and its behavior remains as it is.

Integration

To integrate VotingHooks and to perform certain actions as described above, it is required to implement corresponding trait.

Otherwise, simple

    type VotingHooks = ();

can be used and conviction-voting pallet works as nothing happens.

Review Notes

The hooks are called in try_vote and try_remove_vote with parameters.

on_vote hook needs an account that voted on referendum identified by ref_index and the vote itself.

    fn on_vote(who: &AccountId, ref_index: Index, vote: AccountVote<Balance>) -> DispatchResult;

on_remove_vote needs an account that is removing the vote from referendum identified by ref_index. Also bool flag is added to provide information if the referendum is still ongoing.

    fn on_remove_vote(who: &AccountId, ref_index: Index, ongoing: Option<bool>);

ongoing flag is an option because value None is used when referendum has been cancelled.

Tests

Unit tests are added to verify that each hooks is called with correct parameters.

Benchmarks

VotingHook trait contains:

    #[cfg(feature = "runtime-benchmarks")]
    fn on_vote_worst_case(who: &AccountId);

    #[cfg(feature = "runtime-benchmarks")]
    fn on_remove_vote_worst_case(who: &AccountId);

These are called in benchmarks to set up worst cases for the hooks.

Question/Consideration: WE considered adding BenchmarkHelper instead of having the runtime-benchmarks functions in the trait. But I believe this is easier to understand when and why it needs to implement worst cases scenarios ( only if voting hook is used).