Migrated many e2e tests to integration tests

.changelog/unreleased/improvements/2933-migrated-e2e-to-int.md

@@ -0,0 +1,3 @@
+   This PR moves many e2e tests over to integration test. In the future, it may be possible to move more
+   tests over. Moving some of these tests over revealed issues and these have also been resolved, 
+   including \#2927. ([\#2933](https://github.com/anoma/namada/pull/2933))

.github/workflows/scripts/e2e.json

@@ -6,19 +6,12 @@
     "e2e::ibc_tests::proposal_ibc_token_inflation": 600,
     "e2e::eth_bridge_tests::test_add_to_bridge_pool": 10,
     "e2e::ledger_tests::double_signing_gets_slashed": 12,
-    "e2e::ledger_tests::invalid_transactions": 13,
     "e2e::ledger_tests::ledger_many_txs_in_a_block": 55,
-    "e2e::ledger_tests::ledger_txs_and_queries": 30,
-    "e2e::ledger_tests::masp_txs_and_queries": 82,
     "e2e::ledger_tests::pos_bonds": 77,
-    "e2e::ledger_tests::implicit_account_reveal_pk": 30,
     "e2e::ledger_tests::pos_init_validator": 40,
     "e2e::ledger_tests::proposal_offline": 21,
     "e2e::ledger_tests::rollback": 21,
-    "e2e::ledger_tests::pgf_governance_proposal": 320,
-    "e2e::ledger_tests::proposal_submission": 200,
     "e2e::ledger_tests::proposal_change_shielded_reward": 200,
-    "e2e::pgf_steward_change_commissions": 30,
     "e2e::ledger_tests::run_ledger": 5,
     "e2e::ledger_tests::run_ledger_load_state_and_reset": 23,
     "e2e::ledger_tests::test_namada_shuts_down_if_tendermint_dies": 2,
@@ -27,10 +20,7 @@
     "e2e::ledger_tests::test_epoch_sleep": 12,
     "e2e::ledger_tests::wrapper_disposable_signer": 28,
     "e2e::ledger_tests::deactivate_and_reactivate_validator": 67,
-    "e2e::ledger_tests::change_validator_metadata": 31,
-    "e2e::ledger_tests::pos_rewards": 44,
     "e2e::ledger_tests::test_invalid_validator_txs": 73,
-    "e2e::ledger_tests::test_bond_queries": 95,
     "e2e::ledger_tests::suspend_ledger": 30,
     "e2e::ledger_tests::stop_ledger_at_height": 18,
     "e2e::ledger_tests::change_consensus_key": 91,

crates/apps/src/lib/client/tx.rs

@@ -217,7 +217,8 @@ pub async fn submit_reveal_aux(
         if tx::is_reveal_pk_needed(context.client(), address, args.force)
             .await?
         {
-            println!(
+            display_line!(
+                context.io(),
                 "Submitting a tx to reveal the public key for address \
                  {address}..."
             );

crates/apps/src/lib/node/ledger/mod.rs

@@ -9,16 +9,14 @@ pub mod tendermint_node;
 use std::convert::TryInto;
 use std::net::SocketAddr;
 use std::path::PathBuf;
-use std::str::FromStr;
 use std::thread;
 
 use byte_unit::Byte;
 use data_encoding::HEXUPPER;
 use futures::future::TryFutureExt;
-use namada::core::storage::{BlockHeight, Key};
+use namada::core::storage::BlockHeight;
 use namada::core::time::DateTimeUtc;
 use namada::eth_bridge::ethers::providers::{Http, Provider};
-use namada::governance::storage::keys as governance_storage;
 use namada::state::DB;
 use namada::storage::DbColFam;
 use namada::tendermint::abci::request::CheckTxKind;
@@ -68,34 +66,6 @@ const ENV_VAR_RAYON_THREADS: &str = "NAMADA_RAYON_THREADS";
 //     }
 //```
 impl Shell {
-    fn load_proposals(&mut self) {
-        let proposals_key = governance_storage::get_commiting_proposals_prefix(
-            self.state.in_mem().last_epoch.0,
-        );
-
-        let (proposal_iter, _) = self.state.db_iter_prefix(&proposals_key);
-        for (key, _, _) in proposal_iter {
-            let key =
-                Key::from_str(key.as_str()).expect("Key should be parsable");
-            if governance_storage::get_commit_proposal_epoch(&key).unwrap()
-                != self.state.in_mem().last_epoch.0
-            {
-                // NOTE: `iter_prefix` iterate over the matching prefix. In this
-                // case  a proposal with grace_epoch 110 will be
-                // matched by prefixes  1, 11 and 110. Thus we
-                // have to skip to the next iteration of
-                //  the cycle for all the prefixes that don't actually match
-                //  the desired epoch.
-                continue;
-            }
-
-            let proposal_id = governance_storage::get_commit_proposal_id(&key);
-            if let Some(id) = proposal_id {
-                self.proposal_data.insert(id);
-            }
-        }
-    }
-
     fn call(&mut self, req: Request) -> Result<Response, Error> {
         match req {
             Request::InitChain(init) => {
@@ -131,7 +101,6 @@ impl Shell {
             }
             Request::FinalizeBlock(finalize) => {
                 tracing::debug!("Request FinalizeBlock");
-                self.load_proposals();
                 self.finalize_block(finalize).map(Response::FinalizeBlock)
             }
             Request::Commit => {
@@ -235,7 +204,7 @@ pub fn dump_db(
 #[cfg(feature = "migrations")]
 pub fn query_db(
     config: config::Ledger,
-    key: &Key,
+    key: &namada::core::storage::Key,
     type_hash: &[u8; 32],
     cf: &DbColFam,
 ) {

crates/apps/src/lib/node/ledger/shell/governance.rs

@@ -38,6 +38,7 @@ where
     H: 'static + StorageHasher + Sync,
 {
     if is_new_epoch {
+        shell.load_proposals();
         execute_governance_proposals(shell, events)?;
     }
     Ok(())

crates/apps/src/lib/node/ledger/shell/mod.rs

@@ -554,7 +554,7 @@ where
 
     /// Load the Merkle root hash and the height of the last committed block, if
     /// any. This is returned when ABCI sends an `info` request.
-    pub fn last_state(&mut self) -> response::Info {
+    pub fn last_state(&self) -> response::Info {
         if ledger::migrating_state().is_some() {
             // When migrating state, return a height of 0, such
             // that CometBFT calls InitChain and subsequently
@@ -593,6 +593,38 @@ where
         response
     }
 
+    fn load_proposals(&mut self) {
+        use std::str::FromStr;
+
+        use namada::governance::storage::keys as governance_storage;
+
+        let proposals_key = governance_storage::get_commiting_proposals_prefix(
+            self.state.in_mem().last_epoch.0,
+        );
+
+        let (proposal_iter, _) = self.state.db_iter_prefix(&proposals_key);
+        for (key, _, _) in proposal_iter {
+            let key =
+                Key::from_str(key.as_str()).expect("Key should be parsable");
+            if governance_storage::get_commit_proposal_epoch(&key).unwrap()
+                != self.state.in_mem().last_epoch.0
+            {
+                // NOTE: `iter_prefix` iterate over the matching prefix. In this
+                // case  a proposal with grace_epoch 110 will be
+                // matched by prefixes  1, 11 and 110. Thus we
+                // have to skip to the next iteration of
+                //  the cycle for all the prefixes that don't actually match
+                //  the desired epoch.
+                continue;
+            }
+
+            let proposal_id = governance_storage::get_commit_proposal_id(&key);
+            if let Some(id) = proposal_id {
+                self.proposal_data.insert(id);
+            }
+        }
+    }
+
     /// Read the value for a storage key dropping any error
     pub fn read_storage_key<T>(&self, key: &Key) -> Option<T>
     where

crates/apps/src/lib/node/ledger/shell/testing/node.rs

@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::fmt::{Debug, Formatter};
 use std::future::poll_fn;
 use std::mem::ManuallyDrop;
 use std::path::PathBuf;
@@ -10,6 +11,7 @@ use color_eyre::eyre::{Report, Result};
 use data_encoding::HEXUPPER;
 use itertools::Either;
 use lazy_static::lazy_static;
+use namada::address::Address;
 use namada::control_flow::time::Duration;
 use namada::core::ethereum_events::EthereumEvent;
 use namada::core::ethereum_structs;
@@ -29,7 +31,9 @@ use namada::proof_of_stake::storage::{
     validator_consensus_key_handle,
 };
 use namada::proof_of_stake::types::WeightedValidator;
-use namada::state::{LastBlock, Sha256Hasher, EPOCH_SWITCH_BLOCKS_DELAY};
+use namada::state::{
+    LastBlock, Sha256Hasher, StorageRead, EPOCH_SWITCH_BLOCKS_DELAY,
+};
 use namada::tendermint::abci::response::Info;
 use namada::tendermint::abci::types::VoteInfo;
 use namada_sdk::queries::Client;
@@ -233,7 +237,7 @@ pub fn mock_services(cfg: MockServicesCfg) -> MockServicesPackage {
 }
 
 /// Status of tx
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum NodeResults {
     /// Success
     Ok,
@@ -253,6 +257,14 @@ pub struct MockNode {
     pub auto_drive_services: bool,
 }
 
+impl Debug for MockNode {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("MockNode")
+            .field("shell", &self.shell)
+            .finish()
+    }
+}
+
 impl Drop for MockNode {
     fn drop(&mut self) {
         unsafe {
@@ -354,6 +366,11 @@ impl MockNode {
             .0
     }
 
+    pub fn native_token(&self) -> Address {
+        let locked = self.shell.lock().unwrap();
+        locked.state.get_native_token().unwrap()
+    }
+
     /// Get the address of the block proposer and the votes for the block
     fn prepare_request(&self) -> (Vec<u8>, Vec<VoteInfo>) {
         let (val1, ck) = {
@@ -779,6 +796,10 @@ impl<'a> Client for &'a MockNode {
         } else {
             self.clear_results();
         }
+        // TODO: Figure out why this is necessary and fix it properly
+        // TODO: causes `implicit_account_reveal_pk` and
+        // TODO: `ledger_txs_and_queries` to fail if absent.
+        std::thread::sleep(std::time::Duration::from_secs(2));
         let (proposer_address, _) = self.prepare_request();
         let req = RequestPrepareProposal {
             proposer_address: proposer_address.into(),

crates/apps/src/lib/node/ledger/shell/testing/utils.rs

@@ -1,3 +1,4 @@
+use std::fmt::Display;
 use std::ops::{Deref, DerefMut};
 use std::path::{Path, PathBuf};
 use std::pin::Pin;
@@ -172,15 +173,29 @@ impl<T> CapturedOutput<T> {
         CapturedOutput::of(func)
     }
 
-    /// Check if the captured output contains the regex.
-    pub fn matches(&self, needle: regex::Regex) -> bool {
-        needle.captures(&self.output).is_some()
+    /// Return the first capture of the regex from the output.
+    pub fn matches(&self, needle: &str) -> Option<&str> {
+        let needle = regex::Regex::new(needle).unwrap();
+        needle.find(&self.output).map(|x| x.as_str())
     }
 
     /// Check if the captured output contains the string.
     pub fn contains(&self, needle: &str) -> bool {
+        self.matches(needle).is_some()
+    }
+}
+
+impl<U, E: Display> CapturedOutput<Result<U, E>> {
+    pub fn err_contains(&self, needle: &str) -> bool {
+        if self.result.is_ok() {
+            return false;
+        }
+        let err_str = match self.result.as_ref() {
+            Ok(_) => unreachable!(),
+            Err(e) => e.to_string(),
+        };
         let needle = regex::Regex::new(needle).unwrap();
-        self.matches(needle)
+        needle.find(&err_str).is_some()
     }
 }
 

crates/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs

@@ -218,7 +218,7 @@ pub mod shim {
         #[derive(Debug, Default)]
         pub struct VerifyHeader;
 
-        #[derive(Debug, Default, Clone, PartialEq, Eq)]
+        #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]
         pub struct TxResult {
             pub code: u32,
             pub info: String,

crates/core/src/address.rs

@@ -60,6 +60,8 @@ pub const POS_SLASH_POOL: Address =
     Address::Internal(InternalAddress::PosSlashPool);
 /// Internal Governance address
 pub const GOV: Address = Address::Internal(InternalAddress::Governance);
+/// Internal Public Goods funding address
+pub const PGF: Address = Address::Internal(InternalAddress::Pgf);
 /// Internal MASP address
 pub const MASP: Address = Address::Internal(InternalAddress::Masp);
 /// Internal Multitoken address

crates/proof_of_stake/src/rewards.rs

@@ -475,7 +475,7 @@ where
         credit_tokens(
             storage,
             staking_token,
-            &address::GOV,
+            &address::PGF,
             reward_tokens_remaining,
         )?;
     }

crates/proof_of_stake/src/tests/test_pos.rs

@@ -1397,8 +1397,8 @@ fn test_update_rewards_products_aux(validators: Vec<GenesisValidator>) {
     // Read some data before applying rewards
     let pos_balance_pre =
         read_balance(&s, &staking_token, &address::POS).unwrap();
-    let gov_balance_pre =
-        read_balance(&s, &staking_token, &address::GOV).unwrap();
+    let pgf_balance_pre =
+        read_balance(&s, &staking_token, &address::PGF).unwrap();
 
     let num_consensus_validators = consensus_set.len() as u64;
     let accum_val = Dec::one() / num_consensus_validators;
@@ -1436,17 +1436,17 @@ fn test_update_rewards_products_aux(validators: Vec<GenesisValidator>) {
 
     let pos_balance_post =
         read_balance(&s, &staking_token, &address::POS).unwrap();
-    let gov_balance_post =
-        read_balance(&s, &staking_token, &address::GOV).unwrap();
+    let pgf_balance_post =
+        read_balance(&s, &staking_token, &address::PGF).unwrap();
 
     assert_eq!(
-        pos_balance_pre + gov_balance_pre + inflation,
-        pos_balance_post + gov_balance_post,
-        "Expected inflation to be minted to PoS and left-over amount to Gov"
+        pos_balance_pre + pgf_balance_pre + inflation,
+        pos_balance_post + pgf_balance_post,
+        "Expected inflation to be minted to PoS and left-over amount to PGF"
     );
 
     let pos_credit = pos_balance_post - pos_balance_pre;
-    let gov_credit = gov_balance_post - gov_balance_pre;
+    let gov_credit = pgf_balance_post - pgf_balance_pre;
     assert!(
         pos_credit > gov_credit,
         "PoS must receive more tokens than Gov, but got {} in PoS and {} in \

crates/tests/src/e2e/ibc_tests.rs

@@ -1827,8 +1827,13 @@ fn propose_funding(
     let rpc_a = get_actor_rpc(test_a, Who::Validator(0));
     let epoch = get_epoch(test_a, &rpc_a)?;
     let start_epoch = (epoch.0 + 3) / 3 * 3;
-    let proposal_json_path =
-        prepare_proposal_data(test_a, 0, albert, pgf_funding, start_epoch);
+    let proposal_json_path = prepare_proposal_data(
+        test_a.test_dir.path(),
+        0,
+        albert,
+        pgf_funding,
+        start_epoch,
+    );
 
     let submit_proposal_args = vec![
         "init-proposal",