From 714146263a7db5428483b14b06d1a81964e43044 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 15 May 2023 15:05:29 +0100 Subject: [PATCH 1/6] Implement CLI command to delete a storage value --- apps/src/bin/namada-node/cli.rs | 3 ++ apps/src/lib/cli.rs | 48 +++++++++++++++++++++++++++ apps/src/lib/node/ledger/mod.rs | 59 +++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+) diff --git a/apps/src/bin/namada-node/cli.rs b/apps/src/bin/namada-node/cli.rs index 0f5305e4d4c..5d0b6e5779c 100644 --- a/apps/src/bin/namada-node/cli.rs +++ b/apps/src/bin/namada-node/cli.rs @@ -40,6 +40,9 @@ pub fn main() -> Result<()> { cmds::Ledger::DumpDb(cmds::LedgerDumpDb(args)) => { ledger::dump_db(ctx.config.ledger, args); } + cmds::Ledger::DbDeleteValue(cmds::LedgerDbDeleteValue(args)) => { + ledger::db_delete_value(ctx.config.ledger, args); + } }, cmds::NamadaNode::Config(sub) => match sub { cmds::Config::Gen(cmds::ConfigGen) => { diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index c0f8c0d58d4..0d86acde4a0 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -827,6 +827,7 @@ pub mod cmds { Run(LedgerRun), Reset(LedgerReset), DumpDb(LedgerDumpDb), + DbDeleteValue(LedgerDbDeleteValue), } impl SubCmd for Ledger { @@ -837,8 +838,11 @@ pub mod cmds { let run = SubCmd::parse(matches).map(Self::Run); let reset = SubCmd::parse(matches).map(Self::Reset); let dump_db = SubCmd::parse(matches).map(Self::DumpDb); + let db_delete_value = + SubCmd::parse(matches).map(Self::DbDeleteValue); run.or(reset) .or(dump_db) + .or(db_delete_value) // The `run` command is the default if no sub-command given .or(Some(Self::Run(LedgerRun(args::LedgerRun(None))))) }) @@ -853,6 +857,7 @@ pub mod cmds { .subcommand(LedgerRun::def()) .subcommand(LedgerReset::def()) .subcommand(LedgerDumpDb::def()) + .subcommand(LedgerDbDeleteValue::def()) } } @@ -912,6 +917,29 @@ pub mod cmds { } } + #[derive(Clone, Debug)] + pub struct LedgerDbDeleteValue(pub args::LedgerDbDeleteValue); + + impl SubCmd for LedgerDbDeleteValue { + const CMD: &'static str = "db-delete-value"; + + fn parse(matches: &ArgMatches) -> Option { + matches + .subcommand_matches(Self::CMD) + .map(|matches| Self(args::LedgerDbDeleteValue::parse(matches))) + } + + fn def() -> App { + App::new(Self::CMD) + .about( + "Delete a value from the ledger node's DB at the given \ + key.", + ) + .setting(AppSettings::ArgRequiredElseHelp) + .add_args::() + } + } + #[derive(Clone, Debug)] pub enum Config { Gen(ConfigGen), @@ -2263,6 +2291,26 @@ pub mod args { } } + #[derive(Clone, Debug)] + pub struct LedgerDbDeleteValue { + pub storage_key: storage::Key, + } + + impl Args for LedgerDbDeleteValue { + fn parse(matches: &ArgMatches) -> Self { + let storage_key = STORAGE_KEY.parse(matches); + Self { storage_key } + } + + fn def(app: App) -> App { + app.arg( + STORAGE_KEY + .def() + .about("Storage key to delete a value from."), + ) + } + } + /// Transaction associated results arguments #[derive(Clone, Debug)] pub struct QueryResult { diff --git a/apps/src/lib/node/ledger/mod.rs b/apps/src/lib/node/ledger/mod.rs index 679e48888e5..f334fdbb2e3 100644 --- a/apps/src/lib/node/ledger/mod.rs +++ b/apps/src/lib/node/ledger/mod.rs @@ -222,6 +222,65 @@ pub fn dump_db( db.dump_last_block(out_file_path); } +/// Delete a value from storage. +// TODO: recalculate merkle roots? maybe this should be +// a new argument +pub fn db_delete_value( + config: config::Ledger, + args: args::LedgerDbDeleteValue, +) { + use namada::ledger::storage::DB; + + let chain_id = config.chain_id; + let db_path = config.shell.db_dir(&chain_id); + + let mut db = storage::PersistentDB::open(db_path, None); + let latest_block = match db.read_last_block() { + Ok(Some(data)) => { + tracing::info!( + last_height = ?data.height, + "Read the last committed block's data." + ); + data + } + Ok(None) => { + tracing::error!("No block has been committed yet."); + return; + } + Err(reason) => { + tracing::error!(%reason, "Failed to read the last block's data."); + return; + } + }; + + tracing::info!( + key = %args.storage_key, + last_height = ?latest_block.height, + "Deleting value from storage subspace key..." + ); + if let Err(reason) = + db.delete_subspace_val(latest_block.height, &args.storage_key) + { + tracing::error!( + %reason, + key = %args.storage_key, + "Failed to delete value from database." + ); + return; + } + + tracing::debug!("Flushing changes..."); + if let Err(reason) = db.flush(true) { + tracing::error!(%reason, "Failed to flush database changes."); + return; + } + + tracing::info!( + key = %args.storage_key, + "Value successfully deleted from the database." + ); +} + /// Runs and monitors a few concurrent tasks. /// /// This includes: From 7d56535b6993407cbee3a5a316bb63d428416117 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 15 May 2023 15:06:58 +0100 Subject: [PATCH 2/6] Fix docstr --- apps/src/lib/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 0d86acde4a0..7a4ecad6ba7 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -2407,7 +2407,7 @@ pub mod args { } } - /// A transfer to be added to the Ethereum bridge pool. + /// Submit a validator set update protocol tx. #[derive(Clone, Debug)] pub struct SubmitValidatorSetUpdate { /// The query parameters. From a46abb02fab8915baeb91147bedc725dc5e9c6a4 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 15 May 2023 15:39:41 +0100 Subject: [PATCH 3/6] Run e2e test from existing test dir --- tests/src/e2e/helpers.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/src/e2e/helpers.rs b/tests/src/e2e/helpers.rs index ee31084476e..d2fd016c6d9 100644 --- a/tests/src/e2e/helpers.rs +++ b/tests/src/e2e/helpers.rs @@ -46,6 +46,11 @@ where /// `namadac`. pub fn setup_single_node_test() -> Result<(Test, NamadaBgCmd)> { let test = setup::single_node_net()?; + run_single_node_test_from(test) +} + +/// Same as [`setup_single_node_test`], but use a pre-existing test directory. +pub fn run_single_node_test_from(test: Test) -> Result<(Test, NamadaBgCmd)> { let mut ledger = run_as!(test, Who::Validator(0), Bin::Node, &["ledger"], Some(40))?; ledger.exp_string("Namada ledger node started")?; From 68137915ac253181d5c9cc45b37df445e90b1e15 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 15 May 2023 16:28:50 +0100 Subject: [PATCH 4/6] Add borrowed data param to rpc_client_do --- tests/src/e2e/eth_bridge_tests/helpers.rs | 2 +- tests/src/e2e/helpers.rs | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/src/e2e/eth_bridge_tests/helpers.rs b/tests/src/e2e/eth_bridge_tests/helpers.rs index d68135aa629..8722843627a 100644 --- a/tests/src/e2e/eth_bridge_tests/helpers.rs +++ b/tests/src/e2e/eth_bridge_tests/helpers.rs @@ -239,7 +239,7 @@ pub async fn read_erc20_supply( ledger_addr: &str, asset: &EthAddress, ) -> Result> { - rpc_client_do(ledger_addr, |rpc, client| async move { + rpc_client_do(ledger_addr, &(), |rpc, client, ()| async move { let amount = rpc .shell() .eth_bridge() diff --git a/tests/src/e2e/helpers.rs b/tests/src/e2e/helpers.rs index d2fd016c6d9..4f2e8598e81 100644 --- a/tests/src/e2e/helpers.rs +++ b/tests/src/e2e/helpers.rs @@ -30,14 +30,18 @@ use crate::e2e::setup::{Bin, Who, APPS_PACKAGE}; use crate::{run, run_as}; /// Instantiate a new [`HttpClient`] to perform RPC requests with. -pub async fn rpc_client_do(ledger_address: &str, mut action: A) -> R +pub async fn rpc_client_do<'fut, 'usr: 'fut, B, A, F, R>( + ledger_address: &str, + borrowed_data: &'usr B, + mut action: A, +) -> R where - A: FnMut(Rpc, HttpClient) -> F, - F: Future, + A: FnMut(Rpc, HttpClient, &'usr B) -> F, + F: Future + 'fut, { let client = HttpClient::new(ledger_address).expect("Invalid ledger address"); - action(RPC, client).await + action(RPC, client, borrowed_data).await } /// Sets up a test chain with a single validator node running in the background, From 0641897516b3bcdb9dee695544a761e243e53b17 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 15 May 2023 23:21:32 +0100 Subject: [PATCH 5/6] Allow user data other than refs in rpc requests for e2e tests --- tests/src/e2e/helpers.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/src/e2e/helpers.rs b/tests/src/e2e/helpers.rs index 4f2e8598e81..e625b993282 100644 --- a/tests/src/e2e/helpers.rs +++ b/tests/src/e2e/helpers.rs @@ -30,18 +30,20 @@ use crate::e2e::setup::{Bin, Who, APPS_PACKAGE}; use crate::{run, run_as}; /// Instantiate a new [`HttpClient`] to perform RPC requests with. -pub async fn rpc_client_do<'fut, 'usr: 'fut, B, A, F, R>( +pub async fn rpc_client_do<'fut, 'usr, U, A, F, R>( ledger_address: &str, - borrowed_data: &'usr B, + user_data: U, mut action: A, ) -> R where - A: FnMut(Rpc, HttpClient, &'usr B) -> F, + 'usr: 'fut, + U: 'usr, + A: FnMut(Rpc, HttpClient, U) -> F, F: Future + 'fut, { let client = HttpClient::new(ledger_address).expect("Invalid ledger address"); - action(RPC, client, borrowed_data).await + action(RPC, client, user_data).await } /// Sets up a test chain with a single validator node running in the background, From 53011c2f288c52f2ce5ee16c9c029652295240ac Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 15 May 2023 16:29:04 +0100 Subject: [PATCH 6/6] Add test_submit_validator_set_udpate() e2e test --- tests/src/e2e/eth_bridge_tests.rs | 166 ++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) diff --git a/tests/src/e2e/eth_bridge_tests.rs b/tests/src/e2e/eth_bridge_tests.rs index 46ce50ba9a7..32b0fd84d8e 100644 --- a/tests/src/e2e/eth_bridge_tests.rs +++ b/tests/src/e2e/eth_bridge_tests.rs @@ -1,10 +1,13 @@ mod helpers; use std::num::NonZeroU64; +use std::ops::ControlFlow; use std::str::FromStr; +use borsh::BorshDeserialize; use color_eyre::eyre::{eyre, Result}; use namada::eth_bridge::oracle; +use namada::eth_bridge::storage::vote_tallies; use namada::ledger::eth_bridge::{ ContractVersion, Contracts, EthereumBridgeConfig, MinimumConfirmations, UpgradeableContract, @@ -12,14 +15,17 @@ use namada::ledger::eth_bridge::{ use namada::types::address::wnam; use namada::types::ethereum_events::testing::DAI_ERC20_ETH_ADDRESS; use namada::types::ethereum_events::EthAddress; +use namada::types::storage::Epoch; use namada::types::{address, token}; use namada_apps::config::ethereum_bridge; +use namada_apps::control_flow::timeouts::SleepStrategy; use namada_core::ledger::eth_bridge::ADDRESS as BRIDGE_ADDRESS; use namada_core::types::address::Address; use namada_core::types::ethereum_events::{ EthereumEvent, TransferToEthereum, TransferToNamada, }; use namada_core::types::token::Amount; +use tokio::time::{Duration, Instant}; use super::setup::set_ethereum_bridge_mode; use crate::e2e::eth_bridge_tests::helpers::{ @@ -30,6 +36,7 @@ use crate::e2e::eth_bridge_tests::helpers::{ }; use crate::e2e::helpers::{ find_address, find_balance, get_actor_rpc, init_established_account, + rpc_client_do, run_single_node_test_from, }; use crate::e2e::setup; use crate::e2e::setup::constants::{ @@ -1214,3 +1221,162 @@ async fn test_wdai_transfer_established_to_established() -> Result<()> { Ok(()) } + +/// Test that manually submitting a validator set update protocol +/// transaction works. +#[tokio::test] +async fn test_submit_validator_set_udpate() -> Result<()> { + let (test, bg_ledger) = setup_single_validator_test()?; + + let ledger_addr = get_actor_rpc(&test, &Who::Validator(0)); + let rpc_addr = format!("http://{ledger_addr}"); + + // wait for epoch E > 1 to be installed + let instant = Instant::now() + Duration::from_secs(180); + SleepStrategy::Constant(Duration::from_millis(500)) + .timeout(instant, || async { + match rpc_client_do(&rpc_addr, &(), |rpc, client, ()| async move { + rpc.shell().epoch(&client).await.ok() + }) + .await + { + Some(epoch) if epoch.0 > 0 => ControlFlow::Break(()), + _ => ControlFlow::Continue(()), + } + }) + .await?; + + // check that we have a complete proof for E=1 + let valset_upd_keys = vote_tallies::Keys::from(&Epoch(1)); + let seen_key = valset_upd_keys.seen(); + SleepStrategy::Constant(Duration::from_millis(500)) + .timeout(instant, || async { + rpc_client_do( + &rpc_addr, + &seen_key, + |rpc, client, seen_key| async move { + rpc.shell() + .storage_value(&client, None, None, false, seen_key) + .await + .map_or_else( + |_| { + unreachable!( + "By the end of epoch 0, a proof should be \ + available" + ) + }, + |rsp| { + let seen = + bool::try_from_slice(&rsp.data).unwrap(); + assert!( + seen, + "No valset upd complete proof in storage" + ); + ControlFlow::Break(()) + }, + ) + }, + ) + .await + }) + .await?; + + // shut down ledger + let mut ledger = bg_ledger.foreground(); + ledger.send_control('c')?; + ledger.exp_string("Namada ledger node has shut down.")?; + ledger.exp_eof()?; + drop(ledger); + + // delete the valset upd proof for E=1 from storage + for key in &valset_upd_keys { + let key = key.to_string(); + let delete_args = + vec!["ledger", "db-delete-value", "--storage-key", &key]; + let mut delete_cmd = + run_as!(test, Who::Validator(0), Bin::Node, delete_args, Some(30))?; + delete_cmd + .exp_string("Value successfully deleted from the database.")?; + drop(delete_cmd); + } + + // restart the ledger + let (test, _bg_ledger) = run_single_node_test_from(test)?; + + // check that no complete proof is available for E=1 anymore + SleepStrategy::Constant(Duration::from_millis(500)) + .timeout(instant, || async { + rpc_client_do( + &rpc_addr, + &seen_key, + |rpc, client, seen_key| async move { + rpc.shell() + .storage_value(&client, None, None, false, seen_key) + .await + .map_or_else( + |_| unreachable!("The RPC does not error out"), + |rsp| { + assert_eq!( + rsp.info, + format!( + "No value found for key: {seen_key}" + ) + ); + ControlFlow::Break(()) + }, + ) + }, + ) + .await + }) + .await?; + + // submit valset upd vote extension protocol tx for E=1 + let tx_args = vec![ + "validator-set-update", + "--ledger-address", + &ledger_addr, + "--epoch", + "1", + ]; + let mut namadac_tx = + run_as!(test, Who::Validator(0), Bin::Client, tx_args, Some(30))?; + namadac_tx.exp_string("Transaction added to mempool")?; + drop(namadac_tx); + + // check that a complete proof is once more available for E=1 + SleepStrategy::Constant(Duration::from_millis(500)) + .timeout(instant, || async { + rpc_client_do( + &rpc_addr, + &seen_key, + |rpc, client, seen_key| async move { + rpc.shell() + .storage_value(&client, None, None, false, seen_key) + .await + .map_or_else( + |_| ControlFlow::Continue(()), + |rsp| { + if rsp + .info + .starts_with("No value found for key") + { + return ControlFlow::Continue(()); + } + let seen = + bool::try_from_slice(&rsp.data).unwrap(); + assert!( + seen, + "No valset upd complete proof in storage" + ); + ControlFlow::Break(()) + }, + ) + }, + ) + .await + }) + .await?; + + Ok(()) +}