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

ICS07: Add verification method for client update handler #1321

Merged
merged 84 commits into from
Oct 19, 2021

Conversation

thanethomson
Copy link
Contributor

@thanethomson thanethomson commented Sep 4, 2021

Closes cosmos/ibc-rs#71

Builds on and replaces #1252 with the following changes:

  1. Attempts to make use of the Light Client verification functionality provided by the tendermint-light-client crate in implementing ICS07.
  2. Updates all ibc-rs usage of tendermint-rs to make use of recent flex-error integration.

For contributor use:

  • Added a changelog entry, using unclog.
  • If applicable: Unit tests written, added test to CI.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments.
  • Re-reviewed Files changed in the Github PR explorer.

@thanethomson thanethomson marked this pull request as ready for review October 5, 2021 20:05
@ancazamfir ancazamfir self-requested a review October 12, 2021 09:40
Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good from my side for this PR. Will open a different issue to discuss and fix the client specific store.

modules/src/ics02_client/error.rs Outdated Show resolved Hide resolved
modules/src/ics02_client/error.rs Outdated Show resolved Hide resolved
modules/src/ics02_client/handler/update_client.rs Outdated Show resolved Hide resolved
modules/src/ics07_tendermint/client_def.rs Show resolved Hide resolved
modules/src/ics07_tendermint/client_def.rs Show resolved Hide resolved
modules/src/ics07_tendermint/header.rs Show resolved Hide resolved
modules/src/ics07_tendermint/header.rs Show resolved Hide resolved
modules/src/ics18_relayer/utils.rs Outdated Show resolved Hide resolved
modules/src/mock/context.rs Outdated Show resolved Hide resolved
modules/src/mock/context.rs Outdated Show resolved Hide resolved
@romac romac self-assigned this Oct 12, 2021
Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you so much for all the work, @cezarad @thanethomson and @ancazamfir!

Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work team, thank you everyone for your patience and perseverence with this PR!

Note that I changed the "closing" issue associated to this PR. I think we can keep the changelog as it is.

use crate::timestamp::ZERO_DURATION;
use std::ops::Add;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last concern: should we scrub this PR for std ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in tests, which we will run with std, so it doesn't really matter whether we import from std or core, but in this case it wouldn't hurt to just import from core for consistency.

@adizere adizere merged commit fde0a96 into master Oct 19, 2021
@adizere adizere deleted the thane/1252-ics07 branch October 19, 2021 13:14
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
…tems#1321)

* bla

* on going

* ongoing

* on going

* on going

* timestamp default changed from none to now

* failed ping pong - signs

* Update context.rs

* bla

* blabla

* fixed test client_update_ping_pong

* fmt + clippy

* Update context.rs

* Update context.rs

* remove comments

* Update host.rs

* Update to tendermint-rs v0.20.0

* Update changelog

* Fix tendermint-rs version in changelog

* Update predicates.rs

* Update context.rs

* tests

* tendermint stuff in

* Update Cargo.toml

* clippy + fmt

* moved predicates into ics07 header.rs

* Adapted to latest TM changes

* Fixed MockHeader test

* Fmt & clippy

* Removed irrelevant file

* Bit more cleanup

* fixed tests

* upgraded to new error  model

* fmt

* errors and timestamp changes

* Fix error notation and formatting

Signed-off-by: Thane Thomson <[email protected]>

* Upgrade to tendermint-rs master

Signed-off-by: Thane Thomson <[email protected]>

* Use tendermint-rs from branch thane/ibc-1252

Signed-off-by: Thane Thomson <[email protected]>

* Refactor ICS07 update handler to reuse light client verifier

This commit makes use of the latest code from
informalsystems/tendermint-rs#960 in order to
reuse the light client's verification predicates instead of
reimplementing them in the `ibc` crate.

Signed-off-by: Thane Thomson <[email protected]>

* Update Cargo.lock to address zeroize issue

Signed-off-by: Thane Thomson <[email protected]>

* Bump tendermint-light-client dep to v0.22.0 for ibc module

Signed-off-by: Thane Thomson <[email protected]>

* Refactor to accommodate new context API

Signed-off-by: Thane Thomson <[email protected]>

* Fix missing import

Signed-off-by: Thane Thomson <[email protected]>

* Fix imports

Signed-off-by: Thane Thomson <[email protected]>

* Fix error check in test

Signed-off-by: Thane Thomson <[email protected]>

* Output debug version of error

Signed-off-by: Thane Thomson <[email protected]>

* Remove test as per informalsystems#1321 (comment)

Signed-off-by: Thane Thomson <[email protected]>

* Address comments from Adi

Signed-off-by: Thane Thomson <[email protected]>

* Cosmetic tweaks

Signed-off-by: Thane Thomson <[email protected]>

* Add revision number check

Signed-off-by: Thane Thomson <[email protected]>

* Fix broken test

Signed-off-by: Thane Thomson <[email protected]>

* Check incoming header height against chain ID version from client state

Signed-off-by: Thane Thomson <[email protected]>

* Add revision_number consistency check when deserializing header

Signed-off-by: Thane Thomson <[email protected]>

* Clarify MismatchedRevisions error message

Signed-off-by: Thane Thomson <[email protected]>

* Add changelog entries

Signed-off-by: Thane Thomson <[email protected]>

* Commented import no longer necessary

Signed-off-by: Thane Thomson <[email protected]>

* Add in-the-middle monotonicity checks

Signed-off-by: Thane Thomson <[email protected]>

* Fix broken dep tree relating to Prometheus

Signed-off-by: Thane Thomson <[email protected]>

* Move next/prev consensus state search functionality to ClientReader trait

Signed-off-by: Thane Thomson <[email protected]>

* Move impl of prev/next to the specific implementation and simplify signatures

* Disable `tendermint-light-client` default features, ie. RPC client, std and color-eyre

* Apply suggestions from code review

* Fix compilation

* Cleanup BTreeMap import

* Always show underlying reason in ics07_tendermint errors

* Update modules/src/ics02_client/handler/update_client.rs

* Fix compilation

Co-authored-by: cezarad <[email protected]>
Co-authored-by: Romain Ruetschi <[email protected]>
Co-authored-by: cezarad <[email protected]>
Co-authored-by: Adi Seredinschi <[email protected]>
Co-authored-by: Anca Zamfir <[email protected]>
hu55a1n1 pushed a commit to cosmos/ibc-rs that referenced this pull request Sep 29, 2022
* bla

* on going

* ongoing

* on going

* on going

* timestamp default changed from none to now

* failed ping pong - signs

* Update context.rs

* bla

* blabla

* fixed test client_update_ping_pong

* fmt + clippy

* Update context.rs

* Update context.rs

* remove comments

* Update host.rs

* Update to tendermint-rs v0.20.0

* Update changelog

* Fix tendermint-rs version in changelog

* Update predicates.rs

* Update context.rs

* tests

* tendermint stuff in

* Update Cargo.toml

* clippy + fmt

* moved predicates into ics07 header.rs

* Adapted to latest TM changes

* Fixed MockHeader test

* Fmt & clippy

* Removed irrelevant file

* Bit more cleanup

* fixed tests

* upgraded to new error  model

* fmt

* errors and timestamp changes

* Fix error notation and formatting

Signed-off-by: Thane Thomson <[email protected]>

* Upgrade to tendermint-rs master

Signed-off-by: Thane Thomson <[email protected]>

* Use tendermint-rs from branch thane/ibc-1252

Signed-off-by: Thane Thomson <[email protected]>

* Refactor ICS07 update handler to reuse light client verifier

This commit makes use of the latest code from
informalsystems/tendermint-rs#960 in order to
reuse the light client's verification predicates instead of
reimplementing them in the `ibc` crate.

Signed-off-by: Thane Thomson <[email protected]>

* Update Cargo.lock to address zeroize issue

Signed-off-by: Thane Thomson <[email protected]>

* Bump tendermint-light-client dep to v0.22.0 for ibc module

Signed-off-by: Thane Thomson <[email protected]>

* Refactor to accommodate new context API

Signed-off-by: Thane Thomson <[email protected]>

* Fix missing import

Signed-off-by: Thane Thomson <[email protected]>

* Fix imports

Signed-off-by: Thane Thomson <[email protected]>

* Fix error check in test

Signed-off-by: Thane Thomson <[email protected]>

* Output debug version of error

Signed-off-by: Thane Thomson <[email protected]>

* Remove test as per informalsystems/hermes#1321 (comment)

Signed-off-by: Thane Thomson <[email protected]>

* Address comments from Adi

Signed-off-by: Thane Thomson <[email protected]>

* Cosmetic tweaks

Signed-off-by: Thane Thomson <[email protected]>

* Add revision number check

Signed-off-by: Thane Thomson <[email protected]>

* Fix broken test

Signed-off-by: Thane Thomson <[email protected]>

* Check incoming header height against chain ID version from client state

Signed-off-by: Thane Thomson <[email protected]>

* Add revision_number consistency check when deserializing header

Signed-off-by: Thane Thomson <[email protected]>

* Clarify MismatchedRevisions error message

Signed-off-by: Thane Thomson <[email protected]>

* Add changelog entries

Signed-off-by: Thane Thomson <[email protected]>

* Commented import no longer necessary

Signed-off-by: Thane Thomson <[email protected]>

* Add in-the-middle monotonicity checks

Signed-off-by: Thane Thomson <[email protected]>

* Fix broken dep tree relating to Prometheus

Signed-off-by: Thane Thomson <[email protected]>

* Move next/prev consensus state search functionality to ClientReader trait

Signed-off-by: Thane Thomson <[email protected]>

* Move impl of prev/next to the specific implementation and simplify signatures

* Disable `tendermint-light-client` default features, ie. RPC client, std and color-eyre

* Apply suggestions from code review

* Fix compilation

* Cleanup BTreeMap import

* Always show underlying reason in ics07_tendermint errors

* Update modules/src/ics02_client/handler/update_client.rs

* Fix compilation

Co-authored-by: cezarad <[email protected]>
Co-authored-by: Romain Ruetschi <[email protected]>
Co-authored-by: cezarad <[email protected]>
Co-authored-by: Adi Seredinschi <[email protected]>
Co-authored-by: Anca Zamfir <[email protected]>
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 this pull request may close these issues.

5 participants