Skip to content

Commit

Permalink
Merge branch 'grarco/fix-replay-protection' (#1867)
Browse files Browse the repository at this point in the history
  • Loading branch information
@grarco
grarco committed Oct 23, 2023
2 parents 738fb61 + c40cbc1 commit f4b94dc
Show file tree
Hide file tree
Showing 18 changed files with 259 additions and 481 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/improvements/1867-fix-replay-protection.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Reworked the signature of inner transactions to improve safety and fix replay
protection. ([\#1867](https://github.com/anoma/namada/pull/1867))
4 changes: 2 additions & 2 deletions apps/src/lib/bench_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ pub fn generate_tx(

if let Some(signer) = signer {
tx.add_section(Section::Signature(Signature::new(
tx.sechashes(),
vec![tx.raw_header_hash()],
[(0, signer.clone())].into_iter().collect(),
None,
)));
Expand Down Expand Up @@ -486,7 +486,7 @@ pub fn generate_foreign_key_tx(signer: &SecretKey) -> Tx {
.serialize_to_vec(),
));
tx.add_section(Section::Signature(Signature::new(
tx.sechashes(),
vec![tx.raw_header_hash()],
[(0, signer.clone())].into_iter().collect(),
None,
)));
Expand Down
260 changes: 108 additions & 152 deletions apps/src/lib/node/ledger/shell/finalize_block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2048,11 +2048,9 @@ mod test_finalize_block {
// won't receive votes from TM since we receive votes at a 1-block
// delay, so votes will be empty here
next_block_for_inflation(&mut shell, pkh1.clone(), vec![], None);
assert!(
rewards_accumulator_handle()
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(rewards_accumulator_handle()
.is_empty(&shell.wl_storage)
.unwrap());

// FINALIZE BLOCK 2. Tell Namada that val1 is the block proposer.
// Include votes that correspond to block 1. Make val2 the next block's
Expand All @@ -2062,11 +2060,9 @@ mod test_finalize_block {
assert!(rewards_prod_2.is_empty(&shell.wl_storage).unwrap());
assert!(rewards_prod_3.is_empty(&shell.wl_storage).unwrap());
assert!(rewards_prod_4.is_empty(&shell.wl_storage).unwrap());
assert!(
!rewards_accumulator_handle()
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(!rewards_accumulator_handle()
.is_empty(&shell.wl_storage)
.unwrap());
// Val1 was the proposer, so its reward should be larger than all
// others, which should themselves all be equal
let acc_sum = get_rewards_sum(&shell.wl_storage);
Expand Down Expand Up @@ -2180,11 +2176,9 @@ mod test_finalize_block {
None,
);
}
assert!(
rewards_accumulator_handle()
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(rewards_accumulator_handle()
.is_empty(&shell.wl_storage)
.unwrap());
let rp1 = rewards_prod_1
.get(&shell.wl_storage, &Epoch::default())
.unwrap()
Expand Down Expand Up @@ -2269,25 +2263,21 @@ mod test_finalize_block {
assert_eq!(root_pre.0, root_post.0);

// Check transaction's hash in storage
assert!(
shell
.shell
.wl_storage
.write_log
.has_replay_protection_entry(&decrypted_tx.header_hash())
.unwrap_or_default()
);
assert!(shell
.shell
.wl_storage
.write_log
.has_replay_protection_entry(&decrypted_tx.header_hash())
.unwrap_or_default());
// Check that the hash is present in the merkle tree
assert!(
!shell
.shell
.wl_storage
.storage
.block
.tree
.has_key(&decrypted_hash_key)
.unwrap()
);
assert!(!shell
.shell
.wl_storage
.storage
.block
.tree
.has_key(&decrypted_hash_key)
.unwrap());
}

/// Test replay protection hash handling
Expand Down Expand Up @@ -2422,48 +2412,36 @@ mod test_finalize_block {
.as_str();
assert_eq!(code, String::from(ErrorCodes::WasmRuntimeError).as_str());

assert!(
shell
.wl_storage
.write_log
.has_replay_protection_entry(&invalid_wrapper_hash)
.unwrap_or_default()
);
assert!(
!shell
.wl_storage
.write_log
.has_replay_protection_entry(&decrypted_3_hash)
.unwrap_or_default()
);
assert!(
!shell
.wl_storage
.write_log
.has_replay_protection_entry(&decrypted_hash)
.unwrap_or_default()
);
assert!(
shell
.wl_storage
.write_log
.has_replay_protection_entry(&wrapper_hash)
.unwrap_or_default()
);
assert!(
shell
.wl_storage
.storage
.has_replay_protection_entry(&decrypted_2_hash)
.expect("test failed")
);
assert!(
!shell
.wl_storage
.write_log
.has_replay_protection_entry(&wrapper_2_hash)
.unwrap_or_default()
);
assert!(shell
.wl_storage
.write_log
.has_replay_protection_entry(&invalid_wrapper_hash)
.unwrap_or_default());
assert!(!shell
.wl_storage
.write_log
.has_replay_protection_entry(&decrypted_3_hash)
.unwrap_or_default());
assert!(!shell
.wl_storage
.write_log
.has_replay_protection_entry(&decrypted_hash)
.unwrap_or_default());
assert!(shell
.wl_storage
.write_log
.has_replay_protection_entry(&wrapper_hash)
.unwrap_or_default());
assert!(shell
.wl_storage
.storage
.has_replay_protection_entry(&decrypted_2_hash)
.expect("test failed"));
assert!(!shell
.wl_storage
.write_log
.has_replay_protection_entry(&wrapper_2_hash)
.unwrap_or_default());
}

// Test that if the fee payer doesn't have enough funds for fee payment the
Expand Down Expand Up @@ -2751,11 +2729,9 @@ mod test_finalize_block {
.unwrap(),
Some(ValidatorState::Consensus)
);
assert!(
enqueued_slashes_handle()
.at(&Epoch::default())
.is_empty(&shell.wl_storage)?
);
assert!(enqueued_slashes_handle()
.at(&Epoch::default())
.is_empty(&shell.wl_storage)?);
assert_eq!(
get_num_consensus_validators(&shell.wl_storage, Epoch::default())
.unwrap(),
Expand All @@ -2774,21 +2750,17 @@ mod test_finalize_block {
.unwrap(),
Some(ValidatorState::Jailed)
);
assert!(
enqueued_slashes_handle()
.at(&epoch)
.is_empty(&shell.wl_storage)?
);
assert!(enqueued_slashes_handle()
.at(&epoch)
.is_empty(&shell.wl_storage)?);
assert_eq!(
get_num_consensus_validators(&shell.wl_storage, epoch).unwrap(),
5_u64
);
}
assert!(
!enqueued_slashes_handle()
.at(&processing_epoch)
.is_empty(&shell.wl_storage)?
);
assert!(!enqueued_slashes_handle()
.at(&processing_epoch)
.is_empty(&shell.wl_storage)?);

// Advance to the processing epoch
loop {
Expand All @@ -2811,11 +2783,9 @@ mod test_finalize_block {
// println!("Reached processing epoch");
break;
} else {
assert!(
enqueued_slashes_handle()
.at(&shell.wl_storage.storage.block.epoch)
.is_empty(&shell.wl_storage)?
);
assert!(enqueued_slashes_handle()
.at(&shell.wl_storage.storage.block.epoch)
.is_empty(&shell.wl_storage)?);
let stake1 = read_validator_stake(
&shell.wl_storage,
&params,
Expand Down Expand Up @@ -3374,15 +3344,13 @@ mod test_finalize_block {
)
.unwrap();
assert_eq!(last_slash, Some(Epoch(4)));
assert!(
namada_proof_of_stake::is_validator_frozen(
&shell.wl_storage,
&val1.address,
current_epoch,
&params
)
.unwrap()
);
assert!(namada_proof_of_stake::is_validator_frozen(
&shell.wl_storage,
&val1.address,
current_epoch,
&params
)
.unwrap());
assert!(
namada_proof_of_stake::validator_slashes_handle(&val1.address)
.is_empty(&shell.wl_storage)
Expand Down Expand Up @@ -3891,12 +3859,14 @@ mod test_finalize_block {
assert!(!consensus_val_set.at(&ep).is_empty(storage).unwrap());
// assert!(!below_cap_val_set.at(&ep).is_empty(storage).
// unwrap());
assert!(
!validator_positions.at(&ep).is_empty(storage).unwrap()
);
assert!(
!all_validator_addresses.at(&ep).is_empty(storage).unwrap()
);
assert!(!validator_positions
.at(&ep)
.is_empty(storage)
.unwrap());
assert!(!all_validator_addresses
.at(&ep)
.is_empty(storage)
.unwrap());
}
};

Expand Down Expand Up @@ -3935,33 +3905,25 @@ mod test_finalize_block {
Epoch(1),
Epoch(params.pipeline_len + default_past_epochs + 1),
);
assert!(
!consensus_val_set
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(
validator_positions
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(
all_validator_addresses
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(!consensus_val_set
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap());
assert!(validator_positions
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap());
assert!(all_validator_addresses
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap());

// Advance to the epoch `consensus_val_set_len` + 1
loop {
assert!(
!consensus_val_set
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(!consensus_val_set
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap());
let votes = get_default_true_votes(
&shell.wl_storage,
shell.wl_storage.storage.block.epoch,
Expand All @@ -3972,12 +3934,10 @@ mod test_finalize_block {
}
}

assert!(
consensus_val_set
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(consensus_val_set
.at(&Epoch(0))
.is_empty(&shell.wl_storage)
.unwrap());

// Advance one more epoch
let votes = get_default_true_votes(
Expand All @@ -3986,23 +3946,19 @@ mod test_finalize_block {
);
current_epoch = advance_epoch(&mut shell, &pkh1, &votes, None);
for ep in Epoch::default().iter_range(2) {
assert!(
consensus_val_set
.at(&ep)
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(consensus_val_set
.at(&ep)
.is_empty(&shell.wl_storage)
.unwrap());
}
for ep in Epoch::iter_bounds_inclusive(
Epoch(2),
current_epoch + params.pipeline_len,
) {
assert!(
!consensus_val_set
.at(&ep)
.is_empty(&shell.wl_storage)
.unwrap()
);
assert!(!consensus_val_set
.at(&ep)
.is_empty(&shell.wl_storage)
.unwrap());
}

Ok(())
Expand Down
Loading

0 comments on commit f4b94dc

Please sign in to comment.