Parse out alpenglow votes in vote_parser.rs #95

carllin · 2025-03-17T07:32:25Z

Problem

Gossip doesn't properly parse out alpenglow votes to pass to downstream listeners in repair and replay
Push vote services doesn't properly parse alpenglow votes

Summary of Changes

Parse out votes

Fixes #

carllin · 2025-03-17T07:33:29Z

Hmm for some reason, votes aren't being received by the second node over gossip in #89, even though i see the leader pushing them into gossip...ick

wen-coding · 2025-03-17T16:52:24Z

core/src/cluster_info_vote_listener.rs

+                    if vote.is_alpenglow_vote() {
+                        return None;
+                    }
+                }


nit: is this equivalent to
if (slot >= first_alpenglow_slot) ^ vote.is_alpenglow_vote() {
return None;
}
?

yeah it's equivalent, changed

wen-coding · 2025-03-17T16:58:14Z

vote/src/vote_parser.rs

+            ParsedVoteTransaction::Alpenglow(tx) => match tx {
+                AlpenglowVote::Notarize(vote) => Some((vote.slot(), *vote.replayed_bank_hash())),
+                AlpenglowVote::Finalize(vote) => Some((vote.slot(), *vote.replayed_bank_hash())),
+                AlpenglowVote::Skip(_vote) => None,


We can give Skip Some((vote.end_slot(), Hash::default())) to make the vote sending work? I don't think we care that much about the start_slot changing

If we do that, then I think this function can return (Slot, Hash), since it will always return Some(...)

I don't think you want to send skip votes to repair for fork choice weighting, so that logic should probably be handled separately

Okay, let's add a comment here saying why Skip should return None then.

wen-coding · 2025-03-17T17:00:42Z

vote/src/vote_parser.rs

+
+    pub fn is_full_tower_vote(&self) -> bool {
+        match self {
+            ParsedVoteTransaction::Tower(tx) => tx.is_full_tower_vote(),


Would we still have anything other than full tower vote in TowerBFT side when Alpenglow launches?

no I think we should be ignoring all alpenglow votes for slots < the first alpenglow slot, hence this check https://github.com/anza-xyz/alpenglow/pulls#discussion_r1999224815

wen-coding · 2025-03-17T17:02:15Z

vote/src/vote_parser.rs

-    Some((*key, vote, switch_proof_hash, signature))
+    Some((
+        *key,
+        ParsedVoteTransaction::Tower(vote),


Hmm why don't we need this for Alpenglow?

what's "this"? this is the equivalent for alpenglow:

alpenglow/vote/src/vote_parser.rs

Line 123 in 1558360

ParsedVoteTransaction::Alpenglow(alpenglow_vote),

wen-coding · 2025-03-17T17:04:04Z

vote/src/vote_parser.rs

+    ))
+}
+
+pub fn parse_alpenglow_vote_transaction(tx: &Transaction) -> Option<ParsedVote> {


nit: can this guy and parse_sanitized_alpenglow_vote_transaction share code?

could probably pass some closures to handle custom parsing, but code overlap is small enough where it doesn't seem worth it

wen-coding · 2025-03-17T17:05:03Z

vote/src/vote_parser.rs


+fn parse_alpenglow_vote_instruction_data(vote_instruction_data: &[u8]) -> Option<AlpenglowVote> {
+    alpenglow_vote::vote::Vote::deserialize_simple_vote(vote_instruction_data).ok()
+}


nit:: add TODO to add test cases.

Also can we use alpenglow_vote::vote::Vote as AlpenglowVote?

wen-coding · 2025-03-17T17:11:32Z

core/src/cluster_info_vote_listener.rs

            vec![(
                validator0_keypairs.vote_keypair.pubkey(),
-                VoteTransaction::from(Vote::new(vec![voted_slot], Hash::default())),
+                ParsedVoteTransaction::Tower(VoteTransaction::from(Vote::new(


nit: add TODO for alpenglow test

yeah going to need to go through and write some detailed tests, might hand off to @ksn6

carllin · 2025-03-17T19:35:15Z

alpenglow/gossip/src/cluster_info.rs

Line 2191 in efc48d5

stats.record_received_packet(packet.deserialize_slice::<Protocol, _>(..))?;

gossip failing to deserialize votes here for some reason

Edit: Figured it out, its the deserialize implementation in CrdsData::Vote, fixed here: 9ee113a

Repair now seems to be working with two local cluster nodes!

AshwinSekar

Post merge LGTM

Just a couple nits

AshwinSekar · 2025-03-24T14:50:01Z

vote/src/vote_parser.rs

    }
 }

+fn parse_alpenglow_vote_instruction_data(vote_instruction_data: &[u8]) -> Option<AlpenglowVote> {


nit: we could also check AlpenglowVote::is_simple_vote if we want to distinguish between accounting instructions here.
Also fine to keep it as is, accounting instructions and invalid packets will both turn into None

AshwinSekar · 2025-03-24T14:51:33Z

vote/src/vote_parser.rs

+
+// TODO: add tests
+pub fn parse_alpenglow_vote_transaction(tx: &Transaction) -> Option<ParsedVote> {
+    // Check first instruction for a vote


Since we are doing this from scratch here, we could formalize the requirement that votes must be a single instruction transaction (fail to parse here and in banking stage).

parse out alpenglow votes

b804f1d

carllin requested review from ksn6 and wen-coding March 17, 2025 07:32

Send votes from cluster_info

1558360

wen-coding reviewed Mar 17, 2025

View reviewed changes

carllin changed the title ~~Parse out alpenglow votes from gossip~~ Parse out alpenglow votes in vote_parser.rs Mar 17, 2025

carllin added 2 commits March 17, 2025 16:02

Fixup crds vote deserialize

9ee113a

PR comments

155ce6a

wen-coding approved these changes Mar 18, 2025

View reviewed changes

carllin merged commit 391639a into anza-xyz:master Mar 18, 2025
7 checks passed

AshwinSekar reviewed Mar 24, 2025

View reviewed changes

bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025

Parse out alpenglow votes in vote_parser.rs (anza-xyz#95)

dcf9555

bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025

Parse out alpenglow votes in vote_parser.rs (anza-xyz#95)

9c1070d

bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025

Parse out alpenglow votes in vote_parser.rs (anza-xyz#95)

c6bcaac

bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025

Parse out alpenglow votes in vote_parser.rs (anza-xyz#95)

3dbee46

bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025

Parse out alpenglow votes in vote_parser.rs (anza-xyz#95)

bc50527

bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025

Parse out alpenglow votes in vote_parser.rs (anza-xyz#95)

feaac11

bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 2, 2025

Parse out alpenglow votes in vote_parser.rs (anza-xyz#95)

e2fa32c

Parse out alpenglow votes in vote_parser.rs #95

Parse out alpenglow votes in vote_parser.rs #95

Uh oh!

Conversation

carllin commented Mar 17, 2025

Problem

Summary of Changes

Uh oh!

carllin commented Mar 17, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

AshwinSekar 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

carllin commented Mar 17, 2025 •

edited

Loading