[stateless_validation] Add testing for handling state witness and chunk endorsements #10504
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
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #10504 +/- ##
==========================================
+ Coverage 71.97% 72.00% +0.03%
==========================================
Files 720 720
Lines 146260 146265 +5
Branches 146260 146265 +5
==========================================
+ Hits 105264 105322 +58
+ Misses 36163 36117 -46
+ Partials 4833 4826 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
robin-near
approved these changes
Jan 26, 2024
| ) = msg | ||
| { | ||
| chunk_hashes.remove(endorsement.chunk_hash()); | ||
| for network_adapter in network_adapters.iter() { |
There was a problem hiding this comment.
nit: you can replace network_adapters.iter() with &network_adapters
| &mut self, | ||
| mut chunk_hashes: HashMap<ChunkHash, (BlockHeight, ShardId)>, | ||
| chunk_hash_to_account_ids: &mut HashMap<ChunkHash, HashSet<AccountId>>, |
There was a problem hiding this comment.
do you need this to be a &mut? Could you take the whole object by ownership?
| ) { | ||
| let _span = tracing::debug_span!(target: "test", "get_all_chunk_endorsements").entered(); | ||
| let network_adapters = self.network_adapters.clone(); |
There was a problem hiding this comment.
Do we actually need to clone this?
There was a problem hiding this comment.
Yes as otherwise we would hold a mut reference to self within the loop. rustc wasn't happy
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 26, 2024
…10507) This PR uncomments two checks - We now filter chunks based on whether they have enough stake endorsed or not. - We validate chunk endorsements in block header. Enabling this breaks a bunch of tests mainly for two reasons - Some tests are handled by mocking out parts/doing manual block production which doesn't take care of chunk endorsement signatures - A number of tests that work with multiple validators don't have the appropriate network request handlers for chunk endorsement and state witness processing. These tests are impossible to explore and fix in a single PR and I've created a task list to help with this here: #10506 Note that we need to merge in the changes from #10501 , #10503 and #10504 before we can merge this. Till then probably the tests would keep failing Tip: Probably a good idea to review PR commit by commit
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 improves the testing infrastructure for stateless validation to handle cases of network message propagation of
ChunkStateWitnessandChunkEndorsement.With the changes in validation of signature in chunk header, we need to handle
ChunkEndorsementmessages at the time of block production.