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

[lc/cosmos] consensus state at latest height is overwritten #257

Closed
rnbguy opened this issue Feb 5, 2025 · 0 comments · Fixed by #272
Closed

[lc/cosmos] consensus state at latest height is overwritten #257

rnbguy opened this issue Feb 5, 2025 · 0 comments · Fixed by #272

Comments

@rnbguy
Copy link
Member

rnbguy commented Feb 5, 2025

fn update_state(
ref self: ComponentState<TContractState>,
client_sequence: u64,
client_message: Array<felt252>
) -> UpdateResponse {
let header: CometHeader = CometHeaderImpl::deserialize(client_message);
let header_height = header.clone().signed_header.height;
// TODO: Implement consensus state pruning mechanism.
if !self.consensus_state_exists(client_sequence, header_height.clone()) {
let mut client_state = self.read_client_state(client_sequence);
client_state.update(header_height.clone());
let new_consensus_state: CometConsensusState = header.into();
self._update_state(client_sequence, client_state, new_consensus_state);
}
array![header_height].into()
}

The above code tries to update a consensus state if not present. This tries to conditionally update the client_state's latest height here

client_state.update(header_height.clone());

This may not actually update the latest height, if the update came from an old height.

But in the _update_state, it tries to use client_state's latest_height to store the new consensus state -- which may not really correspond to the client latest height. So for an old update height, this overwrites the latest consensus state.

fn _update_state(
ref self: ComponentState<TContractState>,
client_sequence: u64,
client_state: CometClientState,
consensus_state: CometConsensusState,
) {
self.write_client_state(client_sequence, client_state.clone());
let latest_height = client_state.latest_height;
self.write_update_height(client_sequence, latest_height.clone());
self
.write_consensus_state(
client_sequence, latest_height.clone(), consensus_state.clone()
);

You may be able to reproduce this in your cairo unit tests -- by trying updating the client state at a lower height and then checking if the new consensus state at the update height exists and the latest consensus state hasn't changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants