feat(blobpool): add explicit max blob count limit #31837
Conversation
core/txpool/validation.go
Outdated
| @@ -63,6 +64,9 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types | |||
| if opts.Accept&(1<<tx.Type()) == 0 { | |||
| return fmt.Errorf("%w: tx type %v not supported by this pool", core.ErrTxTypeNotSupported, tx.Type()) | |||
| } | |||
| if blobCount := len(tx.BlobHashes()); blobCount > opts.MaxBlobCount { | |||
| return fmt.Errorf("too many blobs in transaction: have %d, permitted %d", blobCount, opts.MaxBlobCount) | |||
There was a problem hiding this comment.
Please define a specific error type for this case, as it will make error classification and management easier.
There was a problem hiding this comment.
Looks like we do have the blob count validation https://github.com/ethereum/go-ethereum/pull/31837/files#diff-46644e723f975bf890676f7e1b5d5814b6d3137cd4c9ba9fed13a28b52f0e93bR155, please remove the newly added one and modify that instead.
There was a problem hiding this comment.
Yes, I did consider that existing validation. I added a new one for two reasons:
-
The existing check is for
MaxBlobsPerBlock(a protocol limit), while this one is a local blobpool limit.Theoretically these are two different limits and it's not guaranteed which is larger. Though it's true, after Pectra the new check will always be more strict (7 < 9) -- so maybe we can remove the
MaxBlobsPerBlockcheck, but that might break some tests. -
For the new check to be effective, we need to add it before the
opts.MaxSizecheck, or increase the max tx size limit.
Let me know how you want to proceed.
Fixes ethereum#31792. --------- Co-authored-by: lightclient <lightclient@protonmail.com>
Fixes #31792.