From d317aefa6379b98d61b03f25b3a954da2ef0394c Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Tue, 7 Jan 2025 18:51:36 +0000 Subject: [PATCH 1/5] feat: check gas limit in header received from builder --- .../src/api/impl/validator/index.ts | 2 +- .../chain/produceBlock/produceBlockBody.ts | 27 +++++++- .../src/execution/builder/cache.ts | 34 ++++++++++ .../beacon-node/src/execution/builder/http.ts | 15 ++++- .../src/execution/builder/index.ts | 1 + .../src/execution/builder/interface.ts | 5 +- .../src/execution/builder/utils.ts | 14 ++++ .../test/unit/execution/builder/cache.test.ts | 58 ++++++++++++++++ .../test/unit/execution/builder/utils.test.ts | 66 +++++++++++++++++++ 9 files changed, 217 insertions(+), 5 deletions(-) create mode 100644 packages/beacon-node/src/execution/builder/cache.ts create mode 100644 packages/beacon-node/src/execution/builder/utils.ts create mode 100644 packages/beacon-node/test/unit/execution/builder/cache.test.ts create mode 100644 packages/beacon-node/test/unit/execution/builder/utils.test.ts diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index fe26c559661c..ecb0b61928e2 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -1517,7 +1517,7 @@ export function getValidatorApi( ); }); - await chain.executionBuilder.registerValidator(filteredRegistrations); + await chain.executionBuilder.registerValidator(currentEpoch, filteredRegistrations); logger.debug("Forwarded validator registrations to connected builder", { epoch: currentEpoch, diff --git a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts index d7b36e06abc4..a97a4c8260e7 100644 --- a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts +++ b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts @@ -32,11 +32,17 @@ import { ssz, sszTypesFor, } from "@lodestar/types"; -import {Logger, sleep, toHex, toRootHex} from "@lodestar/utils"; +import {Logger, sleep, toHex, toPubkeyHex, toRootHex} from "@lodestar/utils"; import {ZERO_HASH, ZERO_HASH_HEX} from "../../constants/index.js"; import {IEth1ForBlockProduction} from "../../eth1/index.js"; import {numToQuantity} from "../../eth1/provider/utils.js"; -import {IExecutionBuilder, IExecutionEngine, PayloadAttributes, PayloadId} from "../../execution/index.js"; +import { + IExecutionBuilder, + IExecutionEngine, + PayloadAttributes, + PayloadId, + getExpectedGasLimit, +} from "../../execution/index.js"; import {fromGraffitiBuffer} from "../../util/graffiti.js"; import type {BeaconChain} from "../chain.js"; import {CommonBlockBody} from "../interface.js"; @@ -223,6 +229,23 @@ export async function produceBlockBody( fetchedTime, }); + const targetGasLimit = this.executionBuilder.getValidatorRegistration(proposerPubKey)?.gasLimit; + if (!targetGasLimit) { + // This should only happen if cache was cleared due to restart of beacon node + this.logger.warn("Failed to get validator registration, could not check header gas limit", { + slot: blockSlot, + proposerIndex, + proposerPubKey: toPubkeyHex(proposerPubKey), + }); + } else { + const parentGasLimit = (currentState as CachedBeaconStateBellatrix).latestExecutionPayloadHeader.gasLimit; + const expectedGasLimit = getExpectedGasLimit(parentGasLimit, targetGasLimit); + + if (builderRes.header.gasLimit !== expectedGasLimit) { + throw Error(`Incorrect header gas limit ${builderRes.header.gasLimit} != ${expectedGasLimit}`); + } + } + if (ForkSeq[fork] >= ForkSeq.deneb) { const {blobKzgCommitments} = builderRes; if (blobKzgCommitments === undefined) { diff --git a/packages/beacon-node/src/execution/builder/cache.ts b/packages/beacon-node/src/execution/builder/cache.ts new file mode 100644 index 000000000000..9ca73efc56d6 --- /dev/null +++ b/packages/beacon-node/src/execution/builder/cache.ts @@ -0,0 +1,34 @@ +import {BLSPubkey, Epoch, bellatrix} from "@lodestar/types"; +import {toPubkeyHex} from "@lodestar/utils"; + +const REGISTRATION_PRESERVE_EPOCHS = 2; + +export type ValidatorRegistration = { + epoch: Epoch; + /** Preferred gas limit of validator */ + gasLimit: number; +}; + +export class ValidatorRegistrationCache { + private readonly registrationByValidatorPubkey: Map; + constructor() { + this.registrationByValidatorPubkey = new Map(); + } + + add(epoch: Epoch, {pubkey, gasLimit}: bellatrix.ValidatorRegistrationV1): void { + this.registrationByValidatorPubkey.set(toPubkeyHex(pubkey), {epoch, gasLimit}); + } + + prune(epoch: Epoch): void { + for (const [pubkeyHex, registration] of this.registrationByValidatorPubkey.entries()) { + // We only retain an registrations for REGISTRATION_PRESERVE_EPOCHS epochs + if (registration.epoch + REGISTRATION_PRESERVE_EPOCHS < epoch) { + this.registrationByValidatorPubkey.delete(pubkeyHex); + } + } + } + + get(pubkey: BLSPubkey): ValidatorRegistration | undefined { + return this.registrationByValidatorPubkey.get(toPubkeyHex(pubkey)); + } +} diff --git a/packages/beacon-node/src/execution/builder/http.ts b/packages/beacon-node/src/execution/builder/http.ts index 12e45412bafc..d743cedea545 100644 --- a/packages/beacon-node/src/execution/builder/http.ts +++ b/packages/beacon-node/src/execution/builder/http.ts @@ -6,6 +6,7 @@ import {ForkExecution, SLOTS_PER_EPOCH} from "@lodestar/params"; import {parseExecutionPayloadAndBlobsBundle, reconstructFullBlockOrContents} from "@lodestar/state-transition"; import { BLSPubkey, + Epoch, ExecutionPayloadHeader, Root, SignedBeaconBlockOrContents, @@ -19,6 +20,7 @@ import { } from "@lodestar/types"; import {toPrintableUrl} from "@lodestar/utils"; import {Metrics} from "../../metrics/metrics.js"; +import {ValidatorRegistration, ValidatorRegistrationCache} from "./cache.js"; import {IExecutionBuilder} from "./interface.js"; export type ExecutionBuilderHttpOpts = { @@ -60,6 +62,7 @@ const BUILDER_PROPOSAL_DELAY_TOLERANCE = 1000; export class ExecutionBuilderHttp implements IExecutionBuilder { readonly api: BuilderApi; readonly config: ChainForkConfig; + readonly registrations: ValidatorRegistrationCache; readonly issueLocalFcUWithFeeRecipient?: string; // Builder needs to be explicity enabled using updateStatus status = false; @@ -93,6 +96,7 @@ export class ExecutionBuilderHttp implements IExecutionBuilder { ); logger?.info("External builder", {url: toPrintableUrl(baseUrl)}); this.config = config; + this.registrations = new ValidatorRegistrationCache(); this.issueLocalFcUWithFeeRecipient = opts.issueLocalFcUWithFeeRecipient; /** @@ -128,8 +132,17 @@ export class ExecutionBuilderHttp implements IExecutionBuilder { } } - async registerValidator(registrations: bellatrix.SignedValidatorRegistrationV1[]): Promise { + async registerValidator(epoch: Epoch, registrations: bellatrix.SignedValidatorRegistrationV1[]): Promise { (await this.api.registerValidator({registrations})).assertOk(); + + for (const registration of registrations) { + this.registrations.add(epoch, registration.message); + } + this.registrations.prune(epoch); + } + + getValidatorRegistration(pubkey: BLSPubkey): ValidatorRegistration | undefined { + return this.registrations.get(pubkey); } async getHeader( diff --git a/packages/beacon-node/src/execution/builder/index.ts b/packages/beacon-node/src/execution/builder/index.ts index fff66a3e8bc5..929e88461c8a 100644 --- a/packages/beacon-node/src/execution/builder/index.ts +++ b/packages/beacon-node/src/execution/builder/index.ts @@ -2,6 +2,7 @@ import {ChainForkConfig} from "@lodestar/config"; import {Logger} from "@lodestar/logger"; import {Metrics} from "../../metrics/metrics.js"; import {IExecutionBuilder} from "./interface.js"; +export {getExpectedGasLimit} from "./utils.js"; import {ExecutionBuilderHttp, ExecutionBuilderHttpOpts, defaultExecutionBuilderHttpOpts} from "./http.js"; diff --git a/packages/beacon-node/src/execution/builder/interface.ts b/packages/beacon-node/src/execution/builder/interface.ts index 06cdc1da4ed0..f326ec4bc451 100644 --- a/packages/beacon-node/src/execution/builder/interface.ts +++ b/packages/beacon-node/src/execution/builder/interface.ts @@ -1,6 +1,7 @@ import {ForkExecution} from "@lodestar/params"; import { BLSPubkey, + Epoch, ExecutionPayloadHeader, Root, SignedBeaconBlockOrContents, @@ -12,6 +13,7 @@ import { deneb, electra, } from "@lodestar/types"; +import {ValidatorRegistration} from "./cache.js"; export interface IExecutionBuilder { /** @@ -28,7 +30,8 @@ export interface IExecutionBuilder { updateStatus(shouldEnable: boolean): void; checkStatus(): Promise; - registerValidator(registrations: bellatrix.SignedValidatorRegistrationV1[]): Promise; + registerValidator(epoch: Epoch, registrations: bellatrix.SignedValidatorRegistrationV1[]): Promise; + getValidatorRegistration(pubkey: BLSPubkey): ValidatorRegistration | undefined; getHeader( fork: ForkExecution, slot: Slot, diff --git a/packages/beacon-node/src/execution/builder/utils.ts b/packages/beacon-node/src/execution/builder/utils.ts new file mode 100644 index 000000000000..42bbaece53e8 --- /dev/null +++ b/packages/beacon-node/src/execution/builder/utils.ts @@ -0,0 +1,14 @@ +/** + * Calculates expected gas limit based on parent gas limit and target gas limit + */ +export function getExpectedGasLimit(parentGasLimit: number, targetGasLimit: number): number { + const maxGasLimitDifference = Math.max(Math.floor(parentGasLimit / 1024) - 1, 0); + + if (targetGasLimit > parentGasLimit) { + const gasDiff = targetGasLimit - parentGasLimit; + return parentGasLimit + Math.min(gasDiff, maxGasLimitDifference); + } + + const gasDiff = parentGasLimit - targetGasLimit; + return parentGasLimit - Math.min(gasDiff, maxGasLimitDifference); +} diff --git a/packages/beacon-node/test/unit/execution/builder/cache.test.ts b/packages/beacon-node/test/unit/execution/builder/cache.test.ts new file mode 100644 index 000000000000..60667caf0a50 --- /dev/null +++ b/packages/beacon-node/test/unit/execution/builder/cache.test.ts @@ -0,0 +1,58 @@ +import {ssz} from "@lodestar/types"; +import {beforeEach, describe, expect, it} from "vitest"; +import {ValidatorRegistrationCache} from "../../../../src/execution/builder/cache.js"; + +describe("ValidatorRegistrationCache", () => { + const gasLimit1 = 30000000; + const gasLimit2 = 36000000; + + const validatorPubkey1 = new Uint8Array(48).fill(1); + const validatorPubkey2 = new Uint8Array(48).fill(2); + + const validatorRegistration1 = ssz.bellatrix.ValidatorRegistrationV1.defaultValue(); + validatorRegistration1.pubkey = validatorPubkey1; + validatorRegistration1.gasLimit = gasLimit1; + + const validatorRegistration2 = ssz.bellatrix.ValidatorRegistrationV1.defaultValue(); + validatorRegistration2.pubkey = validatorPubkey2; + validatorRegistration2.gasLimit = gasLimit2; + + let cache: ValidatorRegistrationCache; + + beforeEach(() => { + // max 2 items + cache = new ValidatorRegistrationCache(); + cache.add(1, validatorRegistration1); + cache.add(3, validatorRegistration2); + }); + + it("get for registered validator", () => { + expect(cache.get(validatorPubkey1)?.gasLimit).toBe(gasLimit1); + }); + + it("get for unknown validator", () => { + const unknownValidatorPubkey = new Uint8Array(48).fill(3); + expect(cache.get(unknownValidatorPubkey)).toBe(undefined); + }); + + it("override and get latest", () => { + const newGasLimit = 60000000; + const registration = ssz.bellatrix.ValidatorRegistrationV1.defaultValue(); + registration.pubkey = validatorPubkey1; + registration.gasLimit = newGasLimit; + + cache.add(5, registration); + + expect(cache.get(validatorPubkey1)?.gasLimit).toBe(newGasLimit); + }); + + it("prune", () => { + cache.prune(4); + + // No registration as it has been pruned + expect(cache.get(validatorPubkey1)).toBe(undefined); + + // Registration hasn't been pruned + expect(cache.get(validatorPubkey2)?.gasLimit).toBe(gasLimit2); + }); +}); diff --git a/packages/beacon-node/test/unit/execution/builder/utils.test.ts b/packages/beacon-node/test/unit/execution/builder/utils.test.ts new file mode 100644 index 000000000000..90520fb00e1f --- /dev/null +++ b/packages/beacon-node/test/unit/execution/builder/utils.test.ts @@ -0,0 +1,66 @@ +import {describe, expect, it} from "vitest"; +import {getExpectedGasLimit} from "../../../../src/execution/builder/utils.js"; + +describe("execution / builder / utils", () => { + describe("getExpectedGasLimit", () => { + const testCases: { + name: string; + parentGasLimit: number; + targetGasLimit: number; + expected: number; + }[] = [ + { + name: "Increase within limit", + parentGasLimit: 30000000, + targetGasLimit: 30000100, + expected: 30000100, + }, + { + name: "Increase exceeding limit", + parentGasLimit: 30000000, + targetGasLimit: 36000000, + expected: 30029295, // maxGasLimitDifference = (30000000 / 1024) - 1 = 29295 + }, + { + name: "Decrease within limit", + parentGasLimit: 30000000, + targetGasLimit: 29999990, + expected: 29999990, + }, + { + name: "Decrease exceeding limit", + parentGasLimit: 36000000, + targetGasLimit: 30000000, + expected: 35964845, // maxGasLimitDifference = (36000000 / 1024) - 1 = 35155 + }, + { + name: "Target equals parent", + parentGasLimit: 30000000, + targetGasLimit: 30000000, + expected: 30000000, // No change + }, + { + name: "Very small parent gas limit", + parentGasLimit: 1025, + targetGasLimit: 2000, + expected: 1025, + }, + { + name: "Target far below parent but limited", + parentGasLimit: 30000000, + targetGasLimit: 10000000, + expected: 29970705, // maxGasLimitDifference = (30000000 / 1024) - 1 = 29295 + }, + { + name: "Parent gas limit underflows", + parentGasLimit: 1023, + targetGasLimit: 30000000, + expected: 1023, + }, + ]; + + it.each(testCases)("$name", ({parentGasLimit, targetGasLimit, expected}) => { + expect(getExpectedGasLimit(parentGasLimit, targetGasLimit)).toBe(expected); + }); + }); +}); From 3b4f655b0867e2a507dc8ca0410088f3e8e4e675 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 9 Jan 2025 11:58:44 +0000 Subject: [PATCH 2/5] Add constant for gas limit adjustment factor --- packages/beacon-node/src/execution/builder/utils.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/beacon-node/src/execution/builder/utils.ts b/packages/beacon-node/src/execution/builder/utils.ts index 42bbaece53e8..76d75fda2b35 100644 --- a/packages/beacon-node/src/execution/builder/utils.ts +++ b/packages/beacon-node/src/execution/builder/utils.ts @@ -1,8 +1,13 @@ +/** + * From https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1559.md + */ +const gasLimitAdjustmentFactor = 1024; + /** * Calculates expected gas limit based on parent gas limit and target gas limit */ export function getExpectedGasLimit(parentGasLimit: number, targetGasLimit: number): number { - const maxGasLimitDifference = Math.max(Math.floor(parentGasLimit / 1024) - 1, 0); + const maxGasLimitDifference = Math.max(Math.floor(parentGasLimit / gasLimitAdjustmentFactor) - 1, 0); if (targetGasLimit > parentGasLimit) { const gasDiff = targetGasLimit - parentGasLimit; From 107694cbef693d1de12238fdbd98d74093cfe481 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 9 Jan 2025 19:08:33 +0000 Subject: [PATCH 3/5] Relax header gas limit check to lower / upper bound --- .../src/chain/produceBlock/produceBlockBody.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts index a97a4c8260e7..68443c3bb921 100644 --- a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts +++ b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts @@ -238,11 +238,25 @@ export async function produceBlockBody( proposerPubKey: toPubkeyHex(proposerPubKey), }); } else { + const headerGasLimit = builderRes.header.gasLimit; const parentGasLimit = (currentState as CachedBeaconStateBellatrix).latestExecutionPayloadHeader.gasLimit; const expectedGasLimit = getExpectedGasLimit(parentGasLimit, targetGasLimit); - if (builderRes.header.gasLimit !== expectedGasLimit) { - throw Error(`Incorrect header gas limit ${builderRes.header.gasLimit} != ${expectedGasLimit}`); + const lowerBound = Math.min(parentGasLimit, expectedGasLimit); + const upperBound = Math.max(parentGasLimit, expectedGasLimit); + + if (headerGasLimit < lowerBound || headerGasLimit > upperBound) { + throw Error( + `Header gas limit ${headerGasLimit} is outside of acceptable range [${lowerBound}, ${upperBound}]` + ); + } + + if (headerGasLimit !== expectedGasLimit) { + this.logger.warn("Header gas limit does not match the expected value", { + slot: blockSlot, + headerGasLimit, + expectedGasLimit, + }); } } From 500c07dd114b8b4f82cbe4cbde2778be72bfddec Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 9 Jan 2025 19:18:28 +0000 Subject: [PATCH 4/5] Include parent and target gas limit in warning log --- .../beacon-node/src/chain/produceBlock/produceBlockBody.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts index 68443c3bb921..cbd46b1655aa 100644 --- a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts +++ b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts @@ -252,10 +252,12 @@ export async function produceBlockBody( } if (headerGasLimit !== expectedGasLimit) { - this.logger.warn("Header gas limit does not match the expected value", { + this.logger.warn("Header gas limit does not match expected value", { slot: blockSlot, headerGasLimit, expectedGasLimit, + parentGasLimit, + targetGasLimit, }); } } From d00450ba978cbff2060a99c8586b210f7f9cf312 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Tue, 14 Jan 2025 11:02:20 +0000 Subject: [PATCH 5/5] Add note on why validator pubkey used as key instead of index --- packages/beacon-node/src/execution/builder/cache.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/beacon-node/src/execution/builder/cache.ts b/packages/beacon-node/src/execution/builder/cache.ts index 9ca73efc56d6..3fe192460a0b 100644 --- a/packages/beacon-node/src/execution/builder/cache.ts +++ b/packages/beacon-node/src/execution/builder/cache.ts @@ -10,6 +10,11 @@ export type ValidatorRegistration = { }; export class ValidatorRegistrationCache { + /** + * Map to track registrations by validator pubkey which is used here instead of + * validator index as `bellatrix.ValidatorRegistrationV1` does not contain the index + * and builder flow in general prefers to use pubkey over index. + */ private readonly registrationByValidatorPubkey: Map; constructor() { this.registrationByValidatorPubkey = new Map();