Skip to content

Commit

Permalink
Writes only one hash at a time for replay protection
Browse files Browse the repository at this point in the history
  • Loading branch information
grarco committed Oct 10, 2023
1 parent fcabecc commit 895eaf2
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 93 deletions.
116 changes: 38 additions & 78 deletions apps/src/lib/node/ledger/shell/finalize_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,23 +211,17 @@ where
tx_event["gas_used"] = "0".into();
response.events.push(tx_event);
// if the rejected tx was decrypted, remove it
// from the queue of txs to be processed and remove the hash
// from storage
// from the queue of txs to be processed, remove its hash
// from storage and write the hash of the corresponding wrapper
if let TxType::Decrypted(_) = &tx_header.tx_type {
let tx_hash = self
let wrapper_tx = self
.wl_storage
.storage
.tx_queue
.pop()
.expect("Missing wrapper tx in queue")
.tx
.clone()
.update_header(TxType::Raw)
.header_hash();
self.wl_storage
.write_log
.delete_tx_hash(tx_hash)
.expect("Error while deleting tx hash from storage");
.tx;
self.allow_tx_replay(wrapper_tx);
}

#[cfg(not(any(feature = "abciplus", feature = "abcipp")))]
Expand Down Expand Up @@ -436,16 +430,6 @@ where
.map_err(Error::TxApply)
{
Ok(result) => {
if let Some(wrapper) = embedding_wrapper {
// If transaction is decrypted remove the corresponding
// wrapper hash from storage which is not needed anymore
self.wl_storage
.write_log
.delete_tx_hash(wrapper.header_hash())
.expect(
"Error while deleting tx hash key from storage",
);
}
if result.is_accepted() {
if let EventType::Accepted = tx_event.event_type {
// Wrapper transaction
Expand Down Expand Up @@ -523,31 +507,19 @@ where
// If transaction type is Decrypted and failed because of
// out of gas, remove its hash from storage to allow
// rewrapping it
if let Some(mut wrapper) = embedding_wrapper {
if let Some(wrapper) = embedding_wrapper {
if let Error::TxApply(protocol::Error::GasError(_)) =
msg
{
let raw_header_hash = wrapper
.update_header(TxType::Raw)
.header_hash();
self.wl_storage
.write_log
.delete_tx_hash(raw_header_hash)
.expect(
"Error while deleting tx hash key from \
storage",
);
} else {
// Remove the wrapper hash which is not needed
// anymore
self.wl_storage
.write_log
.delete_tx_hash(wrapper.header_hash())
.expect(
"Error while deleting tx hash key from \
storage",
);
self.allow_tx_replay(wrapper);
}
} else if let Some(wrapper) = wrapper {
// In case of error, write the wrapper tx hash to
// storage to prevent replay
self.wl_storage
.write_log
.write_tx_hash(wrapper.header_hash())
.expect("Error while writing tx hash to storage");
}

tx_event["gas_used"] =
Expand Down Expand Up @@ -985,6 +957,20 @@ where
}
Ok(())
}

// Allow to replay a specific wasm transaction. Needs as argument the
// corresponding wrapper transaction to avoid replay of that in the process
fn allow_tx_replay(&mut self, mut wrapper_tx: Tx) {
self.wl_storage
.write_log
.write_tx_hash(wrapper_tx.header_hash())
.expect("Error while deleting tx hash from storage");

self.wl_storage
.write_log
.delete_tx_hash(wrapper_tx.update_header(TxType::Raw).header_hash())
.expect("Error while deleting tx hash from storage");
}
}

/// Convert ABCI vote info to PoS vote info. Any info which fails the conversion
Expand Down Expand Up @@ -2295,11 +2281,7 @@ mod test_finalize_block {

let (wrapper_tx, processed_tx) =
mk_wrapper_tx(&shell, &crate::wallet::defaults::albert_keypair());
let wrapper_hash_key =
replay_protection::get_replay_protection_last_key(
&wrapper_tx.header_hash(),
);
let mut decrypted_tx = wrapper_tx.clone();
let mut decrypted_tx = wrapper_tx;

decrypted_tx.update_header(TxType::Raw);
let decrypted_hash_key =
Expand Down Expand Up @@ -2331,32 +2313,15 @@ mod test_finalize_block {
let root_post = shell.shell.wl_storage.storage.block.tree.root();
assert_eq!(root_pre.0, root_post.0);

// Check transactions' hashes in storage
assert!(
shell
.shell
.wl_storage
.write_log
.has_replay_protection_entry(&wrapper_tx.header_hash())
);
// Check transaction's hash in storage
assert!(
shell
.shell
.wl_storage
.write_log
.has_replay_protection_entry(&decrypted_tx.header_hash())
);
// Check that non of the hashes is present in the merkle tree
assert!(
!shell
.shell
.wl_storage
.storage
.block
.tree
.has_key(&wrapper_hash_key)
.unwrap()
);
// Check that the hash is present in the merkle tree
assert!(
!shell
.shell
Expand Down Expand Up @@ -2421,13 +2386,8 @@ mod test_finalize_block {
.update_header(TxType::Raw)
.header_hash();

// Write wrapper and inner hashes in storage
for hash in [
&decrypted_hash,
&wrapper_hash,
&decrypted_2_hash,
&wrapper_2_hash,
] {
// Write inner hashes in storage
for hash in [&decrypted_hash, &decrypted_2_hash] {
let hash_subkey =
replay_protection::get_replay_protection_last_subkey(hash);
shell
Expand All @@ -2446,16 +2406,17 @@ mod test_finalize_block {
info: "".into(),
},
}];
// Out of gas error triggering inner hash removal
// Out of gas error triggering inner hash removal and wrapper hash
// insert
processed_txs.push(ProcessedTx {
tx: decrypted_tx.to_bytes(),
result: TxResult {
code: ErrorCodes::Ok.into(),
info: "".into(),
},
});
// Wasm error that still leads to inner hash commitment and wrapper hash
// removal
// Wasm error that still leads to inner hash commitment and no wrapper
// hash insert
processed_txs.push(ProcessedTx {
tx: decrypted_tx_2.to_bytes(),
result: TxResult {
Expand Down Expand Up @@ -2526,9 +2487,8 @@ mod test_finalize_block {
assert!(
shell
.wl_storage
.storage
.write_log
.has_replay_protection_entry(&wrapper_hash)
.expect("test failed")
);
assert!(
shell
Expand Down
24 changes: 9 additions & 15 deletions shared/src/ledger/protocol/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,12 @@ where
apply_protocol_tx(protocol_tx.tx, tx.data(), wl_storage)
}
TxType::Wrapper(ref wrapper) => {
let fee_unshielding_transaction =
get_fee_unshielding_transaction(&tx, wrapper);
let changed_keys = apply_wrapper_tx(
tx,
wrapper,
get_fee_unshielding_transaction(&tx, wrapper),
fee_unshielding_transaction,
tx_bytes,
ShellParams {
tx_gas_meter,
Expand Down Expand Up @@ -207,12 +210,14 @@ where
}

/// Performs the required operation on a wrapper transaction:
/// - replay protection
/// - fee payment
/// - gas accounting
/// - replay protection
///
/// Returns the set of changed storage keys.
/// Returns the set of changed storage keys. The caller should write the hash of
/// the wrapper header to storage in case of failure.
pub(crate) fn apply_wrapper_tx<'a, D, H, CA, WLS>(
mut tx: Tx,
wrapper: &WrapperTx,
fee_unshield_transaction: Option<Transaction>,
tx_bytes: &[u8],
Expand All @@ -226,17 +231,6 @@ where
WLS: WriteLogAndStorage<D = D, H = H>,
{
let mut changed_keys = BTreeSet::default();
let mut tx: Tx = tx_bytes.try_into().unwrap();

// Writes wrapper tx hash
shell_params
.wl_storage
.write_log_mut()
.write_tx_hash(tx.header_hash())
.expect("Error while writing tx hash to storage");
changed_keys.insert(replay_protection::get_replay_protection_last_key(
&tx.header_hash(),
));

// Charge fee before performing any fallible operations
charge_fee(
Expand All @@ -257,7 +251,7 @@ where
.write_tx_hash(tx.update_header(TxType::Raw).header_hash())
.expect("Error while writing tx hash to storage");
changed_keys.insert(replay_protection::get_replay_protection_last_key(
&tx.update_header(TxType::Raw).header_hash(),
&tx.header_hash(),
));

Ok(changed_keys)
Expand Down
1 change: 1 addition & 0 deletions shared/src/ledger/queries/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ where
let mut tx_gas_meter =
TxGasMeter::new(wrapper.gas_limit.to_owned());
protocol::apply_wrapper_tx(
tx.clone(),
&wrapper,
None,
&request.data,
Expand Down

0 comments on commit 895eaf2

Please sign in to comment.