Skip to content

Commit

Permalink
fix: increase max attestation inclusion slot for post deneb blocks (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
nflaig committed Mar 8, 2024
1 parent adc0534 commit cae26be
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ export async function produceCommonBlockBody<T extends BlockType>(
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,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
});
Expand All @@ -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);
});
Expand Down

0 comments on commit cae26be

Please sign in to comment.