Skip to content

Commit

Permalink
fixup! Derive inner tx hash using wrapper hash
Browse files Browse the repository at this point in the history
  • Loading branch information
@sug0
sug0 committed Jun 17, 2024
1 parent b044db4 commit 1305d90
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 63 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 2 additions & 5 deletions crates/namada/src/ledger/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ mod test {
use namada_state::testing::TestState;
use namada_state::StorageWrite;
use namada_test_utils::TestWasms;
use namada_tx::data::{compute_inner_tx_hash, TxType};
use namada_tx::data::TxType;
use namada_tx::{Code, Data, Tx};
use tempfile::TempDir;

Expand Down Expand Up @@ -305,10 +305,7 @@ mod test {
result
.data
.batch_results
.get_inner_tx_result(&compute_inner_tx_hash(
None,
either::Right(cmt)
))
.get_inner_tx_result(None, either::Right(cmt))
.unwrap()
.as_ref()
.unwrap()
Expand Down
1 change: 1 addition & 0 deletions crates/node/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ clap = { workspace = true, optional = true }
color-eyre.workspace = true
data-encoding.workspace = true
drain_filter_polyfill.workspace = true
either.workspace = true
ethabi.workspace = true
ethbridge-bridge-events.workspace = true
ethbridge-events.workspace = true
Expand Down
17 changes: 10 additions & 7 deletions crates/node/src/bench_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,19 +913,22 @@ impl Client for BenchShell {
.map(|(idx, (tx, changed_keys))| {
let tx_result = TxResult::<String> {
gas_used: 0.into(),
batch_results: BatchResults(
[(
tx.first_commitments().unwrap().get_hash(),
batch_results: {
let mut batch_results = BatchResults::new();
batch_results.insert_inner_tx_result(
tx.wrapper_hash().as_ref(),
either::Right(
tx.first_commitments().unwrap(),
),
Ok(BatchedTxResult {
changed_keys: changed_keys.to_owned(),
vps_result: VpsResult::default(),
initialized_accounts: vec![],
events: BTreeSet::default(),
}),
)]
.into_iter()
.collect(),
),
);
batch_results
},
};
let event: Event = new_tx_event(tx, height.value())
.with(Batch(&tx_result))
Expand Down
127 changes: 89 additions & 38 deletions crates/node/src/shell/finalize_block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ where
.commitments_len
.checked_sub(
u64::try_from(
extended_tx_result.tx_result.batch_results.0.len(),
extended_tx_result.tx_result.batch_results.len(),
)
.expect("Should be able to convert to u64"),
)
Expand Down Expand Up @@ -517,10 +517,8 @@ where
let unrun_txs = tx_data
.commitments_len
.checked_sub(
u64::try_from(
extended_tx_result.tx_result.batch_results.0.len(),
)
.expect("Should be able to convert to u64"),
u64::try_from(extended_tx_result.tx_result.batch_results.len())
.expect("Should be able to convert to u64"),
)
.expect("Shouldn't underflow");

Expand Down Expand Up @@ -841,6 +839,7 @@ where
let dispatch_result = protocol::dispatch_tx(
&tx,
DispatchArgs::Raw {
wrapper_hash: Some(&tx_hash),
tx_index: TxIndex::must_from_usize(tx_index),
wrapper_tx_result: Some(wrapper_tx_result),
vp_wasm_cache: &mut self.vp_wasm_cache,
Expand Down Expand Up @@ -971,7 +970,7 @@ impl<'finalize> TempTxLogs {
) -> ValidityFlags {
let mut flags = ValidityFlags::default();

for (cmt_hash, batched_result) in &tx_result.batch_results.0 {
for (cmt_hash, batched_result) in tx_result.batch_results.iter() {
match batched_result {
Ok(result) => {
if result.is_accepted() {
Expand Down Expand Up @@ -3258,8 +3257,10 @@ mod test_finalize_block {
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let first_tx_result = inner_tx_result
.batch_results
.0
.get(&wrapper.first_commitments().unwrap().get_hash())
.get_inner_tx_result(
Some(&wrapper.header_hash()),
either::Right(wrapper.first_commitments().unwrap()),
)
.unwrap();
assert!(first_tx_result.as_ref().is_ok_and(|res| !res.is_accepted()));
assert_eq!(*event[1].kind(), APPLIED_TX);
Expand Down Expand Up @@ -3407,8 +3408,10 @@ mod test_finalize_block {
let inner_tx_result = event[1].read_attribute::<Batch<'_>>().unwrap();
let inner_result = inner_tx_result
.batch_results
.0
.get(&unsigned_wrapper.first_commitments().unwrap().get_hash())
.get_inner_tx_result(
Some(&unsigned_wrapper.header_hash()),
either::Right(unsigned_wrapper.first_commitments().unwrap()),
)
.unwrap();
assert!(inner_result.as_ref().is_ok_and(|res| !res.is_accepted()));
assert_eq!(*event[2].kind(), APPLIED_TX);
Expand All @@ -3417,12 +3420,11 @@ mod test_finalize_block {
let inner_tx_result = event[2].read_attribute::<Batch<'_>>().unwrap();
let inner_result = inner_tx_result
.batch_results
.0
.get(
&wrong_commitment_wrapper
.first_commitments()
.unwrap()
.get_hash(),
.get_inner_tx_result(
Some(&wrong_commitment_wrapper.header_hash()),
either::Right(
wrong_commitment_wrapper.first_commitments().unwrap(),
),
)
.unwrap();
assert!(inner_result.is_err());
Expand All @@ -3432,8 +3434,10 @@ mod test_finalize_block {
let inner_tx_result = event[3].read_attribute::<Batch<'_>>().unwrap();
let inner_result = inner_tx_result
.batch_results
.0
.get(&failing_wrapper.first_commitments().unwrap().get_hash())
.get_inner_tx_result(
Some(&failing_wrapper.header_hash()),
either::Right(failing_wrapper.first_commitments().unwrap()),
)
.unwrap();
assert!(inner_result.is_err());

Expand Down Expand Up @@ -3635,8 +3639,10 @@ mod test_finalize_block {
let inner_tx_result = event.read_attribute::<Batch<'_>>().unwrap();
let inner_result = inner_tx_result
.batch_results
.0
.get(&wrapper.first_commitments().unwrap().get_hash())
.get_inner_tx_result(
Some(&wrapper.header_hash()),
either::Right(wrapper.first_commitments().unwrap()),
)
.unwrap();
assert!(inner_result.is_err());

Expand Down Expand Up @@ -5554,12 +5560,15 @@ mod test_finalize_block {
let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::Ok);
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;
let inner_results = inner_tx_result.batch_results;

for cmt in batch.commitments() {
assert!(
inner_results
.get(&cmt.get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(cmt),
)
.unwrap()
.clone()
.is_ok_and(|res| res.is_accepted())
Expand Down Expand Up @@ -5599,25 +5608,36 @@ mod test_finalize_block {
let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::WasmRuntimeError);
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;
let inner_results = inner_tx_result.batch_results;

assert!(
inner_results
.get(&batch.commitments()[0].get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[0]),
)
.unwrap()
.clone()
.is_ok_and(|res| res.is_accepted())
);
assert!(
inner_results
.get(&batch.commitments()[1].get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[1]),
)
.unwrap()
.clone()
.is_err()
);
// Assert that the last tx didn't run
assert!(
!inner_results.contains_key(&batch.commitments()[2].get_hash())
inner_results
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[2]),
)
.is_none()
);

// Check storage modifications are missing
Expand Down Expand Up @@ -5647,25 +5667,34 @@ mod test_finalize_block {
let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::Ok);
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;
let inner_results = inner_tx_result.batch_results;

assert!(
inner_results
.get(&batch.commitments()[0].get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[0]),
)
.unwrap()
.clone()
.is_ok_and(|res| res.is_accepted())
);
assert!(
inner_results
.get(&batch.commitments()[1].get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[1])
)
.unwrap()
.clone()
.is_err()
);
assert!(
inner_results
.get(&batch.commitments()[2].get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[2])
)
.unwrap()
.clone()
.is_ok_and(|res| res.is_accepted())
Expand Down Expand Up @@ -5715,25 +5744,36 @@ mod test_finalize_block {
let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::WasmRuntimeError);
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;
let inner_results = inner_tx_result.batch_results;

assert!(
inner_results
.get(&batch.commitments()[0].get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[0]),
)
.unwrap()
.clone()
.is_ok_and(|res| res.is_accepted())
);
assert!(
inner_results
.get(&batch.commitments()[1].get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[1])
)
.unwrap()
.clone()
.is_err()
);
// Assert that the last tx didn't run
assert!(
!inner_results.contains_key(&batch.commitments()[2].get_hash())
inner_results
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[2])
)
.is_none()
);

// Check storage modifications are missing
Expand Down Expand Up @@ -5762,25 +5802,36 @@ mod test_finalize_block {
let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::WasmRuntimeError);
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;
let inner_results = inner_tx_result.batch_results;

assert!(
inner_results
.get(&batch.commitments()[0].get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[0]),
)
.unwrap()
.clone()
.is_ok_and(|res| res.is_accepted())
);
assert!(
inner_results
.get(&batch.commitments()[1].get_hash())
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[1])
)
.unwrap()
.clone()
.is_err()
);
// Assert that the last tx didn't run
assert!(
!inner_results.contains_key(&batch.commitments()[2].get_hash())
inner_results
.get_inner_tx_result(
Some(&batch.header_hash()),
either::Right(&batch.commitments()[2])
)
.is_none()
);

// Check storage modifications
Expand Down
4 changes: 2 additions & 2 deletions crates/node/src/shell/governance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ where
let dispatch_result = protocol::dispatch_tx(
&tx,
protocol::DispatchArgs::Raw {
wrapper_hash: None,
tx_index: TxIndex::default(),
wrapper_tx_result: None,
vp_wasm_cache: &mut shell.vp_wasm_cache,
Expand All @@ -422,8 +423,7 @@ where
Ok(extended_tx_result) => match extended_tx_result
.tx_result
.batch_results
.0
.get(&cmt.get_hash())
.get_inner_tx_result(None, either::Right(&cmt))
{
Some(Ok(batched_result)) if batched_result.is_accepted() => {
shell.state.commit_tx();
Expand Down
Loading

0 comments on commit 1305d90

Please sign in to comment.