feat(consensus): additional sanity checks for the size of proposed blocks (backport #1408) by mergify[bot] · Pull Request #2139 · cometbft/cometbft

mergify · 2024-01-26T07:49:17Z

This is an automatic backport of pull request #1408 done by Mergify.
Cherry-pick of 28ad4d2 has failed:

On branch mergify/bp/v0.38.x/pr-1408
Your branch is up to date with 'origin/v0.38.x'.

You are currently cherry-picking commit 28ad4d223.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   consensus/state.go
	modified:   consensus/state_test.go
	modified:   crypto/merkle/proof.go
	modified:   evidence/pool_test.go
	modified:   state/execution_test.go
	modified:   types/event_bus_test.go
	modified:   types/part_set.go
	modified:   types/part_set_test.go

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   internal/consensus/errors.go
	both modified:   state/store_test.go
	both modified:   store/store_test.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

@Mergifyio refresh will re-evaluate the rules
@Mergifyio rebase will rebase this PR on its base branch
@Mergifyio update will merge the base branch into this PR
@Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

look at your merge queues
generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

…ocks (#1408) Hardens tests regarding the size of proposed blocks, namely: - The byte size of a proposal block `Part` should be constant (`== types.BlockPartSizeBytes`), except for the last part of a `PartSet` (`<= types.BlockPartSizeBytes`) - A valid `Proposal` should not enclose a `PartSet` enabling the building of a `ProposalBlock` with size larger than the configured `ConsensusParams.Block.MaxBytes`. Notice that building a `ProposalBlock` larger than the allowed would fail in any case, but the proposed changes also invalidate the associated `Proposal`. --- #### PR checklist - [x] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: Andy Nogueira <me@andynogueira.dev> (cherry picked from commit 28ad4d2) # Conflicts: # internal/consensus/errors.go # state/store_test.go # store/store_test.go

…posed blocks (#1408)" This reverts commit 1877a69.

…ocks (#1408) Hardens tests regarding the size of proposed blocks, namely: - The byte size of a proposal block `Part` should be constant (`== types.BlockPartSizeBytes`), except for the last part of a `PartSet` (`<= types.BlockPartSizeBytes`) - A valid `Proposal` should not enclose a `PartSet` enabling the building of a `ProposalBlock` with size larger than the configured `ConsensusParams.Block.MaxBytes`. Notice that building a `ProposalBlock` larger than the allowed would fail in any case, but the proposed changes also invalidate the associated `Proposal`. --- - [x] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: Andy Nogueira <me@andynogueira.dev>

cason

Changes in this backport are identical to the original PR, with the following exceptions:

fillStore is not on state/store_test.go in this branch
TestPruningService is not on store/store_test.go in this branch
we don't have consensus/errors.go in this branch, so the new error was added to consensus/state.go

cason · 2024-01-26T10:39:38Z

consensus/state.go

 	ErrInvalidProposalPOLRound    = errors.New("error invalid proposal POL round")
 	ErrAddingVote                 = errors.New("error adding vote")
 	ErrSignatureFoundInPastBlocks = errors.New("found signature from the same key")
+	ErrProposalTooManyParts       = errors.New("proposal block has too many parts")


Note for reviewers: we don't have consensus/errors.go in this branch.

sergio-mena

Checked diff-of-diffs.
Unlike in #2140 and #2141, you included here the changes to TestLoadBlockPart made in the original PR. Thanks!

…ocks (backport cometbft#1408) (cometbft#2139) This is an automatic backport of pull request cometbft#1408 done by [Mergify](https://mergify.com). Cherry-pick of 28ad4d2 has failed: ``` On branch mergify/bp/v0.38.x/pr-1408 Your branch is up to date with 'origin/v0.38.x'. You are currently cherry-picking commit 28ad4d2. (fix conflicts and run "git cherry-pick --continue") (use "git cherry-pick --skip" to skip this patch) (use "git cherry-pick --abort" to cancel the cherry-pick operation) Changes to be committed: modified: consensus/state.go modified: consensus/state_test.go modified: crypto/merkle/proof.go modified: evidence/pool_test.go modified: state/execution_test.go modified: types/event_bus_test.go modified: types/part_set.go modified: types/part_set_test.go Unmerged paths: (use "git add/rm <file>..." as appropriate to mark resolution) deleted by us: internal/consensus/errors.go both modified: state/store_test.go both modified: store/store_test.go ``` To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally --- <details> <summary>Mergify commands and options</summary> <br /> More conditions and actions can be found in the [documentation](https://docs.mergify.com/). You can also trigger Mergify actions by commenting on this pull request: - `@Mergifyio refresh` will re-evaluate the rules - `@Mergifyio rebase` will rebase this PR on its base branch - `@Mergifyio update` will merge the base branch into this PR - `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you can: - look at your merge queues - generate the Mergify configuration with the config editor. Finally, you can contact us on https://mergify.com </details> --------- Co-authored-by: Daniel <daniel.cason@informal.systems> Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: Andy Nogueira <me@andynogueira.dev>

* feat(consensus): additional sanity checks for the size of proposed blocks (backport cometbft#1408) (cometbft#2139) This is an automatic backport of pull request cometbft#1408 done by [Mergify](https://mergify.com). Cherry-pick of 28ad4d2 has failed: ``` On branch mergify/bp/v0.38.x/pr-1408 Your branch is up to date with 'origin/v0.38.x'. You are currently cherry-picking commit 28ad4d2. (fix conflicts and run "git cherry-pick --continue") (use "git cherry-pick --skip" to skip this patch) (use "git cherry-pick --abort" to cancel the cherry-pick operation) Changes to be committed: modified: consensus/state.go modified: consensus/state_test.go modified: crypto/merkle/proof.go modified: evidence/pool_test.go modified: state/execution_test.go modified: types/event_bus_test.go modified: types/part_set.go modified: types/part_set_test.go Unmerged paths: (use "git add/rm <file>..." as appropriate to mark resolution) deleted by us: internal/consensus/errors.go both modified: state/store_test.go both modified: store/store_test.go ``` To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally --- <details> <summary>Mergify commands and options</summary> <br /> More conditions and actions can be found in the [documentation](https://docs.mergify.com/). You can also trigger Mergify actions by commenting on this pull request: - `@Mergifyio refresh` will re-evaluate the rules - `@Mergifyio rebase` will rebase this PR on its base branch - `@Mergifyio update` will merge the base branch into this PR - `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you can: - look at your merge queues - generate the Mergify configuration with the config editor. Finally, you can contact us on https://mergify.com </details> --------- Co-authored-by: Daniel <daniel.cason@informal.systems> Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: Andy Nogueira <me@andynogueira.dev> * Merge commit from fork --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Daniel <daniel.cason@informal.systems> Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: Andy Nogueira <me@andynogueira.dev> Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>

…ocks (backport cometbft#1408) (cometbft#2139) This is an automatic backport of pull request cometbft#1408 done by [Mergify](https://mergify.com). Cherry-pick of 28ad4d2 has failed: ``` On branch mergify/bp/v0.38.x/pr-1408 Your branch is up to date with 'origin/v0.38.x'. You are currently cherry-picking commit 28ad4d2. (fix conflicts and run "git cherry-pick --continue") (use "git cherry-pick --skip" to skip this patch) (use "git cherry-pick --abort" to cancel the cherry-pick operation) Changes to be committed: modified: consensus/state.go modified: consensus/state_test.go modified: crypto/merkle/proof.go modified: evidence/pool_test.go modified: state/execution_test.go modified: types/event_bus_test.go modified: types/part_set.go modified: types/part_set_test.go Unmerged paths: (use "git add/rm <file>..." as appropriate to mark resolution) deleted by us: internal/consensus/errors.go both modified: state/store_test.go both modified: store/store_test.go ``` To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally --- <details> <summary>Mergify commands and options</summary> <br /> More conditions and actions can be found in the [documentation](https://docs.mergify.com/). You can also trigger Mergify actions by commenting on this pull request: - `@Mergifyio refresh` will re-evaluate the rules - `@Mergifyio rebase` will rebase this PR on its base branch - `@Mergifyio update` will merge the base branch into this PR - `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you can: - look at your merge queues - generate the Mergify configuration with the config editor. Finally, you can contact us on https://mergify.com </details> --------- Co-authored-by: Daniel <daniel.cason@informal.systems> Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: Andy Nogueira <me@andynogueira.dev>

mergify bot requested a review from a team as a code owner January 26, 2024 07:49

mergify bot added the conflicts label Jan 26, 2024

Daniel Cason and others added 2 commits January 26, 2024 10:58

Revert "feat(consensus): additional sanity checks for the size of pro…

247d6e8

…posed blocks (#1408)" This reverts commit 1877a69.

cason approved these changes Jan 26, 2024

View reviewed changes

sergio-mena removed the conflicts label Jan 31, 2024

sergio-mena approved these changes Jan 31, 2024

View reviewed changes

Merge branch 'v0.38.x' into mergify/bp/v0.38.x/pr-1408

5713c59

cason merged commit 1f48ff9 into v0.38.x Jan 31, 2024

cason deleted the mergify/bp/v0.38.x/pr-1408 branch January 31, 2024 21:22

faddat mentioned this pull request Feb 20, 2024

chore: update the v0.38 experimental branch #2389

Closed

4 tasks

faddat mentioned this pull request Oct 15, 2024

chore: use latest cometbft-db in v0.38.x #4296

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(consensus): additional sanity checks for the size of proposed blocks (backport #1408)#2139

feat(consensus): additional sanity checks for the size of proposed blocks (backport #1408)#2139
cason merged 4 commits intov0.38.xfrom
mergify/bp/v0.38.x/pr-1408

mergify bot commented Jan 26, 2024

Uh oh!

cason left a comment

Uh oh!

cason Jan 26, 2024

Uh oh!

sergio-mena left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mergify bot commented Jan 26, 2024

Uh oh!

cason left a comment

Choose a reason for hiding this comment

Uh oh!

cason Jan 26, 2024

Choose a reason for hiding this comment

Uh oh!

sergio-mena left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants