Skip to content

vote-program: handler: init v4 support#8120

Merged
buffalojoec merged 13 commits intoanza-xyz:masterfrom
buffalojoec:vote-state-handler-v4-variant
Sep 23, 2025
Merged

vote-program: handler: init v4 support#8120
buffalojoec merged 13 commits intoanza-xyz:masterfrom
buffalojoec:vote-state-handler-v4-variant

Conversation

@buffalojoec
Copy link
Copy Markdown

@buffalojoec buffalojoec commented Sep 20, 2025

Problem

Now that we have a "vote state handler" that can help us gate new vote state targets behind feature gates, it's time to implement VoteStateV4 within the handler.

Summary of Changes

Implements the VoteStateHandle trait for VoteStateV4 in the program's handler module and does some moderate refactoring to simplify the trait where possible and share functionality across free functions.

This change does not add a V4 variant to the handler yet.

@buffalojoec buffalojoec requested a review from jstarry September 20, 2025 08:27
@buffalojoec buffalojoec added the noCI Suppress CI on this Pull Request label Sep 20, 2025
@mergify
Copy link
Copy Markdown

mergify Bot commented Sep 20, 2025

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

Comment thread programs/vote/src/vote_state/handler.rs Outdated
Comment thread programs/vote/src/vote_state/handler.rs Outdated
Comment thread programs/vote/src/vote_state/handler.rs Outdated
Comment thread programs/vote/src/vote_state/handler.rs
Comment thread programs/vote/src/vote_state/handler.rs
Comment thread programs/vote/src/vote_state/handler.rs Outdated
Comment thread programs/vote/src/vote_state/handler.rs Outdated
Comment on lines +711 to +713
VoteStateTargetVersion::V4 => {
VoteStateV4::default().set_vote_account_state(vote_account)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just zero the vote account data after we do the resize / rent check for prev vote state versions

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this also belongs to follow-up now?

Comment thread programs/vote/src/vote_state/mod.rs Outdated
instruction_context.try_borrow_instruction_account(vote_account_index)?;
let vote_state =
VoteStateHandler::deserialize_and_convert(&vote_account, VoteStateTargetVersion::V3)?;
let vote_state = VoteStateHandler::deserialize_and_convert(&vote_account, target_version)?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need to update the target version below in deinitialize_vote_account_state. I guess it needs tests too?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment belongs to the follow-up PR now.

Comment thread programs/vote/src/vote_state/handler.rs Outdated
assert!(!vote_state_versions.is_uninitialized());

// Verify fields.
if let VoteStateVersions::V4(v4) = vote_state_versions {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add checks for the other initialized v4 fields per simd-185 (collector accounts, etc)

Comment thread programs/vote/src/vote_state/handler.rs Outdated
if result.is_ok() {
let vote_state_versions = vote_account.get_state::<VoteStateVersions>().unwrap();
assert!(matches!(vote_state_versions, VoteStateVersions::V4(_)));
// v4 is always initialized
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After de-init, we shouldn't be able to deserialize a v4 state anymore since it should be a v0 uninitalized account.

Comment thread programs/vote/src/vote_state/handler.rs Outdated
@buffalojoec buffalojoec force-pushed the vote-state-handler-v4-variant branch from 69ba407 to ac69447 Compare September 22, 2025 11:25
@buffalojoec buffalojoec changed the title [DRAFT]: vote-program: handler: add v4 variant vote-program: handler: init v4 support Sep 22, 2025
@buffalojoec
Copy link
Copy Markdown
Author

I repurposed this PR to be identical to #8108.

@jstarry jstarry removed the noCI Suppress CI on this Pull Request label Sep 23, 2025
@buffalojoec buffalojoec marked this pull request as ready for review September 23, 2025 11:03
@buffalojoec
Copy link
Copy Markdown
Author

@jstarry It's a little difficult to tell what comments were for Part 1 of this work or Part 2. I think I got everything for Part 1. Everything else mentioned should pertain to Part 2, where we add the V4 variant to the handler and update the program.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.45247% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.0%. Comparing base (4a70cfb) to head (b05fc66).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #8120    +/-   ##
========================================
  Coverage    82.9%    83.0%            
========================================
  Files         823      823            
  Lines      361216   361498   +282     
========================================
+ Hits       299758   300132   +374     
+ Misses      61458    61366    -92     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread programs/vote/src/vote_state/mod.rs
@buffalojoec buffalojoec merged commit 894d8da into anza-xyz:master Sep 23, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants