feat(state): CometBFT does not store blobs#26
Open
alesforz wants to merge 30 commits intoberachain:bera-v1.xfrom
Open
feat(state): CometBFT does not store blobs#26alesforz wants to merge 30 commits intoberachain:bera-v1.xfrom
alesforz wants to merge 30 commits intoberachain:bera-v1.xfrom
Conversation
* Add `blob` field to `PrepareProposalResponse`. * Add `Blob` type as `[]byte`. * `BlockExecutor.CreateProposalBlock` returns the blob as well. * update supranational/blst to fix compile errors * Add `TestCreateProposalWithBlob` unit test. Tests that a call to `PrepareProposal` returns a blob inr `PrepareProposalResponse`. * update go.sum * Update call sites of `State.createProposalBlock()` * removed unnecessary line from docs --------- Signed-off-by: Alessandro Sforzin <alesforz@gmail.com>
This was referenced Mar 13, 2025
* Update `RoundState` with blob fields * Added methods `Hash` and `String` to type `Blob`. * Update `RoundStateSimple` creation * Update `RoundState.StringIndented()` * `RoundState` fields of type `Blob` are no longer pointers. Type `Blob` is a `[]byte` slice, so there's no need for it to be a pointer. * Update `String()` method of type `Blob` It now returns a different string when the blob is nil, as type `Block` does.
* Move `Blob` type and methods to separate file * Add `BlobID` type * Added protobuf definitions for type `Blob` * Add unit tests for `BlobID` methods * Fix typo pointed out in code review Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> Signed-off-by: Alessandro Sforzin <alesforz@gmail.com> * regenerated protobuf --------- Signed-off-by: Alessandro Sforzin <alesforz@gmail.com> Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
This was referenced Mar 14, 2025
added 2 commits
March 18, 2025 12:44
* Extend type `Proposal` with `BlobID` field * Update `Proposal` methods `ValidateBasic()` and `String()` to use `BlobID` * Add `BlobID` to `Proposal` protobuf. Regenerated the proto files as well. Moved the proto definition of `BlobID` to the `types.proto` file to fix an import cycle. * Add function to canonalize `BlobID` * Update `Proposal` to/from protobuf functions to include `BlobID` * Update `Proposal` unit tests to use `BlobID` * Add method `IsNil()` to type `BlobID` * `State.defaultDecideProposal() assigns an empty `BlobID` to `Proposal`. Since `BlobID` is not used in the consensus logic yet, this change is harmless. However, it allows us to keep this PR focused solely on `Proposal`-related changes. In a future PR, we’ll handle the proper creation of `BlobID` by implementing blob splitting into multiple parts. Additionally, this PR includes a refactor for improved readability. * Add missing `BlobID` param to `decideProposal()` * Add missing `BlobID` param in calls to `NewProposal()` in state_test.go * Add missing `BlobID` param in calls to `NewProposal()` in replay_test.go * Add missing `BlobID` param in calls to `NewProposal()` in pbts_test.go * Add missing `BlobID` param in calls to `NewProposal()` in byzantine_test.go * Fixed unit test TestConsMsgsVectors. The `Proposal` sub-test was using an incorrect hex string to check the test's results. The string changed because we added the `BlobID` field to the `Proposal` type, thus changing its encoding to protobuf. Also rearrenged the code for better readability. * Fixed unit test `TestWALEncoder`. It was using an incorrect hex string to check the test's results. The string changed because we added the `BlobID` field to the `Proposal` type, thus changing its encoding to protobuf. Also rearrenged the code for better readability. * Fixed unit test `TestPrivvalVectors`. The sub-tests "Proposal Request", "Proposal Response", and "Proposal Response with error" were was using an incorrect hex string to check the test's results. The string changed because we added the `BlobID` field to the `Proposal` type, thus changing its encoding to protobuf. Also rearrenged the code for better readability. * Extend `EventDataCompleteProposal` with `BlobID` field * `Hash()` method of type `Blob` checks return `nil` if empty blob. * updated docs * Update `State.isProposalComplete()`. Rationale: If a block has an associated blob, we set the `BlobID` in `Proposal` after returning from `PrepareProposal()`, causing the `IsNil()` check to return false. If the block does not have a blob, the `BlobID` field in `Proposal` will remain its zero value, making `IsNil()` return true. If a blob is present, `cs.ProposalBlob.IsNil()` will always return true until `cs.ProposalBlob` is assigned a non-empty `[]byte` slice. This assignment happens only after we have received all the blob parts and we have successfully reconstructed the blob.
added 5 commits
March 19, 2025 11:43
* Update protobuf definition of `ProcessProposalRequest` * Update calls to `ProcessProposal` to include the blob. * Update test `TestProcessProposal`
…sus state and reactor (#9) * Add `BlobPartMessage` in consensus reactor * Add `BlobPartMessage` case in consensus state `handleMsg()` * add blob part in protobuf messages * Add `HasProposalBlobPartMessage` to consensus reactor * Implement `Wrap()` method for types `BlobPart` and `HasProposalBlobPart` * Added `BlobPartMessage` and `HasBlobPartMessage` to functions serializing/deserializing protobuf messages to/from their corresponding Go types * Add `ProposalBlobPart` to events * Add `BlobParts` to `peerStateStats`. Does it make sense to mark a peer as good if it has sent enough blob parts? See `switch` in `Reactor.peerStatsRoutine()`. * Add `BlobPartMessage` to WAL replay after crash. * Fix unit test `TestMarshalJSONPeerState` * Update unit test `TestMsgToProto` * Add test `TestBlobPartMessageValidateBasic` * Small refactor of `State.readReplayMessage()` * Add a blob ID to some WAL tests * Address comments from code review * Register types `BlobPartMessage` and `HasProposalBlobPartMessage` in consensus reactor `init`() * Add `TODO` in consensus reactor `subscribeToBroadcastEvents()`. In this PR we don't call `FireEvent` for `BlobPartMessage`. We must add this call in a future PR implementing consensus state updates for blobs. Because this code isn't there yet, there isn't a "correct spot" to place the call to `FireEvent` yet.
feat(state): Split `Blob` into blob parts and send it to internal message queue
feat(state): Add blob-related fields to `PeerRoundState`
This was referenced Apr 7, 2025
* Handle reception of BlobPartMessage * Gossip blob part to connecting peers (current round only) * Receive routine fixes * Clear blob on new round * add basic test * different way of checking for complete block * lint * lint2 * Properly test whether partset is complete, fixes tests * lint3 * Added SendBlobPartSetHasBlobPart * lint4 * lint5 * Renamed BlockPartSizeBytes to PartSizeBytes to indicate it is also impacting blobs * block/blob completeness check fixes * trying alternative blob completion check * blob completion check fixes * Reverting to IsComplete when nil * Revert "blob completion check fixes" This reverts commit de950b1. * Revert "trying alternative blob completion check" This reverts commit 35a2819. * Revert "block/blob completeness check fixes" This reverts commit 9cb911f. * Revert "Reverting to IsComplete when nil" This reverts commit dd16f7d. * Attempt to fix the checks * Small fix * PartSetType for blocks and blobs * Use partsettype * supress similar tests as duplicates * Remove proto decoding for blobs * Add unit test with blob * Fix build errors * Use randStateWithBlob * Set ProposalBlobPartSetHeader; note this commit also resets them , that is to be revisited * Unit test to send blob between peers * Reintroduced MaxBlobSizeBytes * lint: removed unused function * blob existence fixes * Revert "blob existence fixes" This reverts commit 499371d. * condition simplified, updateHeight clears blob * feat(consensus): Configurable max blob size bytes (#17) * added blob parameters to params.proto * make proto-gen * extended ConsensusParams in types/params.go * test app fixes for configurable blob size * comment updated in auto-generated protobuf * unit test fix for blob config * typo fix * unit tests for blob bytes config * add detail to BlobParams doc * regenerate protobuf files --------- Co-authored-by: Alessandro Sforzin <alessandro@informal.systems> * removed PartSetType * removed unused function, lint * removed MaxBlobSizeBytes * No BlobID pointer in tests * lint * Set `ProposalBlob` to empty slice rather than `nil` in `enterNewRound()` * Add `TestStateEnterProposeWithBlob()` Checks that we finish the proposal stage without timing out (i.e., we have the proposal) for a block with a corresponding blob. * Fix incorrect formula computing `MaxBlockPartsCount`. The previous formula always added one extra part, even when the block size is an exact multiple of the part size. * Rename `BlockPartSizeBytes` to `PartSizeBytes`. * Fix `TestNewValidBlockMessageValidateBasic` * Add blob in `TestStateFullRound1` * Add blob to `TestStateFullRoundNil` * Add blob to TestStateFullRound2 * Fix race condition in test * Update util `decideProposal()` with blob. - Adds a non-empty `BlobID` to proposal - Returns the blob * Add blob to `TestStateLock_NoPOL` * Add blob to `TestStateLock_POLUpdateLock` * Add blob to `TestStateLock_POLRelock` * Add blob to `TestStateLock_PrevoteNilWhenLockedAndMissProposal` * Add blob to `TestStateLock_PrevoteNilWhenLockedAndDifferentProposal` * Add blob to `TestStateLock_POLDoesNotUnlock` * Add blob to `TestStateLock_MissingProposalWhenPOLSeenDoesNotUpdateLock` * Add blob to `TestStateLock_MissingProposalWhenPOLForLockedBlock` * Add blob to `TestStateLock_DoesNotLockOnOldProposal` * Add blob to `TestStateLock_POLSafety1` * Add `TestBlob()` to kvstore example application * Add blob to `TestStateLock_POLSafety1` * Add blob to `TestStateLock_POLSafety2` * lintwq * Add blob to `TestState_PrevotePOLFromPreviousRound` * Add blob to `TestProposeValidBlock` * Add blob to `TestSetValidBlockOnDelayedPrevote` * `ensureProposalWithTimeout` error string prints BlobID instead of BlockID. * Add blob to `TestSetValidBlockOnDelayedProposal` * Add blob to `TestResetTimeoutPrecommitUponNewHeight` * Add blob to `TestState_MissingProposalValidBlockReceivedTimeout`. * Improve blob testing in `TestState_MissingProposalValidBlockReceivedTimeout` * Add blob to `TestState_MissingProposalValidBlockReceivedPrecommit` * Improve blob testing in `TestStateLock_POLSafety1` * Improve blob testing in `TestStateLock_POLSafety2` * Add blob to `TestStateTimestamp_ProposalNotMatch` * Add blob to `TestStateBadProposal`. Improve blob testing in `TestStateTimestamp_ProposalNotMatch` * Test func `decideProposal` returns app blob instead of generating one. * Add blob `nil` checks in several tests * Fix `TestResetTimeoutPrecommitUponNewHeight` * Add blob to `TestByzantineConflictingProposalsWithPartition` * minor refactor and fix incorrect msg --------- Co-authored-by: Jasmina Malicevic <jasmina@informal.systems> Co-authored-by: Alessandro Sforzin <alessandro@informal.systems>
* Handle reception of BlobPartMessage * Gossip blob part to connecting peers (current round only) * Receive routine fixes * Clear blob on new round * add basic test * different way of checking for complete block * lint * lint2 * Properly test whether partset is complete, fixes tests * lint3 * Added SendBlobPartSetHasBlobPart * lint4 * lint5 * Renamed BlockPartSizeBytes to PartSizeBytes to indicate it is also impacting blobs * e2e test app update to generate and verify blobs * block/blob completeness check fixes * lint1 * trying alternative blob completion check * blob completion check fixes * Reverting to IsComplete when nil * Revert "blob completion check fixes" This reverts commit de950b1. * Revert "trying alternative blob completion check" This reverts commit 35a2819. * Revert "block/blob completeness check fixes" This reverts commit 9cb911f. * Revert "Reverting to IsComplete when nil" This reverts commit dd16f7d. * Attempt to fix the checks * Small fix * PartSetType for blocks and blobs * Use partsettype * supress similar tests as duplicates * Remove proto decoding for blobs * Add unit test with blob * Fix build errors * Use randStateWithBlob * Set ProposalBlobPartSetHeader; note this commit also resets them , that is to be revisited * Unit test to send blob between peers * Reintroduced MaxBlobSizeBytes * e2e app MaxBlobByteSize * e2e app half size blobs * lint: removed unused function * simpler blob generation algorithm * blob fix * blob existence fixes * Revert "blob existence fixes" This reverts commit 499371d. * condition simplified, updateHeight clears blob * feat(consensus): Configurable max blob size bytes (#17) * added blob parameters to params.proto * make proto-gen * extended ConsensusParams in types/params.go * test app fixes for configurable blob size * comment updated in auto-generated protobuf * unit test fix for blob config * typo fix * unit tests for blob bytes config * add detail to BlobParams doc * regenerate protobuf files --------- Co-authored-by: Alessandro Sforzin <alessandro@informal.systems> * removed PartSetType * removed unused function, lint * removed MaxBlobSizeBytes * No BlobID pointer in tests * lint * Set `ProposalBlob` to empty slice rather than `nil` in `enterNewRound()` * Add `TestStateEnterProposeWithBlob()` Checks that we finish the proposal stage without timing out (i.e., we have the proposal) for a block with a corresponding blob. * Fix incorrect formula computing `MaxBlockPartsCount`. The previous formula always added one extra part, even when the block size is an exact multiple of the part size. * Rename `BlockPartSizeBytes` to `PartSizeBytes`. * Fix `TestNewValidBlockMessageValidateBasic` * Fixed check for empty blob * Add blob in `TestStateFullRound1` * Add blob to `TestStateFullRoundNil` * Add blob to TestStateFullRound2 * Fix race condition in test * Update util `decideProposal()` with blob. - Adds a non-empty `BlobID` to proposal - Returns the blob * Add blob to `TestStateLock_NoPOL` * Add blob to `TestStateLock_POLUpdateLock` * Add blob to `TestStateLock_POLRelock` * Add blob to `TestStateLock_PrevoteNilWhenLockedAndMissProposal` * Add blob to `TestStateLock_PrevoteNilWhenLockedAndDifferentProposal` * Add blob to `TestStateLock_POLDoesNotUnlock` * Add blob to `TestStateLock_MissingProposalWhenPOLSeenDoesNotUpdateLock` * Add blob to `TestStateLock_MissingProposalWhenPOLForLockedBlock` * Add blob to `TestStateLock_DoesNotLockOnOldProposal` * Add blob to `TestStateLock_POLSafety1` * Add `TestBlob()` to kvstore example application * Add blob to `TestStateLock_POLSafety1` * Add blob to `TestStateLock_POLSafety2` * lintwq * Add blob to `TestState_PrevotePOLFromPreviousRound` * Add blob to `TestProposeValidBlock` * Add blob to `TestSetValidBlockOnDelayedPrevote` * `ensureProposalWithTimeout` error string prints BlobID instead of BlockID. * Add blob to `TestSetValidBlockOnDelayedProposal` * Add blob to `TestResetTimeoutPrecommitUponNewHeight` * Add blob to `TestState_MissingProposalValidBlockReceivedTimeout`. * Revert "Add blob to `TestState_MissingProposalValidBlockReceivedTimeout`." This reverts commit a17ad30. Wrong branch. * Add blob to `TestState_MissingProposalValidBlockReceivedTimeout`. * Improve blob testing in `TestState_MissingProposalValidBlockReceivedTimeout` * Add blob to `TestState_MissingProposalValidBlockReceivedPrecommit` * Improve blob testing in `TestStateLock_POLSafety1` * Improve blob testing in `TestStateLock_POLSafety2` * Add blob to `TestStateTimestamp_ProposalNotMatch` * Add blob to `TestStateBadProposal`. Improve blob testing in `TestStateTimestamp_ProposalNotMatch` * Test func `decideProposal` returns app blob instead of generating one. * Add blob `nil` checks in several tests * Fix `TestResetTimeoutPrecommitUponNewHeight` * Add blob to `TestByzantineConflictingProposalsWithPartition` * minor refactor and fix incorrect msg --------- Co-authored-by: Jasmina Malicevic <jasmina@informal.systems> Co-authored-by: Alessandro Sforzin <alessandro@informal.systems>
* Handle reception of BlobPartMessage * Gossip blob part to connecting peers (current round only) * Receive routine fixes * Clear blob on new round * add basic test * different way of checking for complete block * lint * lint2 * Properly test whether partset is complete, fixes tests * lint3 * Added SendBlobPartSetHasBlobPart * lint4 * lint5 * Renamed BlockPartSizeBytes to PartSizeBytes to indicate it is also impacting blobs * block/blob completeness check fixes * trying alternative blob completion check * blob completion check fixes * Reverting to IsComplete when nil * Revert "blob completion check fixes" This reverts commit de950b1. * Revert "trying alternative blob completion check" This reverts commit 35a2819. * Revert "block/blob completeness check fixes" This reverts commit 9cb911f. * Revert "Reverting to IsComplete when nil" This reverts commit dd16f7d. * Attempt to fix the checks * Small fix * PartSetType for blocks and blobs * Use partsettype * supress similar tests as duplicates * Remove proto decoding for blobs * Add unit test with blob * Fix build errors * Use randStateWithBlob * Set ProposalBlobPartSetHeader; note this commit also resets them , that is to be revisited * Unit test to send blob between peers * Reintroduced MaxBlobSizeBytes * lint: removed unused function * implement basic blob metrics * blob existence fixes * Revert "blob existence fixes" This reverts commit 499371d. * condition simplified, updateHeight clears blob * feat(consensus): Configurable max blob size bytes (#17) * added blob parameters to params.proto * make proto-gen * extended ConsensusParams in types/params.go * test app fixes for configurable blob size * comment updated in auto-generated protobuf * unit test fix for blob config * typo fix * unit tests for blob bytes config * add detail to BlobParams doc * regenerate protobuf files --------- Co-authored-by: Alessandro Sforzin <alessandro@informal.systems> * removed PartSetType * removed unused function, lint * removed MaxBlobSizeBytes * No BlobID pointer in tests * lint * Set `ProposalBlob` to empty slice rather than `nil` in `enterNewRound()` * Add `TestStateEnterProposeWithBlob()` Checks that we finish the proposal stage without timing out (i.e., we have the proposal) for a block with a corresponding blob. * Fix incorrect formula computing `MaxBlockPartsCount`. The previous formula always added one extra part, even when the block size is an exact multiple of the part size. * Rename `BlockPartSizeBytes` to `PartSizeBytes`. * Fix `TestNewValidBlockMessageValidateBasic` * Add blob in `TestStateFullRound1` * Add blob to `TestStateFullRoundNil` * Add blob to TestStateFullRound2 * Fix race condition in test * Update util `decideProposal()` with blob. - Adds a non-empty `BlobID` to proposal - Returns the blob * Add blob to `TestStateLock_NoPOL` * Add blob to `TestStateLock_POLUpdateLock` * Add blob to `TestStateLock_POLRelock` * Add blob to `TestStateLock_PrevoteNilWhenLockedAndMissProposal` * Add blob to `TestStateLock_PrevoteNilWhenLockedAndDifferentProposal` * Add blob to `TestStateLock_POLDoesNotUnlock` * Add blob to `TestStateLock_MissingProposalWhenPOLSeenDoesNotUpdateLock` * Add blob to `TestStateLock_MissingProposalWhenPOLForLockedBlock` * Add blob to `TestStateLock_DoesNotLockOnOldProposal` * Add blob to `TestStateLock_POLSafety1` * Add `TestBlob()` to kvstore example application * Add blob to `TestStateLock_POLSafety1` * Add blob to `TestStateLock_POLSafety2` * lintwq * Add blob to `TestState_PrevotePOLFromPreviousRound` * Add blob to `TestProposeValidBlock` * Add blob to `TestSetValidBlockOnDelayedPrevote` * `ensureProposalWithTimeout` error string prints BlobID instead of BlockID. * Add blob to `TestSetValidBlockOnDelayedProposal` * Add blob to `TestResetTimeoutPrecommitUponNewHeight` * Add blob to `TestState_MissingProposalValidBlockReceivedTimeout`. * Improve blob testing in `TestState_MissingProposalValidBlockReceivedTimeout` * Add blob to `TestState_MissingProposalValidBlockReceivedPrecommit` * Improve blob testing in `TestStateLock_POLSafety1` * Improve blob testing in `TestStateLock_POLSafety2` * Add blob to `TestStateTimestamp_ProposalNotMatch` * Add blob to `TestStateBadProposal`. Improve blob testing in `TestStateTimestamp_ProposalNotMatch` * Test func `decideProposal` returns app blob instead of generating one. * Add blob `nil` checks in several tests * Fix `TestResetTimeoutPrecommitUponNewHeight` * Add blob to `TestByzantineConflictingProposalsWithPartition` * minor refactor and fix incorrect msg * linting --------- Signed-off-by: Alessandro Sforzin <alessandro@informal.systems> Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com> Co-authored-by: Alessandro Sforzin <alessandro@informal.systems>
6 tasks
3 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 feature branch stores the work we are doing to prevent CometBFT from storing blobs.
We will merge related PRs to this branch rather than
bera-v1.x.Context
Berachain’s ABCI app includes two transactions in each proposal, the second of which is a blob cryptographically linked to the first. Because these blobs are included in blocks as transactions, CometBFT stores them in its block database, leading to a significant storage footprint over time.
A better approach is to avoid storing the blobs in CometBFT’s storage layer at all. This means the ABCI app must not include them in the block itself. However, the app still needs access to these blobs during
ProcessProposalfor state validation. Therefore, we must ensure CometBFT gossips and delivers the blobs to the app alongside the corresponding block.Changes merged so far
PrepareProposalreturns the blob separately from the blocks' txs (PR)RoundStatedata structure (PR 5, PR 8)BlobIDtype (PR)Proposalmsg withBlobID(PR)ProcessProposaldelivers blob to ABCI application (PR)BlobPartMessage(PR)PeerRoundState(PR)BlobPartMessageRoundStateduring consensus roundsNote for Readers
All the code in this PR has already been reviewed and approved in the smaller PRs linked above. All commits are either merge commits of those PRs or merges from the
bera-v1.xbranch to keep this branch up to date.Therefore, to review the changes, I suggest looking at the list above and going to the specific PR that implemented the changes you're interested in. This should be easier than trying to review all the changes at once in this PR.