From efb3fea7c4eae35e83451275a3f86279398d5464 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 8 Mar 2024 12:09:54 +0100 Subject: [PATCH] fix: increase max attestation inclusion slot for post deneb blocks --- .../chain/opPools/aggregatedAttestationPool.ts | 16 +++++++++++++--- .../src/chain/produceBlock/produceBlockBody.ts | 2 +- .../opPools/aggregatedAttestationPool.test.ts | 2 +- .../opPools/aggregatedAttestationPool.test.ts | 11 ++++++----- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts index 03cf447b44f4..c94e5d81e823 100644 --- a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts @@ -1,6 +1,6 @@ import bls from "@chainsafe/bls"; import {toHexString} from "@chainsafe/ssz"; -import {ForkName, MAX_ATTESTATIONS, MIN_ATTESTATION_INCLUSION_DELAY, SLOTS_PER_EPOCH} from "@lodestar/params"; +import {ForkName, ForkSeq, MAX_ATTESTATIONS, MIN_ATTESTATION_INCLUSION_DELAY, SLOTS_PER_EPOCH} from "@lodestar/params"; import {phase0, Epoch, Slot, ssz, ValidatorIndex, RootHex} from "@lodestar/types"; import { CachedBeaconStateAllForks, @@ -117,7 +117,11 @@ export class AggregatedAttestationPool { /** * Get attestations to be included in a block. Returns $MAX_ATTESTATIONS items */ - getAttestationsForBlock(forkChoice: IForkChoice, state: CachedBeaconStateAllForks): phase0.Attestation[] { + getAttestationsForBlock( + fork: ForkName, + forkChoice: IForkChoice, + state: CachedBeaconStateAllForks + ): phase0.Attestation[] { const stateSlot = state.slot; const stateEpoch = state.epochCtx.epoch; const statePrevEpoch = stateEpoch - 1; @@ -144,7 +148,13 @@ export class AggregatedAttestationPool { continue; // Invalid attestations } // validateAttestation condition: Attestation slot not within inclusion window - if (!(slot + MIN_ATTESTATION_INCLUSION_DELAY <= stateSlot && stateSlot <= slot + SLOTS_PER_EPOCH)) { + if ( + !( + slot + MIN_ATTESTATION_INCLUSION_DELAY <= stateSlot && + // Post deneb, attestations are valid for current and previous epoch + (ForkSeq[fork] >= ForkSeq.deneb || stateSlot <= slot + SLOTS_PER_EPOCH) + ) + ) { continue; // Invalid attestations } diff --git a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts index d598ddcb9688..7697e89807ea 100644 --- a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts +++ b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts @@ -606,7 +606,7 @@ export async function produceCommonBlockBody( this.opPool.getSlashingsAndExits(currentState, blockType, this.metrics); const endAttestations = stepsMetrics?.startTimer(); - const attestations = this.aggregatedAttestationPool.getAttestationsForBlock(this.forkChoice, currentState); + const attestations = this.aggregatedAttestationPool.getAttestationsForBlock(fork, this.forkChoice, currentState); endAttestations?.({ step: BlockProductionStep.attestations, }); diff --git a/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts b/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts index 4ed8215ac85b..a9ce54e9d3f4 100644 --- a/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts @@ -159,7 +159,7 @@ describe(`getAttestationsForBlock vc=${vc}`, () => { return {state, pool}; }, fn: ({state, pool}) => { - pool.getAttestationsForBlock(forkchoice, state); + pool.getAttestationsForBlock(state.config.getForkName(state.slot), forkchoice, state); }, }); } diff --git a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts index 8d7718a69ebd..3c248ad4d194 100644 --- a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts @@ -3,7 +3,7 @@ import bls from "@chainsafe/bls"; import {BitArray, fromHexString, toHexString} from "@chainsafe/ssz"; import {describe, it, expect, beforeEach, beforeAll, afterEach, vi} from "vitest"; import {CachedBeaconStateAllForks, newFilledArray} from "@lodestar/state-transition"; -import {FAR_FUTURE_EPOCH, MAX_EFFECTIVE_BALANCE, SLOTS_PER_EPOCH} from "@lodestar/params"; +import {FAR_FUTURE_EPOCH, ForkName, MAX_EFFECTIVE_BALANCE, SLOTS_PER_EPOCH} from "@lodestar/params"; import {ssz, phase0} from "@lodestar/types"; import {CachedBeaconStateAltair} from "@lodestar/state-transition/src/types.js"; import {MockedForkChoice, getMockedForkChoice} from "../../../mocks/mockedBeaconChain.js"; @@ -28,6 +28,7 @@ const validSignature = fromHexString( describe("AggregatedAttestationPool", function () { let pool: AggregatedAttestationPool; + const fork = ForkName.altair; const altairForkEpoch = 2020; const currentEpoch = altairForkEpoch + 10; const currentSlot = SLOTS_PER_EPOCH * currentEpoch; @@ -115,9 +116,9 @@ describe("AggregatedAttestationPool", function () { forkchoiceStub.getBlockHex.mockReturnValue(generateProtoBlock()); forkchoiceStub.getDependentRoot.mockReturnValue(ZERO_HASH_HEX); if (isReturned) { - expect(pool.getAttestationsForBlock(forkchoiceStub, altairState).length).toBeGreaterThan(0); + expect(pool.getAttestationsForBlock(fork, forkchoiceStub, altairState).length).toBeGreaterThan(0); } else { - expect(pool.getAttestationsForBlock(forkchoiceStub, altairState).length).toEqual(0); + expect(pool.getAttestationsForBlock(fork, forkchoiceStub, altairState).length).toEqual(0); } // "forkchoice should be called to check pivot block" expect(forkchoiceStub.getDependentRoot).toHaveBeenCalledTimes(1); @@ -129,7 +130,7 @@ describe("AggregatedAttestationPool", function () { // all attesters are not seen const attestingIndices = [2, 3]; pool.add(attestation, attDataRootHex, attestingIndices.length, committee); - expect(pool.getAttestationsForBlock(forkchoiceStub, altairState)).toEqual([]); + expect(pool.getAttestationsForBlock(fork, forkchoiceStub, altairState)).toEqual([]); // "forkchoice should not be called" expect(forkchoiceStub.iterateAncestorBlocks).not.toHaveBeenCalledTimes(1); }); @@ -140,7 +141,7 @@ describe("AggregatedAttestationPool", function () { pool.add(attestation, attDataRootHex, attestingIndices.length, committee); forkchoiceStub.getBlockHex.mockReturnValue(generateProtoBlock()); forkchoiceStub.getDependentRoot.mockReturnValue("0xWeird"); - expect(pool.getAttestationsForBlock(forkchoiceStub, altairState)).toEqual([]); + expect(pool.getAttestationsForBlock(fork, forkchoiceStub, altairState)).toEqual([]); // "forkchoice should be called to check pivot block" expect(forkchoiceStub.getDependentRoot).toHaveBeenCalledTimes(1); });