From 45e9fc07cf05aae928c3fd2fc09a0c08c5158bf1 Mon Sep 17 00:00:00 2001 From: gajinder Date: Sat, 2 Dec 2023 02:46:40 +0530 Subject: [PATCH 1/4] feat: allow configuring broadcast validation on validator for proposals --- packages/cli/src/cmds/validator/handler.ts | 16 ++++++++++ packages/cli/src/cmds/validator/options.ts | 7 +++++ packages/validator/src/services/block.ts | 29 ++++++++++++------- .../validator/src/services/validatorStore.ts | 2 ++ packages/validator/src/validator.ts | 10 ++++++- .../test/unit/services/block.test.ts | 1 + 6 files changed, 54 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/cmds/validator/handler.ts b/packages/cli/src/cmds/validator/handler.ts index 8537500684ca..fe14cedbcca1 100644 --- a/packages/cli/src/cmds/validator/handler.ts +++ b/packages/cli/src/cmds/validator/handler.ts @@ -169,6 +169,7 @@ export async function validatorHandler(args: IValidatorCliArgs & GlobalArgs): Pr valProposerConfig, distributed: args.distributed, useProduceBlockV3: args.useProduceBlockV3, + broadcastValidation: parseBroadcastValidation(args.broadcastValidation), }, metrics ); @@ -268,3 +269,18 @@ function parseBuilderSelection(builderSelection?: string): routes.validator.Buil } return builderSelection as routes.validator.BuilderSelection; } + +function parseBroadcastValidation(broadcastValidation?: string): routes.beacon.BroadcastValidation | undefined { + if (broadcastValidation) { + switch (broadcastValidation) { + case "gossip": + case "consensus": + case "consensus_and_equivocation": + break; + default: + throw Error("Invalid input for broadcastValidation, check help"); + } + } + + return broadcastValidation as routes.beacon.BroadcastValidation; +} diff --git a/packages/cli/src/cmds/validator/options.ts b/packages/cli/src/cmds/validator/options.ts index f3daded11ac7..25400ecd16d5 100644 --- a/packages/cli/src/cmds/validator/options.ts +++ b/packages/cli/src/cmds/validator/options.ts @@ -47,6 +47,7 @@ export type IValidatorCliArgs = AccountValidatorArgs & "builder.selection"?: string; useProduceBlockV3?: boolean; + broadcastValidation?: string; importKeystores?: string[]; importKeystoresPassword?: string; @@ -250,6 +251,12 @@ export const validatorOptions: CliCommandOptions = { defaultDescription: `${defaultOptions.useProduceBlockV3}`, }, + broadcastValidation: { + type: "string", + description: "Validations to be run by beacon node for the signed block prior to publishing", + defaultDescription: `${defaultOptions.broadcastValidation}`, + }, + importKeystores: { alias: ["keystore"], // Backwards compatibility with old `validator import` cmdx description: "Path(s) to a directory or single file path to validator keystores, i.e. Launchpad validators", diff --git a/packages/validator/src/services/block.ts b/packages/validator/src/services/block.ts index f01df8e0263f..55b7c4cd74ce 100644 --- a/packages/validator/src/services/block.ts +++ b/packages/validator/src/services/block.ts @@ -70,7 +70,7 @@ export class BlockProposingService { private readonly clock: IClock, private readonly validatorStore: ValidatorStore, private readonly metrics: Metrics | null, - private readonly opts: {useProduceBlockV3: boolean} + private readonly opts: {useProduceBlockV3: boolean; broadcastValidation: routes.beacon.BroadcastValidation} ) { this.dutiesService = new BlockDutiesService( config, @@ -158,7 +158,9 @@ export class BlockProposingService { signedBlobs = undefined; } - await this.publishBlockWrapper(signedBlock, signedBlobs).catch((e: Error) => { + await this.publishBlockWrapper(signedBlock, signedBlobs, { + broadcastValidation: this.opts.broadcastValidation, + }).catch((e: Error) => { this.metrics?.blockProposingErrors.inc({error: "publish"}); throw extendError(e, "Failed to publish block"); }); @@ -172,22 +174,29 @@ export class BlockProposingService { private publishBlockWrapper = async ( signedBlock: allForks.FullOrBlindedSignedBeaconBlock, - signedBlobSidecars?: allForks.FullOrBlindedSignedBlobSidecar[] + signedBlobSidecars?: allForks.FullOrBlindedSignedBlobSidecar[], + opts: {broadcastValidation?: routes.beacon.BroadcastValidation} = {} ): Promise => { if (signedBlobSidecars === undefined) { ApiError.assert( isBlindedBeaconBlock(signedBlock.message) - ? await this.api.beacon.publishBlindedBlock(signedBlock as allForks.SignedBlindedBeaconBlock) - : await this.api.beacon.publishBlockV2(signedBlock as allForks.SignedBeaconBlock) + ? await this.api.beacon.publishBlindedBlockV2(signedBlock as allForks.SignedBlindedBeaconBlock, opts) + : await this.api.beacon.publishBlockV2(signedBlock as allForks.SignedBeaconBlock, opts) ); } else { ApiError.assert( isBlindedBeaconBlock(signedBlock.message) - ? await this.api.beacon.publishBlindedBlock({ - signedBlindedBlock: signedBlock, - signedBlindedBlobSidecars: signedBlobSidecars, - } as allForks.SignedBlindedBlockContents) - : await this.api.beacon.publishBlockV2({signedBlock, signedBlobSidecars} as allForks.SignedBlockContents) + ? await this.api.beacon.publishBlindedBlockV2( + { + signedBlindedBlock: signedBlock, + signedBlindedBlobSidecars: signedBlobSidecars, + } as allForks.SignedBlindedBlockContents, + opts + ) + : await this.api.beacon.publishBlockV2( + {signedBlock, signedBlobSidecars} as allForks.SignedBlockContents, + opts + ) ); } }; diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 80efdeca4cbf..e2736a09754a 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -127,6 +127,8 @@ export const defaultOptions = { builderAliasSelection: routes.validator.BuilderSelection.MaxProfit, // turn it off by default, turn it back on once other clients support v3 api useProduceBlockV3: false, + // spec asks for gossip validation by default + broadcastValidation: routes.beacon.BroadcastValidation.gossip, }; /** diff --git a/packages/validator/src/validator.ts b/packages/validator/src/validator.ts index 68ae2135b564..0a367126dc84 100644 --- a/packages/validator/src/validator.ts +++ b/packages/validator/src/validator.ts @@ -57,6 +57,7 @@ export type ValidatorOptions = { valProposerConfig?: ValidatorProposerConfig; distributed?: boolean; useProduceBlockV3?: boolean; + broadcastValidation?: routes.beacon.BroadcastValidation; }; // TODO: Extend the timeout, and let it be customizable @@ -208,6 +209,7 @@ export class Validator { const blockProposingService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, metrics, { useProduceBlockV3: opts.useProduceBlockV3 ?? defaultOptions.useProduceBlockV3, + broadcastValidation: opts.broadcastValidation ?? defaultOptions.broadcastValidation, }); const attestationService = new AttestationService( @@ -285,13 +287,19 @@ export class Validator { await assertEqualGenesis(opts, genesis); logger.info("Verified connected beacon node and validator have the same genesisValidatorRoot"); - const {useProduceBlockV3 = defaultOptions.useProduceBlockV3, valProposerConfig} = opts; + const { + useProduceBlockV3 = defaultOptions.useProduceBlockV3, + broadcastValidation = defaultOptions.broadcastValidation, + valProposerConfig, + } = opts; const defaultBuilderSelection = valProposerConfig?.defaultConfig.builder?.selection ?? defaultOptions.builderSelection; const strictFeeRecipientCheck = valProposerConfig?.defaultConfig.strictFeeRecipientCheck ?? false; const suggestedFeeRecipient = valProposerConfig?.defaultConfig.feeRecipient ?? defaultOptions.suggestedFeeRecipient; + logger.info("Initializing validator", { useProduceBlockV3, + broadcastValidation, defaultBuilderSelection, suggestedFeeRecipient, strictFeeRecipientCheck, diff --git a/packages/validator/test/unit/services/block.test.ts b/packages/validator/test/unit/services/block.test.ts index 0a533b140a9c..c3d2cdf83043 100644 --- a/packages/validator/test/unit/services/block.test.ts +++ b/packages/validator/test/unit/services/block.test.ts @@ -52,6 +52,7 @@ describe("BlockDutiesService", function () { // use produceBlockV3 const blockService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, null, { useProduceBlockV3: true, + broadcastValidation: "consensus", }); const signedBlock = ssz.phase0.SignedBeaconBlock.defaultValue(); From 0cd4e7e17229269bbf4b6c98726d96628685ed9a Mon Sep 17 00:00:00 2001 From: gajinder Date: Sat, 2 Dec 2023 11:17:15 +0530 Subject: [PATCH 2/4] fix test --- packages/validator/test/unit/services/block.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/validator/test/unit/services/block.test.ts b/packages/validator/test/unit/services/block.test.ts index c3d2cdf83043..0c78fdc82eec 100644 --- a/packages/validator/test/unit/services/block.test.ts +++ b/packages/validator/test/unit/services/block.test.ts @@ -6,7 +6,7 @@ import {createChainForkConfig} from "@lodestar/config"; import {config as mainnetConfig} from "@lodestar/config/default"; import {sleep} from "@lodestar/utils"; import {ssz} from "@lodestar/types"; -import {HttpStatusCode} from "@lodestar/api"; +import {HttpStatusCode, routes} from "@lodestar/api"; import {ForkName} from "@lodestar/params"; import {BlockProposingService} from "../../../src/services/block.js"; import {ValidatorStore} from "../../../src/services/validatorStore.js"; @@ -52,7 +52,7 @@ describe("BlockDutiesService", function () { // use produceBlockV3 const blockService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, null, { useProduceBlockV3: true, - broadcastValidation: "consensus", + broadcastValidation: routes.beacon.BroadcastValidation.consensus, }); const signedBlock = ssz.phase0.SignedBeaconBlock.defaultValue(); @@ -79,6 +79,9 @@ describe("BlockDutiesService", function () { // Must have submitted the block received on signBlock() expect(api.beacon.publishBlockV2.callCount).to.equal(1, "publishBlock() must be called once"); - expect(api.beacon.publishBlockV2.getCall(0).args).to.deep.equal([signedBlock], "wrong publishBlock() args"); + expect(api.beacon.publishBlockV2.getCall(0).args).to.deep.equal( + [signedBlock, {broadcastValidation: routes.beacon.BroadcastValidation.consensus}], + "wrong publishBlock() args" + ); }); }); From ac561fe258691787a5cee6188140c182a4fabfde Mon Sep 17 00:00:00 2001 From: gajinder Date: Sat, 2 Dec 2023 16:59:26 +0530 Subject: [PATCH 3/4] add bodyroot log --- packages/beacon-node/src/chain/chain.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index a6d92760b3f8..b73a36eace7b 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -1,5 +1,5 @@ import path from "node:path"; -import {CompositeTypeAny, fromHexString, TreeView, Type} from "@chainsafe/ssz"; +import {CompositeTypeAny, fromHexString, toHexString, TreeView, Type} from "@chainsafe/ssz"; import { BeaconStateAllForks, CachedBeaconStateAllForks, @@ -504,6 +504,17 @@ export class BeaconChain implements IBeaconChain { proposerPubKey, }); + // The hashtree root computed here for debug log will get cached and hence won't introduce additional delays + const bodyRoot = + blockType === BlockType.Full + ? this.config.getForkTypes(slot).BeaconBlockBody.hashTreeRoot(body) + : this.config.getBlindedForkTypes(slot).BeaconBlockBody.hashTreeRoot(body as allForks.BlindedBeaconBlockBody); + this.logger.debug("Computing block post state from tje produced body", { + slot, + bodyRoot: toHexString(bodyRoot), + blockType, + }); + const block = { slot, proposerIndex, @@ -511,7 +522,6 @@ export class BeaconChain implements IBeaconChain { stateRoot: ZERO_HASH, body, } as AssembledBlockType; - block.stateRoot = computeNewStateRoot(this.metrics, state, block); const blockRoot = blockType === BlockType.Full From 401eac84a237c69203d8864ebd1bc541fec8493f Mon Sep 17 00:00:00 2001 From: gajinder Date: Sat, 2 Dec 2023 18:17:47 +0530 Subject: [PATCH 4/4] typo --- packages/beacon-node/src/chain/chain.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index b73a36eace7b..3c86a7aaa2d4 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -509,7 +509,7 @@ export class BeaconChain implements IBeaconChain { blockType === BlockType.Full ? this.config.getForkTypes(slot).BeaconBlockBody.hashTreeRoot(body) : this.config.getBlindedForkTypes(slot).BeaconBlockBody.hashTreeRoot(body as allForks.BlindedBeaconBlockBody); - this.logger.debug("Computing block post state from tje produced body", { + this.logger.debug("Computing block post state from the produced body", { slot, bodyRoot: toHexString(bodyRoot), blockType,