-
Notifications
You must be signed in to change notification settings - Fork 228
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
Incorrect validator set hash #831
Comments
one other suspect here is "paging":
|
Running into this when light client validates a trusted block during initially instance building for some hermes operations (client create or client update). The two chains are
Looks like something goes wrong here tendermint-rs/light-client/src/predicates.rs Lines 30 to 35 in e6a3168
---> tendermint-rs/light-client/src/operations/hasher.rs Lines 26 to 35 in e6a3168
[ nb: temp |
Could we feed a unit test with a real chain block and debug this please? |
As @tomtau pointed out, it looks like this happens when there are more than 30 validators in the validator set, due to the following:
This should be fixed if we allow for paging in the |
@thanethomson Could we get an option on Something like this perhaps: async fn validators<H>(&self, height: H, paging: Paging) -> Result<validators::Response> { /* ... */ }
enum Paging {
NoPaging,
AllPages,
Paged {
page: usize,
per_page: usize,
}
}
impl Default for Paging {
fn default() -> Self {
Self::NoPaging
}
} alternatively: async fn validators<H>(&self, height: H, paging: Option<Paging>) -> Result<validators::Response> { /* ... */ }
enum Paging {
AllPages,
Paged {
page: usize,
per_page: usize,
}
} |
Fwiw I'm also seeing something that looks like this in the go relayer |
Yeah we spoke about this earlier today. The pagination's definitely an issue on the tendermint-rs side, but it might not be the only issue - we'll know once I fix the pagination issue 🙂 |
I believe this is the case. The go relayer was incorrectly pulling the trusted validator set from the trusted height and it looks like hermes might have been doing the same. The on-chain consensus state stores the hash for the Ironically the ibc-go tests happened to be doing this correctly It makes sense why this was a hard issue to track down. It was only a problem when the validator set changed between trusted height X and X + 1 |
thanks Colin!!! opened informalsystems/hermes#770 to fix this. |
I was trying Hermes CLI (v0.18.1) against one public testnet (running Tendermint v0.34.3) and got:
Steps to reproduce
Feed some Croeseid public testnet block to the light client:
e.g. https://testnet-croeseid.crypto.org:26657/block?height=1190060
and the corresponding validators: https://testnet-croeseid.crypto.org:26657/validators?height=1190060
What's the definition of "done" for this issue?
The validator set hash is calculated correctly -- not sure if this is a duplicate of this issue: #687
The text was updated successfully, but these errors were encountered: