Skip to content

Commit

Permalink
Fix VC not properly handled getAggregatedAttestation's 404 error. (#6254
Browse files Browse the repository at this point in the history
)

* Fix VC not properly handled getAggregatedAttestation's 404 error.

* Update AllTests.
  • Loading branch information
cheatfate authored May 1, 2024
1 parent 1ab6f16 commit f3da063
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 3 deletions.
5 changes: 3 additions & 2 deletions AllTests-mainnet.md
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,7 @@ OK: 1/1 Fail: 0/1 Skip: 0/1
+ /eth/v1/validator/sync_committee_selections serialization/deserialization test OK
+ bestSuccess() API timeout test OK
+ firstSuccessParallel() API timeout test OK
+ getAggregatedAttestationDataScore() default test OK
+ getAggregatedAttestationDataScore() test vectors OK
+ getAttestationDataScore() test vectors OK
+ getLiveness() response deserialization test OK
Expand All @@ -899,7 +900,7 @@ OK: 1/1 Fail: 0/1 Skip: 0/1
+ getUniqueVotes() test vectors OK
+ normalizeUri() test vectors OK
```
OK: 11/11 Fail: 0/11 Skip: 0/11
OK: 12/12 Fail: 0/12 Skip: 0/12
## Validator change pool testing suite
```diff
+ addValidatorChangeMessage/getAttesterSlashingMessage OK
Expand Down Expand Up @@ -1019,4 +1020,4 @@ OK: 2/2 Fail: 0/2 Skip: 0/2
OK: 9/9 Fail: 0/9 Skip: 0/9

---TOTAL---
OK: 684/689 Fail: 0/689 Skip: 5/689
OK: 685/690 Fail: 0/690 Skip: 5/690
7 changes: 7 additions & 0 deletions beacon_chain/spec/eth2_apis/rest_types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ const

const
preferSSZ* = "application/octet-stream,application/json;q=0.9"
LowestScoreAggregatedAttestation* =
phase0.Attestation(
aggregation_bits: CommitteeValidatorsBits(BitSeq.init(1)))

static:
doAssert(ClientMaximumValidatorIds <= ServerMaximumValidatorIds)
Expand Down Expand Up @@ -615,6 +618,10 @@ type
fork_choice_nodes*: seq[RestNode]
extra_data*: RestExtraData

func isLowestScoreAggregatedAttestation*(a: phase0.Attestation): bool =
(a.data.slot == Slot(0)) and (a.data.index == 0'u64) and
(a.data.source.epoch == Epoch(0)) and (a.data.target.epoch == Epoch(0))

func `==`*(a, b: RestValidatorIndex): bool =
uint64(a) == uint64(b)

Expand Down
7 changes: 7 additions & 0 deletions beacon_chain/validator_client/api.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,13 @@ proc getAggregatedAttestation*(
handle400()
ApiResponse[GetAggregatedAttestationResponse].err(
ResponseInvalidError)
of 404:
# A 404 error must be returned if no attestation is available for the
# requested `attestation_data_root`. To address the issue #6184, we
# use empty GetAggregatedAttestationResponse.
ApiResponse[GetAggregatedAttestationResponse].ok(
GetAggregatedAttestationResponse(
data: LowestScoreAggregatedAttestation))
of 500:
handle500()
ApiResponse[GetAggregatedAttestationResponse].err(
Expand Down
6 changes: 6 additions & 0 deletions beacon_chain/validator_client/attestation_service.nim
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,12 @@ proc produceAndPublishAggregates(service: AttestationServiceRef,
err_name = exc.name, err_msg = exc.msg
return

if isLowestScoreAggregatedAttestation(aggAttestation):
warn "Aggregated attestation with the root was not seen by the " &
"beacon node",
attestation_root = shortLog(attestationRoot)
return

let pendingAggregates =
block:
var res: seq[Future[bool]]
Expand Down
11 changes: 10 additions & 1 deletion tests/test_validator_client.nim
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,15 @@ suite "Validator Client test suite":
score = shortScore(getAggregatedAttestationDataScore(adata))
check score == vector[1]

test "getAggregatedAttestationDataScore() default test":
let
adata = GetAggregatedAttestationResponse(
data: LowestScoreAggregatedAttestation)
score = shortScore(getAggregatedAttestationDataScore(adata))
check:
score == "0.0000"
isLowestScoreAggregatedAttestation(adata.data) == true

test "getSyncCommitteeContributionDataScore() test vectors":
for vector in ContributionDataVectors:
let
Expand Down Expand Up @@ -905,4 +914,4 @@ suite "Validator Client test suite":
res.isOk()
len(res.get().data) == 1
res.get().data[0].index == 100000
res.get().data[0].is_live == true
res.get().data[0].is_live == true

0 comments on commit f3da063

Please sign in to comment.