8000 Block Proposal Failure（proposal block parts exceeds maximum block bytes） · Issue #1675 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Block Proposal Failure（proposal block parts exceeds maximum block bytes） #1675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
lllleven opened this issue Nov 22, 2023 · 8 comments
Closed

Block Proposal Failure（proposal block parts exceeds maximum block bytes） #1675

lllleven opened this issue Nov 22, 2023 · 8 comments
Assignees
@lasarojc @glnro
Labels
consensus mempool

Comments

@lllleven
Copy link
Contributor
lllleven commented Nov 22, 2023 •
edited
Loading

Environment Information

  • Cometbft Version: v0.37.2
  • Cosmos Sdk Version: v0.47.3
  • Cosmos Mempool Used: SenderNonceMempool
  • Number of Validator: 3

Err log

ERR failed to process message err="total size of proposal block parts exceeds maximum block bytes (2097374 > 2097152)" height=2771 module=consensus msg_type=*consensus.BlockPartMessage peer=b9018b4260f19b5e0e8b0429c603d4156e6c2702 round=259
ERR failed to process message err="total size of proposal block parts exceeds maximum block bytes (2097354 > 2097152)" height=2771 module=consensus msg_type=*consensus.BlockPartMessage peer=1af9376873ae9bfa5cbb33a954f344afe96d55c6 round=260

https://github.com/cometbft/cometbft/blob/v0.37.2/consensus/state.go#L1931

Consensus Params

for all validators are configured as follows
{"jsonrpc":"2.0","id":-1,"result":{"block_height":"2770","consensus_params":{"block":{"max_bytes":"2097152","max_gas":"-1"},"evidence":{"max_age_num_blocks":"100000","max_age_duration":"172800000000000","max_bytes":"1048576"},"validator":{"pub_key_types":["ed25519"]},"version":{"app":"0"}}}}
{"jsonrpc":"2.0","id":-1,"result":{"block_height":"2771","consensus_params":{"block":{"max_bytes":"2097152","max_gas":"-1"},"evidence":{"max_age_num_blocks":"100000","max_age_duration":"172800000000000","max_bytes":"1048576"},"validator":{"pub_key_types":["ed25519"]},"version":{"app":"0"}}}}
@lllleven lllleven added bug Something isn't working needs-triage This issue/PR has not yet been triaged by the team. labels Nov 22, 2023
@andynog andynog added mempool consensus needs-information Waiting for additional information or feedback and removed needs-triage This issue/PR has not yet been triaged by the team. bug Something isn't working labels Nov 22, 2023
@andynog
Copy link
Contributor
andynog commented Nov 22, 2023

Hi @LX-Xiang, thanks for reporting this. I believe the message in the log is just letting you know that the max_bytes configured might not be enough to accomodate the total size of proposal block parts, so doesn't seem to be a bug. But I'm not sure how you are testing/running this so I can't tell if it's just a matter of increasing that parameter value.

Also, the SenderNonceMempool that you've mentioned in this issue, it is something implemented at the cosmos-sdk level. If you still think this is a bug, have you tried to open an issue in their repo?
https://github.com/cosmos/cosmos-sdk/blob/main/types/mempool/sender_nonce.go

@lllleven
Copy link
Contributor Author
lllleven commented Nov 23, 2023 •
edited
Loading

I discovered that in the cosmos-sdk DefaultProposalHandler, the size is determined by the MaxTxBytes field of RequestPrepareProposal. I suspect that when CometBFT calls cosmos-sdk, it may be providing an incorrect MaxTxBytes

If it's an issue with the cosmos-sdk, it should be detected at this position: https://github.com/cometbft/cometbft/blob/v0.37.2/state/execution.go#L139.

I suspect that there might be an issue with this function: https://github.com/cometbft/cometbft/blob/main/types/block.go#L278

@lllleven lllleven mentioned this issue Nov 23, 2023
Closed
@lllleven
Copy link
Contributor Author
lllleven commented Nov 23, 2023 •
edited
Loading

I added logs using the below method, and indeed, the txs size increased during the call to blockExec.proxyApp.PrepareProposalSync. However, it still remains smaller than the specified maxDataBytes input.

I believe there is an overestimation in the calculation of types.MaxDataBytes

Add log

CreateProposalBlock

func (blockExec *BlockExecutor) CreateProposalBlock(
	height int64,
	state State,
	commit *types.Commit,
	proposerAddr []byte,
) (*types.Block, error) {

	maxBytes := state.ConsensusParams.Block.MaxBytes
	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())

	txs := blockExec.mempool.ReapMaxBytesMaxGas(maxDataBytes, maxGas)
	block := state.MakeBlock(height, txs, commit, evidence, proposerAddr)
	
	localLastCommit := buildLastCommitInfo(block, blockExec.store, state.InitialHeight)
	rpp, err := blockExec.proxyApp.PrepareProposalSync(
		abci.RequestPrepareProposal{
			MaxTxBytes:         maxDataBytes,
			Txs:                block.Txs.ToSliceOfBytes(),
			LocalLastCommit:    extendedCommitInfo(localLastCommit),
			Misbehavior:        block.Evidence.Evidence.ToABCI(),
			Height:             block.Height,
			Time:               block.Time,
			NextValidatorsHash: block.NextValidatorsHash,
			ProposerAddress:    block.ProposerAddress,
		},
	)
	if err != nil {
		// The App MUST ensure that only valid (and hence 'processable') transactions
		// enter the mempool. Hence, at this point, we can't have any non-processable
		// transaction causing an error.
		//
		// Also, the App can simply skip any transaction that could cause any kind of trouble.
		// Either way, we cannot recover in a meaningful way, unless we skip proposing
		// this block, repair what caused the error and try again. Hence, we return an
		// error for now (the production code calling this function is expected to panic).
		return nil, err
	}

	txl := types.ToTxs(rpp.Txs)
	if err := txl.Validate(maxDataBytes); err != nil {
		return nil, err
	}
	var txlSize int64
	for _, tx := range txl {
		txlSize += int64(len(tx))
	}
	var txsSize int64
	for _, tx := range txs {
		txsSize += int64(len(tx))
	}
	newBlock := state.MakeBlock(height, txl, commit, evidence, proposerAddr)
	blockExec.logger.Info("CreateProposalBlock", "maxBytes", maxBytes, "maxDataBytes", maxDataBytes,
		"txs", txsSize, "txl", txlSize, "old block", block.Size(), "new block", newBlock.Size())
	return newBlock, nil

Log

CreateProposalBlock maxBytes=2097152 maxDataBytes=2096088 module=state new block=2098854 old block=2095293 txl=2095438 txs=2091887

@lllleven
Copy link
Contributor Author

I discovered that the inconsistency in byte calculations for tx between Cosmos SDK and CometBFT is causing the issue. I have submitted a pull request to Cosmos SDK.
ref: cosmos/cosmos-sdk#18551

@lllleven lllleven mentioned this issue Nov 24, 2023
Merged
3 tasks
@andynog
Copy link
Contributor
andynog commented Nov 30, 2023

Thanks for investigating, @LX-Xiang. I noticed that the SDK has merged your PR. Please let us know if you think the issue is resolved after testing, so we can close it.

@andynog andynog added wontfix This will not be worked on and removed needs-information Waiting for additional information or feedback wontfix This will not be worked on labels Nov 30, 2023
@lllleven
Copy link
Contributor Author
lllleven commented Dec 1, 2023

@andynog my issue has been solved

@lasarojc
Copy link
Contributor
lasarojc commented Dec 1, 2023

Pls don't close yet. We should consider his PR on CometBFT for merge.

@melekes melekes moved this from Todo to Ready for Review in CometBFT 2023 Dec 4, 2023
@lasarojc
Copy link
Contributor
lasarojc commented Dec 5, 2023

Before this fix, if the app returns a slightly oversized set of transactions, an invalid block would be created and proposed, only being caught by the receiver with the following message.

failed to process message ... msg_type=*consensus.BlockPartMessage err="total size of proposal block parts exceeds maximum block bytes

After the fix, even slightly oversized set of transactions will be caught by the proposer and result in
CONSENSUS FAILURE!!! module=consensus err="transaction data size exceeds maximum "

cc: @alexanderbez

@lasarojc lasarojc closed this as completed Dec 5, 2023
@github-project-automation github-project-automation bot moved this from Ready for Review to Done in CometBFT 2023 Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lasarojc lasarojc

@glnro glnro

Labels
consensus mempool
Projects
No open projects
CometBFT 2023
Status: Done
Milestone
No milestone
Development

No branches or pull requests
4 participants
@lasarojc @glnro @andynog @lllleven
0