Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add conviction voting Split, SplitAbstain vote options #2169

Merged
merged 9 commits into from
Mar 23, 2023

Conversation

4meta5
Copy link
Contributor

@4meta5 4meta5 commented Mar 17, 2023

Adds precompile method coverage for all possible votes expressible by pallet_conviction::AccountVote:

/// A vote for a referendum of a particular account.
#[derive(Encode, Decode, Copy, Clone, Eq, PartialEq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
pub enum AccountVote<Balance> {
	/// A standard vote, one-way (approve or reject) with a given amount of conviction.
	Standard { vote: Vote, balance: Balance },
	/// A split vote with balances given for both ways, and with no conviction, useful for
	/// parachains when voting.
	Split { aye: Balance, nay: Balance },
	/// A split vote with balances given for both ways as well as abstentions, and with no
	/// conviction, useful for parachains when voting, other off-chain aggregate accounts and
	/// individuals who wish to abstain.
	SplitAbstain { aye: Balance, nay: Balance, abstain: Balance },
}

This PR also makes use of this type inside the conviction voting precompile to simplify the code.

The idea behind SplitAbstain is to vote to end the referendum soon without expressing an opinion on it. substrate PR

@4meta5 4meta5 added A3-inprogress Pull request is in progress. No review needed at this stage. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit breaking Needs to be mentioned in breaking changes labels Mar 17, 2023
@4meta5
Copy link
Contributor Author

4meta5 commented Mar 18, 2023

resolved

@tmpolaczyk
Copy link
Contributor

I modifed that test to make the error message less useless:

---- tests::vote_logs_work stdout ----
thread 'tests::vote_logs_work' panicked at 'Expected event not found: RuntimeEvent::Evm(Event::Log { log: Log { address: 0x0000000000000000000000000000000000000001, topics: [0x3839f7832b2a6263aa1fd5040f37d10fd4f9e9c4a9ef07ec384cb1cef9fb4c0e, 0x0000000000000000000000000000000000000000000000000000000000000003], data: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 134, 160, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] } })
All events:
[RuntimeEvent::Evm(Event::Log { log: Log { address: 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, topics: [0x3839f7832b2a6263aa1fd5040f37d10fd4f9e9c4a9ef07ec384cb1cef9fb4c0e, 0x0000000000000000000000000000000000000000000000000000000000000003], data: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 134, 160, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] } }), RuntimeEvent::Evm(Event::Executed { address: 0x0000000000000000000000000000000000000001 }), RuntimeEvent::Evm(Event::Log { log: Log { address: 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, topics: [0x3839f7832b2a6263aa1fd5040f37d10fd4f9e9c4a9ef07ec384cb1cef9fb4c0e, 0x0000000000000000000000000000000000000000000000000000000000000003], data: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 170, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 130, 184, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] } }), RuntimeEvent::Evm(Event::Executed { address: 0x0000000000000000000000000000000000000001 })]', precompiles/conviction-voting/src/tests.rs:141:17

These are the code changes if you want to include them:

precompiles/conviction-voting/src/tests.rs
// Assert vote events are emitted.
let expected_events = vec![
    EvmEvent::Log {
        log: log2(
            Precompile1,
            SELECTOR_LOG_VOTED,
            H256::from_low_u64_be(ONGOING_POLL_INDEX as u64),
            EvmDataWriter::new()
                .write::<Address>(H160::from(Alice).into()) // caller
                .write::<bool>(true) // vote
                .write::<U256>(100_000.into()) // amount
                .write::<u8>(0) // conviction
                .build(),
        ),
    }
    .into(),
    EvmEvent::Log {
        log: log2(
            Precompile1,
            SELECTOR_LOG_VOTED,
            H256::from_low_u64_be(ONGOING_POLL_INDEX as u64),
            EvmDataWriter::new()
                .write::<Address>(H160::from(Alice).into()) // caller
                .write::<bool>(false) // vote
                .write::<U256>(99_000.into()) // amount
                .write::<u8>(1) // conviction
                .build(),
        ),
    }
    .into(),
];
for log in expected_events {
    assert!(
        events().contains(&log),
        "Expected event not found: {:?}\nAll events:\n{:?}",
        log,
        events()
    );
}

@4meta5 4meta5 added A0-pleasereview Pull request needs code review. and removed A3-inprogress Pull request is in progress. No review needed at this stage. labels Mar 21, 2023
@4meta5 4meta5 requested a review from tgmichel March 21, 2023 15:42
@4meta5 4meta5 marked this pull request as ready for review March 21, 2023 15:42
@4meta5 4meta5 requested a review from tmpolaczyk March 21, 2023 17:04
4meta5 and others added 2 commits March 22, 2023 09:45
@4meta5 4meta5 merged commit a29a8ff into master Mar 23, 2023
@4meta5 4meta5 deleted the amar-abstain-cv-precompile branch March 23, 2023 19:33
@crystalin crystalin added not-breaking Does not need to be mentioned in breaking changes and removed breaking Needs to be mentioned in breaking changes labels Apr 12, 2023
imstar15 pushed a commit to AvaProtocol/moonbeam that referenced this pull request May 16, 2023
…dation#2169)

* init

* Update precompiles/conviction-voting/src/lib.rs

* fix

* test split vote logs

* clean

* Update precompiles/conviction-voting/src/lib.rs

Co-authored-by: tmpolaczyk <44604217+tmpolaczyk@users.noreply.github.com>

* add events to sol file and use in field wherever relevant

* add unreachable handling of new AccountVote variants for trivial backwards compatibility if new variant added

* revert unnecessary commit

---------

Co-authored-by: tmpolaczyk <44604217+tmpolaczyk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit not-breaking Does not need to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants