Skip to content

Commit

Permalink
refactor: checked transaction lps is never none (#1487)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jstarry
jstarry authored May 26, 2024
1 parent a175ff1 commit a4a009e
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 90 deletions.
14 changes: 7 additions & 7 deletions core/src/banking_stage/consumer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2516,16 +2516,16 @@ mod tests {
Err(TransactionError::BlockhashNotFound),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: None
lamports_per_signature: 0
}),
Err(TransactionError::BlockhashNotFound),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: None
lamports_per_signature: 0
}),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: None
lamports_per_signature: 0
}),
]),
[2, 4, 5]
Expand All @@ -2535,21 +2535,21 @@ mod tests {
Consumer::filter_valid_transaction_indexes(&[
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: None
lamports_per_signature: 0,
}),
Err(TransactionError::BlockhashNotFound),
Err(TransactionError::BlockhashNotFound),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: None
lamports_per_signature: 0,
}),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: None
lamports_per_signature: 0,
}),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: None
lamports_per_signature: 0,
}),
]),
[0, 3, 4, 5]
Expand Down
4 changes: 2 additions & 2 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3494,14 +3494,14 @@ impl Bank {
if let Some(hash_info) = hash_queue.get_hash_info_if_valid(recent_blockhash, max_age) {
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(hash_info.lamports_per_signature()),
lamports_per_signature: hash_info.lamports_per_signature(),
})
} else if let Some((nonce, nonce_data)) =
self.check_and_load_message_nonce_account(tx.message(), next_durable_nonce)
{
Ok(CheckedTransactionDetails {
nonce: Some(nonce),
lamports_per_signature: Some(nonce_data.get_lamports_per_signature()),
lamports_per_signature: nonce_data.get_lamports_per_signature(),
})
} else {
error_counters.blockhash_not_found += 1;
Expand Down
53 changes: 16 additions & 37 deletions svm/src/account_loader.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub type TransactionLoadResult = Result<LoadedTransaction>;
#[derive(PartialEq, Eq, Debug, Clone)]
pub struct CheckedTransactionDetails {
pub nonce: Option<NoncePartial>,
pub lamports_per_signature: Option<u64>,
pub lamports_per_signature: u64,
}

#[derive(PartialEq, Eq, Debug, Clone)]
Expand Down Expand Up @@ -141,20 +141,16 @@ pub(crate) fn load_accounts<CB: TransactionProcessingCallback>(
}),
) => {
let message = tx.message();
let fee = if let Some(lamports_per_signature) = lamports_per_signature {
fee_structure.calculate_fee(
message,
*lamports_per_signature,
&process_compute_budget_instructions(message.program_instructions_iter())
.unwrap_or_default()
.into(),
feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
feature_set.is_active(&remove_rounding_in_fee_calculation::id()),
)
} else {
return Err(TransactionError::BlockhashNotFound);
};
let fee = fee_structure.calculate_fee(
message,
*lamports_per_signature,
&process_compute_budget_instructions(message.program_instructions_iter())
.unwrap_or_default()
.into(),
feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
feature_set.is_active(&remove_rounding_in_fee_calculation::id()),
);

// load transactions
load_transaction_accounts(
Expand Down Expand Up @@ -557,7 +553,7 @@ mod tests {
&[sanitized_tx],
&[Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(lamports_per_signature),
lamports_per_signature,
})],
error_counters,
fee_structure,
Expand Down Expand Up @@ -1045,7 +1041,7 @@ mod tests {
&[tx],
&[Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(10),
lamports_per_signature: 10,
})],
&mut error_counters,
&FeeStructure::default(),
Expand Down Expand Up @@ -2049,7 +2045,7 @@ mod tests {
&[sanitized_tx.clone()],
&[Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(0),
lamports_per_signature: 0,
})],
&mut error_counters,
&FeeStructure::default(),
Expand Down Expand Up @@ -2129,7 +2125,7 @@ mod tests {
);
let check_result = Ok(CheckedTransactionDetails {
nonce: Some(NoncePartial::default()),
lamports_per_signature: Some(20),
lamports_per_signature: 20,
});

let results = load_accounts(
Expand Down Expand Up @@ -2198,27 +2194,10 @@ mod tests {
false,
);

let check_result = Ok(CheckedTransactionDetails {
nonce: Some(NoncePartial::default()),
lamports_per_signature: None,
});
let fee_structure = FeeStructure::default();

let result = load_accounts(
&mock_bank,
&[sanitized_transaction.clone()],
&[check_result],
&mut TransactionErrorMetrics::default(),
&fee_structure,
None,
&ProgramCacheForTxBatch::default(),
);

assert_eq!(result, vec![Err(TransactionError::BlockhashNotFound)]);

let check_result = Ok(CheckedTransactionDetails {
nonce: Some(NoncePartial::default()),
lamports_per_signature: Some(20),
lamports_per_signature: 20,
});

let result = load_accounts(
Expand Down
63 changes: 23 additions & 40 deletions svm/src/transaction_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,31 +354,24 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
) -> HashMap<Pubkey, u64> {
let mut result: HashMap<Pubkey, u64> = HashMap::new();
check_results.iter_mut().zip(txs).for_each(|etx| {
if let (Ok(checked_details), tx) = etx {
if checked_details.lamports_per_signature.is_some() {
tx.message()
.account_keys()
.iter()
.for_each(|key| match result.entry(*key) {
Entry::Occupied(mut entry) => {
let count = entry.get_mut();
saturating_add_assign!(*count, 1);
}
Entry::Vacant(entry) => {
if callbacks
.account_matches_owners(key, program_owners)
.is_some()
{
entry.insert(1);
}
if let (Ok(_), tx) = etx {
tx.message()
.account_keys()
.iter()
.for_each(|key| match result.entry(*key) {
Entry::Occupied(mut entry) => {
let count = entry.get_mut();
saturating_add_assign!(*count, 1);
}
Entry::Vacant(entry) => {
if callbacks
.account_matches_owners(key, program_owners)
.is_some()
{
entry.insert(1);
}
});
} else {
// If the transaction's nonce account was not valid, and blockhash is not found,
// the transaction will fail to process. Let's not load any programs from the
// transaction, and update the status of the transaction.
*etx.0 = Err(TransactionError::BlockhashNotFound);
}
}
});
}
});
result
Expand Down Expand Up @@ -1259,21 +1252,16 @@ mod tests {
let transactions = vec![
sanitized_transaction_1.clone(),
sanitized_transaction_2.clone(),
sanitized_transaction_2,
sanitized_transaction_1,
];
let mut lock_results = vec![
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(25),
lamports_per_signature: 25,
}),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(25),
}),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: None,
lamports_per_signature: 25,
}),
Err(TransactionError::ProgramAccountNotFound),
];
Expand All @@ -1286,7 +1274,6 @@ mod tests {
&owners,
);

assert_eq!(lock_results[2], Err(TransactionError::BlockhashNotFound));
assert_eq!(result.len(), 2);
assert_eq!(result[&key1], 2);
assert_eq!(result[&key2], 1);
Expand Down Expand Up @@ -1368,11 +1355,11 @@ mod tests {
&mut [
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(0),
lamports_per_signature: 0,
}),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(0),
lamports_per_signature: 0,
}),
],
owners,
Expand Down Expand Up @@ -1467,12 +1454,9 @@ mod tests {
let mut lock_results = vec![
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(0),
}),
Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: None,
lamports_per_signature: 0,
}),
Err(TransactionError::BlockhashNotFound),
];
let programs =
TransactionBatchProcessor::<TestForkGraph>::filter_executable_program_accounts(
Expand All @@ -1490,7 +1474,6 @@ mod tests {
.expect("failed to find the program account"),
&1
);
assert_eq!(lock_results[1], Err(TransactionError::BlockhashNotFound));
}

#[test]
Expand Down
8 changes: 4 additions & 4 deletions svm/tests/integration_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ fn prepare_transactions(
all_transactions.push(sanitized_transaction);
transaction_checks.push(Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(20),
lamports_per_signature: 20,
}));

// The transaction fee payer must have enough funds
Expand Down Expand Up @@ -321,7 +321,7 @@ fn prepare_transactions(
all_transactions.push(sanitized_transaction);
transaction_checks.push(Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(20),
lamports_per_signature: 20,
}));

// Setting up the accounts for the transfer
Expand Down Expand Up @@ -363,7 +363,7 @@ fn prepare_transactions(
all_transactions.push(sanitized_transaction);
transaction_checks.push(Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(20),
lamports_per_signature: 20,
}));

let mut account_data = AccountSharedData::default();
Expand Down Expand Up @@ -407,7 +407,7 @@ fn prepare_transactions(
all_transactions.push(sanitized_transaction.clone());
transaction_checks.push(Ok(CheckedTransactionDetails {
nonce: None,
lamports_per_signature: Some(20),
lamports_per_signature: 20,
}));

// fee payer
Expand Down

0 comments on commit a4a009e

Please sign in to comment.