cometbft · sergio-mena · Jun 20, 2023 · Jun 20, 2023
@@ -0,0 +1,9 @@
+- `[mempool]` Application can now set `ConsensusParams.Block.MaxBytes` to -1
+  to have visibility on all transactions in the
+  mempool at `PrepareProposal` time.
+  This means that the total size of transactions sent via `RequestPrepareProposal`
+  might exceed `RequestPrepareProposal.max_tx_bytes`.
+  If that is the case, the application MUST make sure that the total size of transactions
+  returned in `ResponsePrepareProposal.txs` does not exceed `RequestPrepareProposal.max_tx_bytes`,
+  otherwise CometBFT will panic.
+  ([\#980](https://github.com/cometbft/cometbft/issues/980))
@@ -0,0 +1,5 @@
+- `[mempool]` Application can now set `ConsensusParams.Block.MaxBytes` to -1
+  to gain more control on the max size of transactions in a block.
+  It also allows the application to have visibility on all transactions in the
+  mempool at `PrepareProposal` time.
+  ([\#980](https://github.com/cometbft/cometbft/pull/980))
@@ -1946,9 +1946,13 @@ func (cs *State) addProposalBlockPart(msg *BlockPartMessage, peerID p2p.ID) (add
 		cs.metrics.DuplicateBlockPart.Add(1)
 	}
 
-	if cs.ProposalBlockParts.ByteSize() > cs.state.ConsensusParams.Block.MaxBytes {
+	maxBytes := cs.state.ConsensusParams.Block.MaxBytes
+	if maxBytes == -1 {
+		maxBytes = int64(types.MaxBlockSizeBytes)
+	}
+	if cs.ProposalBlockParts.ByteSize() > maxBytes {
 		return added, fmt.Errorf("total size of proposal block parts exceeds maximum block bytes (%d > %d)",
-			cs.ProposalBlockParts.ByteSize(), cs.state.ConsensusParams.Block.MaxBytes,
+			cs.ProposalBlockParts.ByteSize(), maxBytes,
 		)
 	}
 	if added && cs.ProposalBlockParts.IsComplete() {

@@ -264,10 +264,12 @@ func TestCreateProposalBlock(t *testing.T) {
 	stateStore := sm.NewStore(stateDB, sm.StoreOptions{
 		DiscardABCIResponses: false,
 	})
-	maxBytes := 16384
-	var partSize uint32 = 256
-	maxEvidenceBytes := int64(maxBytes / 2)
-	state.ConsensusParams.Block.MaxBytes = int64(maxBytes)
+	var (
+		partSize uint32 = 256
+		maxBytes int64  = 16384
+	)
+	maxEvidenceBytes := maxBytes / 2
+	state.ConsensusParams.Block.MaxBytes = maxBytes
 	state.ConsensusParams.Evidence.MaxBytes = maxEvidenceBytes
 	proposerAddr, _ := state.Validators.GetByIndex(0)
 
@@ -305,7 +307,7 @@ func TestCreateProposalBlock(t *testing.T) {
 	// fill the mempool with more txs
 	// than can fit in a block
 	txLength := 100
-	for i := 0; i <= maxBytes/txLength; i++ {
+	for i := 0; i <= int(maxBytes)/txLength; i++ {
 		tx := cmtrand.Bytes(txLength)
 		err := mempool.CheckTx(tx, nil, mempl.TxInfo{})
 		assert.NoError(t, err)
@@ -333,7 +335,7 @@ func TestCreateProposalBlock(t *testing.T) {
 	// check that the part set does not exceed the maximum block size
 	partSet, err := block.MakePartSet(partSize)
 	require.NoError(t, err)
-	assert.Less(t, partSet.ByteSize(), int64(maxBytes))
+	assert.Less(t, partSet.ByteSize(), maxBytes)
 
 	partSetFromHeader := types.NewPartSetFromHeader(partSet.Header())
 	for partSetFromHeader.Count() < partSetFromHeader.Total() {

@@ -61,21 +61,22 @@ Process *p*'s prepared proposal can differ in two different rounds where *p* is
 Full execution of blocks at `PrepareProposal` time stands on CometBFT's critical path. Thus,
 Requirement 1 ensures the Application or operator will set a value for `TimeoutPropose` such that the time it takes
 to fully execute blocks in `PrepareProposal` does not interfere with CometBFT's propose timer.
-Note that the violation of Requirement 1 may lead to further rounds, but will not 
+Note that the violation of Requirement 1 may lead to further rounds, but will not
 compromise liveness because even though `TimeoutPropose` is used as the initial
 value for proposal timeouts, CometBFT will be dynamically adjust these timeouts
 such that they will eventually be enough for completing `PrepareProposal`.
 
 * Requirement 2 [`PrepareProposal`, tx-size]: When *p*'s Application calls `ResponsePrepareProposal`, the
   total size in bytes of the transactions returned does not exceed `RequestPrepareProposal.max_tx_bytes`.
 
-Busy blockchains might seek to maximize the amount of transactions included in each block. Under those conditions,
-CometBFT might choose to increase the transactions passed to the Application via `RequestPrepareProposal.txs`
-beyond the `RequestPrepareProposal.max_tx_bytes` limit. The idea is that, if the Application drops some of
-those transactions, it can still return a transaction list whose byte size is as close to
-`RequestPrepareProposal.max_tx_bytes` as possible. Thus, Requirement 2 ensures that the size in bytes of the
-transaction list returned by the application will never cause the resulting block to go beyond its byte size
-limit.
+Busy blockchains might seek to gain full visibility into transactions in CometBFT's mempool,
+rather than having visibility only on *a* subset of those transactions that fit in a block.
+The application can do so by setting `ConsensusParams.Block.MaxBytes` to -1.
+This instructs CometBFT (a) to enforce the maximum possible value for `MaxBytes` (100 MB) at CometBFT level,
+and (b) to provide *all* transactions in the mempool when calling `RequestPrepareProposal`.
+Under these settings, the aggregated size of all transactions may exceed `RequestPrepareProposal.max_tx_bytes`.
+Hence, Requirement 2 ensures that the size in bytes of the transaction list returned by the application will never
+cause the resulting block to go beyond its byte size limit.
 
 * Requirement 3 [`PrepareProposal`, `ProcessProposal`, coherence]: For any two correct processes *p* and *q*,
   if *q*'s CometBFT calls `RequestProcessProposal` on *u<sub>p</sub>*,
@@ -600,7 +601,19 @@ This is enforced by the consensus algorithm.
 This implies a maximum transaction size that is this `MaxBytes`, less the expected size of
 the header, the validator set, and any included evidence in the block.
 
-Must have `0 < MaxBytes < 100 MB`.
+If the Application wants full control over the size of blocks,
+it can do so by enforcing a byte limit set up at the Application level.
+This Application-internal limit is used by `PrepareProposal` to bound the total size
+of transactions it returns, and by `ProcessProposal` to reject any received block
+whose total transaction size is bigger than the enforced limit.
+In such case, the Application MAY set `MaxBytes` to -1.
+
+If the Application sets value -1, consensus will:
+
+- consider that the actual value to enforce is 100 MB
+- will provide *all* transactions in the mempool in calls to `PrepareProposal`
+
+Must have `MaxBytes == -1` OR `0 < MaxBytes <= 100 MB`.
 
 ##### BlockParams.MaxGas
 

@@ -44,7 +44,7 @@ title: Methods
     | version             | string | The application software semantic version           | 2            |
     | app_version         | uint64 | The application protocol version                    | 3            |
     | last_block_height   | int64  | Latest height for which the app persisted its state | 4            |
-    | last_block_app_hash | bytes  | Latest AppHash returned by `Commit`                 | 5            |
+    | last_block_app_hash | bytes  | Latest AppHash returned by `FinalizeBlock`          | 5            |
 
 * **Usage**:
     * Return information about the application state.
@@ -355,12 +355,16 @@ title: Methods
             traceability, it is its responsibility's to support it. For instance, the Application
             could attach to a transformed transaction a list with the hashes of the transactions it
             derives from.
-    * CometBFT MAY include a list of transactions in `RequestPrepareProposal.txs` whose total
-      size in bytes exceeds `RequestPrepareProposal.max_tx_bytes`.
+    * The Application MAY configure CometBFT to include a list of transactions in `RequestPrepareProposal.txs`
+      whose total size in bytes exceeds `RequestPrepareProposal.max_tx_bytes`.
+      If the Application sets `ConsensusParams.Block.MaxBytes` to -1, CometBFT
+      will include _all_ transactions currently in the mempool in `RequestPrepareProposal.txs`,
+      which may not fit in `RequestPrepareProposal.max_tx_bytes`.
       Therefore, if the size of `RequestPrepareProposal.txs` is greater than
       `RequestPrepareProposal.max_tx_bytes`, the Application MUST remove transactions to ensure
       that the `RequestPrepareProposal.max_tx_bytes` limit is respected by those transactions
-      returned in `ResponsePrepareProposal.txs` .
+      returned in `ResponsePrepareProposal.txs`.
+      This is specified in [Requirement 2](./abci%2B%2B_app_requirements.md).
     * As a result of executing the prepared proposal, the Application may produce block events or transaction events.
       The Application must keep those events until a block is decided and then pass them on to CometBFT via
       `ResponseFinalizeBlock`.

@@ -4,40 +4,58 @@ order: 2
 # Creating a proposal
 
 A block consists of a header, transactions, votes (the commit),
-and a list of evidence of malfeasance (ie. signing conflicting votes).
+and a list of evidence of malfeasance (eg. signing conflicting votes).
 
-We include no more than 1/10th of the maximum block size
-(`ConsensusParams.Block.MaxBytes`) of evidence with each block.
+Outstanding evidence items get priority over outstanding transactions in the mempool.
+All in all, the block MUST NOT exceed  `ConsensusParams.Block.MaxBytes`,
+or 100MB if `ConsensusParams.Block.MaxBytes == -1`.
 
 ## Reaping transactions from the mempool
 
 When we reap transactions from the mempool, we calculate maximum data
 size by subtracting maximum header size (`MaxHeaderBytes`), the maximum
-amino overhead for a block (`MaxAminoOverheadForBlock`), the size of
+protobuf overhead for a block (`MaxOverheadForBlock`), the size of
 the last commit (if present) and evidence (if present). While reaping
-we account for amino overhead for each transaction.
+we account for protobuf overhead for each transaction.
 
 ```go
-func MaxDataBytes(maxBytes int64, valsCount, evidenceCount int) int64 {
- return maxBytes -
+func MaxDataBytes(maxBytes, evidenceBytes int64, valsCount int) int64 {
+  return maxBytes -
   MaxOverheadForBlock -
   MaxHeaderBytes -
-  int64(valsCount)*MaxVoteBytes -
-  int64(evidenceCount)*MaxEvidenceBytes
+  MaxCommitBytes(valsCount) -
+  evidenceBytes
 }
 ```
 
+If `ConsensusParams.Block.MaxBytes == -1`, we reap *all* outstanding transactions from the mempool
+
+## Preparing the proposal
+
+Once the transactions have been reaped from the mempool according to the rules described above,
+CometBFT calls `PrepareProposal` to the application with the transaction list that has just been reaped.
+As part of this call the application can remove, add, or reorder transactions in the transaction list.
+
+The `RequestPrepareProposal` contains two important fields:
+
+* `MaxTxBytes`, which contains the value returned by `MaxDataBytes` described above.
+  The application MUST NOT return a list of transactions whose size exceeds this number.
+* `Txs`, which contains the list of reaped transactions.
+
+For more details on `PrepareProposal`, please see the
+[relevant part of the spec](../abci/abci%2B%2B_methods.md#prepareproposal)
+
 ## Validating transactions in the mempool
 
-Before we accept a transaction in the mempool, we check if it's size is no more
+Before we accept a transaction in the mempool, we check if its size is no more
 than {MaxDataSize}. {MaxDataSize} is calculated using the same formula as
-above, except we subtract the max number of evidence, {MaxNum} by the maximum size of evidence
+above, except we assume there is no evidence.
 
 ```go
-func MaxDataBytesUnknownEvidence(maxBytes int64, valsCount int) int64 {
- return maxBytes -
-  MaxOverheadForBlock -
-  MaxHeaderBytes -
-  (maxNumEvidence * MaxEvidenceBytes)
+func MaxDataBytesNoEvidence(maxBytes int64, valsCount int) int64 {
+  return maxBytes -
+    MaxOverheadForBlock -
+    MaxHeaderBytes -
+    MaxCommitBytes(valsCount)
 }
 ```
@@ -104,6 +104,12 @@ The total size of a block is limited in bytes by the `ConsensusParams.Block.MaxB
 Proposed blocks must be less than this size, and will be considered invalid
 otherwise.
 
+The Application may set `ConsensusParams.Block.MaxBytes` to -1.
+In that case, the actual block limit is set to 100 MB,
+and CometBFT will provide all transactions in the mempool as part of `PrepareProposal`.
+The application has to be careful to return a list of transactions in `ResponsePrepareProposal`
+whose size is less than or equal to `RequestPrepareProposal.MaxTxBytes`.
+
 Blocks should additionally be limited by the amount of "gas" consumed by the
 transactions in the block, though this is not yet implemented.
 

@@ -94,7 +94,7 @@ func (blockExec *BlockExecutor) SetEventBus(eventBus types.BlockEventPublisher)
 
 // CreateProposalBlock calls state.MakeBlock with evidence from the evpool
 // and txs from the mempool. The max bytes must be big enough to fit the commit.
-// Up to 1/10th of the block space is allocated for maximum sized evidence.
+// The block space is first allocated to outstanding evidence.
 // The rest is given to txs, up to the max gas.
 //
 // Contract: application will not return more bytes than are sent over the wire.
@@ -107,14 +107,23 @@ func (blockExec *BlockExecutor) CreateProposalBlock(
 ) (*types.Block, error) {
 
 	maxBytes := state.ConsensusParams.Block.MaxBytes
+	emptyMaxBytes := maxBytes == -1
+	if emptyMaxBytes {
+		maxBytes = int64(types.MaxBlockSizeBytes)
+	}
+
 	maxGas := state.ConsensusParams.Block.MaxGas
 
 	evidence, evSize := blockExec.evpool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes)
 
 	// Fetch a limited amount of valid txs
 	maxDataBytes := types.MaxDataBytes(maxBytes, evSize, state.Validators.Size())
+	maxReapBytes := maxDataBytes
+	if emptyMaxBytes {
+		maxReapBytes = -1
+	}
 
-	txs := blockExec.mempool.ReapMaxBytesMaxGas(maxDataBytes, maxGas)
+	txs := blockExec.mempool.ReapMaxBytesMaxGas(maxReapBytes, maxGas)
 	commit := lastExtCommit.ToCommit()
 	block := state.MakeBlock(height, txs, commit, evidence, proposerAddr)
 	rpp, err := blockExec.proxyApp.PrepareProposal(

@@ -8,8 +8,12 @@ import (
 // TxPreCheck returns a function to filter transactions before processing.
 // The function limits the size of a transaction to the block's maximum data size.
 func TxPreCheck(state State) mempl.PreCheckFunc {
+	maxBytes := state.ConsensusParams.Block.MaxBytes
+	if maxBytes == -1 {
+		maxBytes = int64(types.MaxBlockSizeBytes)
+	}
 	maxDataBytes := types.MaxDataBytesNoEvidence(
-		state.ConsensusParams.Block.MaxBytes,
+		maxBytes,
 		state.Validators.Size(),
 	)
 	return mempl.PreCheckMaxBytes(maxDataBytes)

@@ -294,8 +294,7 @@ func MaxDataBytes(maxBytes, evidenceBytes int64, valsCount int) int64 {
 }
 
 // MaxDataBytesNoEvidence returns the maximum size of block's data when
-// evidence count is unknown. MaxEvidencePerBlock will be used for the size
-// of evidence.
+// evidence count is unknown (will be assumed to be 0).
 //
 // XXX: Panics on negative result.
 func MaxDataBytesNoEvidence(maxBytes int64, valsCount int) int64 {
@@ -306,7 +305,7 @@ func MaxDataBytesNoEvidence(maxBytes int64, valsCount int) int64 {
 
 	if maxDataBytes < 0 {
 		panic(fmt.Sprintf(
-			"Negative MaxDataBytesUnknownEvidence. Block.MaxBytes=%d is too small to accommodate header&lastCommit&evidence=%d",
+			"Negative MaxDataBytesNoEvidence. Block.MaxBytes=%d is too small to accommodate header&lastCommit&evidence=%d",
 			maxBytes,
 			-(maxDataBytes - maxBytes),
 		))

@@ -143,8 +143,12 @@ func IsValidPubkeyType(params ValidatorParams, pubkeyType string) bool {
 // Validate validates the ConsensusParams to ensure all values are within their
 // allowed limits, and returns an error if they are not.
 func (params ConsensusParams) ValidateBasic() error {
-	if params.Block.MaxBytes <= 0 {
-		return fmt.Errorf("block.MaxBytes must be greater than 0. Got %d",
+	if params.Block.MaxBytes == 0 {
+		return fmt.Errorf("block.MaxBytes cannot be 0")
+	}
+	if params.Block.MaxBytes < -1 {
+		return fmt.Errorf("block.MaxBytes must be -1 or greater than 0. Got %d",
+
 			params.Block.MaxBytes)
 	}
 	if params.Block.MaxBytes > MaxBlockSizeBytes {
@@ -167,7 +171,11 @@ func (params ConsensusParams) ValidateBasic() error {
 			params.Evidence.MaxAgeDuration)
 	}
 
-	if params.Evidence.MaxBytes > params.Block.MaxBytes {
+	maxBytes := params.Block.MaxBytes
+	if maxBytes == -1 {
+		maxBytes = int64(MaxBlockSizeBytes)
+	}
+	if params.Evidence.MaxBytes > maxBytes {
 		return fmt.Errorf("evidence.MaxBytesEvidence is greater than upper bound, %d > %d",
 			params.Evidence.MaxBytes, params.Block.MaxBytes)
 	}

@@ -39,6 +39,8 @@ func TestConsensusParamsValidation(t *testing.T) {
 		11: {makeParams(1, 0, 2, 0, []string{}, 0), false},
 		// test invalid pubkey type provided
 		12: {makeParams(1, 0, 2, 0, []string{"potatoes make good pubkeys"}, 0), false},
+		13: {makeParams(-1, 0, 2, 0, valEd25519, 0), true},
+		14: {makeParams(-2, 0, 2, 0, valEd25519, 0), false},
 	}
 	for i, tc := range testCases {
 		if tc.valid {