Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement consensus state pruning for Tendermint clients #922

Merged
merged 23 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a0020f4
Add prune_oldest_consensus_state to ClientStateValidation
seanchen1991 Oct 3, 2023
1b71e7d
Add consensus_state_heights fn signature to CommonContext
seanchen1991 Oct 4, 2023
6faa5d2
Stub prune_oldest_consensus_state fn
seanchen1991 Oct 4, 2023
126c496
Add comments for delete_update_time and delete_update_height
seanchen1991 Oct 4, 2023
cf831e8
Clean up code structure a bit
seanchen1991 Oct 4, 2023
c0b4c27
Fix some compilation errors
seanchen1991 Oct 4, 2023
df5879c
Address all compilation errors resulting from prune_old_consensus_states
seanchen1991 Oct 5, 2023
742379e
Move delete_update_height and delete_update_time methods to Execution…
seanchen1991 Oct 6, 2023
1692864
Clean up all compilation errors
seanchen1991 Oct 11, 2023
528630f
Fix merge conflicts
seanchen1991 Oct 12, 2023
b5d08c5
Disambiguate some method calls
seanchen1991 Oct 12, 2023
c68f9c1
Fix fn formatting
seanchen1991 Oct 12, 2023
42a2419
Merge branch 'main' of https://github.com/cosmos/ibc-rs into sean/con…
seanchen1991 Oct 13, 2023
2f79431
Run cargo fmt
seanchen1991 Oct 13, 2023
5fdda8d
Flip consensus state condition check
seanchen1991 Oct 18, 2023
f0626df
Add changelog entry
seanchen1991 Oct 18, 2023
1bd8588
Incorporate PR feedback
seanchen1991 Oct 19, 2023
1496c42
Merge branch 'main' into sean/consensus-state-pruning
seanchen1991 Oct 19, 2023
9fe77f4
test: add test_cons_state_pruning
Farhad-Shabani Oct 19, 2023
cb0dac4
Simplify consensus state pruning unit test a bit
seanchen1991 Oct 19, 2023
0d3dae8
Document `test_consensus_state_pruning` function
seanchen1991 Oct 19, 2023
6d36d82
Correct a detail in documentation
seanchen1991 Oct 19, 2023
52ea3ef
Merge branch 'main' of https://github.com/cosmos/ibc-rs into sean/con…
seanchen1991 Oct 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Implement consensus state pruning for Tendermint light clients ([#600](https://github.com/cosmos/ibc-rs/issues/600))
32 changes: 20 additions & 12 deletions crates/ibc/src/clients/ics07_tendermint/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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;

Expand Down Expand Up @@ -488,7 +490,7 @@ where

impl<E> ClientStateExecution<E> for ClientState
where
E: TmExecutionContext,
E: TmExecutionContext + ExecutionContext,
<E as ClientExecutionContext>::AnyClientState: From<ClientState>,
<E as ClientExecutionContext>::AnyConsensusState: From<TmConsensusState>,
{
Expand All @@ -498,19 +500,18 @@ 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())?;
ctx.store_consensus_state(
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(())
}
Expand All @@ -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() {
Expand All @@ -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)?;

Expand All @@ -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])
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -163,4 +165,47 @@ impl ClientState {
}
}
}

pub fn prune_oldest_consensus_state<E>(
&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(())
}
}
4 changes: 4 additions & 0 deletions crates/ibc/src/clients/ics07_tendermint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -27,6 +28,9 @@ pub trait CommonContext {
&self,
client_cons_state_path: &ClientConsensusStatePath,
) -> Result<Self::AnyConsensusState, ContextError>;

/// Returns all the heights at which a consensus state is stored
fn consensus_state_heights(&self, client_id: &ClientId) -> Result<Vec<Height>, ContextError>;
}

/// Client's context required during validation
Expand Down
24 changes: 24 additions & 0 deletions crates/ibc/src/core/ics02_client/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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>;
}
92 changes: 92 additions & 0 deletions crates/ibc/src/core/ics02_client/handler/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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();
Expand Down
Loading