oak-audit: fix issues 2, 12; Updated Grandpa light client with authority set history #382
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
The GRANDPA light-client logic and storage is updated to maintain a history of authorities set changes. This feature is necessary to support a more robust system, especially for evidence checks. In the previous version, the client could only store the current authorities, preventing it from correctly verifying whether misbehavior had taken place when a set change occurs. The history of authorities is stored in the client state, with information of the block height, the timestamp of changes, the new set id and the new authorities. This commit also enhances the misbehaviour detection functionality by checking every header in the submitted headers for evidence instead of only the first few unknown headers. Additional fixes contain correcting the finality proof verification for client update and modifying relevant test logic accommodating the new client features.
blasrodri
reviewed
Aug 1, 2023
| @@ -257,6 +257,33 @@ where | |||
| Ok(()) | |||
| } | |||
|
|
|||
| /// The function checks if the given chain is canonical: | |||
blasrodri
reviewed
Aug 1, 2023
| @@ -162,37 +158,32 @@ impl grandpa_client_primitives::HostFunctions for HostFunctionsManager { | |||
| pub_key.verify(&msg, sig) | |||
| } | |||
|
|
|||
| fn insert_relay_header_hashes(new_hashes: &[<Self::Header as Header>::Hash]) { | |||
| fn insert_relay_header_hashes(now_ms: u64, new_hashes: &[<Self::Header as Header>::Hash]) { | |||
There was a problem hiding this comment.
why do you need to pass now_ms as an argument? Where does it come from?
blasrodri
reviewed
Aug 2, 2023
hyperspace/parachain/src/provider.rs
Outdated
| .query_client_state(latest_cp_height, client_id.clone()) | ||
| .await | ||
| .map_err(|e| Error::Custom(e.to_string())) | ||
| .unwrap(); |
There was a problem hiding this comment.
either map err or unwrap ser
…ties' into vmarkushin/grandpa-track-authorities
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.
This PR should ONLY be merged just before and when we're fully ready for the runtime upgrade, because otherwise the bridge may be broken
The GRANDPA light-client logic and storage is updated to maintain a history of authorities set changes. This feature is necessary to support a more robust system, especially for evidence checks. In the previous version, the client could only store the current authorities, preventing it from correctly verifying whether misbehavior had taken place when a set change occurs.
The history of authorities is stored in the client state, with information of the block height, the timestamp of changes, the new set id and the new authorities.
This commit also enhances the misbehaviour detection functionality by checking every header in the submitted headers for evidence instead of only the first few unknown headers.
Additional fixes contain correcting the finality proof verification for client update and modifying relevant test logic accommodating the new client features.
Issue 2: "Impossible to report misbehaviors older than 500 blocks"
Instead of storing the last N headers, we now store all the headers for a fixed period of time (HEADER_ITEM_LIFETIME), which allows the fisherman to submit misbehavior even if validators reached the previous limit by submitting many blocks at once. Also, previous authority sets are now stored using the same method.
Also, misbehaviour validation was fixed: before the fix, it was only comparing the base blocks (they should have the same hash) and target block (hashes should be different), but it didn't check that these chains are actually diverging (one of them is a fork of another).