From 9e59883c05ee4f91122f8f2234bd010171468bbc Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 7 Apr 2024 15:01:15 +0100 Subject: [PATCH 1/2] fix: return 404 error if no sync committee contribution is available --- .../beacon-node/src/api/impl/validator/index.ts | 15 ++++++++++----- .../src/chain/opPools/syncCommitteeMessagePool.ts | 10 +++++++--- .../test/unit/chain/opPools/syncCommittee.test.ts | 5 +++-- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 658c86b905ff..20fa6cbf7689 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -866,21 +866,26 @@ export function getValidatorApi({ * @param beaconBlockRoot The block root for which to produce the contribution. */ async produceSyncCommitteeContribution(slot, subcommitteeIndex, beaconBlockRoot) { + const blockRootHex = toHexString(beaconBlockRoot); // when a validator is configured with multiple beacon node urls, this beaconBlockRoot may come from another beacon node // and it hasn't been in our forkchoice since we haven't seen / processing that block // see https://github.com/ChainSafe/lodestar/issues/5063 if (!chain.forkChoice.hasBlock(beaconBlockRoot)) { - const rootHex = toHexString(beaconBlockRoot); - network.searchUnknownSlotRoot({slot, root: rootHex}); + network.searchUnknownSlotRoot({slot, root: blockRootHex}); // if result of this call is false, i.e. block hasn't seen after 1 slot then the below notOnOptimisticBlockRoot call will throw error - await chain.waitForBlock(slot, rootHex); + await chain.waitForBlock(slot, blockRootHex); } // Check the execution status as validator shouldn't contribute on an optimistic head notOnOptimisticBlockRoot(beaconBlockRoot); - const contribution = chain.syncCommitteeMessagePool.getContribution(subcommitteeIndex, slot, beaconBlockRoot); - if (!contribution) throw new ApiError(500, "No contribution available"); + const contribution = chain.syncCommitteeMessagePool.getContribution(subcommitteeIndex, slot, blockRootHex); + if (!contribution) { + throw new ApiError( + 404, + `No sync committee contribution for slot=${slot}, subnet=${subcommitteeIndex}, blockRoot=${blockRootHex}` + ); + } metrics?.production.producedSyncContributionParticipants.observe( contribution.aggregationBits.getTrueBitIndexes().length diff --git a/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts b/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts index 03552992a72a..66026ded490a 100644 --- a/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts +++ b/packages/beacon-node/src/chain/opPools/syncCommitteeMessagePool.ts @@ -2,7 +2,7 @@ import {PointFormat, Signature} from "@chainsafe/bls/types"; import bls from "@chainsafe/bls"; import {BitArray, toHexString} from "@chainsafe/ssz"; import {SYNC_COMMITTEE_SIZE, SYNC_COMMITTEE_SUBNET_COUNT} from "@lodestar/params"; -import {altair, Root, Slot, SubcommitteeIndex} from "@lodestar/types"; +import {altair, RootHex, Slot, SubcommitteeIndex} from "@lodestar/types"; import {MapDef} from "@lodestar/utils"; import {IClock} from "../../util/clock.js"; import {InsertOutcome, OpPoolError, OpPoolErrorCode} from "./types.js"; @@ -99,8 +99,12 @@ export class SyncCommitteeMessagePool { /** * This is for the aggregator to produce ContributionAndProof. */ - getContribution(subnet: SubcommitteeIndex, slot: Slot, prevBlockRoot: Root): altair.SyncCommitteeContribution | null { - const contribution = this.contributionsByRootBySubnetBySlot.get(slot)?.get(subnet)?.get(toHexString(prevBlockRoot)); + getContribution( + subnet: SubcommitteeIndex, + slot: Slot, + prevBlockRoot: RootHex + ): altair.SyncCommitteeContribution | null { + const contribution = this.contributionsByRootBySubnetBySlot.get(slot)?.get(subnet)?.get(prevBlockRoot); if (!contribution) { return null; } diff --git a/packages/beacon-node/test/unit/chain/opPools/syncCommittee.test.ts b/packages/beacon-node/test/unit/chain/opPools/syncCommittee.test.ts index 3cb5d496bf9b..dd375a582e9d 100644 --- a/packages/beacon-node/test/unit/chain/opPools/syncCommittee.test.ts +++ b/packages/beacon-node/test/unit/chain/opPools/syncCommittee.test.ts @@ -40,7 +40,8 @@ describe("chain / opPools / SyncCommitteeMessagePool", function () { it("should propagate SyncCommitteeContribution", () => { clockStub.secFromSlot.mockReturnValue(0); - let contribution = cache.getContribution(subcommitteeIndex, syncCommittee.slot, syncCommittee.beaconBlockRoot); + const blockRootHex = toHexString(syncCommittee.beaconBlockRoot); + let contribution = cache.getContribution(subcommitteeIndex, syncCommittee.slot, blockRootHex); expect(contribution).not.toBeNull(); const newSecretKey = bls.SecretKey.fromBytes(Buffer.alloc(32, 2)); const newSyncCommittee: altair.SyncCommitteeMessage = { @@ -52,7 +53,7 @@ describe("chain / opPools / SyncCommitteeMessagePool", function () { }; const newIndicesInSubSyncCommittee = [1]; cache.add(subcommitteeIndex, newSyncCommittee, newIndicesInSubSyncCommittee[0]); - contribution = cache.getContribution(subcommitteeIndex, syncCommittee.slot, syncCommittee.beaconBlockRoot); + contribution = cache.getContribution(subcommitteeIndex, syncCommittee.slot, blockRootHex); expect(contribution).not.toBeNull(); if (contribution) { expect(contribution.slot).toBe(syncCommittee.slot); From ea5d763a4ca2db511b686a994c583d809a9c6572 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 8 Apr 2024 00:24:20 +0100 Subject: [PATCH 2/2] Return 404 error if block not in fork choice --- .../beacon-node/src/api/impl/validator/index.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 20fa6cbf7689..7296f7520a3d 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -33,6 +33,7 @@ import { isBlindedBeaconBlock, isBlockContents, phase0, + RootHex, } from "@lodestar/types"; import {ExecutionStatus} from "@lodestar/fork-choice"; import {toHex, resolveOrRacePromises, prettyWeiToEth} from "@lodestar/utils"; @@ -303,10 +304,10 @@ export function getValidatorApi({ * is still in flux and will be updated as and when other CL's figure this out. */ - function notOnOptimisticBlockRoot(beaconBlockRoot: Root): void { - const protoBeaconBlock = chain.forkChoice.getBlock(beaconBlockRoot); + function notOnOptimisticBlockRoot(beaconBlockRoot: RootHex): void { + const protoBeaconBlock = chain.forkChoice.getBlockHex(beaconBlockRoot); if (!protoBeaconBlock) { - throw new ApiError(400, "Block not in forkChoice"); + throw new ApiError(404, `Block not in forkChoice blockRoot=${beaconBlockRoot}`); } if (protoBeaconBlock.executionStatus === ExecutionStatus.Syncing) @@ -837,7 +838,7 @@ export function getValidatorApi({ // Check the execution status as validator shouldn't vote on an optimistic head // Check on target is sufficient as a valid target would imply a valid source - notOnOptimisticBlockRoot(targetRoot); + notOnOptimisticBlockRoot(toHexString(targetRoot)); // To get the correct source we must get a state in the same epoch as the attestation's epoch. // An epoch transition may change state.currentJustifiedCheckpoint @@ -870,14 +871,14 @@ export function getValidatorApi({ // when a validator is configured with multiple beacon node urls, this beaconBlockRoot may come from another beacon node // and it hasn't been in our forkchoice since we haven't seen / processing that block // see https://github.com/ChainSafe/lodestar/issues/5063 - if (!chain.forkChoice.hasBlock(beaconBlockRoot)) { + if (!chain.forkChoice.hasBlockHex(blockRootHex)) { network.searchUnknownSlotRoot({slot, root: blockRootHex}); // if result of this call is false, i.e. block hasn't seen after 1 slot then the below notOnOptimisticBlockRoot call will throw error await chain.waitForBlock(slot, blockRootHex); } // Check the execution status as validator shouldn't contribute on an optimistic head - notOnOptimisticBlockRoot(beaconBlockRoot); + notOnOptimisticBlockRoot(blockRootHex); const contribution = chain.syncCommitteeMessagePool.getContribution(subcommitteeIndex, slot, blockRootHex); if (!contribution) {