Skip to content

Commit

Permalink
undo validator changes as per feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
g11tech committed Jul 30, 2023
1 parent c8be41d commit 88a406a
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 61 deletions.
3 changes: 0 additions & 3 deletions packages/cli/src/cmds/validator/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
getHttpMetricsServer,
MonitoringService,
} from "@lodestar/beacon-node";
import {routes} from "@lodestar/api";
import {getNodeLogger} from "@lodestar/logger/node";
import {getBeaconConfigFromArgs} from "../../config/index.js";
import {GlobalArgs} from "../../options/index.js";
Expand Down Expand Up @@ -168,8 +167,6 @@ export async function validatorHandler(args: IValidatorCliArgs & GlobalArgs): Pr
disableAttestationGrouping: args.disableAttestationGrouping,
valProposerConfig,
distributed: args.distributed,
broadcastValidation:
routes.beacon.BroadcastValidation[args.broadcastValidation as keyof typeof routes.beacon.BroadcastValidation],
},
metrics
);
Expand Down
9 changes: 0 additions & 9 deletions packages/cli/src/cmds/validator/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export type IValidatorCliArgs = AccountValidatorArgs &
strictFeeRecipientCheck?: boolean;
doppelgangerProtection?: boolean;
defaultGasLimit?: number;
broadcastValidation?: string;

builder?: boolean;
"builder.selection"?: string;
Expand Down Expand Up @@ -128,14 +127,6 @@ export const validatorOptions: CliCommandOptions<IValidatorCliArgs> = {
...logOptions,
...keymanagerOptions,

broadcastValidation: {
// TODO: flag hidden till validations fully implemented
hidden: true,
description: "Broadcast validation to be requested before publishing the block",
defaultDescription: defaultOptions.broadcastValidation,
type: "string",
},

keystoresDir: {
hidden: true,
description: "Directory for storing validator keystores.",
Expand Down
30 changes: 8 additions & 22 deletions packages/validator/src/services/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
SignedBlockContents,
BlockContents,
BlindedBlockContents,
routes,
} from "@lodestar/api";
import {IClock, LoggerVc} from "../util/index.js";
import {PubkeyHex} from "../types.js";
Expand Down Expand Up @@ -60,16 +59,14 @@ type ProduceBlockOpts = {
*/
export class BlockProposingService {
private readonly dutiesService: BlockDutiesService;
private readonly broadcastValidation?: routes.beacon.BroadcastValidation;

constructor(
private readonly config: ChainForkConfig,
private readonly logger: LoggerVc,
private readonly api: Api,
private readonly clock: IClock,
private readonly validatorStore: ValidatorStore,
private readonly metrics: Metrics | null,
broadcastValidation?: routes.beacon.BroadcastValidation
private readonly metrics: Metrics | null
) {
this.dutiesService = new BlockDutiesService(
logger,
Expand All @@ -79,8 +76,6 @@ export class BlockProposingService {
metrics,
this.notifyBlockProductionFn
);

this.broadcastValidation = broadcastValidation;
}

removeDutiesForKey(pubkey: PubkeyHex): void {
Expand Down Expand Up @@ -178,26 +173,17 @@ export class BlockProposingService {
if (signedBlobSidecars === undefined) {
ApiError.assert(
isBlindedBeaconBlock(signedBlock.message)
? await this.api.beacon.publishBlindedBlockV2(signedBlock as allForks.SignedBlindedBeaconBlock, {
broadcastValidation: this.broadcastValidation,
})
: await this.api.beacon.publishBlockV2(signedBlock as allForks.SignedBeaconBlock, {
broadcastValidation: this.broadcastValidation,
})
? await this.api.beacon.publishBlindedBlock(signedBlock as allForks.SignedBlindedBeaconBlock)
: await this.api.beacon.publishBlock(signedBlock as allForks.SignedBeaconBlock)
);
} else {
ApiError.assert(
isBlindedBeaconBlock(signedBlock.message)
? await this.api.beacon.publishBlindedBlockV2(
{
signedBlindedBlock: signedBlock,
signedBlindedBlobSidecars: signedBlobSidecars,
} as SignedBlindedBlockContents,
{broadcastValidation: this.broadcastValidation}
)
: await this.api.beacon.publishBlockV2({signedBlock, signedBlobSidecars} as SignedBlockContents, {
broadcastValidation: this.broadcastValidation,
})
? await this.api.beacon.publishBlindedBlock({
signedBlindedBlock: signedBlock,
signedBlindedBlobSidecars: signedBlobSidecars,
} as SignedBlindedBlockContents)
: await this.api.beacon.publishBlock({signedBlock, signedBlobSidecars} as SignedBlockContents)
);
}
};
Expand Down
1 change: 0 additions & 1 deletion packages/validator/src/services/validatorStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ export const defaultOptions = {
suggestedFeeRecipient: "0x0000000000000000000000000000000000000000",
defaultGasLimit: 30_000_000,
builderSelection: BuilderSelection.MaxProfit,
broadcastValidation: routes.beacon.BroadcastValidation.none,
};

/**
Expand Down
11 changes: 1 addition & 10 deletions packages/validator/src/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export type ValidatorOptions = {
closed?: boolean;
valProposerConfig?: ValidatorProposerConfig;
distributed?: boolean;
broadcastValidation?: routes.beacon.BroadcastValidation;
};

// TODO: Extend the timeout, and let it be customizable
Expand Down Expand Up @@ -122,15 +121,7 @@ export class Validator {

const chainHeaderTracker = new ChainHeaderTracker(logger, api, emitter);

this.blockProposingService = new BlockProposingService(
config,
loggerVc,
api,
clock,
validatorStore,
metrics,
opts.broadcastValidation
);
this.blockProposingService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, metrics);

this.attestationService = new AttestationService(
loggerVc,
Expand Down
21 changes: 5 additions & 16 deletions packages/validator/test/unit/services/block.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, routes} from "@lodestar/api";
import {HttpStatusCode} from "@lodestar/api";
import {BlockProposingService} from "../../../src/services/block.js";
import {ValidatorStore} from "../../../src/services/validatorStore.js";
import {getApiClientStub} from "../../utils/apiStub.js";
Expand Down Expand Up @@ -48,15 +48,7 @@ describe("BlockDutiesService", function () {
});

const clock = new ClockMock();
const blockService = new BlockProposingService(
config,
loggerVc,
api,
clock,
validatorStore,
null,
routes.beacon.BroadcastValidation.none
);
const blockService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, null);

const signedBlock = ssz.phase0.SignedBeaconBlock.defaultValue();
validatorStore.signRandao.resolves(signedBlock.message.body.randaoReveal);
Expand All @@ -66,7 +58,7 @@ describe("BlockDutiesService", function () {
ok: true,
status: HttpStatusCode.OK,
});
api.beacon.publishBlockV2.resolves();
api.beacon.publishBlock.resolves();

// Trigger block production for slot 1
const notifyBlockProductionFn = blockService["dutiesService"]["notifyBlockProductionFn"];
Expand All @@ -76,10 +68,7 @@ describe("BlockDutiesService", function () {
await sleep(20, controller.signal);

// Must have submitted the block received on signBlock()
expect(api.beacon.publishBlockV2.callCount).to.equal(1, "publishBlockV2() must be called once");
expect(api.beacon.publishBlockV2.getCall(0).args).to.deep.equal(
[signedBlock, {broadcastValidation: routes.beacon.BroadcastValidation.none}],
"wrong publishBlockV2() args"
);
expect(api.beacon.publishBlock.callCount).to.equal(1, "publishBlock() must be called once");
expect(api.beacon.publishBlock.getCall(0).args).to.deep.equal([signedBlock], "wrong publishBlock() args");
});
});

0 comments on commit 88a406a

Please sign in to comment.