Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to update the fee recipient for execution via beacon and/or validator defaults #3958

Merged
merged 16 commits into from
May 24, 2022
Merged
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 {
twoeths marked this conversation as resolved.
Show resolved Hide resolved
this.feeRecipientByValidatorIndex.set(validatorIndex, {epoch, feeRecipient});
}

prune(epoch: Epoch): void {
twoeths marked this conversation as resolved.
Show resolved Hide resolved
// 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;
twoeths marked this conversation as resolved.
Show resolved Hide resolved
}
}
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