-
Notifications
You must be signed in to change notification settings - Fork 346
Introduce ExecutionPayloadBid
for Gloas
#9912
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
Introduce ExecutionPayloadBid
for Gloas
#9912
Conversation
ExecutionPayloadBid
for Gloas
51d667f
to
1c05a55
Compare
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. just a question
*/ | ||
|
||
package tech.pegasys.teku.spec.datastructures.execution.versions.gloas; | ||
package tech.pegasys.teku.spec.datastructures.epbs.versions.gloas; |
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.
why epbs package here?
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.
it's a new package we introduced for all epbs objects, we could move them around in the future.
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.
If it is temporary, ok:)
a3a8392
to
148e15e
Compare
randomSlot(), | ||
randomUInt64(), | ||
randomBytes32()); | ||
} |
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.
Bug: Fee Recipient Type Mismatch
The randomExecutionPayloadBid()
method calls an undefined randomEth1Address()
for the feeRecipient
, causing a compilation error. This also introduces a type inconsistency in ExecutionPayloadBid
, where the constructor expects Bytes20
for feeRecipient
but the getter returns Eth1Address
.
Additional Locations (1)
// No SignedExecutionPayloadHeader in phase 0 | ||
public BeaconBlockBodyBuilder signedExecutionPayloadBid( | ||
final SignedExecutionPayloadBid signedExecutionPayloadBid) { | ||
// No SignedExecutionPayloadBid in phase 0 |
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.
Bug: Builder Method Returns Null Instead Of This
The signedExecutionPayloadBid
method in BeaconBlockBodyBuilderPhase0
returns null
instead of this
. This breaks the builder pattern, which could lead to NullPointerException
if method calls are chained. Other unsupported builder methods in this class return this
for consistency.
PR Description
Related to ethereum/consensus-specs#4525
Reference tests haven't been released yet, so ignored for Gloas. Can re-enable after we merge the
BeaconState
change. Raised an issue for tracking #9915 . For the moment, we can ignore the failing check-specrefs.Fixed Issue(s)
N/A
Documentation
doc-change-required
label to this PR if updates are required.Changelog