Remove the PartSetHeader from VoteSetMaj23, VoteSetBits, and state.State #479
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 Report
@@ Coverage Diff @@
## master #479 +/- ##
==========================================
- Coverage 62.86% 62.80% -0.06%
==========================================
Files 263 263
Lines 32499 32455 -44
==========================================
- Hits 20430 20383 -47
- Misses 10525 10531 +6
+ Partials 1544 1541 -3
|
|
Is the proto breaking CI an issue? https://github.com/celestiaorg/lazyledger-core/pull/479/checks?check_run_id=3192469663 |
|
it only indicates that the protos were changed (and it's a breaking change). |
adlerjohn
reviewed
Aug 3, 2021
| @@ -142,7 +142,7 @@ func TestValidateBlockCommit(t *testing.T) { | |||
| wrongHeightVote, err := types.MakeVote( | |||
| height, | |||
| state.LastBlockID, | |||
| state.LastPartSetHeader, | |||
| types.PartSetHeader{}, | |||
There was a problem hiding this comment.
I know the title of the PR doesn't include Votes proper, but is there a reason not to remove it from votes too?
There was a problem hiding this comment.
That's a great question! The next step in removing the PartSetHeader will be removing from Vote and Commit. Unfortunately, that will include more meaningful changes to how the consensus reactor passes data around, and I figured I should get as many easily isolated changes out of the way as possible to make those more complex changes easier to review.
Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>
adlerjohn
approved these changes
Aug 5, 2021
This was referenced Aug 23, 2021
evan-forbes
added a commit
that referenced
this pull request
Aug 25, 2021
* Revert "Remove the PartSetHeader from VoteSetMaj23, VoteSetBits, and state.State (#479)" This reverts commit 3ba47c2. * Revert "Stop Signing over the `PartSetHeader` for Votes and remove it from the `Header` (#457)" This reverts commit 818be04. * Revert "Decouple PartSetHeader from BlockID (#441)" This reverts commit 9d4265d.
evan-forbes
added a commit
that referenced
this pull request
Aug 25, 2021
* Revert "Remove the PartSetHeader from VoteSetMaj23, VoteSetBits, and state.State (#479)" This reverts commit 3ba47c2. * Revert "Stop Signing over the `PartSetHeader` for Votes and remove it from the `Header` (#457)" This reverts commit 818be04. * Revert "Decouple PartSetHeader from BlockID (#441)" This reverts commit 9d4265d. * Revert "Save and load block data using IPFS (#374)" This reverts commit 8da1644. * fix merge error * Revert "Add the ipfs dag api object in Blockstore (#356)" and get rid of embedded ipfs objects This reverts commit 40acb17. * remove ipfs node provider from new node * stop init ipfs repos * remove IPFS node config * clean up remaining ipfs stuff
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.
Description
This PR is the next step in #418 in removing the PartSetHeader as much as possible, as it removes it from VoteSetMaj23, VoteSetBits, and state.State. Once again, I was careful that I didn't actually change anything meaningful, and only removed the PartSetHeader from the mentioned structs.
I think that removing the PartSetHeader further will require slightly more meaningful changes, so I figured I try to get as many of the cumbersome but easy changes out of the way as possible.