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

feat: synchronize keys between validator client and external signer #6672

Merged
merged 17 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions packages/cli/src/cmds/validator/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,22 @@ export async function validatorHandler(args: IValidatorCliArgs & GlobalArgs): Pr

// Ensure the validator has at least one key
if (signers.length === 0) {
if (args["keymanager"]) {
logger.warn("No local keystores or remote signers found with current args, expecting to be added via keymanager");
if (args["keymanager"] && !args["externalSigner.fetch"]) {
logger.warn("No local keystores or remote keys found with current args, expecting to be added via keymanager");
} else if (!args["keymanager"] && args["externalSigner.fetch"]) {
logger.warn("No remote keys found with current args, expecting to be added to external signer and fetched later");
} else if (args["keymanager"] && args["externalSigner.fetch"]) {
logger.warn(
"No local keystores or remote keys found with current args, expecting to be added via keymanager or fetched from external signer later"
);
} else {
if (args["externalSigner.url"]) {
throw new YargsError(
"No remote keys found with current args, start with --externalSigner.fetch to automatically fetch from external signer"
);
}
throw new YargsError(
"No local keystores and remote signers found with current args, start with --keymanager if intending to add them later (via keymanager)"
"No local keystores and remote keys found with current args, start with --keymanager if intending to add them later (via keymanager)"
);
}
}
Expand Down Expand Up @@ -172,6 +183,11 @@ export async function validatorHandler(args: IValidatorCliArgs & GlobalArgs): Pr
useProduceBlockV3: args.useProduceBlockV3,
broadcastValidation: parseBroadcastValidation(args.broadcastValidation),
blindedLocal: args.blindedLocal,
externalSigner: {
url: args["externalSigner.url"],
fetch: args["externalSigner.fetch"],
fetchInterval: args["externalSigner.fetchInterval"],
},
},
metrics
);
Expand Down
19 changes: 15 additions & 4 deletions packages/cli/src/cmds/validator/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export type IValidatorCliArgs = AccountValidatorArgs &
"externalSigner.url"?: string;
"externalSigner.pubkeys"?: string[];
"externalSigner.fetch"?: boolean;
"externalSigner.fetchInterval"?: number;

distributed?: boolean;

Expand Down Expand Up @@ -303,15 +304,16 @@ export const validatorOptions: CliCommandOptions<IValidatorCliArgs> = {
type: "boolean",
},

// Remote signer
// External signer
Copy link
Member Author

@nflaig nflaig Apr 17, 2024

Choose a reason for hiding this comment

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

I tried to make the wording a bit more consistent within the code, and also few logs. We generally use the term "external signer" to describe an external/(remote) server that handles the signing operations for a specific public key. Whereas the term "remote signer" is also used to describe a remote signer in the local validator store.

nflaig marked this conversation as resolved.
Show resolved Hide resolved

"externalSigner.url": {
Copy link
Member Author

Choose a reason for hiding this comment

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

We could consider supporting multiple external signers in the future, our implementation can easily be adjusted to support this but I think it's better to not include it in this PR. Opened an issue to keep track of it and for further discussion #6679.

description: "URL to connect to an external signing server",
type: "string",
group: "externalSignerUrl",
group: "externalSigner",
},

"externalSigner.pubkeys": {
implies: ["externalSigner.url"],
description:
"List of validator public keys used by an external signer. May also provide a single string of comma-separated public keys",
type: "array",
Expand All @@ -322,15 +324,24 @@ export const validatorOptions: CliCommandOptions<IValidatorCliArgs> = {
.map((item) => item.split(","))
.flat(1)
.map(ensure0xPrefix),
group: "externalSignerUrl",
group: "externalSigner",
},

"externalSigner.fetch": {
implies: ["externalSigner.url"],
conflicts: ["externalSigner.pubkeys"],
description:
"Fetch the list of public keys to validate from an external signer. Cannot be used in combination with `--externalSigner.pubkeys`",
type: "boolean",
group: "externalSignerUrl",
group: "externalSigner",
},

"externalSigner.fetchInterval": {
implies: ["externalSigner.fetch"],
description:
"Interval in milliseconds between fetching the list of public keys from external signer, once per epoch by default",
type: "number",
group: "externalSigner",
},

// Distributed validator
Expand Down
16 changes: 8 additions & 8 deletions packages/cli/src/cmds/validator/signers/logSigners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ export function logSigners(logger: Pick<Logger, LogLevel.info>, signers: Signer[
}
}

for (const {url, pubkeys} of groupExternalSignersByUrl(remoteSigners)) {
logger.info(`External signers on URL: ${toSafePrintableUrl(url)}`);
for (const {url, pubkeys} of groupRemoteSignersByUrl(remoteSigners)) {
logger.info(`Remote signers on URL: ${toSafePrintableUrl(url)}`);
for (const pubkey of pubkeys) {
logger.info(pubkey);
}
Expand All @@ -37,16 +37,16 @@ export function logSigners(logger: Pick<Logger, LogLevel.info>, signers: Signer[
/**
* Only used for logging remote signers grouped by URL
*/
function groupExternalSignersByUrl(externalSigners: SignerRemote[]): {url: string; pubkeys: string[]}[] {
function groupRemoteSignersByUrl(remoteSigners: SignerRemote[]): {url: string; pubkeys: string[]}[] {
const byUrl = new Map<string, {url: string; pubkeys: string[]}>();

for (const externalSigner of externalSigners) {
let x = byUrl.get(externalSigner.url);
for (const remoteSigner of remoteSigners) {
let x = byUrl.get(remoteSigner.url);
if (!x) {
x = {url: externalSigner.url, pubkeys: []};
byUrl.set(externalSigner.url, x);
x = {url: remoteSigner.url, pubkeys: []};
byUrl.set(remoteSigner.url, x);
}
x.pubkeys.push(externalSigner.pubkey);
x.pubkeys.push(remoteSigner.pubkey);
}

return Array.from(byUrl.values());
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/cmds/validator/voluntaryExit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ If no `pubkeys` are provided, it will exit all validators that have been importe
command:
"validator voluntary-exit --network goerli --externalSigner.url http://signer:9000 --externalSigner.fetch --pubkeys 0xF00",
description:
"Perform a voluntary exit for the validator who has a public key 0xF00 and its secret key is on a remote signer",
"Perform a voluntary exit for the validator who has a public key 0xF00 and its secret key is on an external signer",
},
],

Expand Down Expand Up @@ -92,7 +92,7 @@ If no `pubkeys` are provided, it will exit all validators that have been importe
throw new YargsError(`No validators to exit found with current args.
Ensure --dataDir and --network match values used when importing keys via validator import
or alternatively, import keys by providing --importKeystores arg to voluntary-exit command.
If attempting to exit validators on a remote signer, make sure values are provided for
If attempting to exit validators on an external signer, make sure values are provided for
the necessary --externalSigner options.
`);
}
Expand Down
79 changes: 79 additions & 0 deletions packages/validator/src/services/externalSignerSync.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import bls from "@chainsafe/bls";
import {CoordType} from "@chainsafe/bls/types";
import {fromHexString} from "@chainsafe/ssz";
import {ChainForkConfig} from "@lodestar/config";
import {SLOTS_PER_EPOCH} from "@lodestar/params";
import {toSafePrintableUrl} from "@lodestar/utils";

import {LoggerVc} from "../util/index.js";
import {externalSignerGetKeys} from "../util/externalSignerClient.js";
import {ValidatorOptions} from "../validator.js";
import {SignerType, ValidatorStore} from "./validatorStore.js";

/**
* This service is responsible for keeping the keys managed by the connected
* external signer and the validator client in sync by adding newly discovered keys
* and removing no longer present keys on external signer from the validator store.
*/
export function pollExternalSignerPubkeys(
config: ChainForkConfig,
logger: LoggerVc,
signal: AbortSignal,
validatorStore: ValidatorStore,
opts: ValidatorOptions["externalSigner"]
): void {
const externalSigner = opts ?? {};

if (!externalSigner.url || !externalSigner.fetch) {
return; // Disabled
}

async function fetchExternalSignerPubkeys(): Promise<void> {
// External signer URL is already validated earlier
const externalSignerUrl = externalSigner.url as string;
const printableUrl = toSafePrintableUrl(externalSignerUrl);

try {
logger.debug("Fetching public keys from external signer", {url: printableUrl});
const externalPubkeys = await externalSignerGetKeys(externalSignerUrl);
assertValidPubkeysHex(externalPubkeys);
logger.debug("Received public keys from external signer", {url: printableUrl, count: externalPubkeys.length});

const localPubkeys = validatorStore.getRemoteSignerPubkeys(externalSignerUrl);
logger.debug("Local public keys stored for external signer", {url: printableUrl, count: localPubkeys.length});

const localPubkeysSet = new Set(localPubkeys);
for (const pubkey of externalPubkeys) {
if (!localPubkeysSet.has(pubkey)) {
await validatorStore.addSigner({type: SignerType.Remote, pubkey, url: externalSignerUrl});
logger.info("Added remote signer for newly discovered public key", {url: printableUrl, pubkey});
}
}

const externalPubkeysSet = new Set(externalPubkeys);
for (const pubkey of localPubkeys) {
if (!externalPubkeysSet.has(pubkey)) {
validatorStore.removeSigner(pubkey);
logger.info("Removed remote signer for no longer present public key", {url: printableUrl, pubkey});
}
}
} catch (e) {
logger.error("Failed to fetch public keys from external signer", {url: printableUrl}, e as Error);
}
}

const interval = setInterval(
fetchExternalSignerPubkeys,
externalSigner?.fetchInterval ??
// Once per epoch by default
SLOTS_PER_EPOCH * config.SECONDS_PER_SLOT * 1000
);
signal.addEventListener("abort", () => clearInterval(interval), {once: true});
}

function assertValidPubkeysHex(pubkeysHex: string[]): void {
for (const pubkeyHex of pubkeysHex) {
const pubkeyBytes = fromHexString(pubkeyHex);
bls.PublicKey.fromBytes(pubkeyBytes, CoordType.jacobian, true);
}
}
10 changes: 10 additions & 0 deletions packages/validator/src/services/validatorStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,16 @@ export class ValidatorStore {
return this.validators.has(pubkeyHex);
}

getRemoteSignerPubkeys(signerUrl: string): PubkeyHex[] {
const pubkeysHex = [];
for (const {signer} of this.validators.values()) {
if (signer.type === SignerType.Remote && signer.url === signerUrl) {
pubkeysHex.push(signer.pubkey);
}
}
return pubkeysHex;
}

async signBlock(
pubkey: BLSPubkey,
blindedOrFull: allForks.FullOrBlindedBeaconBlock,
Expand Down
7 changes: 7 additions & 0 deletions packages/validator/src/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {AttestationService} from "./services/attestation.js";
import {IndicesService} from "./services/indices.js";
import {SyncCommitteeService} from "./services/syncCommittee.js";
import {pollPrepareBeaconProposer, pollBuilderValidatorRegistration} from "./services/prepareBeaconProposer.js";
import {pollExternalSignerPubkeys} from "./services/externalSignerSync.js";
import {Interchange, InterchangeFormatVersion, ISlashingProtection} from "./slashingProtection/index.js";
import {assertEqualParams, getLoggerVc, NotEqualParamsError} from "./util/index.js";
import {ChainHeaderTracker} from "./services/chainHeaderTracker.js";
Expand Down Expand Up @@ -59,6 +60,11 @@ export type ValidatorOptions = {
useProduceBlockV3?: boolean;
broadcastValidation?: routes.beacon.BroadcastValidation;
blindedLocal?: boolean;
externalSigner?: {
url?: string;
fetch?: boolean;
fetchInterval?: number;
};
};

// TODO: Extend the timeout, and let it be customizable
Expand Down Expand Up @@ -200,6 +206,7 @@ export class Validator {
);
pollPrepareBeaconProposer(config, loggerVc, api, clock, validatorStore, metrics);
pollBuilderValidatorRegistration(config, loggerVc, api, clock, validatorStore, metrics);
pollExternalSignerPubkeys(config, loggerVc, controller.signal, validatorStore, opts.externalSigner);

const emitter = new ValidatorEventEmitter();
// Validator event emitter can have more than 10 listeners in a normal course of operation
Expand Down
Loading
Loading