Skip to content

Commit

Permalink
fix: ignore already known errors in API (ChainSafe#6321)
Browse files Browse the repository at this point in the history
* Ignore known attestations submitted through API

* Ignore known sync committee aggregate

* Add log contex to ignored aggregate attestations

* Fix typo in submitPoolSyncCommitteeSignatures
  • Loading branch information
nflaig authored and ensi321 committed Jan 22, 2024
1 parent a94972e commit b936c51
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 25 deletions.
24 changes: 17 additions & 7 deletions packages/beacon-node/src/api/impl/beacon/pool/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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];
}
Expand Down
46 changes: 28 additions & 18 deletions packages/beacon-node/src/api/impl/validator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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");
}
Expand Down

0 comments on commit b936c51

Please sign in to comment.