SIMD-0185: Vote Account v4 by jstarry · Pull Request #185 · solana-foundation/solana-improvement-documents

jstarry · 2024-10-17T19:42:34Z

No description provided.

AshwinSekar · 2024-10-18T02:11:27Z

+`authorized_voters` field that correspond to epochs less than the current epoch,
+only purge entries less than the previous epoch (current epoch - 1). This will
+mean that the `authorized_voters` field can now hold up to 4 entries for the
+epochs in the range `[current_epoch - 1, current_epoch + 2]`. Keeping the


is there a reason we need to store current_epoch + 2?

Yeah that's the current design and I think it kinda makes sense because otherwise you could set a new authorized voter at the end of an epoch and immediately start using it in the next block in the new epoch and leaders would need an up to date view on any new authorized voters for each fork crossing the epoch boundary

diman-io · 2024-10-23T19:26:24Z

I'm sorry, why is block_tips_collector here? Are tips somehow planned in the protocol level / voting program?

jstarry · 2024-10-23T20:11:56Z

I'm sorry, why is block_tips_collector here? Are tips somehow planned in the protocol level / voting program?

Up for debate still, but I put it there for now since it's easier to update the vote account all in one go. If that field remains unused by the protocol it can be used for something else in the future.

diman-io · 2024-10-23T20:20:40Z

Is this about a new instruction to pay the leader who landed a successful transaction?

segfaultdoc

Overall good proposal, trying to think longer term if the Tips commission piece makes sense or becomes obsolete

jstarry · 2024-11-05T16:49:55Z

I updated the proposal to use basis points for commission values and to remove any special handling for block tips for now.

jstarry · 2025-01-24T09:34:10Z

@AshwinSekar I've updated the SIMD to only remove the prior_voters field in a new version of vote state. This way the SIMD is much simpler and we can make progress towards adding other new fields to the vote state without needing to reallocations for existing vote accounts like we had to for TVC.

AshwinSekar · 2025-01-24T17:50:07Z

@AshwinSekar I've updated the SIMD to only remove the prior_voters field in a new version of vote state. This way the SIMD is much simpler and we can make progress towards adding other new fields to the vote state without needing to reallocations for existing vote accounts like we had to for TVC.

Got it, happy to approve. I do think it would be a lot faster if we could get away with one migration to remove prior_voters and add the new fields. Are you facing any hurdles with that approach?

lidatong

approving for FD

jstarry · 2025-02-17T20:09:13Z

Got it, happy to approve. I do think it would be a lot faster if we could get away with one migration to remove prior_voters and add the new fields. Are you facing any hurdles with that approach?

Yeah maybe that's still the best approach. There's just a lot of changes I want to make:

Remove prior_voters to make room for other state without increasing vote state size
Additionally store the authorized voter for the previous epoch now that prior voters is going away
Store new fields for fee collector addresses, commissions, and pending delegator rewards
New instructions for setting collector addresses and commission

Would we use a single feature gate for all of these changes? I think that could be fine. We will have a separate feature gate for SIMD-0123 anyways to actually make use of these new fields.

This reverts commit 68a231b.

AshwinSekar

Would we use a single feature gate for all of these changes? I think that could be fine. We will have a separate feature gate for SIMD-0123 anyways to actually make use of these new fields.

This sounds like a good approach. lgtm

michaelh-laine · 2025-03-04T23:39:40Z

Can this be incorporated into this SIMD?

#254

jstarry · 2025-03-05T00:44:52Z

Can this be incorporated into this SIMD?

#254

I already have a separate SIMD written up for this actually: #249

jstarry · 2025-04-16T05:13:48Z

Just pushed another update to this SIMD to add more detail to the required changes section and some other minor cleanup and renaming. It'd be great to get another review pass from anza devs @AshwinSekar @buffalojoec @t-nelson and fd devs @lidatong @topointon-jump

jstarry · 2025-04-16T05:18:30Z

+The existing withdraw instruction MUST be modified to completely zero vote
+account data for fully withdrawn vote accounts. The old behavior partially
+zeroed the account data following the vote state version discriminant and is
+less intuitive.


I felt this was a nice to have change. This way we know that any vote account with the version discriminant bytes 03 00 00 00 is still initialized. Other vote state versions require checking fields for initialized values to be sure of initialization which is a footgun IMO.

Seems reasonable to me, unless there's some reason it's done like this currently? Seems strange to keep that discriminator set afterward.

No explicit reason that I'm aware of. We will need an extra look from auditors at this though.

jstarry · 2025-04-16T05:22:08Z

+If a modified vote account's size is smaller than `3762` bytes, first resize the
+account to `3762` bytes before updating the account data. The vote program does
+not need to check if the resulting account is rent exempt, the runtime will
+enforce that check. This differs from the prior vote program implementation which
+falls back to store vote state as v2 if the account size cannot be resized while
+keeping the account rent exempt.


I felt this was nice to have because it simplifies the vote program implementation and ensures that all v4 vote accounts have the same size. Here's the current code in question that this will clean up:

https://github.com/anza-xyz/agave/blame/0df4a2d963e06ced795247719ef1aa16c6d71d15/programs/vote/src/vote_state/mod.rs#L40-L54

buffalojoec

Looks good to me. I left a few clarifying comments or ideas, but no major concerns on my end!

buffalojoec · 2025-04-30T09:45:45Z

+The existing withdraw instruction MUST be modified to completely zero vote
+account data for fully withdrawn vote accounts. The old behavior partially
+zeroed the account data following the vote state version discriminant and is
+less intuitive.


Seems reasonable to me, unless there's some reason it's done like this currently? Seems strange to keep that discriminator set afterward.

buffalojoec

Thanks for updating. I left one more question, then I can stamp.

buffalojoec · 2025-05-05T06:28:27Z

Approved as long as we are certain there are little to no Vote State v2 or older vote accounts in active use. Throwing an error on those accounts not being rent-exempt could affect those nodes' timely vote credits.

jstarry · 2025-05-05T09:07:48Z

Approved as long as we are certain there are little to no Vote State v2 or older vote accounts in active use. Throwing an error on those accounts not being rent-exempt could affect those nodes' timely vote credits.

New vote accounts must be created as v3 and note that v2 vote state accounts wouldn't have timely vote credits because v3 was introduced to add that feature. Because of that, we don't need to be certain about whether v2 accounts are in active use for this proposal, they would already be severely disadvantaged in consensus when tvc was enabled and have plenty of incentive to upgrade if they hadn't already. But in any case, I did check mainnet and there are no active vote accounts using v2 still.

wen-coding · 2025-07-08T05:14:47Z

    pub pending_delegator_rewards: u64,

+    /// NEW: compressed bls pubkey for alpenglow
+    pub bls_pubkey_compressed: Option<[u8; 48]>


Looks good, compressed should be fine. But I think @AshwinSekar proposed we make this field mandatory...

As I said in the other thread option is fine. Since V4 migration happens automatically on first vote instruction so we can't enforce that this field is populated by operators, we'll have to detect it in Agave when alpenglow is active and instruct operators to fill it in.

AshwinSekar

Looks good. If possible can we also squeeze in the additional instruction here?

    /// Update the BLS Pubkey for the vote account
    ///
    /// # Account references
    ///   0. `[WRITE]` Vote account to be updated
    ///   1. `[SIGNER]` Withdraw authority
    UpdateBLSPubkey([u8; 48]),

Can also defer this to the Alpenglow SIMD if you prefer.

jstarry · 2025-07-09T02:09:20Z

Looks good

Thanks for looking!

If possible can we also squeeze in the additional instruction here?
[..]
Can also defer this to the Alpenglow SIMD if you prefer.

I prefer to defer this instruction to another SIMD. I've deferred the new instructions for updating commission and collector accounts to other SIMD's as well.

buffalojoec

This is ready to go from my side, but I left a few small comments on the recent additions. None affect the spec.

buffalojoec

Lgtm, ship it.

SIMD-0185: Vote Account v4

9d8dedb

jstarry force-pushed the vote-account-v4 branch from 3fc627d to 9d8dedb Compare October 17, 2024 19:43

jstarry changed the title ~~SIMD-XXXX: Vote Account v4~~ SIMD-0185: Vote Account v4 Oct 17, 2024

jstarry mentioned this pull request Oct 17, 2024

SIMD-0123: Block Revenue Distribution #123

Merged

buffalu reviewed Oct 18, 2024

View reviewed changes

Comment thread proposals/0185-vote-account-v4.md Outdated

AshwinSekar reviewed Oct 18, 2024

View reviewed changes

github-actions Bot mentioned this pull request Oct 21, 2024

Upstream Updates - Mon Oct 21 00:14:35 UTC 2024 smartcontractkit/chainlink-solana#898

Closed

segfaultdoc reviewed Nov 5, 2024

View reviewed changes

remove tips, use bps

ddc80c2

simplify simd

68a231b

jstarry marked this pull request as ready for review January 24, 2025 09:34

lidatong approved these changes Feb 4, 2025

View reviewed changes

jstarry added 2 commits February 18, 2025 18:04

Revert "simplify simd"

2651ce6

This reverts commit 68a231b.

Remove rewards pool bump seed

08d0088

AshwinSekar approved these changes Feb 25, 2025

View reviewed changes

add more implementation detail

b66ad3a

jstarry added 2 commits April 16, 2025 05:26

clarify init account checks

a106539

clean up update comm ix section

0b2c2bd

jstarry commented Apr 16, 2025

View reviewed changes

buffalojoec reviewed Apr 30, 2025

View reviewed changes

feedback

de1e88f

buffalojoec reviewed May 5, 2025

View reviewed changes

return error if not rent exempt

09d8017

buffalojoec previously approved these changes May 5, 2025

View reviewed changes

lidatong approved these changes May 5, 2025

View reviewed changes

rename block rewards to block revenue

3b9bad3

jstarry dismissed buffalojoec’s stale review via 3b9bad3 May 6, 2025 06:50

buffalojoec previously approved these changes May 13, 2025

View reviewed changes

jstarry dismissed buffalojoec’s stale review via b68e95e May 23, 2025 20:43

jstarry added 2 commits May 23, 2025 20:43

cleanup and commission calculation note

b68e95e

add alpenglow bls pubkey

07edc01

wen-coding reviewed Jul 8, 2025

View reviewed changes

use appropriate default collector values

34d0b02

AshwinSekar reviewed Jul 8, 2025

View reviewed changes

buffalojoec reviewed Jul 9, 2025

View reviewed changes

Comment thread proposals/0185-vote-account-v4.md Outdated

Comment thread proposals/0185-vote-account-v4.md Outdated

Comment thread proposals/0185-vote-account-v4.md

feedback

cb99089

jstarry requested review from buffalojoec and lidatong July 11, 2025 02:05

buffalojoec approved these changes Jul 11, 2025

View reviewed changes

lidatong approved these changes Jul 11, 2025

View reviewed changes

Benhawkins18 merged commit c06f7d2 into solana-foundation:main Jul 11, 2025
2 checks passed

github-actions Bot mentioned this pull request Jul 14, 2025

Upstream Updates - Mon Jul 14 00:18:27 UTC 2025 smartcontractkit/chainlink-solana#1298

Closed

jstarry deleted the vote-account-v4 branch July 14, 2025 01:59

wen-coding mentioned this pull request Aug 1, 2025

SIMD-0326: Alpenglow #326

Merged

This was referenced Sep 24, 2025

vote-program: outfit to fully support vote state v4 anza-xyz/agave#8163

Closed

vote-program: handler: add v4 variant anza-xyz/agave#8178

Merged

github-actions Bot locked as resolved and limited conversation to collaborators Jan 19, 2026

Conversation

jstarry commented Oct 17, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diman-io commented Oct 23, 2024

Uh oh!

jstarry commented Oct 23, 2024

Uh oh!

diman-io commented Oct 23, 2024

Uh oh!

segfaultdoc left a comment

Choose a reason for hiding this comment

Uh oh!

jstarry commented Nov 5, 2024

Uh oh!

jstarry commented Jan 24, 2025

Uh oh!

AshwinSekar commented Jan 24, 2025

Uh oh!

lidatong left a comment

Choose a reason for hiding this comment

Uh oh!

jstarry commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AshwinSekar left a comment

Choose a reason for hiding this comment

Uh oh!

michaelh-laine commented Mar 4, 2025

Uh oh!

jstarry commented Mar 5, 2025

Uh oh!

jstarry commented Apr 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

buffalojoec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

buffalojoec left a comment

Choose a reason for hiding this comment

Uh oh!

buffalojoec commented May 5, 2025

Uh oh!

jstarry commented May 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AshwinSekar left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jstarry commented Jul 9, 2025

Uh oh!

buffalojoec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

buffalojoec left a comment

jstarry commented Feb 17, 2025 •

edited

Loading

AshwinSekar left a comment •

edited

Loading