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

Duplicate vote evidence is retrieving the trusted consensus state height from wrong chain #3999

Closed
5 tasks
ancazamfir opened this issue May 23, 2024 · 1 comment · Fixed by #3998
Closed
5 tasks

Comments

@ancazamfir
Copy link
Collaborator

ancazamfir commented May 23, 2024

Summary of Bug

Thanks Hypha (Dante) and @insumity for finding this.
His analysis:
Regarding the submission of double voting evidence in Hermes I had the following questions:

  • handle_duplicate_vote loops over counterparty_clients and for each client submit_duplicate_vote_evidence. Why is this the case? It seems that a duplicate vote evidence needs to be submitted only once to the provider chain.
  • In submit_duplicate_vote_evidence, the consensus state heights of counterparty_client_id are retrieved. It seems that the consensus state heights are retrieved from chain although, I think, they should be retrieved from counterparty_chain_handle instead that has the counterparty_client_id. Am I missing something?
    The reason I'm asking about this is because we had an issue where Hermes could not submit double voting evidence (see here). I believe if we were to make the change mentioned in 2. this would solve the issue.

Version

Steps to Reproduce

At the time of this PR submission hermes could be run with the config) where the issue was found.

  • Before the fix:
2024-05-23T15:50:22.902192Z DEBUG ThreadId(01) checking for evidence at height 1-570
2024-05-23T15:50:23.088247Z DEBUG ThreadId(01) checking for evidence at height 1-569
2024-05-23T15:50:23.174009Z  WARN ThreadId(01) found duplicate vote evidence
2024-05-23T15:50:23.335408Z DEBUG ThreadId(01) found 2 connections
2024-05-23T15:50:23.335456Z DEBUG ThreadId(01) found connection `connection-0` with client `07-tendermint-0` and counterparty client `07-tendermint-76`
2024-05-23T15:50:23.335475Z DEBUG ThreadId(01) fetching client state for client `07-tendermint-0` on connection `connection-0`
2024-05-23T15:50:23.420743Z  INFO ThreadId(01) found counterparty client `07-tendermint-76` which lives on counterparty chain `provider`
2024-05-23T15:50:23.420795Z DEBUG ThreadId(01) found connection `connection-localhost` with client `09-localhost` and counterparty client `09-localhost`
2024-05-23T15:50:23.420811Z DEBUG ThreadId(01) fetching client state for client `09-localhost` on connection `connection-localhost`
2024-05-23T15:50:23.503687Z ERROR ThreadId(01) failed to fetch client state for client `09-localhost`, skipping...
2024-05-23T15:50:23.503734Z ERROR ThreadId(01) reason: error decoding protobuf: error converting message type into domain type: unknown client state type: /ibc.lightclients.localhost.v2.ClientState
2024-05-23T15:50:23.503769Z  INFO ThreadId(01) counterparty_clients [(ChainId { id: "provider", version: 0 }, ClientId("07-tendermint-76"))]
2024-05-23T15:50:24.175873Z ERROR ThreadId(01) cannot build infraction block header for client `07-tendermint-76` on chain `provider`,reason: could not find consensus state at highest height smaller than infraction height 568
2024-05-23T15:50:24.280409Z DEBUG ThreadId(01) checking for evidence at height 1-568

Acceptance Criteria

Double vote submission should succeed.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@insumity
Copy link

Thanks @ancazamfir!
Just a small note that Hypha first found this issue on the testnet they're running and the provided config and setup was given to us by Dante.

@github-project-automation github-project-automation bot moved this from 🩹 Triage to ✅ Done in Hermes May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants