vote-program: plumb target_version through the program#8191
Conversation
|
The Firedancer team maintains a line-for-line reimplementation of the |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8191 +/- ##
========================================
Coverage 83.1% 83.2%
========================================
Files 828 828
Lines 365080 365167 +87
========================================
+ Hits 303700 303836 +136
+ Misses 61380 61331 -49 🚀 New features to boost your workflow:
|
buffalojoec
force-pushed
the
vote-state-target-version-with-unit-tests
branch
from
a9d52b1 to
9b8f3cf
Compare
| /// Given a proposed new commission, returns true if this would be a commission increase, false otherwise | ||
| pub fn is_commission_increase(vote_state: &VoteStateV3, commission: u8) -> bool { | ||
| commission > vote_state.commission | ||
| } | ||
|
|
There was a problem hiding this comment.
This appears unused across the monorepo.
| let mut borrowed_account = instruction_context | ||
| .try_borrow_instruction_account(0) | ||
| .unwrap(); | ||
| let vote_pubkey = *borrowed_account.get_key(); |
There was a problem hiding this comment.
Pretty odd that this test sets up the tx context with the vote address set to the node_pubkey. Can you update the test to specify a vote pubkey and use that to setup the context?
Referring to this:
let mut transaction_context = TransactionContext::new(
vec![(id(), processor_account), (node_pubkey, vote_account)],There was a problem hiding this comment.
Sure thing. The goal was to keep the existing tests as close to the same as they were. I also felt like this was a bit strange though. I'll update it.
| // V4 has fields that V1_14_11 does not have, so the vote states | ||
| // will vary slightly. | ||
| check_converted_vote_state_v4_fields(&converted_vote_state); |
There was a problem hiding this comment.
Why not have the vote_state_new_for_test helper take a vote_pubkey param which can be used to initialize the inflation collector address? That way we can just do assert!(vote_state == converted_vote_state); no matter the target version.
| ); | ||
| let vote_state_version = borrowed_account.get_state::<VoteStateVersions>().unwrap(); | ||
| assert_matches!(vote_state_version, VoteStateVersions::V1_14_11(_)); | ||
| // Again, re-set the vote account state, knowing the account only has |
There was a problem hiding this comment.
I just left the test as it was! I can remove the redundant step, though. It was already doing this.
3 participants
Problem
Building on the back of #8178, the next step is to outfit the program to be dynamic over some target vote state version, so we can feed the handler with the desired state version to deserialize to and target for state changes.
Summary of Changes
The main change of this PR is in the first commit, where I plumb the
target_version: VoteStateTargetVersionargument through all of the processor functions that operate on vote state.After that, I update all of the tests (mostly using
test_casewherever possible) to test all of thevote_state/modfunctions individually with each target version V3 and V4.In the top-level of the program processor, the target version is fixed to V3, so the Vote program is still functionally unchanged after this PR.