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

fix: return 404 error if no sync committee contribution is available #6649

Draft
wants to merge 2 commits into
base: unstable
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions packages/beacon-node/src/api/impl/validator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -866,21 +867,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});
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, rootHex);
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, 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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);
Expand Down
Loading