Payload Attestation Sync package changes #13989
Conversation
potuz
requested review from
nalepae,
rauljordan and
james-prysm
and removed request for
a team
| ptc, err := helpers.GetPayloadTimelinessCommittee(ctx, st, a.Data.Slot) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| idx := slice.Index(ptc, a.ValidatorIndex) | ||
| if idx == -1 { | ||
| return errInvalidValidatorIndex | ||
| } |
There was a problem hiding this comment.
Why do we need to check if the validator belongs in a PTC again if this was already done in InPayloadTimelinessCommittee and in validatePayloadAttestation
There was a problem hiding this comment.
because we need the index in the PTC, not the validator index.
| agg.AggregationBits.SetBitAt(idx, true) | ||
| sig, err := aggregateSigFromMessage(agg, att) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| agg.Signature = sig |
There was a problem hiding this comment.
I know you assume that the signature is already verified, but a safer ordering might be the following, so you don't set it until everything passes. I don't see any downside here.
sig, err := aggregateSigFromMessage(agg, att)
if err != nil {
return err
}
agg.Signature = sig
agg.AggregationBits.SetBitAt(idx, true)
There was a problem hiding this comment.
I don't understand, why is this safer? aggregateSigFromMessage does not check either.
There was a problem hiding this comment.
My point is if aggregateSigFromMessage returns an error then AggregationBits is already set
There was a problem hiding this comment.
Ah but your code then will have a bug since you need to aggregate the incoming bit too.
potuz
force-pushed
the
payload_attestation_sync
branch
from
c66a6c9
to
918287f
Compare
potuz
force-pushed
the
payload_attestation_sync
branch
2 times, most recently
from
936684a
to
215ba50
Compare
potuz
force-pushed
the
payload_attestation_sync
branch
3 times, most recently
from
d196aed
to
ed8f8ca
Compare
This implements a helper to get the ptc committee from a state. It uses the cached beacon committees if possible It also implements a helper to compute the largest power of two of a uint64 and a helper to test for nil payload attestation messages
* Add ePBS to db
potuz
force-pushed
the
payload_attestation_sync
branch
from
ed8f8ca
to
f1c3e9b
Compare
potuz
force-pushed
the
epbs
branch
3 times, most recently
from
a97480c
to
966e322
Compare
This PR introduces the minimal changes needed to process payload attestation messages over the wire.
The design of the cache is such that a single root is tracked, this is because PTC attestations over the wire are only useful for the current slot. Any attempt to add to the cache a new root removes the previous cache.