From ee92645e050f552bee9acdd17ba6468a624c710c Mon Sep 17 00:00:00 2001 From: Rano | Ranadeep Date: Fri, 19 Jan 2024 08:35:16 -0500 Subject: [PATCH] imp(ibc-testkit): deprecate `MockContext` multiple init methods (#1047) * deprecate default arg init methods in MockContext * refactor tests * add changelog * chore: trim redundant method calls, opting for default values --------- Co-authored-by: Farhad Shabani --- .../1042-deprecate-mockcontext-new-methods.md | 2 + ibc-testkit/src/fixtures/core/context.rs | 2 + ibc-testkit/src/relayer/context.rs | 47 ++--- ibc-testkit/src/testapp/ibc/core/types.rs | 131 +++++++------- .../tests/core/ics02_client/update_client.rs | 168 +++++++++--------- .../core/ics03_connection/conn_open_ack.rs | 13 +- .../core/ics03_connection/conn_open_try.rs | 13 +- 7 files changed, 186 insertions(+), 190 deletions(-) create mode 100644 .changelog/unreleased/improvements/1042-deprecate-mockcontext-new-methods.md diff --git a/.changelog/unreleased/improvements/1042-deprecate-mockcontext-new-methods.md b/.changelog/unreleased/improvements/1042-deprecate-mockcontext-new-methods.md new file mode 100644 index 000000000..7acac9522 --- /dev/null +++ b/.changelog/unreleased/improvements/1042-deprecate-mockcontext-new-methods.md @@ -0,0 +1,2 @@ +- [ibc-testkit] Deprecate `MockContext::new*` in favor of `MockContextConfig`. + ([\#1042](https://github.com/cosmos/ibc-rs/issues/1042)) diff --git a/ibc-testkit/src/fixtures/core/context.rs b/ibc-testkit/src/fixtures/core/context.rs index bf3ed2a7d..a886777e7 100644 --- a/ibc-testkit/src/fixtures/core/context.rs +++ b/ibc-testkit/src/fixtures/core/context.rs @@ -21,6 +21,7 @@ pub struct MockContextConfig { #[builder(default = HostType::Mock)] host_type: HostType, + #[builder(default = ChainId::new("mockgaia-0").expect("Never fails"))] host_id: ChainId, #[builder(default = Duration::from_secs(DEFAULT_BLOCK_TIME_SECS))] @@ -33,6 +34,7 @@ pub struct MockContextConfig { #[builder(default, setter(strip_option))] validator_set_history: Option>>, + #[builder(default = Height::new(0, 5).expect("Never fails"))] latest_height: Height, #[builder(default = Timestamp::now())] diff --git a/ibc-testkit/src/relayer/context.rs b/ibc-testkit/src/relayer/context.rs index d2576f264..c80737603 100644 --- a/ibc-testkit/src/relayer/context.rs +++ b/ibc-testkit/src/relayer/context.rs @@ -52,12 +52,12 @@ mod tests { use tracing::debug; use super::RelayerContext; + use crate::fixtures::core::context::MockContextConfig; use crate::hosts::block::{HostBlock, HostType}; use crate::relayer::context::ClientId; use crate::relayer::error::RelayerError; use crate::testapp::ibc::clients::mock::client_state::client_type as mock_client_type; use crate::testapp::ibc::core::router::MockRouter; - use crate::testapp::ibc::core::types::MockContext; /// Builds a `ClientMsg::UpdateClient` for a client with id `client_id` running on the `dest` /// context, assuming that the latest header on the source context is `src_header`. @@ -121,31 +121,32 @@ mod tests { let chain_id_b = ChainId::new("mockgaiaB-1").unwrap(); // Create two mock contexts, one for each chain. - let mut ctx_a = - MockContext::new(chain_id_a.clone(), HostType::Mock, 5, chain_a_start_height) - .with_client_parametrized_with_chain_id( - chain_id_b.clone(), - &client_on_a_for_b, - client_on_a_for_b_height, - Some(tm_client_type()), // The target host chain (B) is synthetic TM. - Some(client_on_a_for_b_height), - ); + let mut ctx_a = MockContextConfig::builder() + .host_id(chain_id_a.clone()) + .latest_height(chain_a_start_height) + .build() + .with_client_parametrized_with_chain_id( + chain_id_b.clone(), + &client_on_a_for_b, + client_on_a_for_b_height, + Some(tm_client_type()), // The target host chain (B) is synthetic TM. + Some(client_on_a_for_b_height), + ); // dummy; not actually used in client updates let mut router_a = MockRouter::new_with_transfer(); - let mut ctx_b = MockContext::new( - chain_id_b, - HostType::SyntheticTendermint, - 5, - chain_b_start_height, - ) - .with_client_parametrized_with_chain_id( - chain_id_a, - &client_on_b_for_a, - client_on_b_for_a_height, - Some(mock_client_type()), // The target host chain is mock. - Some(client_on_b_for_a_height), - ); + let mut ctx_b = MockContextConfig::builder() + .host_id(chain_id_b) + .host_type(HostType::SyntheticTendermint) + .latest_height(chain_b_start_height) + .build() + .with_client_parametrized_with_chain_id( + chain_id_a, + &client_on_b_for_a, + client_on_b_for_a_height, + Some(mock_client_type()), // The target host chain is mock. + Some(client_on_b_for_a_height), + ); // dummy; not actually used in client updates let mut router_b = MockRouter::new_with_transfer(); diff --git a/ibc-testkit/src/testapp/ibc/core/types.rs b/ibc-testkit/src/testapp/ibc/core/types.rs index 64f39a6ff..e9c914cb1 100644 --- a/ibc-testkit/src/testapp/ibc/core/types.rs +++ b/ibc-testkit/src/testapp/ibc/core/types.rs @@ -33,6 +33,7 @@ use super::client_ctx::{MockClientRecord, PortChannelIdMap}; use crate::fixtures::clients::tendermint::{ dummy_tm_client_state_from_header, ClientStateConfig as TmClientStateConfig, }; +use crate::fixtures::core::context::MockContextConfig; use crate::hosts::block::{HostBlock, HostType}; use crate::relayer::error::RelayerError; use crate::testapp::ibc::clients::mock::client_state::{ @@ -147,12 +148,7 @@ pub struct MockClientConfig { /// creation of new domain objects. impl Default for MockContext { fn default() -> Self { - Self::new( - ChainId::new("mockgaia-0").expect("Never fails"), - HostType::Mock, - 5, - Height::new(0, 5).expect("Never fails"), - ) + MockContextConfig::builder().build() } } @@ -183,6 +179,10 @@ impl MockContext { /// the chain maintain in its history, which also determines the pruning window. Parameter /// `latest_height` determines the current height of the chain. This context /// has support to emulate two type of underlying chains: Mock or SyntheticTendermint. + #[deprecated( + since = "0.49.2", + note = "Please use `MockContextConfig::builder().build()` instead" + )] pub fn new( host_id: ChainId, host_type: HostType, @@ -239,6 +239,10 @@ impl MockContext { /// Note: the validator history is used accordingly for current validator set and next validator set. /// `validator_history[i]` and `validator_history[i+1]` is i'th block's current and next validator set. /// The number of blocks will be `validator_history.len() - 1` due to the above. + #[deprecated( + since = "0.49.2", + note = "Please use `MockContextConfig::builder().build()` instead" + )] pub fn new_with_validator_history( host_id: ChainId, host_type: HostType, @@ -816,93 +820,88 @@ mod tests { let tests: Vec = vec![ Test { name: "Empty history, small pruning window".to_string(), - ctx: MockContext::new( - mock_chain_id.clone(), - HostType::Mock, - 2, - Height::new(cv, 1).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id.clone()) + .max_history_size(2) + .latest_height(Height::new(cv, 1).expect("Never fails")) + .build(), }, Test { name: "[Synthetic TM host] Empty history, small pruning window".to_string(), - ctx: MockContext::new( - mock_chain_id.clone(), - HostType::SyntheticTendermint, - 2, - Height::new(cv, 1).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id.clone()) + .host_type(HostType::SyntheticTendermint) + .max_history_size(2) + .latest_height(Height::new(cv, 1).expect("Never fails")) + .build(), }, Test { name: "Large pruning window".to_string(), - ctx: MockContext::new( - mock_chain_id.clone(), - HostType::Mock, - 30, - Height::new(cv, 2).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id.clone()) + .max_history_size(30) + .latest_height(Height::new(cv, 2).expect("Never fails")) + .build(), }, Test { name: "[Synthetic TM host] Large pruning window".to_string(), - ctx: MockContext::new( - mock_chain_id.clone(), - HostType::SyntheticTendermint, - 30, - Height::new(cv, 2).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id.clone()) + .host_type(HostType::SyntheticTendermint) + .max_history_size(30) + .latest_height(Height::new(cv, 2).expect("Never fails")) + .build(), }, Test { name: "Small pruning window".to_string(), - ctx: MockContext::new( - mock_chain_id.clone(), - HostType::Mock, - 3, - Height::new(cv, 30).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id.clone()) + .max_history_size(3) + .latest_height(Height::new(cv, 30).expect("Never fails")) + .build(), }, Test { name: "[Synthetic TM host] Small pruning window".to_string(), - ctx: MockContext::new( - mock_chain_id.clone(), - HostType::SyntheticTendermint, - 3, - Height::new(cv, 30).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id.clone()) + .host_type(HostType::SyntheticTendermint) + .max_history_size(3) + .latest_height(Height::new(cv, 30).expect("Never fails")) + .build(), }, Test { name: "Small pruning window, small starting height".to_string(), - ctx: MockContext::new( - mock_chain_id.clone(), - HostType::Mock, - 3, - Height::new(cv, 2).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id.clone()) + .max_history_size(3) + .latest_height(Height::new(cv, 2).expect("Never fails")) + .build(), }, Test { name: "[Synthetic TM host] Small pruning window, small starting height".to_string(), - ctx: MockContext::new( - mock_chain_id.clone(), - HostType::SyntheticTendermint, - 3, - Height::new(cv, 2).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id.clone()) + .host_type(HostType::SyntheticTendermint) + .max_history_size(3) + .latest_height(Height::new(cv, 2).expect("Never fails")) + .build(), }, Test { name: "Large pruning window, large starting height".to_string(), - ctx: MockContext::new( - mock_chain_id.clone(), - HostType::Mock, - 50, - Height::new(cv, 2000).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id.clone()) + .max_history_size(50) + .latest_height(Height::new(cv, 2000).expect("Never fails")) + .build(), }, Test { name: "[Synthetic TM host] Large pruning window, large starting height".to_string(), - ctx: MockContext::new( - mock_chain_id, - HostType::SyntheticTendermint, - 50, - Height::new(cv, 2000).expect("Never fails"), - ), + ctx: MockContextConfig::builder() + .host_id(mock_chain_id) + .host_type(HostType::SyntheticTendermint) + .max_history_size(50) + .latest_height(Height::new(cv, 2000).expect("Never fails")) + .build(), }, ]; diff --git a/ibc-testkit/tests/core/ics02_client/update_client.rs b/ibc-testkit/tests/core/ics02_client/update_client.rs index ab04489ec..95c983f08 100644 --- a/ibc-testkit/tests/core/ics02_client/update_client.rs +++ b/ibc-testkit/tests/core/ics02_client/update_client.rs @@ -203,23 +203,25 @@ fn test_update_synthetic_tendermint_client_adjacent_ok() { let update_height = Height::new(1, 21).unwrap(); let chain_id_b = ChainId::new("mockgaiaB-1").unwrap(); - let mut ctx = MockContext::new( - ChainId::new("mockgaiaA-1").unwrap(), - HostType::Mock, - 5, - Height::new(1, 1).unwrap(), - ) - .with_client_parametrized_with_chain_id( - chain_id_b.clone(), - &client_id, - client_height, - Some(tm_client_type()), // The target host chain (B) is synthetic TM. - Some(client_height), - ); + let mut ctx = MockContextConfig::builder() + .host_id(ChainId::new("mockgaiaA-1").unwrap()) + .latest_height(Height::new(1, 1).unwrap()) + .build() + .with_client_parametrized_with_chain_id( + chain_id_b.clone(), + &client_id, + client_height, + Some(tm_client_type()), // The target host chain (B) is synthetic TM. + Some(client_height), + ); let mut router = MockRouter::new_with_transfer(); - let ctx_b = MockContext::new(chain_id_b, HostType::SyntheticTendermint, 5, update_height); + let ctx_b = MockContextConfig::builder() + .host_id(chain_id_b) + .host_type(HostType::SyntheticTendermint) + .latest_height(update_height) + .build(); let signer = dummy_account_id(); @@ -433,23 +435,25 @@ fn test_update_synthetic_tendermint_client_non_adjacent_ok() { let update_height = Height::new(1, 21).unwrap(); let chain_id_b = ChainId::new("mockgaiaB-1").unwrap(); - let mut ctx = MockContext::new( - ChainId::new("mockgaiaA-1").unwrap(), - HostType::Mock, - 5, - Height::new(1, 1).unwrap(), - ) - .with_client_parametrized_history_with_chain_id( - chain_id_b.clone(), - &client_id, - client_height, - Some(tm_client_type()), // The target host chain (B) is synthetic TM. - Some(client_height), - ); + let mut ctx = MockContextConfig::builder() + .host_id(ChainId::new("mockgaiaA-1").unwrap()) + .latest_height(Height::new(1, 1).unwrap()) + .build() + .with_client_parametrized_history_with_chain_id( + chain_id_b.clone(), + &client_id, + client_height, + Some(tm_client_type()), // The target host chain (B) is synthetic TM. + Some(client_height), + ); let mut router = MockRouter::new_with_transfer(); - let ctx_b = MockContext::new(chain_id_b, HostType::SyntheticTendermint, 5, update_height); + let ctx_b = MockContextConfig::builder() + .host_id(chain_id_b) + .host_type(HostType::SyntheticTendermint) + .latest_height(update_height) + .build(); let signer = dummy_account_id(); @@ -491,7 +495,10 @@ fn test_update_synthetic_tendermint_client_duplicate_ok() { let ctx_b_chain_id = ChainId::new("mockgaiaB-1").unwrap(); let start_height = Height::new(1, 11).unwrap(); - let mut ctx_a = MockContext::new(ctx_a_chain_id, HostType::Mock, 5, start_height) + let mut ctx_a = MockContextConfig::builder() + .host_id(ctx_a_chain_id) + .latest_height(start_height) + .build() .with_client_parametrized_with_chain_id( ctx_b_chain_id.clone(), &client_id, @@ -502,12 +509,11 @@ fn test_update_synthetic_tendermint_client_duplicate_ok() { let mut router_a = MockRouter::new_with_transfer(); - let ctx_b = MockContext::new( - ctx_b_chain_id, - HostType::SyntheticTendermint, - 5, - client_height, - ); + let ctx_b = MockContextConfig::builder() + .host_id(ctx_b_chain_id) + .host_type(HostType::SyntheticTendermint) + .latest_height(client_height) + .build(); let signer = dummy_account_id(); @@ -617,27 +623,24 @@ fn test_update_synthetic_tendermint_client_lower_height() { let chain_start_height = Height::new(1, 11).unwrap(); - let ctx = MockContext::new( - ChainId::new("mockgaiaA-1").unwrap(), - HostType::Mock, - 5, - chain_start_height, - ) - .with_client_parametrized( - &client_id, - client_height, - Some(tm_client_type()), // The target host chain (B) is synthetic TM. - Some(client_height), - ); + let ctx = MockContextConfig::builder() + .host_id(ChainId::new("mockgaiaA-1").unwrap()) + .latest_height(chain_start_height) + .build() + .with_client_parametrized( + &client_id, + client_height, + Some(tm_client_type()), // The target host chain (B) is synthetic TM. + Some(client_height), + ); let router = MockRouter::new_with_transfer(); - let ctx_b = MockContext::new( - ChainId::new("mockgaiaB-1").unwrap(), - HostType::SyntheticTendermint, - 5, - client_height, - ); + let ctx_b = MockContextConfig::builder() + .host_id(ChainId::new("mockgaiaB-1").unwrap()) + .host_type(HostType::SyntheticTendermint) + .latest_height(client_height) + .build(); let signer = dummy_account_id(); @@ -774,29 +777,26 @@ fn test_misbehaviour_synthetic_tendermint_equivocation() { let chain_id_b = ChainId::new("mockgaiaB-1").unwrap(); // Create a mock context for chain-A with a synthetic tendermint light client for chain-B - let mut ctx_a = MockContext::new( - ChainId::new("mockgaiaA-1").unwrap(), - HostType::Mock, - 5, - Height::new(1, 1).unwrap(), - ) - .with_client_parametrized_with_chain_id( - chain_id_b.clone(), - &client_id, - client_height, - Some(tm_client_type()), - Some(client_height), - ); + let mut ctx_a = MockContextConfig::builder() + .host_id(ChainId::new("mockgaiaA-1").unwrap()) + .latest_height(Height::new(1, 1).unwrap()) + .build() + .with_client_parametrized_with_chain_id( + chain_id_b.clone(), + &client_id, + client_height, + Some(tm_client_type()), + Some(client_height), + ); let mut router_a = MockRouter::new_with_transfer(); // Create a mock context for chain-B - let ctx_b = MockContext::new( - chain_id_b.clone(), - HostType::SyntheticTendermint, - 5, - misbehaviour_height, - ); + let ctx_b = MockContextConfig::builder() + .host_id(chain_id_b.clone()) + .host_type(HostType::SyntheticTendermint) + .latest_height(misbehaviour_height) + .build(); // Get chain-B's header at `misbehaviour_height` let header1: TmHeader = { @@ -838,19 +838,17 @@ fn test_misbehaviour_synthetic_tendermint_bft_time() { let chain_id_b = ChainId::new("mockgaiaB-1").unwrap(); // Create a mock context for chain-A with a synthetic tendermint light client for chain-B - let mut ctx_a = MockContext::new( - ChainId::new("mockgaiaA-1").unwrap(), - HostType::Mock, - 5, - Height::new(1, 1).unwrap(), - ) - .with_client_parametrized_with_chain_id( - chain_id_b.clone(), - &client_id, - client_height, - Some(tm_client_type()), - Some(client_height), - ); + let mut ctx_a = MockContextConfig::builder() + .host_id(ChainId::new("mockgaiaA-1").unwrap()) + .latest_height(Height::new(1, 1).unwrap()) + .build() + .with_client_parametrized_with_chain_id( + chain_id_b.clone(), + &client_id, + client_height, + Some(tm_client_type()), + Some(client_height), + ); let mut router_a = MockRouter::new_with_transfer(); diff --git a/ibc-testkit/tests/core/ics03_connection/conn_open_ack.rs b/ibc-testkit/tests/core/ics03_connection/conn_open_ack.rs index 62f0ca2f3..1f7dd7828 100644 --- a/ibc-testkit/tests/core/ics03_connection/conn_open_ack.rs +++ b/ibc-testkit/tests/core/ics03_connection/conn_open_ack.rs @@ -14,8 +14,8 @@ use ibc::core::host::ValidationContext; use ibc::core::primitives::prelude::*; use ibc::core::primitives::ZERO_DURATION; use ibc_testkit::fixtures::core::connection::dummy_msg_conn_open_ack; +use ibc_testkit::fixtures::core::context::MockContextConfig; use ibc_testkit::fixtures::{Expect, Fixture}; -use ibc_testkit::hosts::block::HostType; use ibc_testkit::testapp::ibc::core::router::MockRouter; use ibc_testkit::testapp::ibc::core::types::MockContext; use test_log::test; @@ -38,7 +38,6 @@ fn conn_open_ack_fixture(ctx: Ctx) -> Fixture { // Parametrize the host chain to have a height at least as recent as the // the height of the proofs in the Ack msg. let latest_height = proof_height.increment(); - let max_history_size = 5; // A connection end that will exercise the successful path. let default_conn_end = ConnectionEnd::new( @@ -59,12 +58,10 @@ fn conn_open_ack_fixture(ctx: Ctx) -> Fixture { conn_end_open.set_state(State::Open); // incorrect field let ctx_default = MockContext::default(); - let ctx_new = MockContext::new( - ChainId::new(&format!("mockgaia-{}", latest_height.revision_number())).unwrap(), - HostType::Mock, - max_history_size, - latest_height, - ); + let ctx_new = MockContextConfig::builder() + .host_id(ChainId::new(&format!("mockgaia-{}", latest_height.revision_number())).unwrap()) + .latest_height(latest_height) + .build(); let ctx = match ctx { Ctx::New => ctx_new, Ctx::NewWithConnection => ctx_new diff --git a/ibc-testkit/tests/core/ics03_connection/conn_open_try.rs b/ibc-testkit/tests/core/ics03_connection/conn_open_try.rs index 63487221f..18c700ba4 100644 --- a/ibc-testkit/tests/core/ics03_connection/conn_open_try.rs +++ b/ibc-testkit/tests/core/ics03_connection/conn_open_try.rs @@ -4,12 +4,11 @@ use ibc::core::connection::types::State; use ibc::core::entrypoint::{execute, validate}; use ibc::core::handler::types::events::{IbcEvent, MessageEvent}; use ibc::core::handler::types::msgs::MsgEnvelope; -use ibc::core::host::types::identifiers::ChainId; use ibc::core::host::ValidationContext; use ibc::core::primitives::prelude::*; use ibc_testkit::fixtures::core::connection::dummy_msg_conn_open_try; +use ibc_testkit::fixtures::core::context::MockContextConfig; use ibc_testkit::fixtures::{Expect, Fixture}; -use ibc_testkit::hosts::block::HostType; use ibc_testkit::testapp::ibc::core::router::MockRouter; use ibc_testkit::testapp::ibc::core::types::MockContext; use test_log::test; @@ -51,12 +50,10 @@ fn conn_open_try_fixture(ctx_variant: Ctx, msg_variant: Msg) -> Fixture MockContext::default(), Ctx::WithClient => ctx_new.with_client(