Skip to content

vote-program: outfit to fully support vote state v4#8163

Closed
buffalojoec wants to merge 7 commits intoanza-xyz:masterfrom
buffalojoec:vote-state-handler-v4-variant-configure
Closed

vote-program: outfit to fully support vote state v4#8163
buffalojoec wants to merge 7 commits intoanza-xyz:masterfrom
buffalojoec:vote-state-handler-v4-variant-configure

Conversation

@buffalojoec
Copy link
Copy Markdown

Problem

Now that #8120 has landed, VoteStateV4 is now compatible with the VoteStateHandle API used by the Vote program. However, both the handler and the program themselves don't directly support v4 vote state.

In order to enable this support and thus complete the implementation of SIMD-0185, the Vote program must be outfitted to support VoteStateV4.

Summary of Changes

First add a new TargetVoteStateVersion::V4 variant to the VoteStateHandler struct and flesh out the unit tests for the handler. Then, plumb a target_version: TargetVoteStateVersion parameter throughout the Vote program.

With the addition of the new parameter I overhauled a bunch of tests but tried to keep everything pinned to v3-only, until finally adding the feature gate in the second-last commit and converting the rest of the tests.

In the final commit, I took a stab at addressing some offline discussion @jstarry and I were having about v1 vote account states being accidentally deserialized as initialized v4 states, since v4 are always initialized.

@buffalojoec buffalojoec requested a review from jstarry September 24, 2025 00:24
@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.

@buffalojoec buffalojoec changed the title Vote state handler v4 variant configure vote-program: outfit to fully support vote state v4 Sep 24, 2025
@buffalojoec buffalojoec force-pushed the vote-state-handler-v4-variant-configure branch from 1a55688 to 6f1fcb3 Compare September 24, 2025 00:53
@buffalojoec buffalojoec marked this pull request as ready for review September 24, 2025 00:53
Comment on lines +1289 to +1294
if vote_state_v4_enabled {
// v4 is always "initialized" per SIMD-0185.
let v4 = VoteStateV4::deserialize(accounts[0].data(), &vote_pubkey).unwrap();
// After deinitialize, it contains default state.
assert_eq!(v4, VoteStateV4::default());
} else {
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 test is very strange to me. Why not keep the same check for v4?

let post_state: VoteStateVersions = accounts[0].state().unwrap();
assert!(post_state.is_uninitialized());

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.

Because .is_uninitialized() is always false for v4. Maybe it's a smell?

Copy link
Copy Markdown

@jstarry jstarry Sep 24, 2025

Choose a reason for hiding this comment

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

But it would be deserialized as v1 since we zero the entire account

) -> Result<VoteStateHandler, InstructionError> {
let mut vote_state =
VoteStateHandler::deserialize_and_convert(vote_account, VoteStateTargetVersion::V3)?;
let mut 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.

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.

@buffalojoec
Copy link
Copy Markdown
Author

Closing in favor of several other piecemeal PRs to land this feature.

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.

2 participants