Merge branch 'james/mainline/ethbridge-e2e-cleanup' (#796)

* james/mainline/ethbridge-e2e-cleanup:
  Add changelog
  Add comment for the test
  Simplify test and remove references to queue key
  Remove references to anoman/anomac
  Remove no longer used eth_bridge::storage module
  Rename test to unauthorized_tx_cannot_write_storage
  Don't use duplicate ETH_BRIDGE_ADDRESS

.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))

shared/src/ledger/eth_bridge/mod.rs

@@ -1,4 +1,3 @@
 //! Bridge from Ethereum
 
-pub mod storage;
 pub mod vp;

tests/src/e2e/eth_bridge_tests.rs

@@ -1,7 +1,8 @@
+use std::str::FromStr;
+
 use borsh::BorshSerialize;
 use namada::ledger::eth_bridge;
 use namada_core::types::storage;
-use namada_core::types::storage::KeySeg;
 use namada_test_utils::tx_data::TxWriteData;
 
 use crate::e2e::helpers::get_actor_rpc;
@@ -10,38 +11,44 @@ use crate::e2e::setup::constants::{wasm_abs_path, ALBERT, TX_WRITE_WASM};
 use crate::e2e::setup::{Bin, Who};
 use crate::{run, run_as};
 
-const ETH_BRIDGE_ADDRESS: &str = "atest1v9hx7w36g42ysgzzwf5kgem9ypqkgerjv4ehxgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpq8f99ew";
+/// # Examples
+///
+/// ```
+/// let storage_key = storage_key("queue");
+/// assert_eq!(storage_key, "#atest1v9hx7w36g42ysgzzwf5kgem9ypqkgerjv4ehxgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpq8f99ew/queue");
+/// ```
+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 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);
 
     let test = setup::single_node_net().unwrap();
 
-    let mut namadan_ledger = run_as!(
+    let mut ledger = run_as!(
         test,
         SOLE_VALIDATOR,
         Bin::Node,
         &["ledger"],
         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");
+    let tx_data_path = test.test_dir.path().join("arbitrary_storage_key.txt");
     std::fs::write(
         &tx_data_path,
         TxWriteData {
-            key: storage::Key::from(eth_bridge::vp::ADDRESS.to_db_key()),
+            key: storage::Key::from_str(&storage_key("arbitrary")).unwrap(),
             value: b"arbitrary value".to_vec(),
         }
         .try_to_vec()
@@ -66,32 +73,19 @@ fn everything() {
         &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 namadac_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 {
-            namadac_tx.exp_string("Transaction accepted").unwrap();
-            namadac_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
-            .exp_string(&format!("Rejected: {}", ETH_BRIDGE_ADDRESS))
-            .unwrap();
-        namadac_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();
 }