Skip to content

Commit

Permalink
Add ability to update the fee recipient for execution via beacon and/…
Browse files Browse the repository at this point in the history
…or validator defaults (#3958)

* Add and use a default fee recipient for a validator process

* transfer the proposer cache to beacon chain

* mock chain fixes

* test and perf fixes

* fee recipient validation change

* track and use free recipient as string instead of ExecutionAddress

* fix unit test

* fix merge test

* use dummy address

* refac and add proposer cache pruning

* tests for beacon proposer cache

* merge interop fee recipient check

* fix the optional

* feeRecipient confirmation and small refac

* add the missing map

* add flag to enable strict fee recipient check
  • Loading branch information
g11tech authored May 24, 2022
1 parent 70bbc5d commit d4d72fe
Show file tree
Hide file tree
Showing 34 changed files with 383 additions and 42 deletions.
21 changes: 21 additions & 0 deletions packages/api/src/routes/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
WithVersion,
reqOnlyBody,
ReqSerializers,
jsonType,
} from "../utils";

// See /packages/api/src/routes/index.ts for reasoning and instructions to add new routes
Expand All @@ -44,6 +45,15 @@ export type SyncCommitteeSubscription = {
untilEpoch: Epoch;
};

/**
* The types used here are string instead of ssz based because the use of proposer data
* is just validator --> beacon json api call for `beaconProposerCache` cache update.
*/
export type ProposerPreparationData = {
validatorIndex: string;
feeRecipient: string;
};

export type ProposerDuty = {
slot: Slot;
validatorIndex: ValidatorIndex;
Expand Down Expand Up @@ -197,6 +207,8 @@ export type Api = {
prepareBeaconCommitteeSubnet(subscriptions: BeaconCommitteeSubscription[]): Promise<void>;

prepareSyncCommitteeSubnets(subscriptions: SyncCommitteeSubscription[]): Promise<void>;

prepareBeaconProposer(proposers: ProposerPreparationData[]): Promise<void>;
};

/**
Expand All @@ -215,6 +227,7 @@ export const routesData: RoutesData<Api> = {
publishContributionAndProofs: {url: "/eth/v1/validator/contribution_and_proofs", method: "POST"},
prepareBeaconCommitteeSubnet: {url: "/eth/v1/validator/beacon_committee_subscriptions", method: "POST"},
prepareSyncCommitteeSubnets: {url: "/eth/v1/validator/sync_committee_subscriptions", method: "POST"},
prepareBeaconProposer: {url: "/eth/v1/validator/prepare_beacon_proposer", method: "POST"},
};

/* eslint-disable @typescript-eslint/naming-convention */
Expand All @@ -231,6 +244,7 @@ export type ReqTypes = {
publishContributionAndProofs: {body: unknown};
prepareBeaconCommitteeSubnet: {body: unknown};
prepareSyncCommitteeSubnets: {body: unknown};
prepareBeaconProposer: {body: unknown};
};

export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
Expand Down Expand Up @@ -330,6 +344,13 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
publishContributionAndProofs: reqOnlyBody(ArrayOf(ssz.altair.SignedContributionAndProof), Schema.ObjectArray),
prepareBeaconCommitteeSubnet: reqOnlyBody(ArrayOf(BeaconCommitteeSubscription), Schema.ObjectArray),
prepareSyncCommitteeSubnets: reqOnlyBody(ArrayOf(SyncCommitteeSubscription), Schema.ObjectArray),
prepareBeaconProposer: {
writeReq: (items: ProposerPreparationData[]) => ({body: items.map((item) => jsonType("snake").toJson(item))}),
parseReq: ({body}) => [
(body as Record<string, unknown>[]).map((item) => jsonType("snake").fromJson(item) as ProposerPreparationData),
],
schema: {body: Schema.ObjectArray},
},
};
}

Expand Down
4 changes: 4 additions & 0 deletions packages/api/test/unit/validator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ describe("validator", () => {
args: [[{validatorIndex: 1, syncCommitteeIndices: [2], untilEpoch: 3}]],
res: undefined,
},
prepareBeaconProposer: {
args: [[{validatorIndex: "1", feeRecipient: "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b"}]],
res: undefined,
},
});

// TODO: Extra tests to implement maybe
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/cmds/validator/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import {RegistryMetricCreator, collectNodeJSMetrics, HttpMetricsServer} from "@c
import {getBeaconConfigFromArgs} from "../../config";
import {IGlobalArgs} from "../../options";
import {YargsError, getDefaultGraffiti, initBLS, mkdir, getCliLogger} from "../../util";
import {onGracefulShutdown} from "../../util";
import {onGracefulShutdown, parseFeeRecipient} from "../../util";
import {getVersionData} from "../../util/version";
import {getBeaconPaths} from "../beacon/paths";
import {getValidatorPaths} from "./paths";
import {IValidatorCliArgs, validatorMetricsDefaultOptions} from "./options";
import {IValidatorCliArgs, validatorMetricsDefaultOptions, defaultDefaultFeeRecipient} from "./options";
import {getLocalSecretKeys, getExternalSigners, groupExternalSignersByUrl} from "./keys";

/**
Expand All @@ -21,6 +21,7 @@ export async function validatorHandler(args: IValidatorCliArgs & IGlobalArgs): P
await initBLS();

const graffiti = args.graffiti || getDefaultGraffiti();
const defaultFeeRecipient = parseFeeRecipient(args.defaultFeeRecipient ?? defaultDefaultFeeRecipient);

const validatorPaths = getValidatorPaths(args);
const beaconPaths = getBeaconPaths(args);
Expand Down Expand Up @@ -129,6 +130,7 @@ export async function validatorHandler(args: IValidatorCliArgs & IGlobalArgs): P
signers,
graffiti,
afterBlockDelaySlotFraction: args.afterBlockDelaySlotFraction,
defaultFeeRecipient,
},
controller.signal,
metrics
Expand Down
18 changes: 18 additions & 0 deletions packages/cli/src/cmds/validator/options.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {defaultOptions} from "@chainsafe/lodestar";
import {ICliCommandOptions, ILogArgs} from "../../util";
import {defaultValidatorPaths} from "./paths";
import {accountValidatorOptions, IAccountValidatorArgs} from "../account/cmds/validator/options";
Expand All @@ -11,6 +12,8 @@ export const validatorMetricsDefaultOptions = {
address: "127.0.0.1",
};

export const defaultDefaultFeeRecipient = defaultOptions.chain.defaultFeeRecipient;

export type IValidatorCliArgs = IAccountValidatorArgs &
ILogArgs & {
logFile: IBeaconPaths["logFile"];
Expand All @@ -19,6 +22,9 @@ export type IValidatorCliArgs = IAccountValidatorArgs &
force: boolean;
graffiti: string;
afterBlockDelaySlotFraction?: number;
defaultFeeRecipient?: string;
strictFeeRecipientCheck?: boolean;

importKeystoresPath?: string[];
importKeystoresPassword?: string;
externalSignerUrl?: string;
Expand Down Expand Up @@ -68,6 +74,18 @@ export const validatorOptions: ICliCommandOptions<IValidatorCliArgs> = {
type: "number",
},

defaultFeeRecipient: {
description:
"Specify fee recipient default for collecting the EL block fees and rewards (a hex string representing 20 bytes address: ^0x[a-fA-F0-9]{40}$). It would be possible (WIP) to override this per validator key using config or keymanager API.",
defaultDescription: defaultDefaultFeeRecipient,
type: "string",
},

strictFeeRecipientCheck: {
description: "Enable strict checking of the validator's feeRecipient with the one returned by engine",
type: "boolean",
},

importKeystoresPath: {
description: "Path(s) to a directory or single filepath to validator keystores, i.e. Launchpad validators",
defaultDescription: "./keystores/*.json",
Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/options/beaconNodeOptions/chain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export interface IChainArgs {
"chain.disableBlsBatchVerify": boolean;
"chain.persistInvalidSszObjects": boolean;
"chain.proposerBoostEnabled": boolean;
"chain.defaultFeeRecipient": string;
"safe-slots-to-import-optimistically": number;
// this is defined as part of IBeaconPaths
// "chain.persistInvalidSszObjectsDir": string;
Expand All @@ -21,6 +22,7 @@ export function parseArgs(args: IChainArgs): IBeaconNodeOptions["chain"] {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any
persistInvalidSszObjectsDir: undefined as any,
proposerBoostEnabled: args["chain.proposerBoostEnabled"],
defaultFeeRecipient: args["chain.defaultFeeRecipient"],
safeSlotsToImportOptimistically: args["safe-slots-to-import-optimistically"],
};
}
Expand Down Expand Up @@ -66,6 +68,14 @@ Will double processing times. Use only for debugging purposes.",
group: "chain",
},

"chain.defaultFeeRecipient": {
description:
"Specify fee recipient default for collecting the EL block fees and rewards (a hex string representing 20 bytes address: ^0x[a-fA-F0-9]{40}$) in case validator fails to update for a validator index before calling produceBlock.",
defaultDescription: defaultOptions.chain.defaultFeeRecipient,
type: "string",
group: "chain",
},

"safe-slots-to-import-optimistically": {
hidden: true,
type: "number",
Expand Down
4 changes: 4 additions & 0 deletions packages/cli/src/options/beaconNodeOptions/execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ export function parseArgs(args: ExecutionEngineArgs): IBeaconNodeOptions["execut
return {
urls: args["execution.urls"],
timeout: args["execution.timeout"],
/**
* jwtSecret is parsed as hex instead of bytes because the merge with defaults
* in beaconOptions messes up the bytes array as as index => value object
*/
jwtSecretHex: args["jwt-secret"]
? extractJwtHexSecret(fs.readFileSync(args["jwt-secret"], "utf-8").trim())
: undefined,
Expand Down
6 changes: 6 additions & 0 deletions packages/cli/src/util/feeRecipient.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export function parseFeeRecipient(feeRecipientHex: string): string {
if (!/^0x[a-fA-F0-9]{40}$/i.test(feeRecipientHex)) {
throw Error(`Invalid feeRecipient= ${feeRecipientHex}, expected format: ^0x[a-fA-F0-9]{40}$`);
}
return feeRecipientHex.toLowerCase();
}
1 change: 1 addition & 0 deletions packages/cli/src/util/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ export * from "./stripOffNewlines";
export * from "./types";
export * from "./bls";
export * from "./jwt";
export * from "./feeRecipient";
2 changes: 2 additions & 0 deletions packages/cli/test/unit/options/beaconNodeOptions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe("options / beaconNodeOptions", () => {
"chain.disableBlsBatchVerify": true,
"chain.persistInvalidSszObjects": true,
"chain.proposerBoostEnabled": false,
"chain.defaultFeeRecipient": "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"safe-slots-to-import-optimistically": 256,

"eth1.enabled": true,
Expand Down Expand Up @@ -79,6 +80,7 @@ describe("options / beaconNodeOptions", () => {
persistInvalidSszObjects: true,
proposerBoostEnabled: false,
safeSlotsToImportOptimistically: 256,
defaultFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
},
eth1: {
enabled: true,
Expand Down
28 changes: 28 additions & 0 deletions packages/cli/test/unit/validator/options.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {expect} from "chai";
import {parseFeeRecipient} from "../../../src/util";

const feeRecipient = Buffer.from(Array.from({length: 20}, () => Math.round(Math.random() * 255)));
const feeRecipientString = feeRecipient.toString("hex");

describe("validator / parseFeeRecipient", () => {
const testCases: string[] = [`0x${feeRecipientString}`, `0X${feeRecipientString}`];
for (const testCase of testCases) {
it(`parse ${testCase}`, () => {
expect(`0x${feeRecipientString}`).to.be.deep.equal(parseFeeRecipient(testCase));
});
}
});

describe("validator / invalid feeRecipient", () => {
const testCases: string[] = [
feeRecipientString,
`X0${feeRecipientString}`,
`0x${feeRecipientString}13`,
`0x${feeRecipientString.substr(0, 38)}`,
];
for (const testCase of testCases) {
it(`should error on ${testCase}`, () => {
expect(() => parseFeeRecipient(testCase)).to.throw();
});
}
});
6 changes: 4 additions & 2 deletions packages/lodestar/src/api/impl/validator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,6 @@ export function getValidatorApi({chain, config, logger, metrics, network, sync}:
slot,
randaoReveal,
graffiti: toGraffitiBuffer(graffiti || ""),
// TODO - TEMP
feeRecipient: Buffer.alloc(20, 0),
}
);
metrics?.blockProductionSuccess.inc();
Expand Down Expand Up @@ -584,5 +582,9 @@ export function getValidatorApi({chain, config, logger, metrics, network, sync}:

network.prepareSyncCommitteeSubnets(subs);
},

async prepareBeaconProposer(proposers) {
await chain.updateBeaconProposerData(chain.clock.currentEpoch, proposers);
},
};
}
36 changes: 36 additions & 0 deletions packages/lodestar/src/chain/beaconProposerCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import {MapDef} from "../util/map";
import {Epoch} from "@chainsafe/lodestar-types";
import {IMetrics} from "../metrics";
import {routes} from "@chainsafe/lodestar-api";

const PROPOSER_PRESERVE_EPOCHS = 2;

export type ProposerPreparationData = routes.validator.ProposerPreparationData;

export class BeaconProposerCache {
private readonly feeRecipientByValidatorIndex: MapDef<string, {epoch: Epoch; feeRecipient: string}>;
constructor(opts: {defaultFeeRecipient: string}, private readonly metrics?: IMetrics | null) {
this.feeRecipientByValidatorIndex = new MapDef<string, {epoch: Epoch; feeRecipient: string}>(() => ({
epoch: 0,
feeRecipient: opts.defaultFeeRecipient,
}));
}

add(epoch: Epoch, {validatorIndex, feeRecipient}: ProposerPreparationData): void {
this.feeRecipientByValidatorIndex.set(validatorIndex, {epoch, feeRecipient});
}

prune(epoch: Epoch): void {
// This is not so optimized function, but could maintain a 2d array may be?
for (const [validatorIndex, feeRecipientEntry] of this.feeRecipientByValidatorIndex.entries()) {
// We only retain an entry for PROPOSER_PRESERVE_EPOCHS epochs
if (feeRecipientEntry.epoch + PROPOSER_PRESERVE_EPOCHS < epoch) {
this.feeRecipientByValidatorIndex.delete(validatorIndex);
}
}
}

get(proposerIndex: number | string): string {
return this.feeRecipientByValidatorIndex.getOrDefault(`${proposerIndex}`).feeRecipient;
}
}
15 changes: 13 additions & 2 deletions packages/lodestar/src/chain/chain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
} from "@chainsafe/lodestar-beacon-state-transition";
import {IBeaconConfig} from "@chainsafe/lodestar-config";
import {IForkChoice} from "@chainsafe/lodestar-fork-choice";
import {allForks, UintNum64, Root, phase0, Slot, RootHex} from "@chainsafe/lodestar-types";
import {allForks, UintNum64, Root, phase0, Slot, RootHex, Epoch} from "@chainsafe/lodestar-types";
import {ILogger} from "@chainsafe/lodestar-utils";
import {fromHexString} from "@chainsafe/ssz";
import {AbortController} from "@chainsafe/abort-controller";
Expand All @@ -25,7 +25,7 @@ import {BlockProcessor, PartiallyVerifiedBlockFlags} from "./blocks";
import {IBeaconClock, LocalClock} from "./clock";
import {ChainEventEmitter} from "./emitter";
import {handleChainEvents} from "./eventHandlers";
import {IBeaconChain, SSZObjectType} from "./interface";
import {IBeaconChain, SSZObjectType, ProposerPreparationData} from "./interface";
import {IChainOptions} from "./options";
import {IStateRegenerator, QueuedStateRegenerator, RegenCaller} from "./regen";
import {initializeForkChoice} from "./forkChoice";
Expand All @@ -52,6 +52,7 @@ import {IExecutionEngine} from "../executionEngine";
import {PrecomputeNextEpochTransitionScheduler} from "./precomputeNextEpochTransition";
import {ReprocessController} from "./reprocess";
import {SeenAggregatedAttestations} from "./seenCache/seenAggregateAndProof";
import {BeaconProposerCache} from "./beaconProposerCache";

export class BeaconChain implements IBeaconChain {
readonly genesisTime: UintNum64;
Expand Down Expand Up @@ -91,6 +92,8 @@ export class BeaconChain implements IBeaconChain {
readonly pubkey2index: PubkeyIndexMap;
readonly index2pubkey: Index2PubkeyCache;

readonly beaconProposerCache: BeaconProposerCache;

protected readonly blockProcessor: BlockProcessor;
protected readonly db: IBeaconDb;
protected readonly logger: ILogger;
Expand Down Expand Up @@ -148,6 +151,8 @@ export class BeaconChain implements IBeaconChain {
this.pubkey2index = new PubkeyIndexMap();
this.index2pubkey = [];

this.beaconProposerCache = new BeaconProposerCache(opts);

// Restore state caches
const cachedState = createCachedBeaconState(anchorState, {
config,
Expand Down Expand Up @@ -316,4 +321,10 @@ export class BeaconChain implements IBeaconChain {
}
return fileName;
}

async updateBeaconProposerData(epoch: Epoch, proposers: ProposerPreparationData[]): Promise<void> {
proposers.forEach((proposer) => {
this.beaconProposerCache.add(epoch, proposer);
});
}
}
1 change: 1 addition & 0 deletions packages/lodestar/src/chain/eventHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export function onClockEpoch(this: BeaconChain, currentEpoch: Epoch): void {
this.seenAttesters.prune(currentEpoch);
this.seenAggregators.prune(currentEpoch);
this.seenAggregatedAttestations.prune(currentEpoch);
this.beaconProposerCache.prune(currentEpoch);
}

export function onForkVersion(this: BeaconChain, version: Version): void {
Expand Down
Loading

0 comments on commit d4d72fe

Please sign in to comment.