Skip to content

Commit

Permalink
Move unknown block retry back to handlers
Browse files Browse the repository at this point in the history
  • Loading branch information
@dapplion
dapplion committed Jan 13, 2022
1 parent 7760c41 commit dfc5af6
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 39 deletions.
2 changes: 1 addition & 1 deletion packages/lodestar/src/chain/validation/aggregateAndProof.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export async function validateGossipAggregateAndProof(

// [IGNORE] The block being voted for (attestation.data.beacon_block_root) has been seen (via both gossip
// and non-gossip sources) (a client MAY queue attestations for processing once block is retrieved).
const attHeadBlock = await verifyHeadBlockAndTargetRoot(chain, attData, attTarget.root, attEpoch);
const attHeadBlock = verifyHeadBlockAndTargetRoot(chain, attData.beaconBlockRoot, attTarget.root, attEpoch);

// [REJECT] The current finalized_checkpoint is an ancestor of the block defined by aggregate.data.beacon_block_root
// -- i.e. get_ancestor(store, aggregate.data.beacon_block_root, compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)) == store.finalized_checkpoint.root
Expand Down
50 changes: 14 additions & 36 deletions packages/lodestar/src/chain/validation/attestation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import {AttestationError, AttestationErrorCode, GossipAction} from "../errors";
import {MAXIMUM_GOSSIP_CLOCK_DISPARITY_SEC} from "../../constants";
import {RegenCaller} from "../regen";

const MAX_UNKNOWN_BLOCK_ROOT_RETRIES = 1;

const {EpochContextError, EpochContextErrorCode, computeSubnetForSlot, getIndexedAttestationSignatureSet} = allForks;

export async function validateGossipAttestation(
Expand Down Expand Up @@ -74,7 +72,7 @@ export async function validateGossipAttestation(

// [IGNORE] The block being voted for (attestation.data.beacon_block_root) has been seen (via both gossip
// and non-gossip sources) (a client MAY queue attestations for processing once block is retrieved).
const attHeadBlock = await verifyHeadBlockAndTargetRoot(chain, attData, attTarget.root, attEpoch);
const attHeadBlock = verifyHeadBlockAndTargetRoot(chain, attData.beaconBlockRoot, attTarget.root, attEpoch);

// [REJECT] The block being voted for (attestation.data.beacon_block_root) passes validation.
// > Altready check in `verifyHeadBlockAndTargetRoot()`
Expand Down Expand Up @@ -203,13 +201,13 @@ export function verifyPropagationSlotRange(chain: IBeaconChain, attestationSlot:
* 1. head block is known
* 2. attestation's target block is an ancestor of the block named in the LMD vote
*/
export async function verifyHeadBlockAndTargetRoot(
export function verifyHeadBlockAndTargetRoot(
chain: IBeaconChain,
attData: phase0.AttestationData,
beaconBlockRoot: Root,
targetRoot: Root,
attestationEpoch: Epoch
): Promise<IProtoBlock> {
const headBlock = await verifyHeadBlockIsKnown(chain, attData);
): IProtoBlock {
const headBlock = verifyHeadBlockIsKnown(chain, beaconBlockRoot);
verifyAttestationTargetRoot(headBlock, targetRoot, attestationEpoch);
return headBlock;
}
Expand All @@ -226,38 +224,18 @@ export async function verifyHeadBlockAndTargetRoot(
* it's still fine to reject here because there's no need for us to handle attestations that are
* already finalized.
*/
async function verifyHeadBlockIsKnown(chain: IBeaconChain, attData: phase0.AttestationData): Promise<IProtoBlock> {
const beaconBlockRootHex = toHexString(attData.beaconBlockRoot);

function verifyHeadBlockIsKnown(chain: IBeaconChain, beaconBlockRoot: Root): IProtoBlock {
// TODO (LH): Enforce a maximum skip distance for unaggregated attestations.

// If an attestation refers to a block root that's not known, it will wait for 1 slot max
// See https://github.com/ChainSafe/lodestar/pull/3564 for reasoning and results
// Waiting here requires minimal code and automatically affects attestation, and aggregate validation
// both from gossip and the API. I also prevents having to catch and re-throw in multiple places.
let unknownBlockRootRetries = 0;

// eslint-disable-next-line no-constant-condition
while (true) {
const headBlock = chain.forkChoice.getBlockHex(beaconBlockRootHex);
if (headBlock === null) {
if (unknownBlockRootRetries++ < MAX_UNKNOWN_BLOCK_ROOT_RETRIES) {
const foundBlock = await chain.waitForBlockOfAttestation(attData.slot, beaconBlockRootHex);
// Returns true if the block was found on time. In that case, try to get it from the fork-choice again.
// Otherwise, throw the error below.
if (foundBlock) {
continue;
}
}

throw new AttestationError(GossipAction.IGNORE, {
code: AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT,
root: beaconBlockRootHex,
});
}

return headBlock;
const headBlock = chain.forkChoice.getBlock(beaconBlockRoot);
if (headBlock === null) {
throw new AttestationError(GossipAction.IGNORE, {
code: AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT,
root: toHexString(beaconBlockRoot.valueOf() as typeof beaconBlockRoot),
});
}

return headBlock;
}

/**
Expand Down
83 changes: 81 additions & 2 deletions packages/lodestar/src/network/gossip/handlers/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {OpSource} from "../../../metrics/validatorMonitor";
import {IBeaconChain} from "../../../chain";
import {
AttestationError,
AttestationErrorCode,
BlockError,
BlockErrorCode,
BlockGossipError,
Expand Down Expand Up @@ -37,6 +38,8 @@ type ValidatorFnsModules = {
metrics: IMetrics | null;
};

const MAX_UNKNOWN_BLOCK_ROOT_RETRIES = 1;

/**
* Gossip handlers perform validation + handling in a single function.
* - This gossip handlers MUST only be registered as validator functions. No handler is registered for any topic.
Expand Down Expand Up @@ -121,7 +124,7 @@ export function getGossipHandlers(modules: ValidatorFnsModules): GossipHandlers
[GossipType.beacon_aggregate_and_proof]: async (signedAggregateAndProof, _topic, _peer, seenTimestampSec) => {
let validationResult: {indexedAttestation: phase0.IndexedAttestation; committeeIndices: number[]};
try {
validationResult = await validateGossipAggregateAndProof(chain, signedAggregateAndProof);
validationResult = await validateGossipAggregateAndProofRetryUnknownRoot(chain, signedAggregateAndProof);
} catch (e) {
if (e instanceof AttestationError && e.action === GossipAction.REJECT) {
const archivedPath = chain.persistInvalidSszObject(
Expand Down Expand Up @@ -153,7 +156,7 @@ export function getGossipHandlers(modules: ValidatorFnsModules): GossipHandlers
[GossipType.beacon_attestation]: async (attestation, {subnet}, _peer, seenTimestampSec) => {
let validationResult: {indexedAttestation: phase0.IndexedAttestation; subnet: number};
try {
validationResult = await validateGossipAttestation(chain, attestation, subnet);
validationResult = await validateGossipAttestationRetryUnknownRoot(chain, attestation, subnet);
} catch (e) {
if (e instanceof AttestationError && e.action === GossipAction.REJECT) {
const archivedPath = chain.persistInvalidSszObject(
Expand Down Expand Up @@ -269,3 +272,79 @@ export function getGossipHandlers(modules: ValidatorFnsModules): GossipHandlers
},
};
}

/**
* If an attestation refers to a block root that's not known, it will wait for 1 slot max
* See https://github.com/ChainSafe/lodestar/pull/3564 for reasoning and results
* Waiting here requires minimal code and automatically affects attestation, and aggregate validation
* both from gossip and the API. I also prevents having to catch and re-throw in multiple places.
*/
async function validateGossipAggregateAndProofRetryUnknownRoot(
chain: IBeaconChain,
signedAggregateAndProof: phase0.SignedAggregateAndProof
): Promise<ReturnType<typeof validateGossipAggregateAndProof>> {
let unknownBlockRootRetries = 0;
// eslint-disable-next-line no-constant-condition
while (true) {
try {
return await validateGossipAggregateAndProof(chain, signedAggregateAndProof);
} catch (e) {
if (e instanceof AttestationError && e.type.code === AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT) {
if (unknownBlockRootRetries++ < MAX_UNKNOWN_BLOCK_ROOT_RETRIES) {
// Trigger unknown block root search here

const attestation = signedAggregateAndProof.message.aggregate;
const foundBlock = await chain.waitForBlockOfAttestation(
attestation.data.slot,
toHexString(attestation.data.beaconBlockRoot)
);
// Returns true if the block was found on time. In that case, try to get it from the fork-choice again.
// Otherwise, throw the error below.
if (foundBlock) {
continue;
}
}
}

throw e;
}
}
}

/**
* If an attestation refers to a block root that's not known, it will wait for 1 slot max
* See https://github.com/ChainSafe/lodestar/pull/3564 for reasoning and results
* Waiting here requires minimal code and automatically affects attestation, and aggregate validation
* both from gossip and the API. I also prevents having to catch and re-throw in multiple places.
*/
async function validateGossipAttestationRetryUnknownRoot(
chain: IBeaconChain,
attestation: phase0.Attestation,
subnet: number | null
): Promise<ReturnType<typeof validateGossipAttestation>> {
let unknownBlockRootRetries = 0;
// eslint-disable-next-line no-constant-condition
while (true) {
try {
return await validateGossipAttestation(chain, attestation, subnet);
} catch (e) {
if (e instanceof AttestationError && e.type.code === AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT) {
if (unknownBlockRootRetries++ < MAX_UNKNOWN_BLOCK_ROOT_RETRIES) {
// Trigger unknown block root search here

const foundBlock = await chain.waitForBlockOfAttestation(
attestation.data.slot,
toHexString(attestation.data.beaconBlockRoot)
);
// Returns true if the block was found on time. In that case, try to get it from the fork-choice again.
// Otherwise, throw the error below.
if (foundBlock) {
continue;
}
}
}

throw e;
}
}
}

0 comments on commit dfc5af6

Please sign in to comment.