-
Notifications
You must be signed in to change notification settings - Fork 897
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.
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
Ban zero blob transactions #5425
Conversation
Signed-off-by: Fabio Di Fabio <[email protected]>
|
Signed-off-by: Fabio Di Fabio <[email protected]>
builder.maxFeePerGas(maxFeePerGas.orElse(Wei.of(5000))); | ||
builder.accessList(accessListEntries.orElse(List.of())); | ||
builder.maxFeePerDataGas(maxFeePerDataGas.orElse(Wei.ONE)); | ||
builder.versionedHashes(versionedHashes.orElse(List.of(Hash.wrap(Bytes32.random())))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random will produce variable test data. I'd recommend something else like a converted ascii string "Versioned Hash Not Set", padded to 32 bytes.
Also, when random the version byte will almost always be invalid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I forgot to properly format the versioned hash, fixing
Unit tests need help. |
… 4844-0-blob-txs-invalid
Signed-off-by: Fabio Di Fabio <[email protected]>
Test fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. one comment on a var name in test
...ore/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java
Outdated
Show resolved
Hide resolved
...ore/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java
Outdated
Show resolved
Hide resolved
…net/MainnetTransactionValidatorTest.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]>
…net/MainnetTransactionValidatorTest.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]>
* Ban zero blob transactions Signed-off-by: Fabio Di Fabio <[email protected]> * Update CHANGELOG Signed-off-by: Fabio Di Fabio <[email protected]> * Fix tests Signed-off-by: Fabio Di Fabio <[email protected]> * Update ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> * Update ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> --------- Signed-off-by: Fabio Di Fabio <[email protected]> Co-authored-by: Stefan Pingel <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]>
* Ban zero blob transactions Signed-off-by: Fabio Di Fabio <[email protected]> * Update CHANGELOG Signed-off-by: Fabio Di Fabio <[email protected]> * Fix tests Signed-off-by: Fabio Di Fabio <[email protected]> * Update ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> * Update ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> --------- Signed-off-by: Fabio Di Fabio <[email protected]> Co-authored-by: Stefan Pingel <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]>
PR description
An update to EIP-4844 made zero blob transactions invalid, so this PR adds the check to verify that at least one blob is specified, this check could be done statically when creating the transaction.
Fixed Issue(s)
fixes #5401