Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions packages/beacon-node/src/network/processor/gossipHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -713,9 +713,32 @@ function getSequentialHandlers(modules: ValidatorFnsModules, options: GossipHand
const {serializedData} = gossipData;

const signedBlock = sszDeserialize(topic, serializedData);
const blockInput = await validateBeaconBlock(signedBlock, topic.boundary.fork, peerIdStr, seenTimestampSec);
chain.serializedCache.set(signedBlock, serializedData);
handleValidBeaconBlock(blockInput, peerIdStr, seenTimestampSec);
try {
const blockInput = await validateBeaconBlock(signedBlock, topic.boundary.fork, peerIdStr, seenTimestampSec);
chain.serializedCache.set(signedBlock, serializedData);
handleValidBeaconBlock(blockInput, peerIdStr, seenTimestampSec);
} catch (e) {
// Spec: IGNORE the block, ie not to re-publish to peers
// but we should still import an equivocating (REPEAT_PROPOSAL) block into fork choice because we don't
// know if this block (or the 1st known block with same slot) will become canonical yet
if (
e instanceof BlockGossipError &&
e.type.code === BlockErrorCode.REPEAT_PROPOSAL &&
// this is make sure the block's proposer signature was verified, it should be true anyway
chain.seenBlockProposers.hasBlockRoot(signedBlock.message.slot, e.type.proposerIndex, e.type.root)
) {
// blockInput was optimistically seeded in validateBeaconBlock and retained on IGNORE
const blockInput = chain.seenBlockInputCache.get(e.type.root);
if (blockInput) {
chain.serializedCache.set(signedBlock, serializedData);
// this is technically not a valid gossip block but gossip validation is a cheap subset of checks
// this runs the full state transition, so importing an equivocating-but-valid block here is safe.
handleValidBeaconBlock(blockInput, peerIdStr, seenTimestampSec);

@nflaig nflaig Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to revisit the ordering in validateGossipBlock, so right now, if we throw a REPEAT_PROPOSAL that happens quite early, so the beacon block isn't fully validated here?

also it's before this check

 if (blockState.getBeaconProposer(blockSlot) !== proposerIndex) {
    throw new BlockGossipError(GossipAction.REJECT, {code: BlockErrorCode.INCORRECT_PROPOSER, proposerIndex});
  }

which seems questionable?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the p2p rules are just quick checks to forward the gossip block without having to run state transition
if the block is really invalid, it should be caught inside the state transition itself
in this case it's

it's worth to mention this in the code

@nflaig nflaig Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that is probably fine was thinking the same way, but we should be more explicit about it, so adding a comment as suggested makes sense.

there was one worry that we could receive many invalid blocks and fill our cache but due to how gossip re-propagation works this should be unlikely unless a peer sends them directly to us, but I think we still wanna make sure that we at least validate the proposer signature in any case, although my worry here is that we run blockState.getBeaconProposer(blockSlot) !== proposerIndex later, so technically any validator can produce a valid signature (of course they would get slashed too if they do more >=2 so the incentives for doing so are low) and we downscore them, so it's likely fine

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I think we still wanna make sure that we at least validate the proposer signature in any case

it's already checked at line 718 above

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah that looks fine

}
}
// rethrow so gossipValidatorFn maps IGNORE -> TopicValidatorResult.Ignore (message not forwarded)
throw e;
}
},

[GossipType.blob_sidecar]: async ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {toRootHex} from "@lodestar/utils";
import {BlockInputBlobs} from "../../../../src/chain/blocks/blockInput/blockInput.js";
import {BlockInputSource} from "../../../../src/chain/blocks/blockInput/types.js";
import {BlockError, BlockErrorCode} from "../../../../src/chain/errors/blockError.js";
import {BlockGossipError, GossipAction} from "../../../../src/chain/errors/index.js";
import {ChainEventEmitter, IBeaconChain} from "../../../../src/chain/index.js";
import {SeenBlockProposers} from "../../../../src/chain/seenCache/seenBlockProposers.js";
import {SeenBlockInput} from "../../../../src/chain/seenCache/seenGossipBlockInput.js";
Expand Down Expand Up @@ -62,6 +63,22 @@ describe("getGossipHandlers", () => {
expect(core.reportPeer).toHaveBeenCalledOnce();
expect(core.reportPeer).toHaveBeenCalledWith(peerIdStr, PeerAction.LowToleranceError, "ExecutionEngineInvalid");
});

it("imports a signature-verified REPEAT_PROPOSAL (equivocating) block into fork choice but keeps IGNORE", async () => {
const {processBlock, threw} = await runBeaconBlockRepeatProposal(denebConfig, {recorded: true});

// imported so LMD-GHOST can weigh it ...
expect(processBlock).toHaveBeenCalledOnce();
// ... but the gossip result stays IGNORE (handler re-throws), so the message is not forwarded
expect(threw).toBe(true);
});

it("does not import a REPEAT_PROPOSAL block whose root was not recorded (unverified 3rd+ proposal)", async () => {
const {processBlock, threw} = await runBeaconBlockRepeatProposal(denebConfig, {recorded: false});

expect(processBlock).not.toHaveBeenCalled();
expect(threw).toBe(true);
});
});

async function runBeaconBlockProcessingError(
Expand Down Expand Up @@ -138,6 +155,103 @@ async function runBeaconBlockProcessingError(
return {core, peerIdStr};
}

async function runBeaconBlockRepeatProposal(
config: BeaconConfig,
{recorded}: {recorded: boolean}
): Promise<{processBlock: ReturnType<typeof vi.fn>; threw: boolean}> {
const logger = testLogger();
const peerIdStr = "16Uiu2HAmTestGossipPeer" as PeerIdStr;
const signedBlock = ssz.deneb.SignedBeaconBlock.defaultValue();
signedBlock.message.slot = 1;
signedBlock.message.proposerIndex = 3;
const blockRootHex = toRootHex(ssz.deneb.BeaconBlock.hashTreeRoot(signedBlock.message));
const blockInput = BlockInputBlobs.createFromBlock({
block: signedBlock,
blockRootHex,
forkName: ForkName.deneb,
daOutOfRange: false,
source: BlockInputSource.gossip,
seenTimestampSec: 0,
peerIdStr,
});

// gossip validation rejects the 2nd distinct block for this (proposer, slot) with REPEAT_PROPOSAL
vi.mocked(validateGossipBlock).mockRejectedValue(
new BlockGossipError(GossipAction.IGNORE, {
code: BlockErrorCode.REPEAT_PROPOSAL,
proposerIndex: signedBlock.message.proposerIndex,
root: blockRootHex,
})
);

const seenBlockProposers = new SeenBlockProposers();
if (recorded) {
// observeBlockRoot runs only after the proposer signature is verified, so hasBlockRoot(root)
// being true is the handler's proof the signature was checked (the 2nd distinct block)
seenBlockProposers.observeBlockRoot(
signedBlock.message.slot,
signedBlock.message.proposerIndex,
blockRootHex,
ssz.phase0.SignedBeaconBlockHeader.defaultValue()
);
}

const processBlock = vi.fn().mockResolvedValue(undefined);
const chain = {
clock: new ClockStopped(1),
custodyConfig: {sampledColumns: [], custodyColumns: []} as unknown as CustodyConfig,
emitter: new ChainEventEmitter(),
getBlobsTracker: {triggerGetBlobs: vi.fn()},
logger,
processBlock,
processProposerEquivocation: vi.fn(),
seenBlockProposers,
seenBlockInputCache: {
getByBlock: vi.fn().mockReturnValue(blockInput),
get: vi.fn().mockReturnValue(blockInput),
prune: vi.fn(),
} as unknown as SeenBlockInput,
seenPayloadEnvelopeInputCache: {
add: vi.fn(),
get: vi.fn().mockReturnValue(undefined),
prune: vi.fn(),
} as unknown as IBeaconChain["seenPayloadEnvelopeInputCache"],
serializedCache: {set: vi.fn()},
} as unknown as IBeaconChain;

const handlers = getGossipHandlers(
{
aggregatorTracker: {} as AggregatorTracker,
chain,
config,
core: {reportPeer: vi.fn()} as unknown as INetworkCore,
events: new NetworkEventBus(),
logger,
metrics: null,
},
{}
);
const beaconBlockHandler = handlers[GossipType.beacon_block] as SequentialGossipHandler<GossipType.beacon_block>;

let threw = false;
try {
await beaconBlockHandler({
gossipData: {serializedData: ssz.deneb.SignedBeaconBlock.serialize(signedBlock)},
peerIdStr,
seenTimestampSec: 0,
topic: {
boundary: {fork: ForkName.deneb, epoch: 0},
type: GossipType.beacon_block,
},
});
} catch {
threw = true;
}
await new Promise((resolve) => setTimeout(resolve, 0));

return {processBlock, threw};
}

function getExecutionBlockError(
signedBlock: SignedBeaconBlock<typeof ForkName.deneb>,
code: BlockErrorCode.EXECUTION_ENGINE_ERROR | BlockErrorCode.EXECUTION_ENGINE_INVALID
Expand Down
Loading