From f9fd97652c57bc0bb0c5155a0682a2be9e8d8bae Mon Sep 17 00:00:00 2001 From: James Hiew Date: Wed, 16 Nov 2022 20:41:16 +0000 Subject: [PATCH 1/7] Don't use duplicate ETH_BRIDGE_ADDRESS --- tests/src/e2e/eth_bridge_tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/src/e2e/eth_bridge_tests.rs b/tests/src/e2e/eth_bridge_tests.rs index 7cc1bd6aee..39cf23fbcb 100644 --- a/tests/src/e2e/eth_bridge_tests.rs +++ b/tests/src/e2e/eth_bridge_tests.rs @@ -1,3 +1,5 @@ +use namada::ledger::eth_bridge; + use crate::e2e::helpers::get_actor_rpc; use crate::e2e::setup; use crate::e2e::setup::constants::{ @@ -6,8 +8,6 @@ use crate::e2e::setup::constants::{ use crate::e2e::setup::{Bin, Who}; use crate::{run, run_as}; -const ETH_BRIDGE_ADDRESS: &str = "atest1v9hx7w36g42ysgzzwf5kgem9ypqkgerjv4ehxgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpq8f99ew"; - /// # Examples /// /// ``` @@ -15,7 +15,7 @@ const ETH_BRIDGE_ADDRESS: &str = "atest1v9hx7w36g42ysgzzwf5kgem9ypqkgerjv4ehxgpq /// assert_eq!(storage_key, "#atest1v9hx7w36g42ysgzzwf5kgem9ypqkgerjv4ehxgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpq8f99ew/queue"); /// ``` fn storage_key(path: &str) -> String { - format!("#{ETH_BRIDGE_ADDRESS}/{}", path) + format!("#{}/{}", eth_bridge::vp::ADDRESS, path) } #[test] @@ -87,7 +87,7 @@ fn everything() { // stdout. namadac_tx.exp_string("Transaction is invalid").unwrap(); namadac_tx - .exp_string(&format!("Rejected: {}", ETH_BRIDGE_ADDRESS)) + .exp_string(&format!("Rejected: {}", eth_bridge::vp::ADDRESS)) .unwrap(); namadac_tx.assert_success(); } From 89fc491c3c44c6acae21147e6fb5cd937458a443 Mon Sep 17 00:00:00 2001 From: James Hiew Date: Wed, 16 Nov 2022 20:42:07 +0000 Subject: [PATCH 2/7] Rename test to unauthorized_tx_cannot_write_storage --- tests/src/e2e/eth_bridge_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/e2e/eth_bridge_tests.rs b/tests/src/e2e/eth_bridge_tests.rs index 39cf23fbcb..012c3a8cef 100644 --- a/tests/src/e2e/eth_bridge_tests.rs +++ b/tests/src/e2e/eth_bridge_tests.rs @@ -19,7 +19,7 @@ fn storage_key(path: &str) -> String { } #[test] -fn everything() { +fn test_unauthorized_tx_cannot_write_storage() { const LEDGER_STARTUP_TIMEOUT_SECONDS: u64 = 30; const CLIENT_COMMAND_TIMEOUT_SECONDS: u64 = 30; const SOLE_VALIDATOR: Who = Who::Validator(0); From 5a0887aebf792f7854e137af5fd01cffd540ba06 Mon Sep 17 00:00:00 2001 From: James Hiew Date: Wed, 16 Nov 2022 20:43:49 +0000 Subject: [PATCH 3/7] Remove no longer used eth_bridge::storage module --- shared/src/ledger/eth_bridge/mod.rs | 1 - shared/src/ledger/eth_bridge/storage.rs | 12 ------------ 2 files changed, 13 deletions(-) delete mode 100644 shared/src/ledger/eth_bridge/storage.rs diff --git a/shared/src/ledger/eth_bridge/mod.rs b/shared/src/ledger/eth_bridge/mod.rs index ff8505b08e..817623e54c 100644 --- a/shared/src/ledger/eth_bridge/mod.rs +++ b/shared/src/ledger/eth_bridge/mod.rs @@ -1,4 +1,3 @@ //! Bridge from Ethereum -pub mod storage; pub mod vp; diff --git a/shared/src/ledger/eth_bridge/storage.rs b/shared/src/ledger/eth_bridge/storage.rs deleted file mode 100644 index e67abf921c..0000000000 --- a/shared/src/ledger/eth_bridge/storage.rs +++ /dev/null @@ -1,12 +0,0 @@ -//! storage helpers -use super::vp::ADDRESS; -use crate::types::storage::{Key, KeySeg}; - -const QUEUE_STORAGE_KEY: &str = "queue"; - -/// Get the key corresponding to @EthBridge/queue -pub fn queue_key() -> Key { - Key::from(ADDRESS.to_db_key()) - .push(&QUEUE_STORAGE_KEY.to_owned()) - .expect("Cannot obtain a storage key") -} From 8f92e19ddce1a0c65157046eac8d0ac48aea5da9 Mon Sep 17 00:00:00 2001 From: James Hiew Date: Wed, 16 Nov 2022 20:44:54 +0000 Subject: [PATCH 4/7] Remove references to anoman/anomac --- tests/src/e2e/eth_bridge_tests.rs | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/tests/src/e2e/eth_bridge_tests.rs b/tests/src/e2e/eth_bridge_tests.rs index 012c3a8cef..9cfb45c4f3 100644 --- a/tests/src/e2e/eth_bridge_tests.rs +++ b/tests/src/e2e/eth_bridge_tests.rs @@ -26,7 +26,7 @@ fn test_unauthorized_tx_cannot_write_storage() { let test = setup::single_node_net().unwrap(); - let mut namadan_ledger = run_as!( + let mut ledger = run_as!( test, SOLE_VALIDATOR, Bin::Node, @@ -34,14 +34,10 @@ fn test_unauthorized_tx_cannot_write_storage() { Some(LEDGER_STARTUP_TIMEOUT_SECONDS) ) .unwrap(); - namadan_ledger - .exp_string("Namada ledger node started") - .unwrap(); - namadan_ledger - .exp_string("Tendermint node started") - .unwrap(); - namadan_ledger.exp_string("Committed block hash").unwrap(); - let _bg_ledger = namadan_ledger.background(); + ledger.exp_string("Namada ledger node started").unwrap(); + ledger.exp_string("Tendermint node started").unwrap(); + ledger.exp_string("Committed block hash").unwrap(); + let _bg_ledger = ledger.background(); let tx_data_path = test.test_dir.path().join("queue_storage_key.txt"); std::fs::write(&tx_data_path, &storage_key("queue")[..]).unwrap(); @@ -69,7 +65,7 @@ fn test_unauthorized_tx_cannot_write_storage() { } else { tx_args.clone() }; - let mut namadac_tx = run!( + let mut client_tx = run!( test, Bin::Client, tx_args, @@ -78,17 +74,17 @@ fn test_unauthorized_tx_cannot_write_storage() { .unwrap(); if !dry_run { - namadac_tx.exp_string("Transaction accepted").unwrap(); - namadac_tx.exp_string("Transaction applied").unwrap(); + client_tx.exp_string("Transaction accepted").unwrap(); + client_tx.exp_string("Transaction applied").unwrap(); } // TODO: we should check here explicitly with the ledger via a // Tendermint RPC call that the path `value/#EthBridge/queue` // is unchanged rather than relying solely on looking at namadac // stdout. - namadac_tx.exp_string("Transaction is invalid").unwrap(); - namadac_tx + client_tx.exp_string("Transaction is invalid").unwrap(); + client_tx .exp_string(&format!("Rejected: {}", eth_bridge::vp::ADDRESS)) .unwrap(); - namadac_tx.assert_success(); + client_tx.assert_success(); } } From b1133141a1bc694e1b1485cdaeaadd4dd6023d55 Mon Sep 17 00:00:00 2001 From: James Hiew Date: Wed, 16 Nov 2022 20:51:47 +0000 Subject: [PATCH 5/7] Simplify test and remove references to queue key --- tests/src/e2e/eth_bridge_tests.rs | 45 +++++++++++-------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/tests/src/e2e/eth_bridge_tests.rs b/tests/src/e2e/eth_bridge_tests.rs index 9cfb45c4f3..bd973c13f4 100644 --- a/tests/src/e2e/eth_bridge_tests.rs +++ b/tests/src/e2e/eth_bridge_tests.rs @@ -39,8 +39,8 @@ fn test_unauthorized_tx_cannot_write_storage() { ledger.exp_string("Committed block hash").unwrap(); let _bg_ledger = ledger.background(); - let tx_data_path = test.test_dir.path().join("queue_storage_key.txt"); - std::fs::write(&tx_data_path, &storage_key("queue")[..]).unwrap(); + let tx_data_path = test.test_dir.path().join("arbitrary_storage_key.txt"); + std::fs::write(&tx_data_path, &storage_key("arbitrary")[..]).unwrap(); let tx_code_path = wasm_abs_path(TX_WRITE_STORAGE_KEY_WASM); @@ -59,32 +59,19 @@ fn test_unauthorized_tx_cannot_write_storage() { &ledger_addr, ]; - for &dry_run in &[true, false] { - let tx_args = if dry_run { - vec![tx_args.clone(), vec!["--dry-run"]].concat() - } else { - tx_args.clone() - }; - let mut client_tx = run!( - test, - Bin::Client, - tx_args, - Some(CLIENT_COMMAND_TIMEOUT_SECONDS) - ) - .unwrap(); + let mut client_tx = run!( + test, + Bin::Client, + tx_args, + Some(CLIENT_COMMAND_TIMEOUT_SECONDS) + ) + .unwrap(); - if !dry_run { - client_tx.exp_string("Transaction accepted").unwrap(); - client_tx.exp_string("Transaction applied").unwrap(); - } - // TODO: we should check here explicitly with the ledger via a - // Tendermint RPC call that the path `value/#EthBridge/queue` - // is unchanged rather than relying solely on looking at namadac - // stdout. - client_tx.exp_string("Transaction is invalid").unwrap(); - client_tx - .exp_string(&format!("Rejected: {}", eth_bridge::vp::ADDRESS)) - .unwrap(); - client_tx.assert_success(); - } + client_tx.exp_string("Transaction accepted").unwrap(); + client_tx.exp_string("Transaction applied").unwrap(); + client_tx.exp_string("Transaction is invalid").unwrap(); + client_tx + .exp_string(&format!("Rejected: {}", eth_bridge::vp::ADDRESS)) + .unwrap(); + client_tx.assert_success(); } From c1da3933a4dad4fd7b547b7c592683c6724b2a7f Mon Sep 17 00:00:00 2001 From: James Hiew Date: Wed, 16 Nov 2022 20:52:25 +0000 Subject: [PATCH 6/7] Add comment for the test --- tests/src/e2e/eth_bridge_tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/e2e/eth_bridge_tests.rs b/tests/src/e2e/eth_bridge_tests.rs index bd973c13f4..54647eacbc 100644 --- a/tests/src/e2e/eth_bridge_tests.rs +++ b/tests/src/e2e/eth_bridge_tests.rs @@ -18,6 +18,8 @@ fn storage_key(path: &str) -> String { format!("#{}/{}", eth_bridge::vp::ADDRESS, path) } +/// Test that a regular transaction cannot modify arbitrary keys of the Ethereum +/// bridge VP. #[test] fn test_unauthorized_tx_cannot_write_storage() { const LEDGER_STARTUP_TIMEOUT_SECONDS: u64 = 30; From 8efb474421232131077a7aa4bd5d16454669378b Mon Sep 17 00:00:00 2001 From: James Hiew Date: Mon, 28 Nov 2022 13:22:54 +0000 Subject: [PATCH 7/7] Add changelog --- .../unreleased/miscellaneous/796-ethbridge-e2e-cleanup.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/miscellaneous/796-ethbridge-e2e-cleanup.md diff --git a/.changelog/unreleased/miscellaneous/796-ethbridge-e2e-cleanup.md b/.changelog/unreleased/miscellaneous/796-ethbridge-e2e-cleanup.md new file mode 100644 index 0000000000..738678102c --- /dev/null +++ b/.changelog/unreleased/miscellaneous/796-ethbridge-e2e-cleanup.md @@ -0,0 +1,2 @@ +- Clean up some code relating to the Ethereum bridge + ([#796](https://github.com/anoma/namada/pull/796)) \ No newline at end of file