v1.9: Enforce tx metadata upload to bigtable (solana-labs#23212)

* Enforce tx metadata upload with static types (solana-labs#23028)

* resolve conflicts

* fix test

Author: jstarry

client-test/tests/client.rs

@@ -8,7 +8,7 @@ use {
             RpcAccountInfoConfig, RpcBlockSubscribeConfig, RpcBlockSubscribeFilter,
             RpcProgramAccountsConfig,
         },
-        rpc_response::{RpcBlockUpdate, SlotInfo},
+        rpc_response::SlotInfo,
     },
     solana_ledger::{blockstore::Blockstore, get_tmp_ledger_path},
     solana_rpc::{
@@ -34,7 +34,9 @@ use {
     },
     solana_streamer::socket::SocketAddrSpace,
     solana_test_validator::TestValidator,
-    solana_transaction_status::{TransactionDetails, UiTransactionEncoding},
+    solana_transaction_status::{
+        ConfirmedBlockWithOptionalMetadata, TransactionDetails, UiTransactionEncoding,
+    },
     std::{
         collections::HashSet,
         net::{IpAddr, SocketAddr},
@@ -212,6 +214,7 @@ fn test_block_subscription() {
         ..
     } = create_genesis_config(10_000);
     let bank = Bank::new_for_tests(&genesis_config);
+    let rent_exempt_amount = bank.get_minimum_balance_for_rent_exemption(0);
     let bank_forks = Arc::new(RwLock::new(BankForks::new(bank)));
 
     // setup Blockstore
@@ -225,6 +228,8 @@ fn test_block_subscription() {
     let keypair2 = Keypair::new();
     let keypair3 = Keypair::new();
     let max_complete_transaction_status_slot = Arc::new(AtomicU64::new(blockstore.max_root()));
+    bank.transfer(rent_exempt_amount, &alice, &keypair2.pubkey())
+        .unwrap();
     let _confirmed_block_signatures = create_test_transactions_and_populate_blockstore(
         vec![&alice, &keypair1, &keypair2, &keypair3],
         0,
@@ -276,22 +281,12 @@ fn test_block_subscription() {
     match maybe_actual {
         Ok(actual) => {
             let complete_block = blockstore.get_complete_block(slot, false).unwrap();
-            let block = complete_block.clone().configure(
-                UiTransactionEncoding::Json,
-                TransactionDetails::Signatures,
-                false,
-            );
-            let expected = RpcBlockUpdate {
-                slot,
-                block: Some(block),
-                err: None,
-            };
-            let block = complete_block.configure(
+            let block = ConfirmedBlockWithOptionalMetadata::from(complete_block).configure(
                 UiTransactionEncoding::Json,
                 TransactionDetails::Signatures,
                 false,
             );
-            assert_eq!(actual.value.slot, expected.slot);
+            assert_eq!(actual.value.slot, slot);
             assert!(block.eq(&actual.value.block.unwrap()));
         }
         Err(e) => {

core/src/banking_stage.rs

@@ -2067,7 +2067,7 @@ mod tests {
             transaction::{Transaction, TransactionError},
         },
         solana_streamer::{recvmmsg::recv_mmsg, socket::SocketAddrSpace},
-        solana_transaction_status::TransactionWithStatusMeta,
+        solana_transaction_status::TransactionWithMetadata,
         solana_vote_program::vote_transaction,
         std::{
             net::SocketAddr,
@@ -3223,39 +3223,23 @@ mod tests {
         genesis_config.rent.lamports_per_byte_year = 50;
         genesis_config.rent.exemption_threshold = 2.0;
         let bank = Arc::new(Bank::new_no_wallclock_throttle_for_tests(&genesis_config));
+        let rent_exempt_minimum = bank.get_minimum_balance_for_rent_exemption(0);
         let pubkey = solana_sdk::pubkey::new_rand();
         let pubkey1 = solana_sdk::pubkey::new_rand();
         let keypair1 = Keypair::new();
 
-        let rent_exempt_amount = bank.get_minimum_balance_for_rent_exemption(0);
-
-        let success_tx = system_transaction::transfer(
-            &mint_keypair,
-            &pubkey,
-            rent_exempt_amount,
-            genesis_config.hash(),
-        );
+        let success_tx =
+            system_transaction::transfer(&mint_keypair, &pubkey, 0, genesis_config.hash());
         let success_signature = success_tx.signatures[0];
         let entry_1 = next_entry(&genesis_config.hash(), 1, vec![success_tx.clone()]);
-        let ix_error_tx = system_transaction::transfer(
-            &keypair1,
-            &pubkey1,
-            2 * rent_exempt_amount,
-            genesis_config.hash(),
-        );
+        let ix_error_tx =
+            system_transaction::transfer(&keypair1, &pubkey1, std::u64::MAX, genesis_config.hash());
         let ix_error_signature = ix_error_tx.signatures[0];
         let entry_2 = next_entry(&entry_1.hash, 1, vec![ix_error_tx.clone()]);
-        let fail_tx = system_transaction::transfer(
-            &mint_keypair,
-            &pubkey1,
-            rent_exempt_amount,
-            genesis_config.hash(),
-        );
-        let entry_3 = next_entry(&entry_2.hash, 1, vec![fail_tx.clone()]);
-        let entries = vec![entry_1, entry_2, entry_3];
+        let entries = vec![entry_1, entry_2];
 
-        let transactions = sanitize_transactions(vec![success_tx, ix_error_tx, fail_tx]);
-        bank.transfer(rent_exempt_amount, &mint_keypair, &keypair1.pubkey())
+        let transactions = sanitize_transactions(vec![success_tx, ix_error_tx]);
+        bank.transfer(rent_exempt_minimum, &mint_keypair, &keypair1.pubkey())
             .unwrap();
 
         let ledger_path = get_tmp_ledger_path_auto_delete!();
@@ -3314,27 +3298,24 @@ mod tests {
             transaction_status_service.join().unwrap();
 
             let confirmed_block = blockstore.get_rooted_block(bank.slot(), false).unwrap();
-            assert_eq!(confirmed_block.transactions.len(), 3);
-
-            for TransactionWithStatusMeta { transaction, meta } in
-                confirmed_block.transactions.into_iter()
-            {
-                if transaction.signatures[0] == success_signature {
-                    let meta = meta.unwrap();
-                    assert_eq!(meta.status, Ok(()));
-                } else if transaction.signatures[0] == ix_error_signature {
-                    let meta = meta.unwrap();
-                    assert_eq!(
-                        meta.status,
-                        Err(TransactionError::InstructionError(
-                            0,
-                            InstructionError::Custom(1)
-                        ))
-                    );
-                } else {
-                    assert_eq!(meta, None);
-                }
-            }
+            let actual_tx_results: Vec<_> = confirmed_block
+                .transactions
+                .into_iter()
+                .map(|TransactionWithMetadata { transaction, meta }| {
+                    (transaction.signatures[0], meta.status)
+                })
+                .collect();
+            let expected_tx_results = vec![
+                (success_signature, Ok(())),
+                (
+                    ix_error_signature,
+                    Err(TransactionError::InstructionError(
+                        0,
+                        InstructionError::Custom(1),
+                    )),
+                ),
+            ];
+            assert_eq!(actual_tx_results, expected_tx_results);
 
             poh_recorder
                 .lock()

core/src/replay_stage.rs

@@ -3021,7 +3021,7 @@ pub mod tests {
             transaction::TransactionError,
         },
         solana_streamer::socket::SocketAddrSpace,
-        solana_transaction_status::TransactionWithStatusMeta,
+        solana_transaction_status::TransactionWithMetadata,
         solana_vote_program::{
             vote_state::{VoteState, VoteStateVersions},
             vote_transaction,
@@ -3871,36 +3871,35 @@ pub mod tests {
             let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1));
             let slot = bank1.slot();
 
-            let signatures = create_test_transactions_and_populate_blockstore(
+            let mut test_signatures_iter = create_test_transactions_and_populate_blockstore(
                 vec![&mint_keypair, &keypair1, &keypair2, &keypair3],
                 bank0.slot(),
                 bank1,
                 blockstore.clone(),
                 Arc::new(AtomicU64::default()),
-            );
+            )
+            .into_iter();
 
             let confirmed_block = blockstore.get_rooted_block(slot, false).unwrap();
-            assert_eq!(confirmed_block.transactions.len(), 3);
-
-            for TransactionWithStatusMeta { transaction, meta } in
-                confirmed_block.transactions.into_iter()
-            {
-                if transaction.signatures[0] == signatures[0] {
-                    let meta = meta.unwrap();
-                    assert_eq!(meta.status, Ok(()));
-                } else if transaction.signatures[0] == signatures[1] {
-                    let meta = meta.unwrap();
-                    assert_eq!(
-                        meta.status,
-                        Err(TransactionError::InstructionError(
-                            0,
-                            InstructionError::Custom(1)
-                        ))
-                    );
-                } else {
-                    assert_eq!(meta, None);
-                }
-            }
+            let actual_tx_results: Vec<_> = confirmed_block
+                .transactions
+                .into_iter()
+                .map(|TransactionWithMetadata { transaction, meta }| {
+                    (transaction.signatures[0], meta.status)
+                })
+                .collect();
+            let expected_tx_results = vec![
+                (test_signatures_iter.next().unwrap(), Ok(())),
+                (
+                    test_signatures_iter.next().unwrap(),
+                    Err(TransactionError::InstructionError(
+                        0,
+                        InstructionError::Custom(1),
+                    )),
+                ),
+            ];
+            assert_eq!(actual_tx_results, expected_tx_results);
+            assert!(test_signatures_iter.next().is_none());
         }
         Blockstore::destroy(&ledger_path).unwrap();
     }

ledger-tool/src/bigtable.rs

@@ -16,7 +16,9 @@ use {
     },
     solana_ledger::{blockstore::Blockstore, blockstore_db::AccessType},
     solana_sdk::{clock::Slot, pubkey::Pubkey, signature::Signature},
-    solana_transaction_status::{ConfirmedBlock, EncodedTransaction, UiTransactionEncoding},
+    solana_transaction_status::{
+        ConfirmedBlockWithOptionalMetadata, EncodedTransaction, UiTransactionEncoding,
+    },
     std::{
         collections::HashSet,
         path::Path,
@@ -30,7 +32,6 @@ async fn upload(
     blockstore: Blockstore,
     starting_slot: Slot,
     ending_slot: Option<Slot>,
-    allow_missing_metadata: bool,
     force_reupload: bool,
 ) -> Result<(), Box<dyn std::error::Error>> {
     let bigtable = solana_storage_bigtable::LedgerStorage::new(false, None, None)
@@ -42,7 +43,6 @@ async fn upload(
         bigtable,
         starting_slot,
         ending_slot,
-        allow_missing_metadata,
         force_reupload,
         Arc::new(AtomicBool::new(false)),
     )
@@ -194,7 +194,7 @@ pub async fn transaction_history(
 ) -> Result<(), Box<dyn std::error::Error>> {
     let bigtable = solana_storage_bigtable::LedgerStorage::new(true, None, None).await?;
 
-    let mut loaded_block: Option<(Slot, ConfirmedBlock)> = None;
+    let mut loaded_block: Option<(Slot, ConfirmedBlockWithOptionalMetadata)> = None;
     while limit > 0 {
         let results = bigtable
             .get_confirmed_signatures_for_address(
@@ -306,12 +306,6 @@ impl BigTableSubCommand for App<'_, '_> {
                                 .index(2)
                                 .help("Stop uploading at this slot [default: last available slot]"),
                         )
-                        .arg(
-                            Arg::with_name("allow_missing_metadata")
-                                .long("allow-missing-metadata")
-                                .takes_value(false)
-                                .help("Don't panic if transaction metadata is missing"),
-                        )
                         .arg(
                             Arg::with_name("force_reupload")
                                 .long("force")
@@ -506,7 +500,6 @@ pub fn bigtable_process_command(ledger_path: &Path, matches: &ArgMatches<'_>) {
         ("upload", Some(arg_matches)) => {
             let starting_slot = value_t!(arg_matches, "starting_slot", Slot).unwrap_or(0);
             let ending_slot = value_t!(arg_matches, "ending_slot", Slot).ok();
-            let allow_missing_metadata = arg_matches.is_present("allow_missing_metadata");
             let force_reupload = arg_matches.is_present("force_reupload");
             let blockstore = crate::open_blockstore(
                 &canonicalize_ledger_path(ledger_path),
@@ -518,7 +511,6 @@ pub fn bigtable_process_command(ledger_path: &Path, matches: &ArgMatches<'_>) {
                 blockstore,
                 starting_slot,
                 ending_slot,
-                allow_missing_metadata,
                 force_reupload,
             ))
         }

ledger/src/bigtable_upload.rs

@@ -25,7 +25,6 @@ pub async fn upload_confirmed_blocks(
     bigtable: solana_storage_bigtable::LedgerStorage,
     starting_slot: Slot,
     ending_slot: Option<Slot>,
-    allow_missing_metadata: bool,
     force_reupload: bool,
     exit: Arc<AtomicBool>,
 ) -> Result<(), Box<dyn std::error::Error>> {
@@ -186,20 +185,7 @@ pub async fn upload_confirmed_blocks(
                 num_blocks -= 1;
                 None
             }
-            Some(confirmed_block) => {
-                if confirmed_block
-                    .transactions
-                    .iter()
-                    .any(|transaction| transaction.meta.is_none())
-                {
-                    if allow_missing_metadata {
-                        info!("Transaction metadata missing from slot {}", slot);
-                    } else {
-                        panic!("Transaction metadata missing from slot {}", slot);
-                    }
-                }
-                Some(bigtable.upload_confirmed_block(slot, confirmed_block))
-            }
+            Some(confirmed_block) => Some(bigtable.upload_confirmed_block(slot, confirmed_block)),
         });
 
         for result in futures::future::join_all(uploads).await {

ledger/src/bigtable_upload_service.rs

@@ -72,7 +72,6 @@ impl BigTableUploadService {
                 bigtable_ledger_storage.clone(),
                 start_slot,
                 Some(end_slot),
-                true,
                 false,
                 exit.clone(),
             ));