-
Notifications
You must be signed in to change notification settings - Fork 90
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
Replace check_header_and_update_state
with an architecture similar to ibc-go
#584
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #584 +/- ##
==========================================
+ Coverage 72.93% 73.19% +0.26%
==========================================
Files 126 125 -1
Lines 15725 15660 -65
==========================================
- Hits 11469 11463 -6
+ Misses 4256 4197 -59
... and 3 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
check_{header,misbehaviour}_and_update_state
with a similar architecture of ibc-gocheck_header_and_update_state
with a similar architecture of ibc-go
check_header_and_update_state
with a similar architecture of ibc-gocheck_header_and_update_state
with an architecture similar to ibc-go
Created #600; I won't implement in this PR but it's a very desirable feature which we should have. |
…our.rs Co-authored-by: Anca Zamfir <[email protected]> Signed-off-by: Philippe Laferrière <[email protected]>
…ient.rs Co-authored-by: Anca Zamfir <[email protected]> Signed-off-by: Philippe Laferrière <[email protected]>
…our.rs Co-authored-by: Anca Zamfir <[email protected]> Signed-off-by: Philippe Laferrière <[email protected]>
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.
I left some suggestions. About the different checks discussed here in particular, I didn't go into much detail yet, I assumed there might be updates based on your discussion with Anca.
Greatly appreciate you handling this PR!
…ient.rs Co-authored-by: Anca Zamfir <[email protected]> Signed-off-by: Philippe Laferrière <[email protected]>
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 work 👌🏻
@ancazamfir I will wait for your approval as well before merging |
…to ibc-go (#584) * fix * changelog * verify_header contains a FIXME to double check * fix header.trusted_next_validator_set hash bug * fmt * remove bad comment * add timestamp monotonicity checks to `verify_header()` * clean up verify_header * add revision number check in verify_header * header height extra check * clarifying comment * check_misbehaviour_header scaffolding * remove duplicate methods * verify_misbehaviour done as ibc-go * fix chain ID before commit verification * finish verify_misbehavior * `check_for_misbehaviour` header variant implementation * check_for_misbehaviour, misbehaviour variant * monotonicity of timestamps in update_client * update_state_on_misbehaviour * implement update_state in tendermint light client * cleanup `map_err`s * Rename MsgUpdateClient::client_message * impl raw misbehaviour -> MsgUpdateClient * change updateClientKind enum * move misbehaviour tests to update_client * move misbehaviour type url * remove misbehaviour msg and handler * update_client::validate * implement part of update_client::execute * separate events emitted * finish update_client::execute * remove TODO * implement mock client state * fix mockclientstate * fix event created on update_client * fix mock * fix mock * test var names * fix parts of the problem with test * fix test * clippy * remove old methods * clean Misbehaviour::new() * cleanup unused * move `update_client` method to submodule * move misbehaviour to submodule * move implementation to function * move implementation to function * changelog * add clarifying comment * fmt * Update crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs Co-authored-by: Anca Zamfir <[email protected]> Signed-off-by: Philippe Laferrière <[email protected]> * Update crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs Co-authored-by: Anca Zamfir <[email protected]> Signed-off-by: Philippe Laferrière <[email protected]> * Update crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs Co-authored-by: Anca Zamfir <[email protected]> Signed-off-by: Philippe Laferrière <[email protected]> * fix misbehaviour ctor * improve documentation around `UpdateKind` * fmt * remove unused misbehaviour trait * fix misbehaviour header checks * Update crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs Co-authored-by: Anca Zamfir <[email protected]> Signed-off-by: Philippe Laferrière <[email protected]> * fix test after merge --------- Signed-off-by: Philippe Laferrière <[email protected]> Co-authored-by: Anca Zamfir <[email protected]>
While implementing the new API, I found several bugs. I implicitly fix those by not re-introducing them in the new API implementation.
Main issue:
Closes: #535
Bugs fixed:
Closes: #583
Closes: #585
Closes: #589
Closes: #598
Closes: #601
Description
verify_client_message
check_for_misbehaviour
update_state_on_misbehaviour
update_state
update_client::validate
callsclient_state.verify_client_message
, similar to here in ibc-goupdate_client::execute
callsclient_state.{check_for_misbehaviour, update_state_on_misbehaviour, update_state}
, similar to here in ibc-goClientState::{check_header_and_update_state}
andClientState::{check_misbehaviour_and_update_state}
, and all unusedwith_*
functionsmisbehaviour::{validate, execute}
TmMisbehaviour::new()
; this should be a light constructor.MsgUpdateClient
MsgSubmitMisbehaviour
andMsgUpdateClient
are merged into the "new"MsgUpdateClient
TryFrom<RawMsgSubmitMisbehaviour> for MsgUpdateClient
TryFrom<Any> for MsgEnvelope
MsgUpdateClient.header
toMsgUpdateClient.client_message
, and addupdate_kind
enum fieldPR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.