Skip to content

Commit

Permalink
simplify parallel tx validation per PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@miloserdow
miloserdow committed Jan 14, 2025
1 parent 4d82f86 commit 222dda7
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 70 deletions.
51 changes: 22 additions & 29 deletions chain/chain/src/runtime/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,6 @@ impl RuntimeAdapter for NightshadeRuntime {
match verify_and_charge_transaction(
runtime_config,
&mut state_update,
gas_price,
transaction,
&cost,
// here we do not know which block the transaction will be included
Expand Down Expand Up @@ -775,42 +774,36 @@ impl RuntimeAdapter for NightshadeRuntime {
continue;
}

let res = validate_transaction(
let verify_result = validate_transaction(
runtime_config,
prev_block.next_gas_price,
&tx,
true,
protocol_version,
);
match res {
)
.and_then(|cost| {
verify_and_charge_transaction(
runtime_config,
&mut state_update,
&tx,
&cost,
Some(next_block_height),
protocol_version,
)
});

match verify_result {
Ok(cost) => {
match verify_and_charge_transaction(
runtime_config,
&mut state_update,
prev_block.next_gas_price,
&tx,
&cost,
Some(next_block_height),
protocol_version,
) {
Ok(verification_result) => {
tracing::trace!(target: "runtime", tx=?tx.get_hash(), "including transaction that passed validation");
state_update.commit(StateChangeCause::NotWritableToDisk);
total_gas_burnt += verification_result.gas_burnt;
total_size += tx.get_size();
result.transactions.push(tx);
// Take one transaction from this group, no more.
break;
}
Err(err) => {
tracing::trace!(target: "runtime", tx=?tx.get_hash(), ?err, "discarding transaction that failed verification");
rejected_invalid_tx += 1;
state_update.rollback();
}
}
tracing::trace!(target: "runtime", tx=?tx.get_hash(), "including transaction that passed validation and verification");
state_update.commit(StateChangeCause::NotWritableToDisk);
total_gas_burnt += cost.gas_burnt;
total_size += tx.get_size();
result.transactions.push(tx);
// Take one transaction from this group, no more.
break;
}
Err(err) => {
tracing::trace!(target: "runtime", tx=?tx.get_hash(), ?err, "discarding transaction that failed initial validation");
tracing::trace!(target: "runtime", tx=?tx.get_hash(), ?err, "discarding transaction that failed verification or verification");
rejected_invalid_tx += 1;
state_update.rollback();
}
Expand Down
1 change: 0 additions & 1 deletion runtime/runtime-params-estimator/src/estimator_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,6 @@ impl Testbed<'_> {
node_runtime::verify_and_charge_transaction(
&self.apply_state.config,
&mut state_update,
gas_price,
tx,
&cost,
block_height,
Expand Down
39 changes: 16 additions & 23 deletions runtime/runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,26 +290,26 @@ impl Runtime {
debug!(target: "runtime", "{}", log_str);
}

/// Validates all transactions in parallel and returns a map of transaction hashes
/// to their validation results.
/// Validates all transactions in parallel and returns an iterator of
/// transactions paired with their validation results.
///
/// Returns a `HashMap` of `tx_hash -> Result<TransactionCost, InvalidTxError>`.
fn parallel_validate_transactions(
config: &RuntimeConfig,
/// Returns an `Iterator` of `(&SignedTransaction, Result<TransactionCost, InvalidTxError>)`
fn parallel_validate_transactions<'a>(
config: &'a RuntimeConfig,
gas_price: Balance,
transactions: &[SignedTransaction],
transactions: &'a [SignedTransaction],
current_protocol_version: ProtocolVersion,
) -> HashMap<CryptoHash, Result<TransactionCost, InvalidTxError>> {
tracing::debug!(target: "runtime", "parallel validation: starting threads");

transactions
) -> impl Iterator<Item = (&'a SignedTransaction, Result<TransactionCost, InvalidTxError>)>
{
let results: Vec<_> = transactions
.par_iter()
.map(|tx| {
.map(move |tx| {
let cost_result =
validate_transaction(config, gas_price, tx, true, current_protocol_version);
(tx.get_hash(), cost_result)
(tx, cost_result)
})
.collect()
.collect();
results.into_iter()
}

/// Takes one signed transaction, verifies it and converts it to a receipt.
Expand Down Expand Up @@ -344,7 +344,6 @@ impl Runtime {
match verify_and_charge_transaction(
&apply_state.config,
state_update,
apply_state.gas_price,
signed_transaction,
transaction_cost,
Some(apply_state.block_height),
Expand Down Expand Up @@ -1639,19 +1638,13 @@ impl Runtime {
let apply_state = &mut processing_state.apply_state;
let state_update = &mut processing_state.state_update;

let tx_cost_results = Self::parallel_validate_transactions(
for (signed_transaction, maybe_cost) in Self::parallel_validate_transactions(
&apply_state.config,
apply_state.gas_price,
&processing_state.transactions.transactions,
apply_state.current_protocol_version,
);

for signed_transaction in processing_state.transactions.iter_nonexpired_transactions() {
) {
let tx_hash = signed_transaction.get_hash();
let maybe_cost = tx_cost_results
.get(&tx_hash)
.expect("Must have a validation result for each transaction hash");

let cost = match maybe_cost {
Ok(c) => c,
Err(e) => {
Expand All @@ -1669,7 +1662,7 @@ impl Runtime {
state_update,
apply_state,
signed_transaction,
cost,
&cost,
&mut processing_state.stats,
);
let (receipt, outcome_with_id) = match tx_result {
Expand Down
17 changes: 0 additions & 17 deletions runtime/runtime/src/verifier.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ pub fn validate_transaction(
pub fn verify_and_charge_transaction(
config: &RuntimeConfig,
state_update: &mut TrieUpdate,
_gas_price: Balance,
signed_transaction: &SignedTransaction,
transaction_cost: &TransactionCost,
block_height: Option<BlockHeight>,
Expand Down Expand Up @@ -738,7 +737,6 @@ mod tests {
let err = verify_and_charge_transaction(
config,
state_update,
gas_price,
signed_transaction,
&cost,
None,
Expand Down Expand Up @@ -874,7 +872,6 @@ mod tests {
let verification_result = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -953,7 +950,6 @@ mod tests {
let err = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1023,7 +1019,6 @@ mod tests {
let err = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1056,7 +1051,6 @@ mod tests {
let err = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1131,7 +1125,6 @@ mod tests {
let err = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1183,7 +1176,6 @@ mod tests {
let err = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1230,7 +1222,6 @@ mod tests {
let verification_result = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1274,7 +1265,6 @@ mod tests {
let res = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1332,7 +1322,6 @@ mod tests {
verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand All @@ -1355,7 +1344,6 @@ mod tests {
verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand All @@ -1378,7 +1366,6 @@ mod tests {
verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1423,7 +1410,6 @@ mod tests {
let err = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1475,7 +1461,6 @@ mod tests {
let err = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1526,7 +1511,6 @@ mod tests {
let err = verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down Expand Up @@ -1582,7 +1566,6 @@ mod tests {
verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
&cost,
None,
Expand Down

0 comments on commit 222dda7

Please sign in to comment.