Switching to VoteStateV4. by wen-coding · Pull Request #456 · anza-xyz/alpenglow

wen-coding · 2025-09-18T05:19:53Z

Adding create_v4_account_with_authorized which is actually submitted in Agave #8103, we are just replicating the work here because we need it
Use create_v4_account_with_authorized in genesis_utils and a few other tests
As a result we do need to deserialize v4 account data instead of AlpenglowVoteState deserialize or VoteStateV3 deserialize in a few tests
Also replaced new_random_from_alpenglow_vote_state with new_random_alpenglow() which creates v4 account
Replace AlpenglowVoteState with normal vote_state_view in voting_utils.rs, we don't use AlpenglowVoteState any more
Add bls_pubkey_to_compressed_bytes in genesis_util to simplify bls pubkey conversion

bw-solana

Can we add a PR description?

Also, it would be nice to create multiple, smaller PRs for things that can be separated. For example dropping solana_bls_signatures::signature::Signature as BlsSignature and just using Signature directly is unrelated cosmetic change and could be forked out

bw-solana · 2025-09-22T15:52:28Z

+pub(crate) struct BLSKeypairWithCompressedPubkey {
+    pub keypair: Arc<BLSKeypair>,
+    pub compressed_pubkey: [u8; BLS_PUBLIC_KEY_COMPRESSED_SIZE],
+}


feels a little funky having this since the compressed_pubkey can be derived from the keypair. Should we have separate APIs and let callers decide how they want to access and mix/match things?

Moving this to another PR later.

wen-coding · 2025-09-22T16:14:17Z

Can we add a PR description?

Also, it would be nice to create multiple, smaller PRs for things that can be separated. For example dropping solana_bls_signatures::signature::Signature as BlsSignature and just using Signature directly is unrelated cosmetic change and could be forked out

That's fair. We don't strictly need anything after a24e3ca, I was just bored and started fixing small inefficiencies. I'll remove that from this PR.

ksn6 · 2025-09-22T21:48:08Z

-            vote_state.set_epoch_credits(epoch_credits);
-            vote_state.serialize_into(vote_account.data_as_mut_slice());
+            let current_credits = vote_state.epoch_credits.last_mut().unwrap();
+            current_credits.1 += 16;


Rather than doing current_credits.1, could we do let (x, y, ...) = vote_state.epoch_credits.last_mut().unwrap()?

ksn6 · 2025-09-22T21:50:53Z

-                    account
-                        .bls_pubkey()
-                        .map(|bls_pubkey| (*pubkey, *bls_pubkey, *stake))
+                    account.vote_state_view().and_then(|vote_state| {


This is a bit hard to read - could we unnest / clean this a bit?

ksn6 · 2025-09-22T21:53:17Z

    }

-    pub fn bls_pubkey(&self) -> Option<&BLSPubkey> {
+    pub fn bls_pubkey(&self) -> Option<BLSPubkey> {


This doesn't seem right - shouldn't invoking bls_pubkey() return None for TowerBFT and Some(...) for Alpenglow?

This part is a mess and will go away in the next PR, we will no longer have TowerBFT or Alpenglow option.

ksn6 · 2025-09-22T21:54:48Z

    let vote_account_pubkey = context.vote_account_pubkey;
    let authorized_voter_keypair;
-    let bls_pubkey_in_vote_account;
+    let bls_pubkey_in_vote_account: BLSPubkey;


I don't think we need to forward declare this variable - could we move it to the site of initialization and remove the annotated type, unless really needed?

This is mainly done to release context.authorized_voter_keypairs read lock asap

carllin · 2025-09-22T21:56:54Z

+            VoteAccountState::TowerBFT(vote_state) => vote_state.bls_pubkey_compressed().map(|b| {
+                let bls_pubkey_compressed =
+                    bincode::deserialize::<BLSPubkeyCompressed>(&b).unwrap();
+                BLSPubkeyCompressed::try_as_affine(&bls_pubkey_compressed).unwrap()


@samkim-crypto just double checking this is the right transformation

It's pretty ugly, but should be correct because all local-cluster tests are passing :) We've turned on BLS verification now.

So VoteStateV4 only says "give me a 48 byte buffer as bls pubkey", it doesn't care what's in it or whether it decodes properly for now. This PR (and anza-xyz/agave#8122 on Agave side) decides we serialize the compressed pubkey into the 48 byte buffer.

Yes, I confirm that this is the right transformation.

carllin · 2025-09-22T21:58:48Z

-            AlpenglowVoteState::create_account_with_authorized(
-                &vote_pubkey,
+            vote_state::create_v4_account_with_authorized(
+                &solana_pubkey::new_rand(),


out of curiosity why does this need to change?

AlpenglowVoteState::create_account_with_authorized will create a vote state belonging to our new vote program, we are ditching our new vote program everywhere and replacing it with VoteStateV4 accounts.

AshwinSekar

Thanks for doing this! Can we remove the vote state related stuff from votor-messages now?

wen-coding · 2025-09-23T15:56:11Z

Thanks for doing this! Can we remove the vote state related stuff from votor-messages now?

Not yet, we need to remove the vote-account hack first. I'll get to that today.

Try switching to VoteStateV4.

d700f61

wen-coding marked this pull request as draft September 18, 2025 05:19

wen-coding added 3 commits September 18, 2025 00:20

Merge branch 'master' into try_out_vote_state_v4

9712c53

Fix tests.

def3e62

Fix how voting_utils read vote account.

7a6f159

wen-coding marked this pull request as ready for review September 18, 2025 14:11

wen-coding requested review from AshwinSekar, bw-solana, carllin and ksn6 September 18, 2025 14:11

Remove all uses of AlpenglowVoteState::create_account.

76ff411

wen-coding marked this pull request as draft September 19, 2025 02:28

wen-coding added 3 commits September 19, 2025 12:58

Fix test_bank_update_vote_stake_rewards.

2aa2c3c

Use new create_v4_account_with_authorized API.

f3fe927

Fix stakes tests.

a24e3ca

wen-coding marked this pull request as ready for review September 19, 2025 21:20

wen-coding changed the title ~~Try switching to VoteStateV4.~~ Switching to VoteStateV4. Sep 19, 2025

bw-solana reviewed Sep 22, 2025

View reviewed changes

wen-coding force-pushed the try_out_vote_state_v4 branch from dbdc7ed to a24e3ca Compare September 22, 2025 16:19

wen-coding added 2 commits September 22, 2025 10:20

Add bls_pubkey_to_compressed_bytes and use it where needed.

08a6576

Remove unnecessary dependencies.

308b7bc

wen-coding requested a review from bw-solana September 22, 2025 18:00

Move more things to bls_pubkey_to_compressed_bytes.

cb61167

ksn6 reviewed Sep 22, 2025

View reviewed changes

carllin previously approved these changes Sep 22, 2025

View reviewed changes

Address review comments.

974dadd

wen-coding dismissed carllin’s stale review via 974dadd September 22, 2025 22:11

ksn6 previously approved these changes Sep 22, 2025

View reviewed changes

Sync with upstream #8122.

3926286

wen-coding dismissed ksn6’s stale review via 3926286 September 22, 2025 23:51

Merge branch 'master' into try_out_vote_state_v4

a54dcbe

samkim-crypto approved these changes Sep 23, 2025

View reviewed changes

AshwinSekar approved these changes Sep 23, 2025

View reviewed changes

wen-coding merged commit a240b70 into anza-xyz:master Sep 23, 2025
7 checks passed

wen-coding deleted the try_out_vote_state_v4 branch September 23, 2025 15:56

Conversation

wen-coding commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bw-solana left a comment

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!

wen-coding commented Sep 22, 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

wen-coding Sep 22, 2025 • 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!

AshwinSekar left a comment

Choose a reason for hiding this comment

Uh oh!

wen-coding commented Sep 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wen-coding commented Sep 18, 2025 •

edited

Loading

wen-coding Sep 22, 2025 •

edited

Loading