internal/ethapi: improve error msg when refusing bids#3131
Merged
zzzckck merged 1 commit intobnb-chain:developfrom May 29, 2025
buddh0:improve-error-msg-when-refusing-bids
Merged
internal/ethapi: improve error msg when refusing bids#3131zzzckck merged 1 commit intobnb-chain:developfrom buddh0:improve-error-msg-when-refusing-bids
zzzckck merged 1 commit intobnb-chain:developfrom
buddh0:improve-error-msg-when-refusing-bids
Conversation
galaio
approved these changes
May 29, 2025
zzzckck
approved these changes
May 29, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refines error handling in SendBid by distinguishing between bids for stale blocks, future blocks, and out-of-turn bids.
- Introduce explicit error messages for stale and future block numbers
- Shift
MinerInTurncheck to after block-number validation - Retain existing nil-bid guard and parent-hash validation
Comments suppressed due to low confidence (4)
internal/ethapi/api_mev.go:42
- Add unit tests covering the new 'stale block number' branch to ensure the error message and type are returned correctly when BlockNumber < latestBlockNumber+1.
if latestBlockNumber := currentHeader.Number.Uint64(); rawBid.BlockNumber < latestBlockNumber+1 {
internal/ethapi/api_mev.go:45
- Add unit tests for the 'block in future' branch so builders get the intended error when BlockNumber > latestBlockNumber+1.
} else if rawBid.BlockNumber > latestBlockNumber+1 {
internal/ethapi/api_mev.go:52
- [nitpick] Consider rephrasing to 'future block number: %d (latest: %d)' or capitalizing the message and ending with a period for consistency with other errors.
fmt.Sprintf("block in future: %d, latest block: %d", rawBid.BlockNumber, latestBlockNumber)
internal/ethapi/api_mev.go:53
- Relocating the MinerInTurn check changes error prioritization—ensure that returning block-number errors before out-of-turn errors matches intended UX and downstream logic.
} else if !m.b.MinerInTurn() {
| return common.Hash{}, types.NewInvalidBidError( | ||
| fmt.Sprintf("stale block number: %d, latest block: %d", rawBid.BlockNumber, latestBlockNumber)) | ||
| } else if rawBid.BlockNumber > latestBlockNumber+1 { | ||
| // For the first block of a validator's turn, the previous block must be imported first. |
There was a problem hiding this comment.
[nitpick] The multi-line comment explaining turn import timing is quite detailed; consider condensing it or moving to higher-level documentation to keep the code path focused.
sysvm
pushed a commit
to sysvm/bsc
that referenced
this pull request
Jun 16, 2025
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
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
internal/ethapi: improve error msg when refusing bids
Rationale
It will be helpful for builders.
rawBid.BlockNumber > latestBlockNumber+1,without this PR, error msg will be
ErrMevNotInTurn, this PR fix it.Example
add an example CLI or API response...
Changes
Notable changes: