cosmos / ibc-rs

Rust implementation of the Inter-Blockchain Communication (IBC) protocol.
Apache License 2.0
193 stars 80 forks source link

Add needed unit or integration tests for Tendermint proof verifications #398

Closed Farhad-Shabani closed 4 months ago

Farhad-Shabani commented 1 year ago

Summary

There is a need to cover Tendermint proof verifications with proper tests after issue #396. Mock Chain passes the proof verifications and we are not able to catch Tendermint-specific errors with the existing tests.

To Do

To ensure Tendermint proof verifications are correctly executed, we should add corresponding unit tests or somehow integrate basecoin-rs into our CI process

Blocked on: #677

plafer commented 1 year ago

396 was a core IBC bug; not Tendermint-specific. Did you mean "core handler validation needs to be tested" instead of "Tendermint proof verifications"?

Farhad-Shabani commented 1 year ago

It occurred in the core section, but catching such errors needs that tests go through a proof verification process (e.g. here), which is a client-specific action. So, we can match the output with the fed test data. Note that we already bypass this check for MockClientState by setting the result of such methods to Ok() (e.g. here)

plafer commented 1 year ago

Ah I see; yes that's right.

rnbguy commented 4 months ago

closed by #1109