Enforce tx metadata upload with static types (#23028)
This commit is contained in:
@ -26,7 +26,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>> {
|
||||
@ -187,20 +186,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 {
|
||||
|
@ -72,7 +72,6 @@ impl BigTableUploadService {
|
||||
bigtable_ledger_storage.clone(),
|
||||
start_slot,
|
||||
Some(end_slot),
|
||||
true,
|
||||
false,
|
||||
exit.clone(),
|
||||
));
|
||||
|
@ -42,8 +42,8 @@ use {
|
||||
},
|
||||
solana_storage_proto::{StoredExtendedRewards, StoredTransactionStatusMeta},
|
||||
solana_transaction_status::{
|
||||
ConfirmedTransactionStatusWithSignature, Rewards, TransactionStatusMeta,
|
||||
VersionedConfirmedBlock, VersionedConfirmedTransactionWithStatusMeta,
|
||||
ConfirmedTransactionStatusWithSignature, ConfirmedTransactionWithStatusMeta, Rewards,
|
||||
TransactionStatusMeta, TransactionWithStatusMeta, VersionedConfirmedBlock,
|
||||
VersionedTransactionWithStatusMeta,
|
||||
},
|
||||
std::{
|
||||
@ -2043,9 +2043,8 @@ impl Blockstore {
|
||||
Ok(VersionedTransactionWithStatusMeta {
|
||||
transaction,
|
||||
meta: self
|
||||
.read_transaction_status((signature, slot))
|
||||
.ok()
|
||||
.flatten(),
|
||||
.read_transaction_status((signature, slot))?
|
||||
.ok_or(BlockstoreError::MissingTransactionMetadata)?,
|
||||
})
|
||||
})
|
||||
.collect()
|
||||
@ -2294,7 +2293,7 @@ impl Blockstore {
|
||||
pub fn get_rooted_transaction(
|
||||
&self,
|
||||
signature: Signature,
|
||||
) -> Result<Option<VersionedConfirmedTransactionWithStatusMeta>> {
|
||||
) -> Result<Option<ConfirmedTransactionWithStatusMeta>> {
|
||||
datapoint_info!(
|
||||
"blockstore-rpc-api",
|
||||
("method", "get_rooted_transaction", String)
|
||||
@ -2307,7 +2306,7 @@ impl Blockstore {
|
||||
&self,
|
||||
signature: Signature,
|
||||
highest_confirmed_slot: Slot,
|
||||
) -> Result<Option<VersionedConfirmedTransactionWithStatusMeta>> {
|
||||
) -> Result<Option<ConfirmedTransactionWithStatusMeta>> {
|
||||
datapoint_info!(
|
||||
"blockstore-rpc-api",
|
||||
("method", "get_complete_transaction", String)
|
||||
@ -2324,8 +2323,8 @@ impl Blockstore {
|
||||
&self,
|
||||
signature: Signature,
|
||||
confirmed_unrooted_slots: &[Slot],
|
||||
) -> Result<Option<VersionedConfirmedTransactionWithStatusMeta>> {
|
||||
if let Some((slot, status)) =
|
||||
) -> Result<Option<ConfirmedTransactionWithStatusMeta>> {
|
||||
if let Some((slot, meta)) =
|
||||
self.get_transaction_status(signature, confirmed_unrooted_slots)?
|
||||
{
|
||||
let transaction = self
|
||||
@ -2333,12 +2332,11 @@ impl Blockstore {
|
||||
.ok_or(BlockstoreError::TransactionStatusSlotMismatch)?; // Should not happen
|
||||
|
||||
let block_time = self.get_block_time(slot)?;
|
||||
Ok(Some(VersionedConfirmedTransactionWithStatusMeta {
|
||||
Ok(Some(ConfirmedTransactionWithStatusMeta {
|
||||
slot,
|
||||
tx_with_meta: VersionedTransactionWithStatusMeta {
|
||||
transaction,
|
||||
meta: Some(status),
|
||||
},
|
||||
tx_with_meta: TransactionWithStatusMeta::Complete(
|
||||
VersionedTransactionWithStatusMeta { transaction, meta },
|
||||
),
|
||||
block_time,
|
||||
}))
|
||||
} else {
|
||||
@ -6327,7 +6325,7 @@ pub mod tests {
|
||||
.unwrap();
|
||||
VersionedTransactionWithStatusMeta {
|
||||
transaction,
|
||||
meta: Some(TransactionStatusMeta {
|
||||
meta: TransactionStatusMeta {
|
||||
status: Ok(()),
|
||||
fee: 42,
|
||||
pre_balances,
|
||||
@ -6338,21 +6336,22 @@ pub mod tests {
|
||||
post_token_balances: Some(vec![]),
|
||||
rewards: Some(vec![]),
|
||||
loaded_addresses: LoadedAddresses::default(),
|
||||
}),
|
||||
},
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Even if marked as root, a slot that is empty of entries should return an error
|
||||
let confirmed_block_err = blockstore.get_rooted_block(slot - 1, true).unwrap_err();
|
||||
assert_matches!(confirmed_block_err, BlockstoreError::SlotUnavailable);
|
||||
assert_matches!(
|
||||
blockstore.get_rooted_block(slot - 1, true),
|
||||
Err(BlockstoreError::SlotUnavailable)
|
||||
);
|
||||
|
||||
// The previous_blockhash of `expected_block` is default because its parent slot is a root,
|
||||
// but empty of entries (eg. snapshot root slots). This now returns an error.
|
||||
let confirmed_block_err = blockstore.get_rooted_block(slot, true).unwrap_err();
|
||||
assert_matches!(
|
||||
confirmed_block_err,
|
||||
BlockstoreError::ParentEntriesUnavailable
|
||||
blockstore.get_rooted_block(slot, true),
|
||||
Err(BlockstoreError::ParentEntriesUnavailable)
|
||||
);
|
||||
|
||||
// Test if require_previous_blockhash is false
|
||||
@ -7168,7 +7167,7 @@ pub mod tests {
|
||||
.unwrap();
|
||||
VersionedTransactionWithStatusMeta {
|
||||
transaction,
|
||||
meta: Some(TransactionStatusMeta {
|
||||
meta: TransactionStatusMeta {
|
||||
status: Ok(()),
|
||||
fee: 42,
|
||||
pre_balances,
|
||||
@ -7179,7 +7178,7 @@ pub mod tests {
|
||||
post_token_balances,
|
||||
rewards,
|
||||
loaded_addresses: LoadedAddresses::default(),
|
||||
}),
|
||||
},
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
@ -7188,9 +7187,9 @@ pub mod tests {
|
||||
let signature = tx_with_meta.transaction.signatures[0];
|
||||
assert_eq!(
|
||||
blockstore.get_rooted_transaction(signature).unwrap(),
|
||||
Some(VersionedConfirmedTransactionWithStatusMeta {
|
||||
Some(ConfirmedTransactionWithStatusMeta {
|
||||
slot,
|
||||
tx_with_meta: tx_with_meta.clone(),
|
||||
tx_with_meta: TransactionWithStatusMeta::Complete(tx_with_meta.clone()),
|
||||
block_time: None
|
||||
})
|
||||
);
|
||||
@ -7198,9 +7197,9 @@ pub mod tests {
|
||||
blockstore
|
||||
.get_complete_transaction(signature, slot + 1)
|
||||
.unwrap(),
|
||||
Some(VersionedConfirmedTransactionWithStatusMeta {
|
||||
Some(ConfirmedTransactionWithStatusMeta {
|
||||
slot,
|
||||
tx_with_meta,
|
||||
tx_with_meta: TransactionWithStatusMeta::Complete(tx_with_meta),
|
||||
block_time: None
|
||||
})
|
||||
);
|
||||
@ -7270,7 +7269,7 @@ pub mod tests {
|
||||
.unwrap();
|
||||
VersionedTransactionWithStatusMeta {
|
||||
transaction,
|
||||
meta: Some(TransactionStatusMeta {
|
||||
meta: TransactionStatusMeta {
|
||||
status: Ok(()),
|
||||
fee: 42,
|
||||
pre_balances,
|
||||
@ -7281,7 +7280,7 @@ pub mod tests {
|
||||
post_token_balances,
|
||||
rewards,
|
||||
loaded_addresses: LoadedAddresses::default(),
|
||||
}),
|
||||
},
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
@ -7292,9 +7291,9 @@ pub mod tests {
|
||||
blockstore
|
||||
.get_complete_transaction(signature, slot)
|
||||
.unwrap(),
|
||||
Some(VersionedConfirmedTransactionWithStatusMeta {
|
||||
Some(ConfirmedTransactionWithStatusMeta {
|
||||
slot,
|
||||
tx_with_meta,
|
||||
tx_with_meta: TransactionWithStatusMeta::Complete(tx_with_meta),
|
||||
block_time: None
|
||||
})
|
||||
);
|
||||
@ -8052,6 +8051,16 @@ pub mod tests {
|
||||
.unwrap();
|
||||
transactions.push(transaction.into());
|
||||
}
|
||||
|
||||
let map_result =
|
||||
blockstore.map_transactions_to_statuses(slot, transactions.clone().into_iter());
|
||||
assert!(map_result.is_ok());
|
||||
let map = map_result.unwrap();
|
||||
assert_eq!(map.len(), 4);
|
||||
for (x, m) in map.iter().enumerate() {
|
||||
assert_eq!(m.meta.fee, x as u64);
|
||||
}
|
||||
|
||||
// Push transaction that will not have matching status, as a test case
|
||||
transactions.push(
|
||||
Transaction::new_with_compiled_instructions(
|
||||
@ -8064,14 +8073,9 @@ pub mod tests {
|
||||
.into(),
|
||||
);
|
||||
|
||||
let map_result = blockstore.map_transactions_to_statuses(slot, transactions.into_iter());
|
||||
assert!(map_result.is_ok());
|
||||
let map = map_result.unwrap();
|
||||
assert_eq!(map.len(), 5);
|
||||
for (x, m) in map.iter().take(4).enumerate() {
|
||||
assert_eq!(m.meta.as_ref().unwrap().fee, x as u64);
|
||||
}
|
||||
assert_eq!(map[4].meta, None);
|
||||
let map_result =
|
||||
blockstore.map_transactions_to_statuses(slot, transactions.clone().into_iter());
|
||||
assert_matches!(map_result, Err(BlockstoreError::MissingTransactionMetadata));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
@ -102,6 +102,7 @@ pub enum BlockstoreError {
|
||||
ParentEntriesUnavailable,
|
||||
SlotUnavailable,
|
||||
UnsupportedTransactionVersion,
|
||||
MissingTransactionMetadata,
|
||||
}
|
||||
pub type Result<T> = std::result::Result<T, BlockstoreError>;
|
||||
|
||||
|
Reference in New Issue
Block a user