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

Alter event on client updates to emit full header #8598

Closed
cwgoes opened this issue Feb 16, 2021 · 2 comments · Fixed by #8624
Closed

Alter event on client updates to emit full header #8598

cwgoes opened this issue Feb 16, 2021 · 2 comments · Fixed by #8624

Comments

@cwgoes
Copy link
Contributor

cwgoes commented Feb 16, 2021

This will make misbehaviour handling (which requires tracking these headers) easier.

This should be done on a non-breaking point release off the Stargate branch, so that Stargate full nodes can run it.

@cwgoes cwgoes added the x/ibc label Feb 16, 2021
@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 16, 2021

cc @ancazamfir

@ancazamfir
Copy link

ancazamfir commented Feb 17, 2021

This looks good.
One other question is about client create and the expected validation for the misbehavior detector.
Discussed this with @milosevic offline and we think we do not need the header in the create event and message as we cannot submit misbehavior in this case. The detector would:

  • monitor CreateClient event
  • fetch header at consensus height from full node
  • ensure the consensus state derived from the header is identical to the one on-chain
  • if not then "blacklist" that client

Not clear what "blacklist" means and what can be done. Locally we can make sure the relayer will not use that client.

The other topic we discussed is the misbehavior detector after a restart. If it finds a conflicting header (different trusted validator set) at latest consensus height, it needs to go backwards through the client updates to find where the misbehavior happened.
Trying to get away with an (almost-)stateless relayer here.

Any thoughts on this @cwgoes? @milosevic please feel free to correct/ add.

[maybe not the best github issue to use for this discussion, though I don't mind].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants