From f3eb48a918225ddd0ddf0d9eaa0cebdbb4c5953c Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Wed, 17 Jan 2024 22:30:06 +0100 Subject: [PATCH] fix: ignore already known errors in API (#6321) * Ignore known attestations submitted through API * Ignore known sync committee aggregate * Add log contex to ignored aggregate attestations * Fix typo in submitPoolSyncCommitteeSignatures --- .../src/api/impl/beacon/pool/index.ts | 24 +++++++--- .../src/api/impl/validator/index.ts | 46 +++++++++++-------- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/packages/beacon-node/src/api/impl/beacon/pool/index.ts b/packages/beacon-node/src/api/impl/beacon/pool/index.ts index 170c358fe417..09a66eba4d15 100644 --- a/packages/beacon-node/src/api/impl/beacon/pool/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/pool/index.ts @@ -8,7 +8,12 @@ import {validateApiVoluntaryExit} from "../../../../chain/validation/voluntaryEx import {validateApiBlsToExecutionChange} from "../../../../chain/validation/blsToExecutionChange.js"; import {validateApiSyncCommittee} from "../../../../chain/validation/syncCommittee.js"; import {ApiModules} from "../../types.js"; -import {AttestationError, GossipAction, SyncCommitteeError} from "../../../../chain/errors/index.js"; +import { + AttestationError, + AttestationErrorCode, + GossipAction, + SyncCommitteeError, +} from "../../../../chain/errors/index.js"; import {validateGossipFnRetryUnknownRoot} from "../../../../network/processor/gossipHandlers.js"; export function getBeaconPoolApi({ @@ -77,12 +82,17 @@ export function getBeaconPoolApi({ const sentPeers = await network.publishBeaconAttestation(attestation, subnet); metrics?.onPoolSubmitUnaggregatedAttestation(seenTimestampSec, indexedAttestation, subnet, sentPeers); } catch (e) { + const logCtx = {slot: attestation.data.slot, index: attestation.data.index}; + + if (e instanceof AttestationError && e.type.code === AttestationErrorCode.ATTESTATION_ALREADY_KNOWN) { + logger.debug("Ignoring known attestation", logCtx); + // Attestations might already be published by another node as part of a fallback setup or DVT cluster + // and can reach our node by gossip before the api. The error can be ignored and should not result in a 500 response. + return; + } + errors.push(e as Error); - logger.error( - `Error on submitPoolAttestations [${i}]`, - {slot: attestation.data.slot, index: attestation.data.index}, - e as Error - ); + logger.error(`Error on submitPoolAttestations [${i}]`, logCtx, e as Error); if (e instanceof AttestationError && e.action === GossipAction.REJECT) { chain.persistInvalidSszValue(ssz.phase0.Attestation, attestation, "api_reject"); } @@ -224,7 +234,7 @@ export function getBeaconPoolApi({ ); if (errors.length > 1) { - throw Error("Multiple errors on publishAggregateAndProofs\n" + errors.map((e) => e.message).join("\n")); + throw Error("Multiple errors on submitPoolSyncCommitteeSignatures\n" + errors.map((e) => e.message).join("\n")); } else if (errors.length === 1) { throw errors[0]; } diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index fc5cf953018d..e795d1c5afad 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -36,7 +36,13 @@ import { } from "@lodestar/types"; import {ExecutionStatus} from "@lodestar/fork-choice"; import {toHex, racePromisesWithCutoff, RaceEvent} from "@lodestar/utils"; -import {AttestationError, AttestationErrorCode, GossipAction, SyncCommitteeError} from "../../../chain/errors/index.js"; +import { + AttestationError, + AttestationErrorCode, + GossipAction, + SyncCommitteeError, + SyncCommitteeErrorCode, +} from "../../../chain/errors/index.js"; import {validateApiAggregateAndProof} from "../../../chain/validation/index.js"; import {ZERO_HASH} from "../../../constants/index.js"; import {SyncState} from "../../../sync/index.js"; @@ -1073,20 +1079,18 @@ export function getValidatorApi({ const sentPeers = await network.publishBeaconAggregateAndProof(signedAggregateAndProof); metrics?.onPoolSubmitAggregatedAttestation(seenTimestampSec, indexedAttestation, sentPeers); } catch (e) { + const logCtx = { + slot: signedAggregateAndProof.message.aggregate.data.slot, + index: signedAggregateAndProof.message.aggregate.data.index, + }; + if (e instanceof AttestationError && e.type.code === AttestationErrorCode.AGGREGATOR_ALREADY_KNOWN) { - logger.debug("Ignoring known signedAggregateAndProof"); + logger.debug("Ignoring known signedAggregateAndProof", logCtx); return; // Ok to submit the same aggregate twice } errors.push(e as Error); - logger.error( - `Error on publishAggregateAndProofs [${i}]`, - { - slot: signedAggregateAndProof.message.aggregate.data.slot, - index: signedAggregateAndProof.message.aggregate.data.index, - }, - e as Error - ); + logger.error(`Error on publishAggregateAndProofs [${i}]`, logCtx, e as Error); if (e instanceof AttestationError && e.action === GossipAction.REJECT) { chain.persistInvalidSszValue(ssz.phase0.SignedAggregateAndProof, signedAggregateAndProof, "api_reject"); } @@ -1128,15 +1132,21 @@ export function getValidatorApi({ ); await network.publishContributionAndProof(contributionAndProof); } catch (e) { + const logCtx = { + slot: contributionAndProof.message.contribution.slot, + subcommitteeIndex: contributionAndProof.message.contribution.subcommitteeIndex, + }; + + if ( + e instanceof SyncCommitteeError && + e.type.code === SyncCommitteeErrorCode.SYNC_COMMITTEE_AGGREGATOR_ALREADY_KNOWN + ) { + logger.debug("Ignoring known contributionAndProof", logCtx); + return; // Ok to submit the same aggregate twice + } + errors.push(e as Error); - logger.error( - `Error on publishContributionAndProofs [${i}]`, - { - slot: contributionAndProof.message.contribution.slot, - subcommitteeIndex: contributionAndProof.message.contribution.subcommitteeIndex, - }, - e as Error - ); + logger.error(`Error on publishContributionAndProofs [${i}]`, logCtx, e as Error); if (e instanceof SyncCommitteeError && e.action === GossipAction.REJECT) { chain.persistInvalidSszValue(ssz.altair.SignedContributionAndProof, contributionAndProof, "api_reject"); }