-
Notifications
You must be signed in to change notification settings - Fork 92
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
Add missing Tendermint client state checks #172
Conversation
12ec9c9
to
8bf8a74
Compare
)); | ||
} | ||
|
||
let _ = TendermintTrustThreshold::new(trust_level.numerator(), trust_level.denominator()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note there is currently a tiny bug in tendermint-rs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, the spec also doesn't allow a TrustThreshold == 1
-> https://github.com/cosmos/ibc/blob/main/spec/client/ics-007-tendermint-client/README.md#client-initialisation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linking the spec issue -> cosmos/ibc#857
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff 💯
Bumps [source-map-support](https://github.com/evanw/node-source-map-support) from 0.5.16 to 0.5.21. - [Release notes](https://github.com/evanw/node-source-map-support/releases) - [Commits](evanw/node-source-map-support@v0.5.16...v0.5.21) --- updated-dependencies: - dependency-name: source-map-support dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Add missing validation checks for 07-client-state * Reuse ClientState::new() for validation in TryFrom<RawClientState> * Polish error handling * Make ClientState fields private * Fix tests * Add more tests * Add changelog entry * Polish tests * Update changelog entry * Add `frozen_height` param to `ClientState::new()` * Update changelog entry
Closes: #22
Acceptance Criteria (copied from #22)
Incomplete implementation of
check_header_and_update_state
We should add a check that incoming header's(to be addressed with 02-client-refactor Implement (parts of) ibc-go/ADR006 #173)trusted_validators
matches thenext_validators_hash
that was persisted in the trusted consensus stateChainId
s totendermint::MaxChainID
-> TM chain-id validation ibc-go#177latest_height
'srevision_number
matches theChainId
'sversion
.max_clock_drift
.upgrade_path
must be non-empty and this must also be checked.PR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.