From b95d50347a03d5c18eac620fa36c318f2d2c2e58 Mon Sep 17 00:00:00 2001 From: Conor Schaefer Date: Mon, 7 Oct 2024 11:50:33 -0700 Subject: [PATCH] tests: integration tests for pmonitor Rewriting the bash script in cargo because I'm running them a lot and will continue to run them a lot before merge. Includes generating genesis allocations, and full validation of happy path. Next up, let's write some test cases we expect to fail. At some point these tests should be refactored to use a test mod.rs so the setup/teardown code is more easily reusable. --- Cargo.lock | 3 + crates/bin/pmonitor/Cargo.toml | 5 + crates/bin/pmonitor/tests/common/mod.rs | 321 ++++++++++++++++++ .../bin/pmonitor/tests/common/pcli_helpers.rs | 63 ++++ .../bin/pmonitor/tests/network_integration.rs | 180 ++++++++++ justfile | 6 +- 6 files changed, 577 insertions(+), 1 deletion(-) create mode 100644 crates/bin/pmonitor/tests/common/mod.rs create mode 100644 crates/bin/pmonitor/tests/common/pcli_helpers.rs create mode 100644 crates/bin/pmonitor/tests/network_integration.rs diff --git a/Cargo.lock b/Cargo.lock index e5a6e00b31..6efbb2e877 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5906,12 +5906,14 @@ name = "pmonitor" version = "0.80.6" dependencies = [ "anyhow", + "assert_cmd", "camino", "clap", "colored", "directories", "futures", "indicatif", + "once_cell", "pcli", "penumbra-app", "penumbra-asset", @@ -5926,6 +5928,7 @@ dependencies = [ "regex", "serde", "serde_json", + "tempfile", "tokio", "toml 0.7.8", "tonic", diff --git a/crates/bin/pmonitor/Cargo.toml b/crates/bin/pmonitor/Cargo.toml index 7a3b304d3e..6d1fe75c8f 100644 --- a/crates/bin/pmonitor/Cargo.toml +++ b/crates/bin/pmonitor/Cargo.toml @@ -39,3 +39,8 @@ tracing = {workspace = true} tracing-subscriber = { workspace = true, features = ["env-filter", "ansi"] } url = {workspace = true, features = ["serde"]} uuid = { version = "1.3", features = ["v4", "serde"] } + +[dev-dependencies] +assert_cmd = {workspace = true} +once_cell = {workspace = true} +tempfile = {workspace = true} diff --git a/crates/bin/pmonitor/tests/common/mod.rs b/crates/bin/pmonitor/tests/common/mod.rs new file mode 100644 index 0000000000..e77ac2089b --- /dev/null +++ b/crates/bin/pmonitor/tests/common/mod.rs @@ -0,0 +1,321 @@ +//! Integration test helpers for `pmonitor`. +//! Contains logic to bootstrap a local devnet, complete with genesis +//! allocations for pre-existing wallets, so that `pmonitor` can audit +//! the behavior of those wallets on the target chain. + +use anyhow::{Context, Result}; +use assert_cmd::Command as AssertCommand; +use once_cell::sync::Lazy; +use pcli::config::PcliConfig; +use penumbra_keys::address::Address; +use std::fs::{create_dir_all, remove_dir_all, File}; +use std::io::{BufWriter, Write}; +use std::path::PathBuf; +use std::process::{Child, Command, Stdio}; +use std::time::Duration; +pub mod pcli_helpers; +use crate::common::pcli_helpers::{pcli_init_softkms, pcli_view_address}; + +/// The TCP port for the process-compose API, used to start/stop devnet. +const PROCESS_COMPOSE_PORT: u16 = 8888; + +/// The path in-repo to the `process-compose` manifest used for running a devnet, +/// relative to the current crate root. This is a minimal manifest, that only runs pd & cometbft. +static PROCESS_COMPOSE_MANIFEST_FILEPATH: Lazy = Lazy::new(|| { + let p: PathBuf = [ + env!("CARGO_MANIFEST_DIR"), + "..", + "..", + "..", + "deployments", + "compose", + "process-compose.yml", + ] + .iter() + .collect(); + p +}); + +/// The path to the root of the git repo, used for setting the working directory +/// when running `process-compose`. +static REPO_ROOT: Lazy = Lazy::new(|| { + let p: PathBuf = [env!("CARGO_MANIFEST_DIR"), "../", "../", "../"] + .iter() + .collect(); + p +}); + +/// Manager for running suites of integration tests for `pmonitor`. +/// Only one instance should exist at a time! The test suites +/// assume access to global resources such as 8080/TCP for pd, +/// and a hardcoded directory in `/tmp/` for the pmonitor configs. +pub struct PmonitorTestRunner { + /// Top-level directory for storing all integration test info, + /// such as wallets and pd network state. + pmonitor_integration_test_dir: PathBuf, + /// How many client wallets to create for testing. + num_wallets: u16, +} + +impl PmonitorTestRunner { + /// Create a new test runner environment. + /// Caller must ensure no other instances exist, because this method + /// will destroy existing test data directories. + pub fn new() -> Self { + // Ideally we'd use a tempdir but using a hardcoded dir for debugging. + let p: PathBuf = ["/tmp", "pmonitor-integration-test"].iter().collect(); + // Nuke any pre-existing state + if p.exists() { + remove_dir_all(&p).expect("failed to remove directory for pmonitor integration tests"); + } + // Ensure parent dir exists; other methods will create subdirs as necessary. + create_dir_all(&p).expect("failed to create directory for pmonitor integration tests"); + Self { + pmonitor_integration_test_dir: p, + num_wallets: 10, + } + } + // Return path for pmonitor home directory. + // Does not create the path, because `pmonitor` will fail if its home already exists. + pub fn pmonitor_home(&self) -> PathBuf { + self.pmonitor_integration_test_dir.join("pmonitor") + } + // Create directory and return path for storing client wallets + pub fn wallets_dir(&self) -> Result { + let p = self.pmonitor_integration_test_dir.join("wallets"); + create_dir_all(&p)?; + Ok(p) + } + + /// Initialize local pcli configs for all wallets specified in config. + pub fn create_pcli_wallets(&self) -> anyhow::Result<()> { + for i in 0..self.num_wallets - 1 { + let pcli_home = self.wallets_dir()?.join(format!("wallet-{}", i)); + pcli_init_softkms(&pcli_home)?; + } + Ok(()) + } + + /// Iterate over all client wallets and return a `PcliConfig` for each. + pub fn get_pcli_wallet_configs(&self) -> anyhow::Result> { + let mut results = Vec::::new(); + for i in 0..self.num_wallets - 1 { + let pcli_home = self.wallets_dir()?.join(format!("wallet-{}", i)); + let pcli_config_path = pcli_home.join("config.toml"); + let pcli_config = PcliConfig::load( + pcli_config_path + .to_str() + .expect("failed to convert pcli wallet path to str"), + )?; + results.push(pcli_config); + } + Ok(results) + } + + /// Iterate over all client wallets and return address 0 for each. + pub fn get_pcli_wallet_addresses(&self) -> anyhow::Result> { + let mut results = Vec::
::new(); + for i in 0..self.num_wallets - 1 { + let pcli_home = self.wallets_dir()?.join(format!("wallet-{}", i)); + let penumbra_address = pcli_view_address(&pcli_home)?; + results.push(penumbra_address); + } + Ok(results) + } + /// Iterate over all client wallets, grab an FVK for each, write those + /// FVKs to a local JSON file, and return the path to that file. + pub fn get_pcli_wallet_fvks_filepath(&self) -> anyhow::Result { + let p = self.pmonitor_integration_test_dir.join("fvks.json"); + if !p.exists() { + // We use a Vec rather than Vec so we get the string + // representations + let fvks: Vec = self + .get_pcli_wallet_configs()? + .into_iter() + .map(|c| c.full_viewing_key.to_string()) + .collect(); + let mut w = BufWriter::new(File::create(&p)?); + serde_json::to_writer(&mut w, &fvks)?; + w.flush()?; + } + Ok(p) + } + + /// Create a CSV file of genesis allocations for all pcli test wallets. + pub fn generate_genesis_allocations(&self) -> anyhow::Result { + let allocations_filepath = self.pmonitor_integration_test_dir.join("allocations.csv"); + + // Generate file contents + if !allocations_filepath.exists() { + let mut w = BufWriter::new(File::create(&allocations_filepath)?); + let csv_header = String::from("amount,denom,address\n"); + w.write(csv_header.as_bytes())?; + for a in self.get_pcli_wallet_addresses()? { + let allo = format!("1_000_000__000_000,upenumbra,{}\n1000,test_usd,{}\n", a, a); + w.write(allo.as_bytes())?; + } + w.flush()?; + } + Ok(allocations_filepath) + } + + /// Create a genesis event for the local devnet, with genesis allocations for all pcli wallets. + /// This is a *destructive* action, as it removes the contents of the default pd network_data + /// directory prior to generation. + pub fn generate_network_data(&self) -> anyhow::Result<()> { + // TODO: it'd be nice if we wrote all this network_data to a tempdir, + // but instead we just reuse the default pd home. + + let reset_cmd = AssertCommand::cargo_bin("pd")? + .args(["network", "unsafe-reset-all"]) + .output(); + assert!( + reset_cmd.unwrap().status.success(), + "failed to clear out prior local devnet config" + ); + + let cmd = AssertCommand::cargo_bin("pd")? + .args([ + "network", + "generate", + "--chain-id", + "penumbra-devnet-pmonitor", + "--unbonding-delay", + "50", + "--epoch-duration", + "50", + "--proposal-voting-blocks", + "50", + "--timeout-commit", + "3s", + // we must opt in to fees, in order to test the migration functionality! + "--gas-price-simple", + "500", + // include allocations for the generated pcli wallets + "--allocations-input-file", + &self + .generate_genesis_allocations()? + .to_str() + .expect("failed to convert allocations csv to str"), + ]) + .output(); + assert!( + cmd.unwrap().status.success(), + "failed to generate local devnet config" + ); + Ok(()) + } + + /// Generate a config directory for `pmonitor`, based on input FVKs. + pub fn initialize_pmonitor(&self) -> anyhow::Result<()> { + let cmd = AssertCommand::cargo_bin("pmonitor")? + .args([ + "--home", + self.pmonitor_home() + .to_str() + .expect("failed to convert pmonitor home to str"), + "init", + "--grpc-url", + "http://127.0.0.1:8080", + "--fvks", + self.get_pcli_wallet_fvks_filepath() + .context("failed to get wallet fvks")? + .to_str() + .expect("failed to convert fvks json filepath to str"), + ]) + .output(); + + assert!( + cmd.unwrap().status.success(), + "failed to initialize pmonitor" + ); + Ok(()) + } + + /// Run `pmonitor audit` based on the pcli wallets and associated FVKs. + pub fn pmonitor_audit(&self) -> anyhow::Result<()> { + let p = self.pmonitor_integration_test_dir.join("pmonitor"); + let cmd = AssertCommand::cargo_bin("pmonitor")? + .args([ + "--home", + p.to_str().expect("failed to convert pmonitor home to str"), + "audit", + ]) + .ok(); + if cmd.is_ok() { + Ok(()) + } else { + anyhow::bail!("failed during 'pmonitor audit'") + } + } + + /// Halt any pre-existing local devnet for these integration tests. + /// We assume that the port `8888` is unique to the process-compose API for this test suite. + fn stop_devnet(&self) -> anyhow::Result<()> { + // Confirm that process-compose is installed, otherwise integration tests can't run. + Command::new("process-compose") + .arg("--help") + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .expect("process-compose is not available on PATH; activate the nix dev env"); + + // Stop an existing devnet on the custom port; ignore error, since we don't know one is + // running. + let cmd = Command::new("process-compose") + .env("PC_PORT_NUM", PROCESS_COMPOSE_PORT.to_string()) + .arg("down") + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + + match cmd { + Ok(_c) => { + tracing::trace!( + "'process-compose down' completed, sleeping briefly during teardown" + ); + + std::thread::sleep(Duration::from_secs(3)); + return Ok(()); + } + Err(_e) => { + tracing::trace!( + "'process-compose down' failed, presumably no prior network running" + ); + Ok(()) + } + } + } + + /// Run a local devnet based on input config. Returns a handle to the spawned process, + /// so that cleanup can be handled gracefully. + /// We assume that the port `8888` is unique to the process-compose API for this test suite. + pub fn start_devnet(&self) -> anyhow::Result { + // Ensure no other instance is currently running; + self.stop_devnet()?; + + self.generate_network_data()?; + + // Stop an existing devnet on the custom port; ignore error, since we don't know one is + // running. + let child = Command::new("process-compose") + .env("PC_PORT_NUM", PROCESS_COMPOSE_PORT.to_string()) + .current_dir(REPO_ROOT.as_os_str()) + .args([ + "up", + "--detached", + "--config", + PROCESS_COMPOSE_MANIFEST_FILEPATH + .to_str() + .expect("failed to convert process-compose manifest to str"), + ]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("failed to execute devnet start cmd"); + // Sleep a bit, to let network start + std::thread::sleep(Duration::from_secs(8)); + Ok(child) + } +} diff --git a/crates/bin/pmonitor/tests/common/pcli_helpers.rs b/crates/bin/pmonitor/tests/common/pcli_helpers.rs new file mode 100644 index 0000000000..c62cc4c057 --- /dev/null +++ b/crates/bin/pmonitor/tests/common/pcli_helpers.rs @@ -0,0 +1,63 @@ +//! Convenience methods for wrangling `pcli` CLI invocations, +//! via `cargo bin` commands, for use in integration testing. + +use anyhow::{Context, Result}; +use assert_cmd::Command as AssertCommand; +use penumbra_keys::{address::Address, FullViewingKey}; +use std::path::PathBuf; +use std::str::FromStr; + +/// Initialize a new pcli wallet at the target directory. +/// Discards the generated seed phrase. +pub fn pcli_init_softkms(pcli_home: &PathBuf) -> Result<()> { + let mut cmd = AssertCommand::cargo_bin("pcli")?; + cmd.args([ + "--home", + pcli_home + .to_str() + .expect("can convert wallet path to string"), + "init", + "--grpc-url", + "http://127.0.0.1:8080", + "soft-kms", + "generate", + ]) + // send empty string to accept the interstitial seed phrase display + .write_stdin(""); + cmd.assert().success(); + Ok(()) +} + +/// Convenience method for looking up `address 0` from +/// pcli wallet stored at `pcli_home`. +pub fn pcli_view_address(pcli_home: &PathBuf) -> Result
{ + let output = AssertCommand::cargo_bin("pcli")? + .args(["--home", pcli_home.to_str().unwrap(), "view", "address"]) + .output() + .expect("failed to retrieve address from pcli wallet"); + + // Convert output to String, to trim trailing newline. + let mut a = String::from_utf8_lossy(&output.stdout).to_string(); + if a.ends_with('\n') { + a.pop(); + } + Address::from_str(&a).with_context(|| format!("failed to convert str to Address: '{}'", a)) +} + +/// Perform a `pcli migrate balance` transaction from the wallet at `pcli_home`, +/// transferring funds to the destination `FullViewingKey`. +pub fn pcli_migrate_balance(pcli_home: &PathBuf, fvk: &FullViewingKey) -> Result<()> { + let mut cmd = AssertCommand::cargo_bin("pcli")?; + cmd.args([ + "--home", + pcli_home + .to_str() + .expect("can convert wallet path to string"), + "migrate", + "balance", + ]) + // pipe FVK to stdin + .write_stdin(fvk.to_string()); + cmd.assert().success(); + Ok(()) +} diff --git a/crates/bin/pmonitor/tests/network_integration.rs b/crates/bin/pmonitor/tests/network_integration.rs new file mode 100644 index 0000000000..b7ad0b81f5 --- /dev/null +++ b/crates/bin/pmonitor/tests/network_integration.rs @@ -0,0 +1,180 @@ +//! Integration integration testing of `pmonitor` against a local devnet. +//! Sets up various scenarios of genesis allocations, and ensures the tool reports +//! violations as errors. +use assert_cmd::Command as AssertCommand; +use pcli::config::PcliConfig; +mod common; +use crate::common::pcli_helpers::{pcli_init_softkms, pcli_migrate_balance, pcli_view_address}; +use crate::common::PmonitorTestRunner; + +#[ignore] +#[test] +/// Tests the simplest happy path for pmonitor: all wallets have genesis balances, +/// they never transferred any funds out, nor migrated balances, so all +/// current balances equal the genesis balances. In this case `pmonitor` +/// should exit 0. +fn audit_passes_on_compliant_wallets() -> anyhow::Result<()> { + tracing_subscriber::fmt::try_init().ok(); + let p = PmonitorTestRunner::new(); + p.create_pcli_wallets()?; + let _network = p.start_devnet()?; + p.initialize_pmonitor()?; + p.pmonitor_audit()?; + Ok(()) +} + +#[ignore] +#[test] +/// Tests another happy path for pmonitor: all wallets have genesis balances, +/// one of the wallets ran `pcli migrate balance` once. This means that all +/// wallets still have their genesis balance, save one, which has the genesis +/// balance minus gas fees. In this case, `pmonitor` should exit 0, +/// because it understood the balance migration and updated the FVK. +fn audit_passes_on_wallets_that_migrated_once() -> anyhow::Result<()> { + let p = PmonitorTestRunner::new(); + p.create_pcli_wallets()?; + let _network = p.start_devnet()?; + // Run audit once, to confirm compliance on clean slate. + p.initialize_pmonitor()?; + p.pmonitor_audit()?; + + // Create an empty wallet, with no genesis funds, to which we'll migrate a balance. + let alice_pcli_home = p.wallets_dir()?.join("wallet-alice"); + pcli_init_softkms(&alice_pcli_home)?; + let alice_pcli_config = PcliConfig::load( + alice_pcli_home + .join("config.toml") + .to_str() + .expect("failed to convert alice wallet to str"), + )?; + + // Take the second wallet, and migrate its balance to Alice. + let migrated_wallet = p.wallets_dir()?.join("wallet-1"); + pcli_migrate_balance(&migrated_wallet, &alice_pcli_config.full_viewing_key)?; + + // Now re-run the audit tool: it should report OK again, because all we did was migrate. + p.pmonitor_audit()?; + Ok(()) +} + +#[ignore] +#[test] +/// Tests an unhappy path for `pmonitor`: a single wallet has sent all its funds +/// to non-genesis account, via `pcli tx send` rather than `pcli migrate balance`. +/// In this case, `pmonitor` should exit non-zero. +fn audit_fails_on_misbehaving_wallet_that_sent_funds() -> anyhow::Result<()> { + let p = PmonitorTestRunner::new(); + p.create_pcli_wallets()?; + let _network = p.start_devnet()?; + // Run audit once, to confirm compliance on clean slate. + p.initialize_pmonitor()?; + p.pmonitor_audit()?; + + // Create an empty wallet, with no genesis funds, to which we'll + // manually send balance. + let alice_pcli_home = p.wallets_dir()?.join("wallet-alice"); + pcli_init_softkms(&alice_pcli_home)?; + + let alice_address = pcli_view_address(&alice_pcli_home)?; + + // Take the second wallet, and send most of its funds of staking tokens to Alice. + let misbehaving_wallet = p.wallets_dir()?.join("wallet-1"); + + let send_cmd = AssertCommand::cargo_bin("pcli")? + .args([ + "--home", + misbehaving_wallet.to_str().unwrap(), + "tx", + "send", + "--to", + &alice_address.to_string(), + "900penumbra", + ]) + .output() + .expect("failed to execute sending tx to alice wallet"); + assert!(send_cmd.status.success(), "failed to send tx to alice"); + + // Debugging: sleep so we can test manually + // std::thread::sleep(Duration::from_secs(5000)); + + // Now re-run the audit tool: it should report failure, via a non-zero exit code, + // because of the missing funds. + let result = p.pmonitor_audit(); + assert!( + result.is_err(), + "expected pmonitor to fail due to missing funds" + ); + Ok(()) +} + +#[ignore] +#[test] +/// Tests a happy path for `pmonitor`: a single wallet has sent all its funds +/// to non-genesis account, via `pcli tx send` rather than `pcli migrate balance`, +/// but the receiving wallet then sent those funds back. +/// In this case, `pmonitor` should exit zero. +fn audit_passes_on_misbehaving_wallet_that_sent_funds_but_got_them_back() -> anyhow::Result<()> { + tracing_subscriber::fmt::try_init().ok(); + let p = PmonitorTestRunner::new(); + p.create_pcli_wallets()?; + let _network = p.start_devnet()?; + // Run audit once, to confirm compliance on clean slate. + p.initialize_pmonitor()?; + p.pmonitor_audit()?; + + // Create an empty wallet, with no genesis funds, to which we'll + // manually send balance. + let alice_pcli_home = p.wallets_dir()?.join("wallet-alice"); + pcli_init_softkms(&alice_pcli_home)?; + + let alice_address = pcli_view_address(&alice_pcli_home)?; + + // Take the second wallet, and send most of its funds of staking tokens to Alice. + let misbehaving_wallet = p.wallets_dir()?.join("wallet-1"); + + let send_cmd = AssertCommand::cargo_bin("pcli")? + .args([ + "--home", + misbehaving_wallet.to_str().unwrap(), + "tx", + "send", + "--to", + &alice_address.to_string(), + "900penumbra", + ]) + .output() + .expect("failed to execute sending tx to alice wallet"); + assert!(send_cmd.status.success(), "failed to send tx to alice"); + + // The audit tool detects this state as a failure, since funds are missing. + let result = p.pmonitor_audit(); + assert!( + result.is_err(), + "expected pmonitor to fail due to missing funds" + ); + + // Send the funds from alice back to the misbehaving wallet. + let misbehaving_address = pcli_view_address(&misbehaving_wallet)?; + let refund_cmd = AssertCommand::cargo_bin("pcli")? + .args([ + "--home", + alice_pcli_home.to_str().unwrap(), + "tx", + "send", + "--to", + &misbehaving_address.to_string(), + // We intentionally specify a bit less than we received, to account for gas. + "899.99penumbra", + ]) + .output() + .expect("failed to execute refund tx from alice wallet"); + assert!( + refund_cmd.status.success(), + "failed to send refund tx from alice" + ); + + // The audit tool detects this state as compliant again, because the funds were returned. + p.pmonitor_audit()?; + + Ok(()) +} diff --git a/justfile b/justfile index b352b05c6c..4ecc0e9d79 100644 --- a/justfile +++ b/justfile @@ -4,7 +4,11 @@ default: # Run integration tests for pmonitor tool test-pmonitor: - ./deployments/scripts/pmonitor-integration-test.sh + # warm cache + cargo build --release --bin pd + cargo run --release --bin pd -- network unsafe-reset-all || true + rm -rf /tmp/pmonitor-integration-test + cargo nextest run -p pmonitor --run-ignored=ignored-only --test-threads 1 # Creates and runs a local devnet with solo validator. Includes ancillary services # like metrics, postgres for storing ABCI events, and pindexer for munging those events.