diff --git a/packages/beacon-node/src/chain/builderCircuitBreaker.ts b/packages/beacon-node/src/chain/builderCircuitBreaker.ts index ba09c90e66cc..b632fc182088 100644 --- a/packages/beacon-node/src/chain/builderCircuitBreaker.ts +++ b/packages/beacon-node/src/chain/builderCircuitBreaker.ts @@ -15,11 +15,15 @@ export type BuilderCircuitBreakerModules = { metrics: Metrics | null; }; +/** Four observations is the minimum useful recovery sample for the default ~25% fault budget */ +const MIN_BLOCKS_TO_DEACTIVATE = 4; + /** * Post-gloas circuit breaker for builder bids. The beacon block is produced by the proposer * regardless of bid source, so missed blocks are not a useful builder health signal. Instead - * count blocks whose payload was never revealed and stop selecting builder bids while the - * non-reveal rate in the fault inspection window is too high. + * count blocks whose payload was never revealed. Activate when the non-reveal rate exceeds the + * fault budget, and resume selecting builder bids only when the observed blocks are within budget + * and meet the minimum recovery sample size. */ export class BuilderCircuitBreaker { readonly faultInspectionWindow: number; @@ -58,7 +62,13 @@ export class BuilderCircuitBreaker { const wasActive = this.active; // Scale the fault budget by blocks present so sparse windows still trigger on high non-reveal rates - this.active = faults * this.faultInspectionWindow > this.allowedFaults * blocksPresent; + const exceedsFaultBudget = faults * this.faultInspectionWindow > this.allowedFaults * blocksPresent; + if (exceedsFaultBudget) { + this.active = true; + } else if (blocksPresent >= MIN_BLOCKS_TO_DEACTIVATE) { + // Require a minimum sample within the fault budget before accepting builder bids again + this.active = false; + } this.modules.metrics?.builderCircuitBreaker.active.set(this.active ? 1 : 0); this.modules.metrics?.builderCircuitBreaker.faults.set(faults); @@ -66,6 +76,7 @@ export class BuilderCircuitBreaker { this.modules.metrics?.builderCircuitBreaker.payloadsRevealed.set(payloadsRevealed); const logCtx = { + clockSlot, blocksPresent, faults, faultInspectionWindow: this.faultInspectionWindow, diff --git a/packages/beacon-node/src/chain/options.ts b/packages/beacon-node/src/chain/options.ts index 265d3f5ddd02..2438645eac38 100644 --- a/packages/beacon-node/src/chain/options.ts +++ b/packages/beacon-node/src/chain/options.ts @@ -53,7 +53,7 @@ export type IChainOptions = BlockProcessOpts & nativeStateView?: boolean; /** Builder circuit breaker fault inspection window in slots */ faultInspectionWindow?: number; - /** Allowed unrevealed payloads within the fault inspection window */ + /** Unrevealed payloads allowed per `faultInspectionWindow` observed blocks */ allowedFaults?: number; }; diff --git a/packages/beacon-node/src/execution/builder/http.ts b/packages/beacon-node/src/execution/builder/http.ts index 78d029bdf584..af5bf9e8576e 100644 --- a/packages/beacon-node/src/execution/builder/http.ts +++ b/packages/beacon-node/src/execution/builder/http.ts @@ -71,15 +71,18 @@ export class NoBidReceived extends Error { } /** - * Beacon clients select randomized values from the following ranges when initializing - * the circuit breaker (so at boot time and once for each unique boot). + * Default circuit breaker parameters: * - * ALLOWED_FAULTS: between 1 and SLOTS_PER_EPOCH // 4 - * FAULT_INSPECTION_WINDOW: between SLOTS_PER_EPOCH and 2 * SLOTS_PER_EPOCH + * SLOTS_PER_EPOCH <= FAULT_INSPECTION_WINDOW < 2 * SLOTS_PER_EPOCH (randomized at initialization) + * ALLOWED_FAULTS: FAULT_INSPECTION_WINDOW // 4 * - * The values are randomized per node so builders cannot predict when a given proposer will - * fall back to local blocks. With fixed thresholds a builder could withhold payloads right up - * to the limit without ever tripping the breaker. + * e.g. on mainnet SLOTS_PER_EPOCH is 32, so FAULT_INSPECTION_WINDOW is between 32 and 63, + * and a window of 40 results in ALLOWED_FAULTS = 10. + * + * The randomized default window keeps the exact slots under inspection unpredictable per node, + * so a builder cannot tell when past faults age out of a given proposer's window and time + * withholding around trip or recovery points. Explicitly configured windows are clamped to at + * least SLOTS_PER_EPOCH, and configured allowedFaults is capped at the default ~25% budget. */ export function getFaultInspectionParams(opts: {faultInspectionWindow?: number; allowedFaults?: number}): { faultInspectionWindow: number; @@ -90,10 +93,7 @@ export function getFaultInspectionParams(opts: {faultInspectionWindow?: number; SLOTS_PER_EPOCH ); // allowedFaults should be < faultInspectionWindow, limiting them to faultInspectionWindow/4 - const allowedFaults = Math.min( - opts.allowedFaults ?? Math.floor(faultInspectionWindow / 4), - Math.floor(faultInspectionWindow / 4) - ); + const allowedFaults = Math.min(opts.allowedFaults ?? Infinity, Math.floor(faultInspectionWindow / 4)); return {faultInspectionWindow, allowedFaults}; } diff --git a/packages/beacon-node/test/unit/chain/builderCircuitBreaker.test.ts b/packages/beacon-node/test/unit/chain/builderCircuitBreaker.test.ts index cfde576e7f44..3f7c53b81fae 100644 --- a/packages/beacon-node/test/unit/chain/builderCircuitBreaker.test.ts +++ b/packages/beacon-node/test/unit/chain/builderCircuitBreaker.test.ts @@ -21,12 +21,13 @@ describe("BuilderCircuitBreaker", () => { } const testCases: [string, {blocksPresent: number; payloadsRevealed: number}, boolean][] = [ - ["empty window", {blocksPresent: 0, payloadsRevealed: 0}, false], + ["empty window keeps initial state", {blocksPresent: 0, payloadsRevealed: 0}, false], ["full window, no faults", {blocksPresent: 32, payloadsRevealed: 32}, false], ["full window, faults at budget", {blocksPresent: 32, payloadsRevealed: 24}, false], ["full window, faults above budget", {blocksPresent: 32, payloadsRevealed: 23}, true], ["sparse window, faults within scaled budget", {blocksPresent: 8, payloadsRevealed: 6}, false], ["sparse window, faults above scaled budget", {blocksPresent: 8, payloadsRevealed: 5}, true], + ["single unrevealed payload", {blocksPresent: 1, payloadsRevealed: 0}, true], ["sparse window, all payloads unrevealed", {blocksPresent: 4, payloadsRevealed: 0}, true], ]; @@ -43,6 +44,20 @@ describe("BuilderCircuitBreaker", () => { expect(getPayloadRevealCounts).toHaveBeenCalledWith(100 - faultInspectionWindow, 99); }); + it("requires a minimum sample to deactivate", () => { + const {breaker, getPayloadRevealCounts} = setup({blocksPresent: 1, payloadsRevealed: 0}); + expect(breaker.isActive(100)).toBe(true); + + getPayloadRevealCounts.mockReturnValue({blocksPresent: 0, payloadsRevealed: 0}); + expect(breaker.isActive(101)).toBe(true); + + getPayloadRevealCounts.mockReturnValue({blocksPresent: 3, payloadsRevealed: 3}); + expect(breaker.isActive(102)).toBe(true); + + getPayloadRevealCounts.mockReturnValue({blocksPresent: 4, payloadsRevealed: 3}); + expect(breaker.isActive(103)).toBe(false); + }); + it("only updates once per slot", () => { const {breaker, getPayloadRevealCounts} = setup({blocksPresent: 32, payloadsRevealed: 32}); expect(breaker.isActive(100)).toBe(false); diff --git a/packages/cli/src/options/beaconNodeOptions/builder.ts b/packages/cli/src/options/beaconNodeOptions/builder.ts index 25f7d09efc58..efc5bd4fbcf2 100644 --- a/packages/cli/src/options/beaconNodeOptions/builder.ts +++ b/packages/cli/src/options/beaconNodeOptions/builder.ts @@ -62,7 +62,7 @@ export const options: CliCommandOptions = { "builder.allowedFaults": { type: "number", description: - "Number of missed slots (pre-gloas) or unrevealed payloads (post-gloas) allowed in the `faultInspectionWindow` for builder circuit", + "Number of missed slots allowed within `faultInspectionWindow` before ignoring the external builder (pre-gloas). Post-gloas, sets the tolerated rate of unrevealed payloads, defined as `allowedFaults` out of `faultInspectionWindow` and applied to blocks observed in the window", group: "builder", }, }; diff --git a/packages/fork-choice/src/forkChoice/interface.ts b/packages/fork-choice/src/forkChoice/interface.ts index db40b4a422bd..737a7d3fd9ad 100644 --- a/packages/fork-choice/src/forkChoice/interface.ts +++ b/packages/fork-choice/src/forkChoice/interface.ts @@ -250,8 +250,8 @@ export interface IForkChoice { hasPayloadHexUnsafe(blockRoot: RootHex): boolean; getSlotsPresent(windowStart: number): number; /** - * Count gloas blocks with fromSlot <= slot <= toSlot and how many of them have a revealed - * payload (FULL variant exists). Used by the builder circuit breaker. + * Count Gloas blocks with fromSlot <= slot <= toSlot, and how many of them have a revealed payload + * (FULL variant exists). Used by the builder circuit breaker. */ getPayloadRevealCounts(fromSlot: Slot, toSlot: Slot): {blocksPresent: number; payloadsRevealed: number}; getPTCVotes(blockRootHex: RootHex): (boolean | null)[] | null; diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index b234f70d3b24..990d2e61a2da 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -1,5 +1,5 @@ import {BitArray} from "@chainsafe/ssz"; -import {GENESIS_EPOCH, PTC_SIZE} from "@lodestar/params"; +import {GENESIS_EPOCH, GENESIS_SLOT, PTC_SIZE} from "@lodestar/params"; import {DataAvailabilityStatus, computeEpochAtSlot, computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {Epoch, RootHex, Slot} from "@lodestar/types"; import {bitCount, toRootHex} from "@lodestar/utils"; @@ -691,8 +691,8 @@ export class ProtoArray { } /** - * Count gloas blocks with fromSlot <= slot <= toSlot and how many of them have a revealed - * payload (FULL variant exists). Used by the builder circuit breaker. + * Count Gloas blocks with fromSlot <= slot <= toSlot, and how many of them have a revealed payload + * (FULL variant exists). Used by the builder circuit breaker. */ getPayloadRevealCounts(fromSlot: Slot, toSlot: Slot): {blocksPresent: number; payloadsRevealed: number} { let blocksPresent = 0; @@ -704,6 +704,10 @@ export class ProtoArray { if (node.slot < fromSlot || node.slot > toSlot || node.payloadStatus !== PayloadStatus.PENDING) { continue; } + // Genesis block is always EMPTY + if (node.slot === GENESIS_SLOT) { + continue; + } blocksPresent++; if (this.hasPayload(node.blockRoot)) { payloadsRevealed++; diff --git a/packages/fork-choice/test/unit/protoArray/gloas.test.ts b/packages/fork-choice/test/unit/protoArray/gloas.test.ts index f99537953362..70e0c82dc8d3 100644 --- a/packages/fork-choice/test/unit/protoArray/gloas.test.ts +++ b/packages/fork-choice/test/unit/protoArray/gloas.test.ts @@ -1,6 +1,6 @@ import {beforeEach, describe, expect, it} from "vitest"; import {BitArray} from "@chainsafe/ssz"; -import {PTC_SIZE} from "@lodestar/params"; +import {GENESIS_SLOT, PTC_SIZE} from "@lodestar/params"; import {DataAvailabilityStatus, computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {RootHex} from "@lodestar/types"; import {ExecutionStatus, PayloadStatus, ProtoArray, ProtoBlock, ProtoNode} from "../../../src/index.js"; @@ -226,6 +226,40 @@ describe("Gloas Fork Choice", () => { payloadsRevealed: 0, }); }); + + it("does not count the genesis block", () => { + const currentSlot = GENESIS_SLOT + 1; + const protoArray = ProtoArray.initialize(createTestBlock(GENESIS_SLOT, genesisRoot, "0x00", "0x00"), currentSlot); + + protoArray.onBlock(createTestBlock(currentSlot, "0x02", genesisRoot, genesisRoot), currentSlot, null); + + expect(protoArray.getPayloadRevealCounts(GENESIS_SLOT, currentSlot)).toEqual({ + blocksPresent: 1, + payloadsRevealed: 0, + }); + }); + + it("counts a non-genesis anchor block seeded at initialization", () => { + const anchorRoot = "0x02"; + const currentSlot = gloasForkSlot + 1; + const protoArray = ProtoArray.initialize(createTestBlock(gloasForkSlot, anchorRoot, "0x00", "0x00"), currentSlot); + + protoArray.onExecutionPayload( + anchorRoot, + currentSlot, + "0x02ff", + 1, + 30000000, + null, + ExecutionStatus.Valid, + DataAvailabilityStatus.Available + ); + + expect(protoArray.getPayloadRevealCounts(gloasForkSlot, currentSlot)).toEqual({ + blocksPresent: 1, + payloadsRevealed: 1, + }); + }); }); describe("Pre-Gloas (Fulu) behavior", () => {