-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Synchronously check all transactions to have non-zero length
#3885
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
Changes from 3 commits
e1eaa7f
c746890
1e96d23
a6095bf
f58b5ba
44d5a1b
0f964b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -293,9 +293,13 @@ def verify_and_notify_new_payload(self: ExecutionEngine, | |
| """ | ||
| Return ``True`` if and only if ``new_payload_request`` is valid with respect to ``self.execution_state``. | ||
| """ | ||
| # [Modified in Deneb:EIP4788] | ||
| execution_payload = new_payload_request.execution_payload | ||
| parent_beacon_block_root = new_payload_request.parent_beacon_block_root | ||
etan-status marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| if b'' in execution_payload.transactions: | ||
| return False | ||
|
Comment on lines
+299
to
+300
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I understand the implications of the PR and agree that zero-length transactions should be rejected, but I'm not certain that
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think So I think putting it in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm okay, well in that case I guess it doesn't hurt to add. Let's do it! @etan-status could you merge in dev & make similar changes to electra?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, This is the matching spec that describes the additional check for zero length transactions. |
||
|
|
||
| # [Modified in Deneb:EIP4788] | ||
| if not self.is_valid_block_hash(execution_payload, parent_beacon_block_root): | ||
| return False | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.