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
17 changes: 17 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,11 @@ export type SyncCommitteeSubscription = {
untilEpoch: Epoch;
};

export type ProposerPreparationData = {
validatorIndex: string;
feeRecipient: string;
};

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

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

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

/**
Expand All @@ -215,6 +223,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 +240,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 +340,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
12 changes: 12 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,8 @@ export type IValidatorCliArgs = IAccountValidatorArgs &
force: boolean;
graffiti: string;
afterBlockDelaySlotFraction?: number;
defaultFeeRecipient?: string;

importKeystoresPath?: string[];
importKeystoresPassword?: string;
externalSignerUrl?: string;
Expand Down Expand Up @@ -68,6 +73,13 @@ 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",
},

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);
},
};
}
37 changes: 37 additions & 0 deletions packages/lodestar/src/chain/beaconProposerCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {MapDef} from "../util/map";
import {Epoch} from "@chainsafe/lodestar-types";
import {IMetrics} from "../metrics";

const PROPOSER_PRESERVE_EPOCHS = 2;
export type ProposerPreparationData = {
validatorIndex: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is validatorIndex is typed as string? we have our own ValidatorIndex type

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea i was using that earlier, but Lion suggested to avoid to/fro conversion between validator<>beacon for beacon proposer data as this is mostly to populate a lookup index at the BN.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 so maybe just drop a comment here

feeRecipient: string;
};

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