Gloas modify process_attestations #8283
Conversation
shane-moore
force-pushed
the
epbs-process-attestation
branch
from
2611ffc to
def8e8e
Compare
shane-moore
force-pushed
the
epbs-process-attestation
branch
from
def8e8e to
9d93170
Compare
eserilev
left a comment
eserilev
left a comment
There was a problem hiding this comment.
I haven't done a review against the spec yet, just some nits to hopefully make this PR a little bit easier to review
| @@ -0,0 +1,322 @@ | |||
| use super::*; | |||
There was a problem hiding this comment.
could we keep this stuff in process_operations? Makes the PR a bit easier to review if the diff here is minimized. The changes here are a bit sensitive and I don't want to miss anything
There was a problem hiding this comment.
@eserilev, I feel you on that! moved the logic over to process_operations
| let is_matching_payload = if is_attestation_same_slot(state, data)? { | ||
| // For same-slot attestations, data.index must be 0 | ||
| if data.index != 0 { | ||
| // TODO(EIP7732): consider if we want to use a different error type, since this is more of an overloaded data index scenario as opposed to the InvalidCommitteeIndex previous error. It's more like `AttestationInvalid::BadOverloadedDataIndex` |
There was a problem hiding this comment.
I think it'd be nice to introduce a new error variant
There was a problem hiding this comment.
for sure, I updated to the following but am happy to name it something else if another preference
if data.index != 0 {
return Err(Error::BadOverloadedDataIndex(data.index));
}
| if state.fork_name_unchecked().altair_enabled() { | ||
| initialize_progressive_balances_cache(state, spec)?; | ||
| altair_deneb::process_attestation( | ||
| altair_gloas::altair::process_attestation( |
There was a problem hiding this comment.
we'll write a test case for the gloas case in a subequent PR I assume?
There was a problem hiding this comment.
yep! added a TODO to write some test cases for this. we'll be able to write tests for gloas features once we can properly produce gloas blocks and payloads.
probably within the next couple weeks I would think. At that point, I'll go back and create tests for this and my other recent PR's, which also have TODO's for testing
shane-moore
force-pushed
the
epbs-process-attestation
branch
from
2893a57 to
d08908b
Compare
shane-moore
force-pushed
the
epbs-process-attestation
branch
from
d08908b to
72fec1d
Compare
shane-moore
changed the title
3 participants
Changes per spec
process_attestationsmodifiedis_attestation_same_slotaddedget_attestation_participation_flag_indicesmodifiedHow This Works
The fast path for converting a slot's
builder_pending_paymentto abuilder_pending_withdrawaltriggers when the execution payload envelope for that slot is received and verified. In that case, we immediately queue the withdrawal corresponding to the CL block that carried the bid.However, the envelope may never be released. As a fallback, we want to convert the
builder_pending_paymentif enough validators attested on time to the block of that slot.What this PR Changes
Therefore, the
builder_pending_paymenthas aweightfield. During block processing, this PR modifiesprocess_attestationto updates this weight only for same slot attestations, adding the attester’s balance once per validator. (Note that same slot means that theattestation.data.beacon_block_rootcorresponds to thestate.get_block_root(data.slot)).In a future PR
Later, during epoch processing, we'll check if the weight is above quorum and if so, convert the
builder_pending_paymentto abuilder_pending_withdrawal, which will have awithdrawable_epoch.@ethDreamer