search

informalsystems / hermes-sdk

Apache License 2.0
9 stars 2 forks source link

fix: ConnOpenTry handshake from cosmos to sovereign #360

Closed rnbguy closed 2 weeks ago

rnbguy commented 1 month ago

Closes: #331

Description

PR author checklist:

Reviewer checklist:

rnbguy commented 1 month ago

Now, the test fails at proof verification. Need to debug this by comparing the ConsensusState stored at Sovereign and the Header queried at the height corresponding to the ConsensusState.

rnbguy commented 3 weeks ago

Currently this requires correct state root hash update from sovereign to cosmos.

https://github.com/informalsystems/hermes-sdk/blob/0733dba9c2674c6811faed0b15d142e428847892/crates/sovereign/sovereign-chain-components/src/cosmos/impls/sovereign_to_cosmos/client/update_client_message.rs#L36-L42

This is fixed at informalsystems/hermes-sdk#359.

rnbguy commented 3 weeks ago

Currently, it fails, as the consensus state doesn't match at counterparty chain - as we're using dummy header.

https://github.com/informalsystems/hermes-sdk/blob/f4111975fafba7a280f840eeaa02eb44b7c35425/crates/sovereign/sovereign-chain-components/src/cosmos/impls/sovereign_to_cosmos/client/update_client_message.rs#L50-L55

We have to build the header with proper AggregatedProofConfig.

rnbguy commented 3 weeks ago

My above explanation is wrong. The problem is actually in sovereign-ibc. Only commitment root is stored at ConsensusState - so other fields don't matter.

Currently, sov-ibc creates Sovereign rollup commitment root from Celestia header hash - this is incorrect and probably used as dummy value.

We need to find how we retrieve this value. The begin_slot_hook comes with a pre_state_root: Root which can be converted to [0; 32]. Currently, we are updating the wasm client with user_hash as commitment root. We need to verify what value we get at [u8; 32]::from(pre_state_root).

rnbguy commented 2 weeks ago

I had to bring back -1/+1 in chain status query and proof fetching for sovereigh rollup - as we have to be consistent with the host consensus state storage at sovereign-ibc to verify the consensus state at counterparty.

For now, this works with current sovereign-ibc implementation with sovereign-sdk-wip. Although, I am still not sure, if we are missing anything because we don't validate the aggregated proof at sovereign-ibc.