Skip to content

vote-program: handler: add v4 variant#8178

Merged
buffalojoec merged 5 commits intoanza-xyz:masterfrom
buffalojoec:vote-state-handler-v4-variant-with-unit-tests
Sep 26, 2025
Merged

vote-program: handler: add v4 variant#8178
buffalojoec merged 5 commits intoanza-xyz:masterfrom
buffalojoec:vote-state-handler-v4-variant-with-unit-tests

Conversation

@buffalojoec
Copy link
Copy Markdown

Problem

Building on the back of #8120, we need to outfit the VoteStateHandler with support for VoteStateV4. As we do this, we want to make sure each implementation is 100% compatible with SIMD-0185.

Summary of Changes

Adds the v4 variant to the VoteStateHandler and adds a bunch of unit tests for compatibility with SIMD-0185. Does not update the Vote program to use v4 yet.

@buffalojoec buffalojoec requested a review from jstarry September 24, 2025 20:53
@mergify
Copy link
Copy Markdown

mergify Bot commented Sep 24, 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.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 24, 2025

Codecov Report

❌ Patch coverage is 93.48837% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.0%. Comparing base (7015e9c) to head (5685c5e).
⚠️ Report is 15 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #8178    +/-   ##
========================================
  Coverage    83.0%    83.0%            
========================================
  Files         827      827            
  Lines      362695   363056   +361     
========================================
+ Hits       301233   301592   +359     
- Misses      61462    61464     +2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jstarry
jstarry previously approved these changes Sep 26, 2025
Copy link
Copy Markdown

@jstarry jstarry left a comment

Choose a reason for hiding this comment

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

Looks good, just a few small things

Comment on lines +866 to +870
VoteStateTargetVersion::V4 => {
let vote_state =
VoteStateV4::deserialize(vote_account.get_data(), vote_account.get_key())?;
TargetVoteState::V4(vote_state)
}
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 is fine but the way we use deserialize_and_convert in verify_and_get_vote_state_handler is problematic. So let's keep that in mind for later

#8163 (comment)

The implementation of VoteStateV4::deserialize for zeroed account data is not ideal. It will successfully deserialize into an uninitialized v1 and then convert to VoteStateV4::default() which will incorrectly be considered to be "initialized." Ideally we first deserialize VoteStateVersions here and then do the initialization check before converting to v4.

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.

Okay, covered in follow-up.

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
Co-authored-by: Justin Starry <justin.m.starry@gmail.com>
@buffalojoec buffalojoec merged commit 2b843ca into anza-xyz:master Sep 26, 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