From c54e06355f393096987d5f26d2fb05f72da42d3b Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Thu, 19 May 2022 18:28:46 -0700 Subject: [PATCH] voteSubscribe pubsub notification now includes the vote transaction signature (#25291) --- client/src/rpc_response.rs | 1 + core/src/cluster_info_vote_listener.rs | 12 +++++++++--- docs/src/developing/clients/jsonrpc-api.md | 1 + gossip/src/cluster_info.rs | 2 +- gossip/src/crds_value.rs | 2 +- ledger/src/blockstore_processor.rs | 2 +- local-cluster/tests/local_cluster_slow_1.rs | 2 +- rpc/src/rpc_pubsub.rs | 8 ++++++-- rpc/src/rpc_subscriptions.rs | 9 +++++---- runtime/src/vote_parser.rs | 12 ++++++++---- 10 files changed, 34 insertions(+), 17 deletions(-) diff --git a/client/src/rpc_response.rs b/client/src/rpc_response.rs index cba3eed997..3b7c21be5e 100644 --- a/client/src/rpc_response.rs +++ b/client/src/rpc_response.rs @@ -346,6 +346,7 @@ pub struct RpcVote { pub slots: Vec, pub hash: String, pub timestamp: Option, + pub signature: String, } #[derive(Serialize, Deserialize, Clone, Debug)] diff --git a/core/src/cluster_info_vote_listener.rs b/core/src/cluster_info_vote_listener.rs index 128964768b..a52efadcab 100644 --- a/core/src/cluster_info_vote_listener.rs +++ b/core/src/cluster_info_vote_listener.rs @@ -307,7 +307,7 @@ impl ClusterInfoVoteListener { !packet_batch.packets[0].meta.discard() }) .filter_map(|(tx, packet_batch)| { - let (vote_account_key, vote, _) = vote_parser::parse_vote_transaction(&tx)?; + let (vote_account_key, vote, ..) = vote_parser::parse_vote_transaction(&tx)?; let slot = vote.last_voted_slot()?; let epoch = epoch_schedule.get_epoch(slot); let authorized_voter = root_bank @@ -570,6 +570,7 @@ impl ClusterInfoVoteListener { fn track_new_votes_and_notify_confirmations( vote: VoteTransaction, vote_pubkey: &Pubkey, + vote_transaction_signature: Signature, vote_tracker: &VoteTracker, root_bank: &Bank, subscriptions: &RpcSubscriptions, @@ -678,7 +679,7 @@ impl ClusterInfoVoteListener { } if is_new_vote { - subscriptions.notify_vote(*vote_pubkey, vote); + subscriptions.notify_vote(*vote_pubkey, vote, vote_transaction_signature); let _ = verified_vote_sender.send((*vote_pubkey, vote_slots)); } } @@ -703,10 +704,11 @@ impl ClusterInfoVoteListener { .filter_map(vote_parser::parse_vote_transaction) .zip(repeat(/*is_gossip:*/ true)) .chain(replayed_votes.into_iter().zip(repeat(/*is_gossip:*/ false))); - for ((vote_pubkey, vote, _), is_gossip) in votes { + for ((vote_pubkey, vote, _switch_proof, signature), is_gossip) in votes { Self::track_new_votes_and_notify_confirmations( vote, &vote_pubkey, + signature, vote_tracker, root_bank, subscriptions, @@ -1018,6 +1020,7 @@ mod tests { vote_keypair.pubkey(), VoteTransaction::from(replay_vote.clone()), switch_proof_hash, + Signature::default(), )) .unwrap(); } @@ -1306,6 +1309,7 @@ mod tests { vote_keypair.pubkey(), VoteTransaction::from(Vote::new(vec![vote_slot], Hash::default())), switch_proof_hash, + Signature::default(), )) .unwrap(); } @@ -1401,6 +1405,7 @@ mod tests { validator0_keypairs.vote_keypair.pubkey(), VoteTransaction::from(Vote::new(vec![voted_slot], Hash::default())), None, + Signature::default(), )], &bank, &subscriptions, @@ -1446,6 +1451,7 @@ mod tests { validator_keypairs[1].vote_keypair.pubkey(), VoteTransaction::from(Vote::new(vec![first_slot_in_new_epoch], Hash::default())), None, + Signature::default(), )], &new_root_bank, &subscriptions, diff --git a/docs/src/developing/clients/jsonrpc-api.md b/docs/src/developing/clients/jsonrpc-api.md index cddf45034c..c5ea2eaf85 100644 --- a/docs/src/developing/clients/jsonrpc-api.md +++ b/docs/src/developing/clients/jsonrpc-api.md @@ -4573,6 +4573,7 @@ The notification will be an object with the following fields: - `hash: ` - The vote hash - `slots: ` - The slots covered by the vote, as an array of u64 integers - `timestamp: ` - The timestamp of the vote +- `signature: ` - The signature of the transaction that contained this vote ```json { diff --git a/gossip/src/cluster_info.rs b/gossip/src/cluster_info.rs index d4c44582d6..c90e1cdf3d 100644 --- a/gossip/src/cluster_info.rs +++ b/gossip/src/cluster_info.rs @@ -1037,7 +1037,7 @@ impl ClusterInfo { }; let vote_index = vote_index.unwrap_or(num_crds_votes); if (vote_index as usize) >= MAX_LOCKOUT_HISTORY { - let (_, vote, hash) = vote_parser::parse_vote_transaction(&vote).unwrap(); + let (_, vote, hash, _) = vote_parser::parse_vote_transaction(&vote).unwrap(); panic!( "invalid vote index: {}, switch: {}, vote slots: {:?}, tower: {:?}", vote_index, diff --git a/gossip/src/crds_value.rs b/gossip/src/crds_value.rs index 57b6b9ab79..b1da5d4c58 100644 --- a/gossip/src/crds_value.rs +++ b/gossip/src/crds_value.rs @@ -307,7 +307,7 @@ impl Sanitize for Vote { impl Vote { // Returns None if cannot parse transaction into a vote. pub fn new(from: Pubkey, transaction: Transaction, wallclock: u64) -> Option { - vote_parser::parse_vote_transaction(&transaction).map(|(_, vote, _)| Self { + vote_parser::parse_vote_transaction(&transaction).map(|(_, vote, ..)| Self { from, transaction, wallclock, diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index bcab738547..8812fff24d 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -3585,7 +3585,7 @@ pub mod tests { process_entries_for_tests(&bank1, vec![entry], true, None, Some(&replay_vote_sender)); let successes: BTreeSet = replay_vote_receiver .try_iter() - .map(|(vote_pubkey, _, _)| vote_pubkey) + .map(|(vote_pubkey, ..)| vote_pubkey) .collect(); assert_eq!(successes, expected_successful_voter_pubkeys); } diff --git a/local-cluster/tests/local_cluster_slow_1.rs b/local-cluster/tests/local_cluster_slow_1.rs index 095e87aeeb..dab4f7807e 100644 --- a/local-cluster/tests/local_cluster_slow_1.rs +++ b/local-cluster/tests/local_cluster_slow_1.rs @@ -490,7 +490,7 @@ fn test_duplicate_shreds_broadcast_leader() { // Filter out votes not from the bad leader if label.pubkey() == bad_leader_id { let vote = vote_parser::parse_vote_transaction(&leader_vote_tx) - .map(|(_, vote, _)| vote) + .map(|(_, vote, ..)| vote) .unwrap(); // Filter out empty votes if !vote.is_empty() { diff --git a/rpc/src/rpc_pubsub.rs b/rpc/src/rpc_pubsub.rs index c7b76975c9..0b14f641cb 100644 --- a/rpc/src/rpc_pubsub.rs +++ b/rpc/src/rpc_pubsub.rs @@ -1340,12 +1340,16 @@ mod tests { hash: Hash::default(), timestamp: None, }; - subscriptions.notify_vote(Pubkey::default(), VoteTransaction::from(vote)); + subscriptions.notify_vote( + Pubkey::default(), + VoteTransaction::from(vote), + Signature::default(), + ); let response = receiver.recv(); assert_eq!( response, - r#"{"jsonrpc":"2.0","method":"voteNotification","params":{"result":{"votePubkey":"11111111111111111111111111111111","slots":[1,2],"hash":"11111111111111111111111111111111","timestamp":null},"subscription":0}}"# + r#"{"jsonrpc":"2.0","method":"voteNotification","params":{"result":{"votePubkey":"11111111111111111111111111111111","slots":[1,2],"hash":"11111111111111111111111111111111","timestamp":null,"signature":"1111111111111111111111111111111111111111111111111111111111111111"},"subscription":0}}"# ); } diff --git a/rpc/src/rpc_subscriptions.rs b/rpc/src/rpc_subscriptions.rs index f79909b646..9abac30f5a 100644 --- a/rpc/src/rpc_subscriptions.rs +++ b/rpc/src/rpc_subscriptions.rs @@ -93,7 +93,7 @@ impl From for TimestampedNotificationEntry { pub enum NotificationEntry { Slot(SlotInfo), SlotUpdate(SlotUpdate), - Vote((Pubkey, VoteTransaction)), + Vote((Pubkey, VoteTransaction, Signature)), Root(Slot), Bank(CommitmentSlots), Gossip(Slot), @@ -728,8 +728,8 @@ impl RpcSubscriptions { self.enqueue_notification(NotificationEntry::SignaturesReceived(slot_signatures)); } - pub fn notify_vote(&self, vote_pubkey: Pubkey, vote: VoteTransaction) { - self.enqueue_notification(NotificationEntry::Vote((vote_pubkey, vote))); + pub fn notify_vote(&self, vote_pubkey: Pubkey, vote: VoteTransaction, signature: Signature) { + self.enqueue_notification(NotificationEntry::Vote((vote_pubkey, vote, signature))); } pub fn notify_roots(&self, mut rooted_slots: Vec) { @@ -814,7 +814,7 @@ impl RpcSubscriptions { // These notifications are only triggered by votes observed on gossip, // unlike `NotificationEntry::Gossip`, which also accounts for slots seen // in VoteState's from bank states built in ReplayStage. - NotificationEntry::Vote((vote_pubkey, ref vote_info)) => { + NotificationEntry::Vote((vote_pubkey, ref vote_info, signature)) => { if let Some(sub) = subscriptions .node_progress_watchers() .get(&SubscriptionParams::Vote) @@ -824,6 +824,7 @@ impl RpcSubscriptions { slots: vote_info.slots(), hash: bs58::encode(vote_info.hash()).into_string(), timestamp: vote_info.timestamp(), + signature: signature.to_string(), }; debug!("vote notify: {:?}", vote_info); inc_new_counter_info!("rpc-subscription-notify-vote", 1); diff --git a/runtime/src/vote_parser.rs b/runtime/src/vote_parser.rs index 2587e154d4..c7ae7e246f 100644 --- a/runtime/src/vote_parser.rs +++ b/runtime/src/vote_parser.rs @@ -4,12 +4,13 @@ use { hash::Hash, program_utils::limited_deserialize, pubkey::Pubkey, + signature::Signature, transaction::{SanitizedTransaction, Transaction}, }, solana_vote_program::vote_instruction::VoteInstruction, }; -pub type ParsedVote = (Pubkey, VoteTransaction, Option); +pub type ParsedVote = (Pubkey, VoteTransaction, Option, Signature); // Used for filtering out votes from the transaction log collector pub(crate) fn is_simple_vote_transaction(transaction: &SanitizedTransaction) -> bool { @@ -46,7 +47,8 @@ pub fn parse_sanitized_vote_transaction(tx: &SanitizedTransaction) -> Option Option { let first_account = usize::from(*first_instruction.accounts.first()?); let key = message.account_keys.get(first_account)?; let (vote, switch_proof_hash) = parse_vote_instruction_data(&first_instruction.data)?; - Some((*key, vote, switch_proof_hash)) + let signature = tx.signatures.get(0).cloned().unwrap_or_default(); + Some((*key, vote, switch_proof_hash, signature)) } fn parse_vote_instruction_data( @@ -113,10 +116,11 @@ mod test { &auth_voter_keypair, input_hash, ); - let (key, vote, hash) = parse_vote_transaction(&vote_tx).unwrap(); + let (key, vote, hash, signature) = parse_vote_transaction(&vote_tx).unwrap(); assert_eq!(hash, input_hash); assert_eq!(vote, VoteTransaction::from(Vote::new(vec![42], bank_hash))); assert_eq!(key, vote_keypair.pubkey()); + assert_eq!(signature, vote_tx.signatures[0]); // Test bad program id fails let mut vote_ix = vote_instruction::vote(