Rework bundle receiving and add metrics

bootstrap

@@ -3,7 +3,7 @@ bank_hash=$(./target/release/solana-ledger-tool -l config/bootstrap-validator ba
 
 # NOTE: each developer will need to adjust the tip-payment-program-pubkey and tip-distribution-program-pubkey AND
 # their respective declare_id!() fields in each anchor program based on the private key used to deploy the program on your cluster.
-RUST_LOG=INFO \
+RUST_LOG=INFO,solana_core::bundle_stage=DEBUG \
 	NDEBUG=1 ./multinode-demo/bootstrap-validator.sh  \
   --wait-for-supermajority 0 \
   --expected-bank-hash $bank_hash \
@@ -13,8 +13,8 @@ RUST_LOG=INFO \
   --relayer-address http://127.0.0.1:11226 \
   --rpc-pubsub-enable-block-subscription \
   --enable-rpc-transaction-history \
-  --tip-payment-program-pubkey DThZmRNNXh7kvTQW9hXeGoWGPKktK8pgVAyoTLjH7UrT \
-  --tip-distribution-program-pubkey FjrdANjvo76aCYQ4kf9FM1R8aESUcEE6F8V7qyoVUQcM \
+  --tip-payment-program-pubkey AeehMKWUfPDcuU2mnx7jyuHgr6NjyFvSxvjSnudgkQRo \
+  --tip-distribution-program-pubkey APy2ShJsonjhn9ka1oa9vwEVpcspqra2gkFWx4kvpsW8 \
   --commission-bps 0 \
   --shred-receiver-address 127.0.0.1:1002 \
   --allow-private-addr \

core/src/bundle_account_locker.rs

@@ -225,7 +225,10 @@ mod tests {
         },
         solana_ledger::genesis_utils::create_genesis_config,
         solana_perf::packet::PacketBatch,
-        solana_runtime::{bank::Bank, genesis_utils::GenesisConfigInfo},
+        solana_runtime::{
+            bank::Bank, genesis_utils::GenesisConfigInfo,
+            transaction_error_metrics::TransactionErrorMetrics,
+        },
         solana_sdk::{
             packet::Packet, signature::Signer, signer::keypair::Keypair, system_program,
             system_transaction::transfer, transaction::VersionedTransaction,
@@ -270,18 +273,22 @@ mod tests {
             uuid: Uuid::new_v4(),
         };
 
+        let mut transaction_errors = TransactionErrorMetrics::default();
+
         let sanitized_bundle0 = get_sanitized_bundle(
             &packet_bundle0,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors,
         )
         .expect("sanitize bundle 0");
         let sanitized_bundle1 = get_sanitized_bundle(
             &packet_bundle1,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors,
         )
         .expect("sanitize bundle 1");
 

core/src/bundle_sanitizer.rs

@@ -1,7 +1,7 @@
-use crate::{packet_bundle::PacketBundle, unprocessed_packet_batches::ImmutableDeserializedPacket};
 ///! Turns packets into SanitizedTransactions and ensure they pass sanity checks
 use {
     crate::unprocessed_packet_batches::deserialize_packets,
+    crate::{packet_bundle::PacketBundle, unprocessed_packet_batches::ImmutableDeserializedPacket},
     solana_perf::sigverify::verify_packet,
     solana_runtime::{bank::Bank, transaction_error_metrics::TransactionErrorMetrics},
     solana_sdk::{
@@ -18,25 +18,24 @@ use {
         sync::Arc,
     },
     thiserror::Error,
-    uuid::Uuid,
 };
 
 pub const MAX_PACKETS_PER_BUNDLE: usize = 5;
 
 #[derive(Error, Debug, PartialEq, Eq, Clone)]
 pub enum BundleSanitizerError {
-    #[error("Bundle contains a transaction that failed to serialize: {0}")]
-    FailedToSerializeTransaction(Uuid),
-    #[error("Bundle contains a duplicate transaction: {0}")]
-    DuplicateTransaction(Uuid),
-    #[error("Bundle failed check results: {0}")]
-    FailedCheckResults(Uuid),
-    #[error("Bundle packet batch failed pre-check: {0}")]
-    FailedPacketBatchPreCheck(Uuid),
-    #[error("Bank is in vote-only mode: {0}")]
-    VoteOnlyMode(Uuid),
-    #[error("Bundle mentions blacklisted account: {0}")]
-    BlacklistedAccount(Uuid),
+    #[error("Bank is in vote-only mode")]
+    VoteOnlyMode,
+    #[error("Bundle packet batch failed pre-check")]
+    FailedPacketBatchPreCheck,
+    #[error("Bundle mentions blacklisted account")]
+    BlacklistedAccount,
+    #[error("Bundle contains a transaction that failed to serialize")]
+    FailedToSerializeTransaction,
+    #[error("Bundle contains a duplicate transaction")]
+    DuplicateTransaction,
+    #[error("Bundle failed check_transactions")]
+    FailedCheckTransactions,
 }
 
 pub type BundleSanitizationResult<T> = Result<T, BundleSanitizerError>;
@@ -57,9 +56,10 @@ pub fn get_sanitized_bundle(
     bank: &Arc<Bank>,
     consensus_accounts_cache: &HashSet<Pubkey>,
     blacklisted_accounts: &HashSet<Pubkey>,
+    transaction_error_metrics: &mut TransactionErrorMetrics,
 ) -> BundleSanitizationResult<SanitizedBundle> {
     if bank.vote_only_bank() {
-        return Err(BundleSanitizerError::VoteOnlyMode(packet_bundle.uuid));
+        return Err(BundleSanitizerError::VoteOnlyMode);
     }
 
     if packet_bundle.batch.is_empty()
@@ -70,9 +70,7 @@ pub fn get_sanitized_bundle(
             .iter()
             .any(|p| !verify_packet(&mut p.clone(), false))
     {
-        return Err(BundleSanitizerError::FailedPacketBatchPreCheck(
-            packet_bundle.uuid,
-        ));
+        return Err(BundleSanitizerError::FailedPacketBatchPreCheck);
     }
 
     let packet_indexes = (0..packet_bundle.batch.len()).collect::<Vec<usize>>();
@@ -98,32 +96,27 @@ pub fn get_sanitized_bundle(
     });
 
     if contains_blacklisted_account {
-        return Err(BundleSanitizerError::BlacklistedAccount(packet_bundle.uuid));
+        return Err(BundleSanitizerError::BlacklistedAccount);
     }
 
     if transactions.is_empty() || packet_bundle.batch.len() != transactions.len() {
-        return Err(BundleSanitizerError::FailedToSerializeTransaction(
-            packet_bundle.uuid,
-        ));
+        return Err(BundleSanitizerError::FailedToSerializeTransaction);
     }
 
     if unique_signatures.len() != transactions.len() {
-        return Err(BundleSanitizerError::DuplicateTransaction(
-            packet_bundle.uuid,
-        ));
+        return Err(BundleSanitizerError::DuplicateTransaction);
     }
 
-    // checks for already-processed transaction or expired/invalid blockhash
+    // assume everything locks okay to check for already-processed transaction or expired/invalid blockhash
     let lock_results: Vec<_> = repeat(Ok(())).take(transactions.len()).collect();
-    let mut metrics = TransactionErrorMetrics::default();
     let check_results = bank.check_transactions(
         &transactions,
         &lock_results,
         MAX_PROCESSING_AGE,
-        &mut metrics,
+        transaction_error_metrics,
     );
     if check_results.iter().any(|r| r.0.is_err()) {
-        return Err(BundleSanitizerError::FailedCheckResults(packet_bundle.uuid));
+        return Err(BundleSanitizerError::FailedCheckTransactions);
     }
 
     Ok(SanitizedBundle { transactions })
@@ -165,7 +158,10 @@ mod tests {
         solana_address_lookup_table_program::instruction::create_lookup_table,
         solana_ledger::genesis_utils::create_genesis_config,
         solana_perf::packet::PacketBatch,
-        solana_runtime::{bank::Bank, genesis_utils::GenesisConfigInfo},
+        solana_runtime::{
+            bank::Bank, genesis_utils::GenesisConfigInfo,
+            transaction_error_metrics::TransactionErrorMetrics,
+        },
         solana_sdk::{
             hash::Hash,
             instruction::Instruction,
@@ -204,11 +200,13 @@ mod tests {
             uuid: Uuid::new_v4(),
         };
 
+        let mut transaction_errors = TransactionErrorMetrics::default();
         let sanitized_bundle = get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors,
         )
         .unwrap();
         assert_eq!(sanitized_bundle.transactions.len(), 1);
@@ -244,11 +242,13 @@ mod tests {
         };
 
         let consensus_accounts_cache = HashSet::from([kp.pubkey()]);
+        let mut transaction_errors = TransactionErrorMetrics::default();
         assert!(get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &consensus_accounts_cache,
             &HashSet::default(),
+            &mut transaction_errors
         )
         .is_err());
     }
@@ -280,11 +280,13 @@ mod tests {
         };
 
         // fails to pop because bundle it locks the same transaction twice
+        let mut transaction_errors = TransactionErrorMetrics::default();
         assert!(get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors
         )
         .is_err());
     }
@@ -311,11 +313,13 @@ mod tests {
         };
 
         // fails to pop because bundle has bad blockhash
+        let mut transaction_errors = TransactionErrorMetrics::default();
         assert!(get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors
         )
         .is_err());
     }
@@ -345,11 +349,13 @@ mod tests {
             uuid: Uuid::new_v4(),
         };
 
+        let mut transaction_errors = TransactionErrorMetrics::default();
         let sanitized_bundle = get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors,
         )
         .unwrap();
 
@@ -374,6 +380,7 @@ mod tests {
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors
         )
         .is_err());
     }
@@ -417,11 +424,13 @@ mod tests {
         };
 
         // fails to pop because bundle mentions tip program
+        let mut transaction_errors = TransactionErrorMetrics::default();
         assert!(get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::from_iter([tip_manager.tip_payment_program_id()]),
+            &mut transaction_errors
         )
         .is_err());
     }
@@ -449,11 +458,13 @@ mod tests {
             uuid: Uuid::new_v4(),
         };
 
+        let mut transaction_errors = TransactionErrorMetrics::default();
         assert!(get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors
         )
         .is_ok());
     }
@@ -469,11 +480,13 @@ mod tests {
             uuid: Uuid::new_v4(),
         };
         // fails to pop because empty bundle
+        let mut transaction_errors = TransactionErrorMetrics::default();
         assert!(get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors
         )
         .is_err());
     }
@@ -504,11 +517,13 @@ mod tests {
             uuid: Uuid::new_v4(),
         };
         // fails to pop because too many packets in a bundle
+        let mut transaction_errors = TransactionErrorMetrics::default();
         assert!(get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors
         )
         .is_err());
     }
@@ -540,11 +555,13 @@ mod tests {
         };
 
         // fails to pop because one of the packets is marked as discard
+        let mut transaction_errors = TransactionErrorMetrics::default();
         assert!(get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors
         )
         .is_err());
     }
@@ -581,11 +598,13 @@ mod tests {
             batch: PacketBatch::new(vec![packet]),
             uuid: Uuid::new_v4(),
         };
+        let mut transaction_errors = TransactionErrorMetrics::default();
         assert!(get_sanitized_bundle(
             &packet_bundle,
             &bank,
             &HashSet::default(),
             &HashSet::default(),
+            &mut transaction_errors
         )
         .is_err());
     }