fix: reject gloas attestations/aggregates for EL-invalidated payloads #9637
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import {afterEach, beforeEach, describe, expect, it, vi} from "vitest"; | ||
| import {ProtoBlock} from "@lodestar/fork-choice"; | ||
| import {testLogger} from "@lodestar/logger/test-utils"; | ||
| import {ForkName} from "@lodestar/params"; | ||
| import {DataAvailabilityStatus} from "@lodestar/state-transition"; | ||
| import {ssz} from "@lodestar/types"; | ||
| import {PayloadEnvelopeInputSource} from "../../../../src/chain/blocks/payloadEnvelopeInput/types.js"; | ||
| import type {BeaconChain} from "../../../../src/chain/chain.js"; | ||
| import {ChainEventEmitter} from "../../../../src/chain/emitter.js"; | ||
| import {SeenPayloadEnvelopeInput} from "../../../../src/chain/seenCache/seenPayloadEnvelopeInput.js"; | ||
| import {ExecutionPayloadStatus} from "../../../../src/execution/index.js"; | ||
| import {SerializedCache} from "../../../../src/util/serializedCache.js"; | ||
| import {getMockedClock} from "../../../mocks/clock.js"; | ||
| import {config, generateBlock} from "../../../utils/blocksAndData.js"; | ||
|
|
||
| // verifyExecutionPayloadEnvelope + signature run before the EL call; stub the CL-side verification so | ||
| // the test can isolate the EL response handling (the envelope is "CL-valid"). | ||
| vi.mock("../../../../src/chain/blocks/verifyExecutionPayloadEnvelope.js", () => ({ | ||
| verifyExecutionPayloadEnvelope: vi.fn(), | ||
| verifyExecutionPayloadEnvelopeSignature: vi.fn().mockResolvedValue(true), | ||
| })); | ||
|
|
||
| // Import after vi.mock so the mocked module is wired in. | ||
| const {importExecutionPayload} = await import("../../../../src/chain/blocks/importExecutionPayload.js"); | ||
|
|
||
| describe("importExecutionPayload / EL INVALID marks payload invalid", () => { | ||
| let cache: SeenPayloadEnvelopeInput; | ||
| let abortController: AbortController; | ||
|
|
||
| beforeEach(() => { | ||
| abortController = new AbortController(); | ||
| cache = new SeenPayloadEnvelopeInput({ | ||
| config, | ||
| clock: getMockedClock(), | ||
| forkChoice: {getAllAncestorBlocks: vi.fn()} as any, | ||
| chainEvents: new ChainEventEmitter(), | ||
| signal: abortController.signal, | ||
| serializedCache: new SerializedCache(), | ||
| metrics: null, | ||
| logger: testLogger(), | ||
| }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| abortController.abort(); | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| function makeInput() { | ||
| const {block, rootHex, blockRoot} = generateBlock({forkName: ForkName.gloas, slot: 0}); | ||
| const input = cache.add({ | ||
| blockRootHex: rootHex, | ||
| block, | ||
| forkName: ForkName.gloas, | ||
| sampledColumns: [], | ||
| custodyColumns: [], | ||
| timeCreatedSec: 0, | ||
| }); | ||
| const signedEnvelope = ssz.gloas.SignedExecutionPayloadEnvelope.defaultValue(); | ||
| signedEnvelope.message.beaconBlockRoot = blockRoot; | ||
| input.addPayloadEnvelope({ | ||
| envelope: signedEnvelope, | ||
| source: PayloadEnvelopeInputSource.gossip, | ||
| seenTimestampSec: 0, | ||
| peerIdStr: "peer", | ||
| }); | ||
| return {input, rootHex}; | ||
| } | ||
|
|
||
| function makeChain(status: ExecutionPayloadStatus): BeaconChain { | ||
| return { | ||
| config, | ||
| clock: {currentSlot: 0} as any, | ||
| emitter: {emit: vi.fn()} as any, | ||
| forkChoice: { | ||
| getBlockHexDefaultStatus: (_root: string) => ({slot: 0}) as ProtoBlock, | ||
| } as any, | ||
| regen: { | ||
| getBlockSlotState: () => Promise.resolve({forkName: ForkName.gloas} as any), | ||
| } as any, | ||
| executionEngine: { | ||
| notifyNewPayload: vi.fn().mockResolvedValue({status, validationError: "boom"}), | ||
| } as any, | ||
| } as unknown as BeaconChain; | ||
| } | ||
|
|
||
| it("EL INVALID -> throws EXECUTION_ENGINE_INVALID and flags the payload input invalid", async () => { | ||
| const {input} = makeInput(); | ||
| expect(input.isPayloadInvalid()).toBe(false); | ||
|
|
||
| const chain = makeChain(ExecutionPayloadStatus.INVALID); | ||
| await expect( | ||
| importExecutionPayload.call(chain, input, DataAvailabilityStatus.PreData, {validSignature: true}) | ||
| ).rejects.toMatchObject({type: {code: "PAYLOAD_ERROR_EXECUTION_ENGINE_INVALID"}}); | ||
|
|
||
| expect(input.isPayloadInvalid()).toBe(true); | ||
| }); | ||
|
|
||
| it("EL transient error (INVALID_BLOCK_HASH) does NOT flag the payload input invalid", async () => { | ||
| const {input} = makeInput(); | ||
|
|
||
| const chain = makeChain(ExecutionPayloadStatus.INVALID_BLOCK_HASH); | ||
| await expect( | ||
| importExecutionPayload.call(chain, input, DataAvailabilityStatus.PreData, {validSignature: true}) | ||
| ).rejects.toMatchObject({type: {code: "PAYLOAD_ERROR_EXECUTION_ENGINE_ERROR"}}); | ||
|
|
||
| // Transient EL errors are not "payload failed validation" — must not be marked invalid. | ||
| expect(input.isPayloadInvalid()).toBe(false); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we keep track of this in fork choice, why do you wanna add this to our payload cache?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The immediate EL-INVALID case never reaches fork choice today. When
newPayloadreturnsINVALID,importExecutionPayloadthrows at step 4c before step 6'sonExecutionPayload(the only thing that creates a FULL variant), andtoForkChoiceExecutionStatusmaps onlyVALID/SYNCING/ACCEPTED—INVALIDhits thedefaultthrow. So there's no FULL variant at all:getBlockHex(root, FULL)isnulland fork choice has no record of it.(The LVH-invalidated case — imported
VALID/SYNCINGthen invalidated — does live in fork choice as a FULL variant withexecutionStatus === Invalid, which the validators already check viagetBlockHex(FULL). The cache flag only covers the immediate case.)Your instinct is fair though: we could make fork choice the single source of truth by teaching
onExecutionPayload/toForkChoiceExecutionStatusto acceptInvalidand creating a FULL-Invalid variant on immediate-INVALID (before the throw) — then both cases collapse to the existinggetBlockHex(FULL)?.executionStatus === Invalidcheck and the cache flag goes away. That's a fork-choice contract change (it rejectsInvalidby design today), which is why I kept it on the payload cache — but I'm happy to move it into fork choice if you'd prefer that. Your call since it's your area.