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

fix: add accept: application/json header to remote signer requests and improve invalid response handling #6587

Merged
merged 23 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e062c15
fix: handle remote signer non "application/json" response content type
cnupy Mar 24, 2024
5f54826
Merge branch 'ChainSafe:unstable' into unstable
cnupy Mar 25, 2024
b02f666
Merge branch 'ChainSafe:unstable' into unstable
cnupy Mar 26, 2024
657caab
Merge branch 'ChainSafe:unstable' into unstable
cnupy Mar 27, 2024
ecdcc6a
restrict logger to minimal set and remove cast
cnupy Mar 27, 2024
383b07b
Fix typo - `minamal`; refactor/inline response handling functions; re…
cnupy Mar 27, 2024
23ca7ba
fix: exception message text
cnupy Mar 27, 2024
8ce75ab
Removed logger in externalSignerClient; Improved handling of response…
cnupy Mar 28, 2024
22f7131
fix: use response statusText as error message if response body is emp…
cnupy Mar 28, 2024
6791ae3
fix: exception message text proper epmpty string check
cnupy Mar 28, 2024
f36052c
fix: exception message text proper epmpty string check
cnupy Mar 28, 2024
6742089
fix: lint errors
cnupy Mar 29, 2024
282cc86
reintroduce handleExternalSignerResponse generic function with better…
cnupy Apr 4, 2024
94580d5
Merge branch 'ChainSafe:unstable' into unstable
cnupy Apr 4, 2024
2b8e722
fix: remove forgotten check in externalSignerUpCheck function
cnupy Apr 4, 2024
2696ea8
Simplify checking response media type
nflaig Apr 4, 2024
12b7bd9
Include json parse error message in thrown Error
nflaig Apr 4, 2024
4364c8b
Remove unnecessary string cast of err body
nflaig Apr 4, 2024
bec5c5b
Fix json parsing
nflaig Apr 4, 2024
c3f1aa3
More robust error handling
nflaig Apr 4, 2024
3100d07
Formatting
nflaig Apr 4, 2024
127909d
Simplify err body message parser
nflaig Apr 4, 2024
facf88c
Revert error handling changes
nflaig Apr 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions packages/cli/src/cmds/validator/signers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {importKeystoreDefinitionsFromExternalDir, readPassphraseOrPrompt} from "

const KEYSTORE_IMPORT_PROGRESS_MS = 10000;

type MinimalLogger = Pick<Logger, LogLevel.info | LogLevel.warn | LogLevel.debug>;

/**
* Options processing hierarchy
* --interopIndexes
Expand Down Expand Up @@ -44,7 +46,7 @@ const KEYSTORE_IMPORT_PROGRESS_MS = 10000;
export async function getSignersFromArgs(
args: IValidatorCliArgs & GlobalArgs,
network: string,
{logger, signal}: {logger: Pick<Logger, LogLevel.info | LogLevel.warn | LogLevel.debug>; signal: AbortSignal}
{logger, signal}: {logger: MinimalLogger; signal: AbortSignal}
): Promise<Signer[]> {
const accountPaths = getAccountPaths(args, network);

Expand Down Expand Up @@ -106,7 +108,7 @@ export async function getSignersFromArgs(

// Remote keys are declared manually or will be fetched from external signer
else if (args["externalSigner.pubkeys"] || args["externalSigner.fetch"]) {
return getRemoteSigners(args);
return getRemoteSigners(args, logger);
}

// Read keys from local account manager
Expand Down Expand Up @@ -155,7 +157,7 @@ export function getSignerPubkeyHex(signer: Signer): string {
}
}

async function getRemoteSigners(args: IValidatorCliArgs & GlobalArgs): Promise<Signer[]> {
async function getRemoteSigners(args: IValidatorCliArgs & GlobalArgs, logger?: MinimalLogger): Promise<Signer[]> {
const externalSignerUrl = args["externalSigner.url"];
if (!externalSignerUrl) {
throw new YargsError(
Expand All @@ -171,7 +173,7 @@ async function getRemoteSigners(args: IValidatorCliArgs & GlobalArgs): Promise<S
throw new YargsError("externalSigner.pubkeys is set to an empty list");
}

const pubkeys = args["externalSigner.pubkeys"] ?? (await externalSignerGetKeys(externalSignerUrl));
const pubkeys = args["externalSigner.pubkeys"] ?? (await externalSignerGetKeys(externalSignerUrl, logger));
assertValidPubkeysHex(pubkeys);

return pubkeys.map((pubkey) => ({type: SignerType.Remote, pubkey, url: externalSignerUrl}));
Expand Down
72 changes: 59 additions & 13 deletions packages/validator/src/util/externalSignerClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {BeaconConfig} from "@lodestar/config";
import {computeEpochAtSlot, blindedOrFullBlockToHeader} from "@lodestar/state-transition";
import {allForks, Epoch, Root, RootHex, Slot, ssz} from "@lodestar/types";
import {PubkeyHex} from "../types.js";
import {Logger, LogLevel} from "@lodestar/utils";

/* eslint-disable @typescript-eslint/naming-convention */

Expand All @@ -25,6 +26,8 @@ export enum SignableMessageType {
BLS_TO_EXECUTION_CHANGE = "BLS_TO_EXECUTION_CHANGE",
}

type MinimalLogger = Pick<Logger, LogLevel.info | LogLevel.warn | LogLevel.debug>;

const AggregationSlotType = new ContainerType({
slot: ssz.Slot,
});
Expand Down Expand Up @@ -104,13 +107,28 @@ type Web3SignerSerializedRequest = {
/**
* Return public keys from the server.
*/
export async function externalSignerGetKeys(externalSignerUrl: string): Promise<string[]> {
export async function externalSignerGetKeys(externalSignerUrl: string, logger?: MinimalLogger): Promise<string[]> {
const res = await fetch(`${externalSignerUrl}/api/v1/eth2/publicKeys`, {
method: "GET",
headers: {"Content-Type": "application/json"},
});

return handlerExternalSignerResponse<string[]>(res);
if (!res.ok) {
const errBody = await res.text();
throw Error(`External signer GetKeys: statusCode=${res.status}, errorBody=${errBody}`);
}

const resBody = await res.text();
const resContentType = res.headers?.get("Content-Type")?.split(";", 1)[0].trim().toLowerCase();

logger?.debug("External signer GetKeys response", {contentType: resContentType, body: resBody});

if(resContentType === "application/json") {
return JSON.parse(resBody) as string[];
}
else {
throw Error(`External signer GetKeys: content type ${resContentType} not supported`);
cnupy marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
Expand All @@ -122,7 +140,8 @@ export async function externalSignerPostSignature(
pubkeyHex: PubkeyHex,
signingRoot: Root,
signingSlot: Slot,
signableMessage: SignableMessage
signableMessage: SignableMessage,
logger?: MinimalLogger
): Promise<string> {
const requestObj = serializerSignableMessagePayload(config, signableMessage) as Web3SignerSerializedRequest;

Expand All @@ -147,30 +166,57 @@ export async function externalSignerPostSignature(
body: JSON.stringify(requestObj),
});

const data = await handlerExternalSignerResponse<{signature: string}>(res);
return data.signature;
if (!res.ok) {
const errBody = await res.text();
throw Error(`External signer PostSignature: statusCode=${res.status}, errorBody=${errBody}`);
}

const resBody = await res.text();
const resContentType = res.headers?.get("Content-Type")?.split(";", 1)[0].trim().toLowerCase();

logger?.debug("External signer PostSignature response", {contentType: resContentType, body: resBody});

if(resContentType === "application/json") {
const data = JSON.parse(resBody) as {signature: string};
return data.signature;
}
else if (resContentType === "text/plain") {
return resBody;
}
else {
throw Error(`External signer PostSignature: content type ${resContentType} not supported`);
}
}

/**
* Return upcheck status from server.
*/
export async function externalSignerUpCheck(remoteUrl: string): Promise<boolean> {
export async function externalSignerUpCheck(remoteUrl: string, logger?: MinimalLogger): Promise<boolean> {
const res = await fetch(`${remoteUrl}/upcheck`, {
method: "GET",
headers: {"Content-Type": "application/json"},
});

const data = await handlerExternalSignerResponse<{status: string}>(res);
return data.status === "OK";
}

async function handlerExternalSignerResponse<T>(res: Response): Promise<T> {
if (!res.ok) {
const errBody = await res.text();
throw Error(`${errBody}`);
throw Error(`External signer UpCheck: statusCode=${res.status}, errorBody=${errBody}`);
}

return JSON.parse(await res.text()) as T;
const resBody = await res.text();
const resContentType = res.headers?.get("Content-Type")?.split(";", 1)[0].trim().toLowerCase();

logger?.debug("External signer UpCheck response", {contentType: resContentType, body: resBody});

if(resContentType === "application/json") {
const data = JSON.parse(resBody) as {status: string};
return data.status === "OK";
}
else if (resContentType === "text/plain") {
return resBody === "OK";
}
else{
throw Error(`External signer UpCheck: content type ${resContentType} not supported`);
}
}

function serializerSignableMessagePayload(config: BeaconConfig, payload: SignableMessage): Record<string, unknown> {
Expand Down