Skip to content
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

Add GetAggregateAttestation V2 endpoint version #6511

Merged
merged 13 commits into from
Sep 6, 2024
1 change: 1 addition & 0 deletions AllTests-mainnet.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ OK: 1/1 Fail: 0/1 Skip: 0/1
+ Aggregated attestations with disjoint comittee bits into a single on-chain aggregate [Pres OK
+ Attestations with disjoint comittee bits and equal data into single on-chain aggregate [Pr OK
+ Can add and retrieve simple electra attestations [Preset: mainnet] OK
+ Working with electra aggregates [Preset: mainnet] OK
```
OK: 3/3 Fail: 0/3 Skip: 0/3
## Attestation pool processing [Preset: mainnet]
Expand Down
37 changes: 33 additions & 4 deletions beacon_chain/consensus_object_pools/attestation_pool.nim
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,16 @@ proc addForkChoiceVotes(
# hopefully the fork choice will heal itself over time.
error "Couldn't add attestation to fork choice, bug?", err = v.error()

func candidateIdx(pool: AttestationPool, slot: Slot): Opt[int] =
func candidateIdx(pool: AttestationPool, slot: Slot,
isElectra: bool = false): Opt[int] =
static: doAssert pool.phase0Candidates.len == pool.electraCandidates.len

let poolLength = if isElectra:
pool.electraCandidates.lenu64 else: pool.phase0Candidates.lenu64

if slot >= pool.startingSlot and
slot < (pool.startingSlot + pool.phase0Candidates.lenu64):
Opt.some(int(slot mod pool.phase0Candidates.lenu64))
slot < (pool.startingSlot + poolLength):
Opt.some(int(slot mod poolLength))
else:
Opt.none(int)

Expand Down Expand Up @@ -978,7 +983,8 @@ proc getElectraAttestationsForBlock*(
else:
default(seq[electra.Attestation])

func bestValidation(aggregates: openArray[Phase0Validation]): (int, int) =
func bestValidation(
aggregates: openArray[Phase0Validation | ElectraValidation]): (int, int) =
pedromiguelmiranda marked this conversation as resolved.
Show resolved Hide resolved
# Look for best validation based on number of votes in the aggregate
doAssert aggregates.len() > 0,
"updateAggregates should have created at least one aggregate"
Expand All @@ -993,6 +999,29 @@ func bestValidation(aggregates: openArray[Phase0Validation]): (int, int) =
bestIndex = i
(bestIndex, best)

func getElectraAggregatedAttestation*(
pool: var AttestationPool, slot: Slot,
attestationDataRoot: Eth2Digest, committeeIndex: CommitteeIndex):
Opt[electra.Attestation] =

let candidateIdx = pool.candidateIdx(slot)
if candidateIdx.isNone:
return Opt.none(electra.Attestation)

var res: Opt[electra.Attestation]
for _, entry in pool.electraCandidates[candidateIdx.get].mpairs():
if entry.data.index != committeeIndex.uint64:
pedromiguelmiranda marked this conversation as resolved.
Show resolved Hide resolved
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of the Electra changes is that attestations across committees can be aggregated.

https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.5/specs/phase0/validator.md#construct-aggregate (phase 0, i.e. pre-Electra) states that:

Construct aggregate

If the validator is selected to aggregate (is_aggregator()), they construct an aggregate attestation via the following.

Collect attestations seen via gossip during the slot that have an equivalent attestation_data to that constructed by the validator. If len(attestations) > 0, create an aggregate_attestation: Attestation with the following fields.

Data

Set aggregate_attestation.data = attestation_data where attestation_data is the AttestationData object that is the same for each individual attestation being aggregated.

Aggregation bits

Let aggregate_attestation.aggregation_bits be a Bitlist[MAX_VALIDATORS_PER_COMMITTEE] of length len(committee), where each bit set from each individual attestation is set to 0b1.

But the index field of https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.5/specs/phase0/beacon-chain.md#attestationdata is the committee index, until Electra. So this condition means that aggregation cannot happen across committees pre-Electra.

https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.5/specs/electra/validator.md#construct-attestation changes this:

Construct attestation

  • Set attestation_data.index = 0.
  • Let attestation.aggregation_bits be a Bitlist[MAX_VALIDATORS_PER_COMMITTEE * MAX_COMMITTEES_PER_SLOT] of length len(committee), where the bit of the index of the validator in the committee is set to 0b1.
  • Let attestation.committee_bits be a Bitvector[MAX_COMMITTEES_PER_SLOT], where the bit at the index associated with the validator's committee is set to 0b1.

Note: Calling get_attesting_indices(state, attestation) should return a list of length equal to 1, containing validator_index.

And the procedure for https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.5/specs/electra/validator.md#attestation-aggregation similarly allows cross-committee index-aggregation.

Copy link
Contributor Author

@pedromiguelmiranda pedromiguelmiranda Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Electra changes will allow cross committee index-aggregationss, however, the new getAgregatedAttestationV2 endpoint version is only interested in attestations in its corresponding committee(*) (previous version, could aggregate attestations from multiple committees since committee_index is not in its params, and that 's not the objective wanted for the API according to the spec).

Also, and more important, we are still using the data.index field from phase0 (maybe as "transition/temp" structure while we still have 2 attestations pools):

  • template addAttToPool on _proc addAttestation uses data.index field to save the index obtained from get_committee_index_one
  • toElectraAttestation recovers this index and saves it on committee_bits while building the final electra aggregated attestation

This particular conditional statement in the code is to achieve the objective defined in (*) given the code particularites referred before.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, indeed.


entry.updateAggregates()

let (bestIndex, best) = bestValidation(entry.aggregates)

if res.isNone() or best > res.get().aggregation_bits.countOnes():
res = Opt.some(entry.toElectraAttestation(entry.aggregates[bestIndex]))

res

func getAggregatedAttestation*(
pool: var AttestationPool, slot: Slot, attestation_data_root: Eth2Digest):
Opt[phase0.Attestation] =
Expand Down
3 changes: 3 additions & 0 deletions beacon_chain/rpc/rest_constants.nim
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ const
"Invalid attestation data root value"
UnableToGetAggregatedAttestationError* =
"Unable to retrieve an aggregated attestation"
DeprecatedGetAggregatedAttestation*: string =
"Deprecated endpoint /eth/v1/validator/aggregate_attestation. Replaced with" &
"https://ethereum.github.io/beacon-APIs/#/Validator/getAggregatedAttestationV2"
MissingRandaoRevealValue* =
"Missing `randao_reveal` value"
InvalidRandaoRevealValue* =
Expand Down
57 changes: 57 additions & 0 deletions beacon_chain/rpc/rest_validator_api.nim
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,12 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) =
router.api2(MethodGet, "/eth/v1/validator/aggregate_attestation") do (
attestation_data_root: Option[Eth2Digest],
slot: Option[Slot]) -> RestApiResponse:

let contextFork = node.dag.cfg.consensusForkAtEpoch(node.currentSlot.epoch)
pedromiguelmiranda marked this conversation as resolved.
Show resolved Hide resolved
if contextFork >= ConsensusFork.Electra:
return RestApiResponse.jsonError(Http410,
DeprecatedGetAggregatedAttestation)

pedromiguelmiranda marked this conversation as resolved.
Show resolved Hide resolved
let attestation =
block:
let qslot =
Expand Down Expand Up @@ -804,6 +810,57 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) =
res.get()
RestApiResponse.jsonResponse(attestation)

# https://ethereum.github.io/beacon-APIs/#/Validator/getAggregatedAttestationV2
router.api2(MethodGet, "/eth/v2/validator/aggregate_attestation") do (
attestation_data_root: Option[Eth2Digest],
committee_index: Option[CommitteeIndex],
slot: Option[Slot]) -> RestApiResponse:

let contextFork = node.dag.cfg.consensusForkAtEpoch(node.currentSlot.epoch)
if contextFork < ConsensusFork.Electra:
return RestApiResponse.jsonError(Http400,
UnableToGetAggregatedAttestationError)

let attestation =
block:
let slot =
block:
if slot.isNone():
return RestApiResponse.jsonError(Http400, MissingSlotValueError)
let res = slot.get()
if res.isErr():
return RestApiResponse.jsonError(Http400, InvalidSlotValueError,
$res.error())
res.get()
let committee_index =
block:
if committee_index.isNone():
return RestApiResponse.jsonError(Http400,
MissingCommitteeIndexValueError)
let res = committee_index.get()
if res.isErr():
return RestApiResponse.jsonError(Http400,
InvalidCommitteeIndexValueError,
$res.error())
res.get()
let root =
block:
if attestation_data_root.isNone():
return RestApiResponse.jsonError(Http400,
MissingAttestationDataRootValueError)
let res = attestation_data_root.get()
if res.isErr():
return RestApiResponse.jsonError(Http400,
InvalidAttestationDataRootValueError, $res.error())
res.get()
let res =
node.attestationPool[].getElectraAggregatedAttestation(slot, root, comittee_index)
if res.isNone():
return RestApiResponse.jsonError(Http400,
UnableToGetAggregatedAttestationError)
res.get()
RestApiResponse.jsonResponse(attestation)

# https://ethereum.github.io/beacon-APIs/#/Validator/publishAggregateAndProofs
router.api2(MethodPost, "/eth/v1/validator/aggregate_and_proofs") do (
contentBody: Option[ContentBody]) -> RestApiResponse:
Expand Down
1 change: 1 addition & 0 deletions beacon_chain/spec/eth2_apis/rest_types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@ type
# Types based on the OAPI yaml file - used in responses to requests
GetBeaconHeadResponse* = DataEnclosedObject[Slot]
GetAggregatedAttestationResponse* = DataEnclosedObject[phase0.Attestation]
GetElectraAggregatedAttestationResponse* = DataEnclosedObject[electra.Attestation]
GetAttesterDutiesResponse* = DataRootEnclosedObject[seq[RestAttesterDuty]]
GetBlockAttestationsResponse* = DataEnclosedObject[seq[phase0.Attestation]]
GetBlockHeaderResponse* = DataOptimisticAndFinalizedObject[RestBlockHeaderInfo]
Expand Down
9 changes: 9 additions & 0 deletions beacon_chain/spec/eth2_apis/rest_validator_calls.nim
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,15 @@ proc getAggregatedAttestationPlain*(
meth: MethodGet.}
## https://ethereum.github.io/beacon-APIs/#/Validator/getAggregatedAttestation

proc getAggregatedAttestationPlainV2*(
attestation_data_root: Eth2Digest,
slot: Slot,
committee_index: CommitteeIndex
): RestPlainResponse {.
rest, endpoint: "/eth/v2/validator/aggregate_attestation"
meth: MethodGet.}
## https://ethereum.github.io/beacon-APIs/#/Validator/getAggregatedAttestation
pedromiguelmiranda marked this conversation as resolved.
Show resolved Hide resolved

proc publishAggregateAndProofs*(
body: seq[phase0.SignedAggregateAndProof]
): RestPlainResponse {.
Expand Down
68 changes: 68 additions & 0 deletions ncli/resttest-rules.json
Original file line number Diff line number Diff line change
Expand Up @@ -4523,6 +4523,74 @@
"status": {"operator": "oneof", "value": ["400", "200"]}
}
},
{
"topics": ["validator", "aggregate_attestation"],
"request": {
"url": "/eth/v2/validator/aggregate_attestation",
"headers": {"Accept": "application/json"}
},
"response": {
"status": {"operator": "equals", "value": "400"},
"headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}],
"body": [{"operator": "jstructcmpns", "value": {"code": 400, "message": ""}}]
}
},
{
"topics": ["validator", "aggregate_attestation"],
"request": {
"url": "/eth/v2/validator/aggregate_attestation?slot=0",
"headers": {"Accept": "application/json"}
},
"response": {
"status": {"operator": "equals", "value": "400"},
"headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}],
"body": [{"operator": "jstructcmpns", "value": {"code": 400, "message": ""}}]
}
},
{
"topics": ["validator", "aggregate_attestation"],
"request": {
"url": "/eth/v2/validator/aggregate_attestation?slot=&attestation_data_root=",
"headers": {"Accept": "application/json"}
},
"response": {
"status": {"operator": "equals", "value": "400"},
"headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}],
"body": [{"operator": "jstructcmpns", "value": {"code": 400, "message": ""}}]
}
},
{
"topics": ["validator", "aggregate_attestation"],
"request": {
"url": "/eth/v2/validator/aggregate_attestation?slot=&attestation_data_root=&committee_index=",
"headers": {"Accept": "application/json"}
},
"response": {
"status": {"operator": "equals", "value": "400"},
"headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}],
"body": [{"operator": "jstructcmpns", "value": {"code": 400, "message": ""}}]
}
},
{
"topics": ["validator", "aggregate_attestation"],
"request": {
"url": "/eth/v2/validator/aggregate_attestation?slot=0&attestation_data_root=0x0000000000000000000000000000000000000000000000000000000000000000",
"headers": {"Accept": "application/json"}
},
"response": {
"status": {"operator": "oneof", "value": ["400", "200"]}
}
},
{
"topics": ["validator", "aggregate_attestation"],
"request": {
"url": "/eth/v2/validator/aggregate_attestation?slot=0&attestation_data_root=0x0000000000000000000000000000000000000000000000000000000000000000&committee_index=0",
"headers": {"Accept": "application/json"}
},
"response": {
"status": {"operator": "oneof", "value": ["400", "200"]}
}
},
{
"topics": ["validator", "attester_duties"],
"request": {
Expand Down
84 changes: 78 additions & 6 deletions tests/test_attestation_pool.nim
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import
from std/sequtils import toSeq
from ./testbcutil import addHeadBlock

func combine(tgt: var phase0.Attestation, src: phase0.Attestation) =
func combine(tgt: var (phase0.Attestation | electra.Attestation),
src: phase0.Attestation | electra.Attestation) =
## Combine the signature and participation bitfield, with the assumption that
## the same data is being signed - if the signatures overlap, they are not
## combined.
Expand Down Expand Up @@ -849,10 +850,13 @@ suite "Attestation pool electra processing" & preset():
# We should now get both attestations for the block, but the aggregate
# should be the one with the most votes
pool[].getElectraAttestationsForBlock(state[], cache).len() == 2
# pool[].getAggregatedAttestation(2.Slot, 0.CommitteeIndex).
# get().aggregation_bits.countOnes() == 2
# pool[].getAggregatedAttestation(2.Slot, hash_tree_root(att2.data)).
# get().aggregation_bits.countOnes() == 2
pool[].getElectraAggregatedAttestation(2.Slot, combined[0].data.beacon_block_root,
0.CommitteeIndex).get().aggregation_bits.countOnes() == 2
pool[].getElectraAggregatedAttestation(2.Slot, hash_tree_root(att2.data), 0.CommitteeIndex).
get().aggregation_bits.countOnes() == 2
# requests to get and aggregate from different committees should be empty
pool[].getElectraAggregatedAttestation(
2.Slot, combined[0].data.beacon_block_root, 1.CommitteeIndex).isNone()

let
# Someone votes for a different root
Expand Down Expand Up @@ -949,4 +953,72 @@ suite "Attestation pool electra processing" & preset():
# with same data, 2 committee bits and 3 aggregation bits
attestations.len == 1
attestations[0].aggregation_bits.countOnes() == 3
attestations[0].committee_bits.countOnes() == 2
attestations[0].committee_bits.countOnes() == 2


test "Working with electra aggregates" & preset():
let
# Create an attestation for slot 1!
bc0 = get_beacon_committee(
state[], getStateField(state[], slot), 0.CommitteeIndex, cache)

var
att0 = makeElectraAttestation(
state[], state[].latest_block_root, bc0[0], cache)
att0x = att0
att1 = makeElectraAttestation(
state[], state[].latest_block_root, bc0[1], cache)
att2 = makeElectraAttestation(
state[], state[].latest_block_root, bc0[2], cache)
att3 = makeElectraAttestation(
state[], state[].latest_block_root, bc0[3], cache)

# Both attestations include member 2 but neither is a subset of the other
att0.combine(att2)
att1.combine(att2)

check:
not pool[].covers(att0.data, att0.aggregation_bits)

pool[].addAttestation(
att0, @[bc0[0], bc0[2]], att0.loadSig, att0.data.slot.start_beacon_time)
pool[].addAttestation(
att1, @[bc0[1], bc0[2]], att1.loadSig, att1.data.slot.start_beacon_time)

check:
process_slots(
defaultRuntimeConfig, state[],
getStateField(state[], slot) + MIN_ATTESTATION_INCLUSION_DELAY, cache,
info, {}).isOk()

check:
pool[].getElectraAttestationsForBlock(state[], cache).len() == 1
# Can get either aggregate here, random!
pool[].getElectraAggregatedAttestation(
1.Slot, att0.data.beacon_block_root, 0.CommitteeIndex).isSome()

# Add in attestation 3 - both aggregates should now have it added
pool[].addAttestation(
att3, @[bc0[3]], att3.loadSig, att3.data.slot.start_beacon_time)

block:
let attestations = pool[].getElectraAttestationsForBlock(state[], cache)
check:
attestations.len() == 1
attestations[0].aggregation_bits.countOnes() == 6
# Can get either aggregate here, random!
pool[].getElectraAggregatedAttestation(
1.Slot, attestations[0].data.beacon_block_root, 0.CommitteeIndex).isSome()

# Add in attestation 0 as single - attestation 1 is now a superset of the
# aggregates in the pool, so everything else should be removed
pool[].addAttestation(
att0x, @[bc0[0]], att0x.loadSig, att0x.data.slot.start_beacon_time)

block:
let attestations = pool[].getElectraAttestationsForBlock(state[], cache)
check:
attestations.len() == 1
attestations[0].aggregation_bits.countOnes() == 4
pool[].getElectraAggregatedAttestation(
1.Slot, attestations[0].data.beacon_block_root, 0.CommitteeIndex).isSome()
Loading