From 0962b80fca2a6b454ca4aaed154b17f4c7b04bff Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 5 Jan 2023 09:36:19 -0800 Subject: [PATCH] Stream the executed transaction count in the block notification (#29272) Problem The plugins need to know when all transactions for a block have been all notified to serve getBlock request correctly. As block and transaction notifications are sent asynchronously to each other it will be difficult. Summary of Changes Include the executed transaction count in block notification which can be used to check if all transactions have been notified. --- core/src/replay_stage.rs | 1 + .../src/geyser_plugin_interface.rs | 12 ++ .../src/block_metadata_notifier.rs | 21 +++- .../src/block_metadata_notifier_interface.rs | 1 + runtime/src/bank.rs | 104 ++++++++++++++++++ 5 files changed, 133 insertions(+), 6 deletions(-) diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 9e6b11ab7c51e4..efa947003996cd 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -2644,6 +2644,7 @@ impl ReplayStage { &bank.rewards, Some(bank.clock().unix_timestamp), Some(bank.block_height()), + bank.executed_transaction_count(), ) } bank_complete_time.stop(); diff --git a/geyser-plugin-interface/src/geyser_plugin_interface.rs b/geyser-plugin-interface/src/geyser_plugin_interface.rs index 4b49701b7f4ffa..ab32c159a74702 100644 --- a/geyser-plugin-interface/src/geyser_plugin_interface.rs +++ b/geyser-plugin-interface/src/geyser_plugin_interface.rs @@ -133,8 +133,20 @@ pub struct ReplicaBlockInfo<'a> { pub block_height: Option, } +/// Extending ReplicaBlockInfo by sending the transaction_entries_count. +#[derive(Clone, Debug)] +pub struct ReplicaBlockInfoV2<'a> { + pub slot: u64, + pub blockhash: &'a str, + pub rewards: &'a [Reward], + pub block_time: Option, + pub block_height: Option, + pub executed_transaction_count: u64, +} + pub enum ReplicaBlockInfoVersions<'a> { V0_0_1(&'a ReplicaBlockInfo<'a>), + V0_0_2(&'a ReplicaBlockInfoV2<'a>), } /// Errors returned by plugin calls diff --git a/geyser-plugin-manager/src/block_metadata_notifier.rs b/geyser-plugin-manager/src/block_metadata_notifier.rs index 3e9a85a9ea6df3..8914336a126a20 100644 --- a/geyser-plugin-manager/src/block_metadata_notifier.rs +++ b/geyser-plugin-manager/src/block_metadata_notifier.rs @@ -5,7 +5,7 @@ use { }, log::*, solana_geyser_plugin_interface::geyser_plugin_interface::{ - ReplicaBlockInfo, ReplicaBlockInfoVersions, + ReplicaBlockInfoV2, ReplicaBlockInfoVersions, }, solana_measure::measure::Measure, solana_metrics::*, @@ -28,6 +28,7 @@ impl BlockMetadataNotifier for BlockMetadataNotifierImpl { rewards: &RwLock>, block_time: Option, block_height: Option, + executed_transaction_count: u64, ) { let mut plugin_manager = self.plugin_manager.write().unwrap(); if plugin_manager.plugins.is_empty() { @@ -37,9 +38,15 @@ impl BlockMetadataNotifier for BlockMetadataNotifierImpl { for plugin in plugin_manager.plugins.iter_mut() { let mut measure = Measure::start("geyser-plugin-update-slot"); - let block_info = - Self::build_replica_block_info(slot, blockhash, &rewards, block_time, block_height); - let block_info = ReplicaBlockInfoVersions::V0_0_1(&block_info); + let block_info = Self::build_replica_block_info( + slot, + blockhash, + &rewards, + block_time, + block_height, + executed_transaction_count, + ); + let block_info = ReplicaBlockInfoVersions::V0_0_2(&block_info); match plugin.notify_block_metadata(block_info) { Err(err) => { error!( @@ -89,13 +96,15 @@ impl BlockMetadataNotifierImpl { rewards: &'a [Reward], block_time: Option, block_height: Option, - ) -> ReplicaBlockInfo<'a> { - ReplicaBlockInfo { + executed_transaction_count: u64, + ) -> ReplicaBlockInfoV2<'a> { + ReplicaBlockInfoV2 { slot, blockhash, rewards, block_time, block_height, + executed_transaction_count, } } diff --git a/geyser-plugin-manager/src/block_metadata_notifier_interface.rs b/geyser-plugin-manager/src/block_metadata_notifier_interface.rs index 6d4b9f6ad2f569..3e698498654e5e 100644 --- a/geyser-plugin-manager/src/block_metadata_notifier_interface.rs +++ b/geyser-plugin-manager/src/block_metadata_notifier_interface.rs @@ -14,6 +14,7 @@ pub trait BlockMetadataNotifier { rewards: &RwLock>, block_time: Option, block_height: Option, + executed_transaction_count: u64, ); } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 275a28fa7845d6..cfe3cda5c5e2ef 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -6615,6 +6615,7 @@ impl Bank { ) } + /// Return the accumulated executed transaction count pub fn transaction_count(&self) -> u64 { self.transaction_count.load(Relaxed) } @@ -6623,6 +6624,20 @@ impl Bank { self.non_vote_transaction_count_since_restart.load(Relaxed) } + /// Return the transaction count executed only in this bank + pub fn executed_transaction_count(&self) -> u64 { + let mut executed_transaction_count = self + .transaction_count() + .saturating_sub(self.parent().map_or(0, |parent| parent.transaction_count())); + if !self.bank_transaction_count_fix_enabled() { + // When the feature bank_tranaction_count_fix is enabled, transaction_count() excludes + // the transactions which were executed but landed in error, we add it here. + executed_transaction_count = + executed_transaction_count.saturating_add(self.transaction_error_count()); + } + executed_transaction_count + } + pub fn transaction_error_count(&self) -> u64 { self.transaction_error_count.load(Relaxed) } @@ -10646,6 +10661,95 @@ pub(crate) mod tests { assert_eq!(bank.get_balance(&pubkey), amount); } + #[test] + fn test_executed_transaction_count_pre_bank_transaction_count_fix() { + let mint_amount = sol_to_lamports(1.); + let (genesis_config, mint_keypair) = create_genesis_config(mint_amount); + let mut bank = Bank::new_for_tests(&genesis_config); + bank.deactivate_feature(&feature_set::bank_transaction_count_fix::id()); + let pubkey = solana_sdk::pubkey::new_rand(); + let amount = genesis_config.rent.minimum_balance(0); + bank.transfer(amount, &mint_keypair, &pubkey).unwrap(); + assert_eq!( + bank.transfer((mint_amount - amount) + 1, &mint_keypair, &pubkey), + Err(TransactionError::InstructionError( + 0, + SystemError::ResultWithNegativeLamports.into(), + )) + ); + + // Without bank_transaction_count_fix, transaction_count should include only the successful + // transactions, but executed_transaction_count include all always + assert_eq!(bank.transaction_count(), 1); + assert_eq!(bank.executed_transaction_count(), 2); + assert_eq!(bank.transaction_error_count(), 1); + + let bank = Arc::new(bank); + let bank2 = Bank::new_from_parent( + &bank, + &Pubkey::default(), + genesis_config.epoch_schedule.first_normal_slot, + ); + + assert_eq!( + bank2.transfer((mint_amount - amount) + 2, &mint_keypair, &pubkey), + Err(TransactionError::InstructionError( + 0, + SystemError::ResultWithNegativeLamports.into(), + )) + ); + + // The transaction_count inherited from parent bank is still 1 as it does + // not include the failed ones! + assert_eq!(bank2.transaction_count(), 1); + assert_eq!(bank2.executed_transaction_count(), 1); + assert_eq!(bank2.transaction_error_count(), 1); + } + + #[test] + fn test_executed_transaction_count_post_bank_transaction_count_fix() { + let mint_amount = sol_to_lamports(1.); + let (genesis_config, mint_keypair) = create_genesis_config(mint_amount); + let mut bank = Bank::new_for_tests(&genesis_config); + bank.activate_feature(&feature_set::bank_transaction_count_fix::id()); + let pubkey = solana_sdk::pubkey::new_rand(); + let amount = genesis_config.rent.minimum_balance(0); + bank.transfer(amount, &mint_keypair, &pubkey).unwrap(); + assert_eq!( + bank.transfer((mint_amount - amount) + 1, &mint_keypair, &pubkey), + Err(TransactionError::InstructionError( + 0, + SystemError::ResultWithNegativeLamports.into(), + )) + ); + + // With bank_transaction_count_fix, transaction_count should include both the successful and + // failed transactions. + assert_eq!(bank.transaction_count(), 2); + assert_eq!(bank.executed_transaction_count(), 2); + assert_eq!(bank.transaction_error_count(), 1); + + let bank = Arc::new(bank); + let bank2 = Bank::new_from_parent( + &bank, + &Pubkey::default(), + genesis_config.epoch_schedule.first_normal_slot, + ); + + assert_eq!( + bank2.transfer((mint_amount - amount) + 2, &mint_keypair, &pubkey), + Err(TransactionError::InstructionError( + 0, + SystemError::ResultWithNegativeLamports.into(), + )) + ); + + // The transaction_count inherited from parent bank is 3: 2 from the parent bank and 1 at this bank2 + assert_eq!(bank2.transaction_count(), 3); + assert_eq!(bank2.executed_transaction_count(), 1); + assert_eq!(bank2.transaction_error_count(), 1); + } + #[test] fn test_transfer_to_newb() { solana_logger::setup();