diff --git a/.changelog/unreleased/improvements/600-tendermint-pruning.md b/.changelog/unreleased/improvements/600-tendermint-pruning.md new file mode 100644 index 000000000..58ced3ddb --- /dev/null +++ b/.changelog/unreleased/improvements/600-tendermint-pruning.md @@ -0,0 +1 @@ +- Implement consensus state pruning for Tendermint light clients ([#600](https://github.com/cosmos/ibc-rs/issues/600)) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 7fc11d421..b604470c2 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -29,6 +29,7 @@ use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConse use crate::clients::ics07_tendermint::error::Error; use crate::clients::ics07_tendermint::header::Header as TmHeader; use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour; +use crate::clients::ics07_tendermint::CommonContext; use crate::core::ics02_client::client_state::{ ClientStateCommon, ClientStateExecution, ClientStateValidation, Status, UpdateKind, }; @@ -46,6 +47,7 @@ use crate::core::ics24_host::path::{ ClientConsensusStatePath, ClientStatePath, Path, UpgradeClientPath, }; use crate::core::timestamp::ZERO_DURATION; +use crate::core::ExecutionContext; use crate::prelude::*; use crate::Height; @@ -488,7 +490,7 @@ where impl ClientStateExecution for ClientState where - E: TmExecutionContext, + E: TmExecutionContext + ExecutionContext, ::AnyClientState: From, ::AnyConsensusState: From, { @@ -498,6 +500,9 @@ where client_id: &ClientId, consensus_state: Any, ) -> Result<(), ClientError> { + let host_timestamp = CommonContext::host_timestamp(ctx)?; + let host_height = CommonContext::host_height(ctx)?; + let tm_consensus_state = TmConsensusState::try_from(consensus_state)?; ctx.store_client_state(ClientStatePath::new(client_id), self.clone().into())?; @@ -505,12 +510,8 @@ where ClientConsensusStatePath::new(client_id, &self.latest_height), tm_consensus_state.into(), )?; - ctx.store_update_time( - client_id.clone(), - self.latest_height(), - ctx.host_timestamp()?, - )?; - ctx.store_update_height(client_id.clone(), self.latest_height(), ctx.host_height()?)?; + ctx.store_update_time(client_id.clone(), self.latest_height(), host_timestamp)?; + ctx.store_update_height(client_id.clone(), self.latest_height(), host_height)?; Ok(()) } @@ -524,10 +525,12 @@ where let header = TmHeader::try_from(header)?; let header_height = header.height(); + self.prune_oldest_consensus_state(ctx, client_id)?; + let maybe_existing_consensus_state = { let path_at_header_height = ClientConsensusStatePath::new(client_id, &header_height); - ctx.consensus_state(&path_at_header_height).ok() + CommonContext::consensus_state(ctx, &path_at_header_height).ok() }; if maybe_existing_consensus_state.is_some() { @@ -536,6 +539,9 @@ where // // Do nothing. } else { + let host_timestamp = CommonContext::host_timestamp(ctx)?; + let host_height = CommonContext::host_height(ctx)?; + let new_consensus_state = TmConsensusState::from(header.clone()); let new_client_state = self.clone().with_header(header)?; @@ -544,8 +550,8 @@ where new_consensus_state.into(), )?; ctx.store_client_state(ClientStatePath::new(client_id), new_client_state.into())?; - ctx.store_update_time(client_id.clone(), header_height, ctx.host_timestamp()?)?; - ctx.store_update_height(client_id.clone(), header_height, ctx.host_height()?)?; + ctx.store_update_time(client_id.clone(), header_height, host_timestamp)?; + ctx.store_update_height(client_id.clone(), header_height, host_height)?; } Ok(vec![header_height]) @@ -615,14 +621,16 @@ where ); let latest_height = new_client_state.latest_height; + let host_timestamp = CommonContext::host_timestamp(ctx)?; + let host_height = CommonContext::host_height(ctx)?; ctx.store_client_state(ClientStatePath::new(client_id), new_client_state.into())?; ctx.store_consensus_state( ClientConsensusStatePath::new(client_id, &latest_height), new_consensus_state.into(), )?; - ctx.store_update_time(client_id.clone(), latest_height, ctx.host_timestamp()?)?; - ctx.store_update_height(client_id.clone(), latest_height, ctx.host_height()?)?; + ctx.store_update_time(client_id.clone(), latest_height, host_timestamp)?; + ctx.store_update_height(client_id.clone(), latest_height, host_height)?; Ok(latest_height) } diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs index 34c10e464..a2fabec1a 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs @@ -5,8 +5,10 @@ use super::{check_header_trusted_next_validator_set, ClientState}; use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; use crate::clients::ics07_tendermint::error::{Error, IntoResult}; use crate::clients::ics07_tendermint::header::Header as TmHeader; -use crate::clients::ics07_tendermint::ValidationContext as TmValidationContext; +use crate::clients::ics07_tendermint::{CommonContext, ValidationContext as TmValidationContext}; +use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::error::ClientError; +use crate::core::ics02_client::ClientExecutionContext; use crate::core::ics24_host::identifier::ClientId; use crate::core::ics24_host::path::ClientConsensusStatePath; use crate::prelude::*; @@ -163,4 +165,47 @@ impl ClientState { } } } + + pub fn prune_oldest_consensus_state( + &self, + ctx: &mut E, + client_id: &ClientId, + ) -> Result<(), ClientError> + where + E: ClientExecutionContext + CommonContext, + { + let mut heights = ctx.consensus_state_heights(client_id)?; + + heights.sort(); + + for height in heights { + let client_consensus_state_path = ClientConsensusStatePath::new(client_id, &height); + let consensus_state = + CommonContext::consensus_state(ctx, &client_consensus_state_path)?; + let tm_consensus_state: TmConsensusState = + consensus_state + .try_into() + .map_err(|err| ClientError::Other { + description: err.to_string(), + })?; + + let host_timestamp = ctx.host_timestamp()?; + let tm_consensus_state_expiry = (tm_consensus_state.timestamp() + self.trusting_period) + .map_err(|_| ClientError::Other { + description: String::from("Timestamp overflow error occurred while attempting to parse TmConsensusState") + })?; + + if tm_consensus_state_expiry > host_timestamp { + break; + } else { + let client_id = client_id.clone(); + + ctx.delete_consensus_state(client_consensus_state_path)?; + ctx.delete_update_time(client_id.clone(), height)?; + ctx.delete_update_height(client_id, height)?; + } + } + + Ok(()) + } } diff --git a/crates/ibc/src/clients/ics07_tendermint/context.rs b/crates/ibc/src/clients/ics07_tendermint/context.rs index b98ffe644..449efa7dd 100644 --- a/crates/ibc/src/clients/ics07_tendermint/context.rs +++ b/crates/ibc/src/clients/ics07_tendermint/context.rs @@ -6,6 +6,7 @@ use crate::core::ics24_host::identifier::ClientId; use crate::core::ics24_host::path::ClientConsensusStatePath; use crate::core::timestamp::Timestamp; use crate::core::ContextError; +use crate::prelude::*; use crate::Height; /// Client's context required during both validation and execution @@ -27,6 +28,9 @@ pub trait CommonContext { &self, client_cons_state_path: &ClientConsensusStatePath, ) -> Result; + + /// Returns all the heights at which a consensus state is stored + fn consensus_state_heights(&self, client_id: &ClientId) -> Result, ContextError>; } /// Client's context required during validation diff --git a/crates/ibc/src/core/ics02_client/context.rs b/crates/ibc/src/core/ics02_client/context.rs index febda0799..093cdf610 100644 --- a/crates/ibc/src/core/ics02_client/context.rs +++ b/crates/ibc/src/core/ics02_client/context.rs @@ -53,6 +53,12 @@ pub trait ClientExecutionContext: Sized { consensus_state: Self::AnyConsensusState, ) -> Result<(), ContextError>; + /// Delete the consensus state from the store located at the given `ClientConsensusStatePath` + fn delete_consensus_state( + &mut self, + consensus_state_path: ClientConsensusStatePath, + ) -> Result<(), ContextError>; + /// Called upon successful client update. /// Implementations are expected to use this to record the specified time as the time at which /// this update (or header) was processed. @@ -72,4 +78,22 @@ pub trait ClientExecutionContext: Sized { height: Height, host_height: Height, ) -> Result<(), ContextError>; + + /// Delete the update time associated with the client at the specified height. This update + /// time should be associated with a consensus state through the specified height. + /// + /// Note that this timestamp is determined by the host. + fn delete_update_time( + &mut self, + client_id: ClientId, + height: Height, + ) -> Result<(), ContextError>; + + /// Delete the update height associated with the client at the specified height. This update + /// time should be associated with a consensus state through the specified height. + fn delete_update_height( + &mut self, + client_id: ClientId, + height: Height, + ) -> Result<(), ContextError>; } diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index ddd7bef73..6e5773271 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -139,8 +139,10 @@ mod tests { use crate::core::ics02_client::handler::update_client::{execute, validate}; use crate::core::ics02_client::msgs::misbehaviour::MsgSubmitMisbehaviour; use crate::core::ics02_client::msgs::update_client::MsgUpdateClient; + use crate::core::ics02_client::ClientValidationContext; use crate::core::ics23_commitment::specs::ProofSpecs; use crate::core::ics24_host::identifier::{ChainId, ClientId}; + use crate::core::ics24_host::path::ClientConsensusStatePath; use crate::core::timestamp::Timestamp; use crate::mock::client_state::{client_type as mock_client_type, MockClientState}; use crate::mock::context::{ @@ -180,6 +182,96 @@ mod tests { ); } + /// Tests that the Tendermint client consensus state pruning logic + /// functions correctly. + /// + /// This test sets up a MockContext with host height 1 and a trusting + /// period of 3 seconds. It then advances the state of the MockContext + /// by 2 heights, and thus 6 seconds, due to the DEFAULT_BLOCK_TIME_SECS + /// constant being set to 3 seconds. At this point, the chain is at height + /// 3. Any consensus states associated with a block more than 3 seconds + /// in the past should be expired and pruned from the IBC store. The test + /// thus checks that the consensus state at height 1 is not contained in + /// the store. It also checks that the consensus state at height 2 is + /// contained in the store and has not expired. + #[test] + fn test_consensus_state_pruning() { + let chain_id = ChainId::new("mockgaiaA", 1).unwrap(); + + let client_height = Height::new(1, 1).unwrap(); + + let client_id = ClientId::new(tm_client_type(), 0).unwrap(); + + let mut ctx = MockContextConfig::builder() + .host_id(chain_id.clone()) + .host_type(HostType::SyntheticTendermint) + .latest_height(client_height) + .latest_timestamp(Timestamp::now()) + .max_history_size(u64::MAX) + .build() + .with_client_config( + MockClientConfig::builder() + .client_chain_id(chain_id.clone()) + .client_id(client_id.clone()) + .client_state_height(client_height) + .client_type(tm_client_type()) + .trusting_period(Duration::from_secs(3)) + .build(), + ); + + let start_host_timestamp = ctx.host_timestamp().unwrap(); + + // Move the chain forward by 2 blocks to pass the trusting period. + for _ in 1..=2 { + let signer = get_dummy_account_id(); + + let update_height = ctx.latest_height(); + + ctx.advance_host_chain_height(); + + let mut block = ctx.host_block(&update_height).unwrap().clone(); + + block.set_trusted_height(client_height); + + let msg = MsgUpdateClient { + client_id: client_id.clone(), + client_message: block.clone().into(), + signer, + }; + + let _ = validate(&ctx, MsgUpdateOrMisbehaviour::UpdateClient(msg.clone())); + let _ = execute(&mut ctx, MsgUpdateOrMisbehaviour::UpdateClient(msg.clone())); + } + + // Check that latest expired consensus state is pruned. + let expired_height = Height::new(1, 1).unwrap(); + let client_cons_state_path = ClientConsensusStatePath::new(&client_id, &expired_height); + assert!(ctx + .client_update_height(&client_id, &expired_height) + .is_err()); + assert!(ctx.client_update_time(&client_id, &expired_height).is_err()); + assert!(ctx.consensus_state(&client_cons_state_path).is_err()); + + // Check that latest valid consensus state exists. + let earliest_valid_height = Height::new(1, 2).unwrap(); + let client_cons_state_path = + ClientConsensusStatePath::new(&client_id, &earliest_valid_height); + + assert!(ctx + .client_update_height(&client_id, &earliest_valid_height) + .is_ok()); + assert!(ctx + .client_update_time(&client_id, &earliest_valid_height) + .is_ok()); + assert!(ctx.consensus_state(&client_cons_state_path).is_ok()); + + let end_host_timestamp = ctx.host_timestamp().unwrap(); + assert_eq!( + end_host_timestamp, + (start_host_timestamp + Duration::from_secs(6)).unwrap() + ); + } + #[test] fn test_update_nonexisting_client() { let client_id = ClientId::from_str("mockclient1").unwrap(); diff --git a/crates/ibc/src/mock/context/clients.rs b/crates/ibc/src/mock/context/clients.rs index 14261184f..cac89cd6d 100644 --- a/crates/ibc/src/mock/context/clients.rs +++ b/crates/ibc/src/mock/context/clients.rs @@ -52,6 +52,21 @@ impl TmCommonContext for MockContext { ) -> Result { ValidationContext::consensus_state(self, client_cons_state_path) } + + fn consensus_state_heights(&self, client_id: &ClientId) -> Result, ContextError> { + let ibc_store = self.ibc_store.lock(); + let client_record = + ibc_store + .clients + .get(client_id) + .ok_or_else(|| ClientError::ClientStateNotFound { + client_id: client_id.clone(), + })?; + + let heights = client_record.consensus_states.keys().cloned().collect(); + + Ok(heights) + } } impl TmValidationContext for MockContext { @@ -214,6 +229,56 @@ impl ClientExecutionContext for MockContext { Ok(()) } + fn delete_consensus_state( + &mut self, + consensus_state_path: ClientConsensusStatePath, + ) -> Result<(), ContextError> { + let mut ibc_store = self.ibc_store.lock(); + + let client_record = ibc_store + .clients + .entry(consensus_state_path.client_id) + .or_insert(MockClientRecord { + consensus_states: Default::default(), + client_state: Default::default(), + }); + + let height = Height::new(consensus_state_path.epoch, consensus_state_path.height) + .expect("Never fails"); + + client_record.consensus_states.remove(&height); + + Ok(()) + } + + fn delete_update_height( + &mut self, + client_id: ClientId, + height: Height, + ) -> Result<(), ContextError> { + let _ = self + .ibc_store + .lock() + .client_processed_heights + .remove(&(client_id, height)); + + Ok(()) + } + + fn delete_update_time( + &mut self, + client_id: ClientId, + height: Height, + ) -> Result<(), ContextError> { + let _ = self + .ibc_store + .lock() + .client_processed_times + .remove(&(client_id, height)); + + Ok(()) + } + fn store_update_time( &mut self, client_id: ClientId, @@ -225,6 +290,7 @@ impl ClientExecutionContext for MockContext { .lock() .client_processed_times .insert((client_id, height), timestamp); + Ok(()) } @@ -239,6 +305,7 @@ impl ClientExecutionContext for MockContext { .lock() .client_processed_heights .insert((client_id, height), host_height); + Ok(()) } }