diff --git a/.changelog/unreleased/bug-fixes/ibc/1706-fix-formatting-for-some-tendermint-errors.md b/.changelog/unreleased/bug-fixes/ibc/1706-fix-formatting-for-some-tendermint-errors.md new file mode 100644 index 0000000000..7fcb55e782 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/ibc/1706-fix-formatting-for-some-tendermint-errors.md @@ -0,0 +1,3 @@ +- Fixed the formatting of NotEnoughTimeElapsed and NotEnoughBlocksElapsed + in tendermint errors ([#1706](https://github.com/informalsystems/ibc- + rs/issues/1706)) \ No newline at end of file diff --git a/.changelog/unreleased/improvements/ibc/1706-add-client-state-tests.md b/.changelog/unreleased/improvements/ibc/1706-add-client-state-tests.md new file mode 100644 index 0000000000..4fb330d8ab --- /dev/null +++ b/.changelog/unreleased/improvements/ibc/1706-add-client-state-tests.md @@ -0,0 +1,2 @@ +- Added more unit tests to verify Tendermint ClientState + ([#1706](https://github.com/informalsystems/ibc-rs/issues/1706)) \ No newline at end of file diff --git a/modules/src/clients/ics07_tendermint/client_state.rs b/modules/src/clients/ics07_tendermint/client_state.rs index 0341bc1a64..b4739aed94 100644 --- a/modules/src/clients/ics07_tendermint/client_state.rs +++ b/modules/src/clients/ics07_tendermint/client_state.rs @@ -330,9 +330,11 @@ impl From for RawClientState { #[cfg(test)] mod tests { use crate::prelude::*; + use crate::Height; use core::time::Duration; use test_log::test; + use ibc_proto::ics23::ProofSpec as Ics23ProofSpec; use tendermint_rpc::endpoint::abci_query::AbciQuery; use crate::clients::ics07_tendermint::client_state::{AllowUpdate, ClientState}; @@ -340,8 +342,20 @@ mod tests { use crate::core::ics23_commitment::specs::ProofSpecs; use crate::core::ics24_host::identifier::ChainId; use crate::test::test_serialization_roundtrip; - use crate::timestamp::ZERO_DURATION; - use crate::Height; + use crate::timestamp::{Timestamp, ZERO_DURATION}; + + #[derive(Clone, Debug, PartialEq)] + struct ClientStateParams { + id: ChainId, + trust_level: TrustThreshold, + trusting_period: Duration, + unbonding_period: Duration, + max_clock_drift: Duration, + latest_height: Height, + proof_specs: ProofSpecs, + upgrade_path: Vec, + allow_update: AllowUpdate, + } #[test] fn serialization_roundtrip_no_proof() { @@ -359,19 +373,6 @@ mod tests { #[test] fn client_state_new() { - #[derive(Clone, Debug, PartialEq)] - struct ClientStateParams { - id: ChainId, - trust_level: TrustThreshold, - trusting_period: Duration, - unbonding_period: Duration, - max_clock_drift: Duration, - latest_height: Height, - proof_specs: ProofSpecs, - upgrade_path: Vec, - allow_update: AllowUpdate, - } - // Define a "default" set of parameters to reuse throughout these tests. let default_params: ClientStateParams = ClientStateParams { id: ChainId::default(), @@ -421,6 +422,30 @@ mod tests { params: ClientStateParams { trusting_period: Duration::new(11, 0), unbonding_period: Duration::new(10, 0), + ..default_params.clone() + }, + want_pass: false, + }, + Test { + name: "Invalid (too small) trusting trust threshold".to_string(), + params: ClientStateParams { + trust_level: TrustThreshold::ZERO, + ..default_params.clone() + }, + want_pass: false, + }, + Test { + name: "Invalid (too small) latest height".to_string(), + params: ClientStateParams { + latest_height: Height::zero(), + ..default_params.clone() + }, + want_pass: false, + }, + Test { + name: "Invalid (empty) proof specs".to_string(), + params: ClientStateParams { + proof_specs: ProofSpecs::from(Vec::::new()), ..default_params }, want_pass: false, @@ -454,6 +479,163 @@ mod tests { ); } } + + #[test] + fn client_state_verify_delay_passed() { + #[derive(Debug, Clone)] + struct Params { + current_time: Timestamp, + current_height: Height, + processed_time: Timestamp, + processed_height: Height, + delay_period_time: Duration, + delay_period_blocks: u64, + } + struct Test { + name: String, + params: Params, + want_pass: bool, + } + let now = Timestamp::now(); + + let tests: Vec = vec![ + Test { + name: "Successful delay verification".to_string(), + params: Params { + current_time: (now + Duration::from_nanos(2000)).unwrap(), + current_height: Height::new(0, 5), + processed_time: (now + Duration::from_nanos(1000)).unwrap(), + processed_height: Height::new(0, 3), + delay_period_time: Duration::from_nanos(500), + delay_period_blocks: 2, + }, + want_pass: true, + }, + Test { + name: "Delay period(time) has not elapsed".to_string(), + params: Params { + current_time: (now + Duration::from_nanos(1200)).unwrap(), + current_height: Height::new(0, 5), + processed_time: (now + Duration::from_nanos(1000)).unwrap(), + processed_height: Height::new(0, 3), + delay_period_time: Duration::from_nanos(500), + delay_period_blocks: 2, + }, + want_pass: false, + }, + Test { + name: "Delay period(blocks) has not elapsed".to_string(), + params: Params { + current_time: (now + Duration::from_nanos(2000)).unwrap(), + current_height: Height::new(0, 5), + processed_time: (now + Duration::from_nanos(1000)).unwrap(), + processed_height: Height::new(0, 4), + delay_period_time: Duration::from_nanos(500), + delay_period_blocks: 2, + }, + want_pass: false, + }, + ]; + + for test in tests { + let res = ClientState::verify_delay_passed( + test.params.current_time, + test.params.current_height, + test.params.processed_time, + test.params.processed_height, + test.params.delay_period_time, + test.params.delay_period_blocks, + ); + + assert_eq!( + test.want_pass, + res.is_ok(), + "ClientState::verify_delay_passed() failed for test {}, \nmsg{:?} with error {:?}", + test.name, + test.params.clone(), + res.err(), + ); + } + } + + #[test] + fn client_state_verify_height() { + // Define a "default" set of parameters to reuse throughout these tests. + let default_params: ClientStateParams = ClientStateParams { + id: ChainId::default(), + trust_level: TrustThreshold::ONE_THIRD, + trusting_period: Duration::new(64000, 0), + unbonding_period: Duration::new(128000, 0), + max_clock_drift: Duration::new(3, 0), + latest_height: Height::new(1, 10), + proof_specs: ProofSpecs::default(), + upgrade_path: vec!["".to_string()], + allow_update: AllowUpdate { + after_expiry: false, + after_misbehaviour: false, + }, + }; + + struct Test { + name: String, + height: Height, + setup: Option ClientState>>, + want_pass: bool, + } + + let tests = vec![ + Test { + name: "Successful height verification".to_string(), + height: Height::new(1, 8), + setup: None, + want_pass: true, + }, + Test { + name: "Invalid (too large) client height".to_string(), + height: Height::new(1, 12), + setup: None, + want_pass: false, + }, + Test { + name: "Invalid, client is frozen below current height".to_string(), + height: Height::new(1, 6), + setup: Some(Box::new(|client_state| { + client_state.with_frozen_height(Height::new(1, 5)).unwrap() + })), + want_pass: false, + }, + ]; + + for test in tests { + let p = default_params.clone(); + let client_state = ClientState::new( + p.id, + p.trust_level, + p.trusting_period, + p.unbonding_period, + p.max_clock_drift, + p.latest_height, + p.proof_specs, + p.upgrade_path, + p.allow_update, + ) + .unwrap(); + let client_state = match test.setup { + Some(setup) => (setup)(client_state), + _ => client_state, + }; + let res = client_state.verify_height(test.height); + + assert_eq!( + test.want_pass, + res.is_ok(), + "ClientState::verify_delay_height() failed for test {}, \nmsg{:?} with error {:?}", + test.name, + test.height, + res.err(), + ); + } + } } #[cfg(any(test, feature = "mocks"))] diff --git a/modules/src/clients/ics07_tendermint/error.rs b/modules/src/clients/ics07_tendermint/error.rs index 54966531fd..0ef3b9f29c 100644 --- a/modules/src/clients/ics07_tendermint/error.rs +++ b/modules/src/clients/ics07_tendermint/error.rs @@ -160,14 +160,18 @@ define_error! { current_time: Timestamp, earliest_time: Timestamp, } - |_| { "not enough time elapsed, current timestamp {0} is still less than earliest acceptable timestamp {1}" }, + | e | { + format_args!("not enough time elapsed, current timestamp {0} is still less than earliest acceptable timestamp {1}", e.current_time, e.earliest_time) + }, NotEnoughBlocksElapsed { current_height: Height, earliest_height: Height, } - |_| { "not enough blocks elapsed, current height {0} is still less than earliest acceptable height {1}" }, + | e | { + format_args!("not enough blocks elapsed, current height {0} is still less than earliest acceptable height {1}", e.current_height, e.earliest_height) + }, InvalidHeaderHeight { height: Height }