Skip to content

Commit

Permalink
Merge branch 'bat/feat/migrate-e2e-to-int' (#2933)
Browse files Browse the repository at this point in the history
* origin/bat/feat/migrate-e2e-to-int:
  Update .changelog/unreleased/improvements/2933-migrated-e2e-to-int.md
  Formatting
  Formatting
  Putting duct tape over a mysterious race condition in integrations tests
  Added logging to MockNode for when broadcasting a tx fails
  Added logging to MockNode for when broadcasting a tx fails
  Rebuilding tests wasms, updating gitflows
  [fix]: Fixing broken unit test
  formatting
  Rebasinga and adding changelog
  [feat]: Migrated many e2e test to integration tests

# Conflicts:
#	crates/apps/src/lib/node/ledger/shell/governance.rs
#	crates/apps/src/lib/node/ledger/shell/testing/node.rs
#	crates/tests/src/e2e/ledger_tests.rs
  • Loading branch information
tzemanovic committed Apr 11, 2024
2 parents 64bee8c + 2468321 commit df9db29
Show file tree
Hide file tree
Showing 23 changed files with 1,905 additions and 2,022 deletions.
Original file line number Diff line number Diff line change
@@ -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))
10 changes: 0 additions & 10 deletions .github/workflows/scripts/e2e.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,11 @@
"e2e::ibc_tests::ibc_rate_limit": 500,
"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::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,
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/apps/src/lib/client/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,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}..."
);
Expand Down
4 changes: 2 additions & 2 deletions crates/apps/src/lib/node/ledger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ 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::state::DB;
Expand Down Expand Up @@ -208,7 +208,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,
) {
Expand Down
2 changes: 1 addition & 1 deletion crates/apps/src/lib/node/ledger/shell/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,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
Expand Down
43 changes: 35 additions & 8 deletions crates/apps/src/lib/node/ledger/shell/testing/node.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::fmt::{Debug, Formatter};
use std::future::poll_fn;
use std::mem::ManuallyDrop;
use std::path::PathBuf;
Expand All @@ -9,6 +10,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::collections::HashMap;
use namada::core::ethereum_events::EthereumEvent;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -356,6 +368,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) = {
Expand Down Expand Up @@ -638,6 +655,14 @@ impl MockNode {
.all(|r| *r == NodeResults::Ok)
}

/// Return a tx result if the tx failed in mempool
pub fn is_broadcast_err(&self) -> Option<TxResult> {
self.results.lock().unwrap().iter().find_map(|r| match r {
NodeResults::Ok | NodeResults::Failed(_) => None,
NodeResults::Rejected(tx_result) => Some(tx_result.clone()),
})
}

pub fn clear_results(&self) {
self.results.lock().unwrap().clear();
}
Expand Down Expand Up @@ -761,13 +786,15 @@ impl<'a> Client for &'a MockNode {
};
let tx_bytes: Vec<u8> = tx.into();
self.submit_txs(vec![tx_bytes]);
if !self.success() {
// TODO: submit_txs should return the correct error code + message
resp.code = 1337.into();
return Ok(resp);
} else {
self.clear_results();

// If the error happened during broadcasting, attach its result to
// response
if let Some(TxResult { code, info }) = self.is_broadcast_err() {
resp.code = code.into();
resp.log = info;
}

self.clear_results();
Ok(resp)
}

Expand Down
23 changes: 19 additions & 4 deletions crates/apps/src/lib/node/ledger/shell/testing/utils.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::fmt::Display;
use std::ops::{Deref, DerefMut};
use std::path::{Path, PathBuf};
use std::pin::Pin;
Expand Down Expand Up @@ -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()
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions crates/core/src/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,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
Expand Down
2 changes: 1 addition & 1 deletion crates/proof_of_stake/src/rewards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ where
credit_tokens(
storage,
staking_token,
&address::GOV,
&address::PGF,
reward_tokens_remaining,
)?;
}
Expand Down
16 changes: 8 additions & 8 deletions crates/proof_of_stake/src/tests/test_pos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1398,8 +1398,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;
Expand Down Expand Up @@ -1437,17 +1437,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 \
Expand Down
2 changes: 2 additions & 0 deletions crates/tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ tracing.workspace = true
namada_apps = {path = "../apps", features = ["testing"]}
namada_vm_env = {path = "../vm_env"}
assert_cmd.workspace = true
assert_matches.workspace = true
borsh.workspace = true
borsh-ext.workspace = true
color-eyre.workspace = true
Expand All @@ -79,6 +80,7 @@ pretty_assertions.workspace = true
proptest.workspace = true
proptest-state-machine.workspace = true
rand.workspace = true
test-log.workspace = true
toml.workspace = true

# This is used to enable logging from tests
Expand Down
9 changes: 7 additions & 2 deletions crates/tests/src/e2e/ibc_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1865,8 +1865,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",
Expand Down
Loading

0 comments on commit df9db29

Please sign in to comment.