feat: implement client recovery feature #1127
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rnbguy
reviewed
Mar 27, 2024
… into sean/client-recovery
rnbguy
changed the title
rnbguy
approved these changes
Mar 27, 2024
There was a problem hiding this comment.
Great work @seanchen1991 ! Good to get it merged.
I added a last commit following @Farhad-Shabani 's comments.
Farhad-Shabani
approved these changes
Mar 27, 2024
There was a problem hiding this comment.
Great job @seanchen1991 on this PR, and big thanks to @rnbguy for the thorough reviews. Much appreciated!
Let's proceed with merging the PR once we've incorporated the changes and run integration tests on the basecoin-rs.
Additionally, it would be great to include adequate unit tests as well, though perhaps in a follow-up PR?
|
If the tests are in separate PR, please add the changelog entries before merging 🙂 |
rnbguy
reviewed
Mar 28, 2024
|
Is there anything pending for this PR? otherwise, we can wrap it us by adding changelog entries 🙂 |
* Initial scaffolding for testing client recovery * Add MsgRecoverClient variant to ClientMsg * Stub out `test_recover_client_ok` * Update recover client test fixture * Perform client updates on subject and substitute clients * Create mock headers with timestamps * Extend substitute client state's trusting period * Remove eprintln statements * Add some more tests * Refactor to not require call to `sleep` * Add docstring for `MockClientState::new` * Formatting * Remove recover_client validate and execute calls from dispatch * Remove recover_client validate and execute calls from dispatch * Call recover_client::validate and execute correctly * Change MockClientState default trusting period to 10 seconds
Farhad-Shabani
added a commit
that referenced
this pull request
Sep 9, 2024
* Add `MsgRecoverClient` domain type * Working on client recovery validation handler logic * Implement recover_client::validate function * Add ClientStateValidation::check_substitute function * Working on `recover_client::execute` function * Stub out `update_on_recovery` function * Implement standalone `update_on_recovery` function * Change `update_on_recovery` function param * Remove `ClientState::initialise` call from `execute` * Implement `check_substitute` in ics07 * Update `update_on_recovery` function * Add `MsgRecoverClient` domain type * Working on client recovery validation handler logic * Implement recover_client::validate function * Add ClientStateValidation::check_substitute function * Working on `recover_client::execute` function * Stub out `update_on_recovery` function * Implement standalone `update_on_recovery` function * Change `update_on_recovery` function param * Remove `ClientState::initialise` call from `execute` * Implement `check_substitute` in ics07 * Update `update_on_recovery` function * Resolve errors upon rebasing * Add additional bounds on `check_substitute` and `update_on_recovery` * Implement TryFrom<AnyClientState> for ClientStateType * Attempt to implement check_substitute and update_on_recovery in ibc-derive * fix: introduce fully-qualified path to get ibc-derive work * Change substitute client state function params to Any * Update change_substitute and update_on_recovery call sites * Add additional bounds on other validation and execution context methods * Implement `update_on_recovery` for MockClientState * Silence unused variables warning * Add missing MockClientState fields * Fix some typos in doc comments * Remove unnecessary trait bound * Change `upgrade_client_proposal_handler` -> `execute_upgrade_client_proposal` * Add upgrade proposal type url * Remove commented-out trait bounds * Convert Any to ClientState instead of ClientStateType * Improve error message wording * Improve error message wording * Remove unnused function paramter * Replace `self` with `&self` * Convert substitute client state to MockClientState instead of Tm client state * Update ClientNotInactive error variant message * Improve `check_substitute` implementation * Make `Status` type `Copy` * Remove some imports and streamline mock client state initialization * rm redundant methods * Remove unnecessary TryFrom impl * tests: add unit tests for client recovery (#1151) * Initial scaffolding for testing client recovery * Add MsgRecoverClient variant to ClientMsg * Stub out `test_recover_client_ok` * Update recover client test fixture * Perform client updates on subject and substitute clients * Create mock headers with timestamps * Extend substitute client state's trusting period * Remove eprintln statements * Add some more tests * Refactor to not require call to `sleep` * Add docstring for `MockClientState::new` * Formatting * Remove recover_client validate and execute calls from dispatch * Remove recover_client validate and execute calls from dispatch * Call recover_client::validate and execute correctly * Change MockClientState default trusting period to 10 seconds * Add changelog entry --------- Co-authored-by: Farhad Shabani <[email protected]> Co-authored-by: Ranadeep Biswas <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #1110
Closes: #1111
Closes: #1112
Description
PR author checklist:
unclog.docs/).Reviewer checklist:
Files changedin the GitHub PR explorer.