search

informalsystems / basecoin-rs

An example ABCI application making use of tendermint-rs and ibc-rs
Apache License 2.0
55 stars 17 forks source link

`ConnectionReader::host_consensus_state()` is not implemented #37

Closed hu55a1n1 closed 2 years ago

hu55a1n1 commented 2 years ago

The ConnectionReader::host_consensus_state() impl is currently returning an empty mock result -> https://github.com/informalsystems/basecoin-rs/blob/main/src/app/modules/ibc.rs#L341-L350

fn host_consensus_state(
    &self,
    _height: IbcHeight,
) -> Result<AnyConsensusState, ConnectionError> {
    // FIXME: store host consensus state and return it here
    Ok(AnyConsensusState::Tendermint(ConsensusState::new(
        CommitmentRoot::from_bytes(&[]),
        Time::unix_epoch(),
        Hash::None,
    )))
}

This results in connection handhake verification failure (there are other problems with ibc-rs integration that I'm yet to figure out but this is one of them).

The reason this call was left unimplemented is because Tendermint doesn't provide a way for the ABCI app to query the consensus engine. (see https://github.com/tendermint/tendermint/discussions/5977) The way the Cosmos-SDK gets around this is by having the staking module save consensus states (to private store) while handling the ABCI BeginBlock method. (see https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-017-historical-header-module.md)

I propose a slightly different approach for basecoin - which is to extend the modules trait to add a begin_block method that'll allow the ibc module to store consensus states in its private store.

hu55a1n1 commented 2 years ago

This has been implemented on the hu55a1n1/module-verification branch -> https://github.com/informalsystems/basecoin-rs/commit/cb01c0b897d6a0e42ebae36769f0a28ecf081f57