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 remote signing #4502

Merged
merged 41 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
8b25b21
Merge branch 'unstable' into dadepo/fix-remote-signer-endpoint
dadepo Aug 25, 2022
00fe74f
updated endpoint to retrieve pub keys
dadepo Aug 25, 2022
05a9aa1
exposing previous version in fork info
dadepo Aug 25, 2022
35ad349
passed in genesisValidatorsRoot to ValidatorStore
dadepo Aug 26, 2022
930f46a
Make signable payload available for external signer
dadepo Aug 28, 2022
ced0343
renamed signableRequest to signableMessage
dadepo Aug 29, 2022
d6f1bef
AGGREGATION_SLOT is now signable by the remote signer
dadepo Aug 29, 2022
f532fc6
tests fixed
dadepo Aug 29, 2022
fc13236
add a way to serialize payload that have ssz values
dadepo Aug 30, 2022
36ee913
Merge branch 'unstable' into dadepo/fix-remote-signer-endpoint
dadepo Aug 31, 2022
5d9e3a4
Merge branch 'unstable' into dadepo/fix-remote-signer-endpoint
dadepo Sep 1, 2022
1175260
some formatting
dadepo Sep 1, 2022
dfa382d
merged in unstable and resolved conflicts
dadepo Sep 2, 2022
1275f86
only construct signable message if remote signer is used
dadepo Sep 5, 2022
29e8bd8
merged in unstable; resolved conflicts
dadepo Sep 5, 2022
e03298b
Merge branch 'unstable' into dadepo/fix-remote-signer-endpoint
dadepo Sep 9, 2022
e92e251
enable remote signer also for dev runs
dadepo Sep 9, 2022
beeca38
process request object for BLOCK_V2 type
dadepo Sep 10, 2022
2f77cf9
Blocks are now being signed by the remote signer too
dadepo Sep 12, 2022
a304885
Add base for web3signer signature test
dapplion Sep 13, 2022
f0768b3
Ensure process is killed
dapplion Sep 13, 2022
e344acf
Use testcontainers for managing containers in tests
dadepo Sep 13, 2022
bf6c7eb
fix lint errors
dadepo Sep 13, 2022
2e4aada
Added bellatrix
dadepo Sep 13, 2022
35a8e56
Add test for signAttestation
dadepo Sep 13, 2022
fb2af6c
Add test for signVoluntaryExit
dadepo Sep 13, 2022
70bbad8
Added test for signSyncCommitteeSelectionProof
dadepo Sep 13, 2022
c3ac872
merged in unstable, resolve conflicts
dadepo Sep 13, 2022
4a3f045
typos fixes
dadepo Sep 13, 2022
33560f8
using assertSameSignature for all tests
dadepo Sep 13, 2022
586a6d4
Added test for signAttestationSelectionProof
dadepo Sep 13, 2022
87c944e
add test for signBlock
dadepo Sep 13, 2022
2d1bd21
added tests for signSyncCommitteeSignature
dadepo Sep 13, 2022
bd18f82
added test for signContributionAndProof and signAggregateAndProof
dadepo Sep 13, 2022
b0d80f4
Remove remote signing tests as it does not test a real remote signer
dadepo Sep 13, 2022
6581fce
Review PR
dapplion Sep 14, 2022
f737bec
setting timeout might not be necessary. Remove it. Readd, if needed
dadepo Sep 14, 2022
82b535f
Fix web3signer paths
dapplion Sep 14, 2022
006a7c0
Remove permissions from tmp dir
dapplion Sep 14, 2022
1923153
Test prevVersion
dapplion Sep 14, 2022
4376721
Set 755
dapplion Sep 14, 2022
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
3 changes: 3 additions & 0 deletions packages/beacon-node/test/unit/network/fork.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,23 @@ function getForkConfig({
seq: ForkSeq.phase0,
epoch: phase0,
version: Buffer.from([0, 0, 0, 0]),
prevVersion: Buffer.from([0, 0, 0, 0]),
prevForkName: ForkName.phase0,
},
altair: {
name: ForkName.altair,
seq: ForkSeq.altair,
epoch: altair,
version: Buffer.from([0, 0, 0, 1]),
prevVersion: Buffer.from([0, 0, 0, 0]),
prevForkName: ForkName.phase0,
},
bellatrix: {
name: ForkName.bellatrix,
seq: ForkSeq.bellatrix,
epoch: bellatrix,
version: Buffer.from([0, 0, 0, 2]),
prevVersion: Buffer.from([0, 0, 0, 1]),
prevForkName: ForkName.altair,
},
};
Expand Down
41 changes: 25 additions & 16 deletions packages/cli/src/cmds/validator/signers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ export async function getSignersFromArgs(args: IValidatorCliArgs & IGlobalArgs,
// ONLY USE FOR TESTNETS - Derive interop keys
if (args.interopIndexes) {
const indexes = parseRange(args.interopIndexes);
return indexes.map((index) => ({type: SignerType.Local, secretKey: interopSecretKey(index)}));
// Using a remote signer with TESTNETS
if (args["externalSigner.pubkeys"] || args["externalSigner.fetch"]) {
return getRemoteSigners(args);
} else {
return indexes.map((index) => ({type: SignerType.Local, secretKey: interopSecretKey(index)}));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of this? I don't follow

Copy link
Contributor

Choose a reason for hiding this comment

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

To consume the interop indexes from the web3signer you should load the interop keystores in the web3signer and then use .fetch

}

// UNSAFE, ONLY USE FOR TESTNETS - Derive keys directly from a mnemonic
Expand Down Expand Up @@ -71,21 +76,7 @@ export async function getSignersFromArgs(args: IValidatorCliArgs & IGlobalArgs,

// Remote keys declared manually with --externalSignerPublicKeys
else if (args["externalSigner.pubkeys"] || args["externalSigner.fetch"]) {
const externalSignerUrl = args["externalSigner.url"];
if (!externalSignerUrl) {
throw new YargsError("Must set externalSignerUrl with externalSignerPublicKeys");
}
if (!isValidHttpUrl(externalSignerUrl)) {
throw new YargsError(`Invalid external signer URL ${externalSignerUrl}`);
}
if (args["externalSigner.pubkeys"] && args["externalSigner.pubkeys"].length === 0) {
throw new YargsError("externalSignerPublicKeys is set to an empty list");
}

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

return pubkeys.map((pubkey) => ({type: SignerType.Remote, pubkey, url: externalSignerUrl}));
return getRemoteSigners(args);
}

// Read keys from local account manager
Expand Down Expand Up @@ -114,3 +105,21 @@ export function getSignerPubkeyHex(signer: Signer): string {
return signer.pubkey;
}
}

async function getRemoteSigners(args: IValidatorCliArgs & IGlobalArgs): Promise<Signer[]> {
const externalSignerUrl = args["externalSigner.url"];
if (!externalSignerUrl) {
throw new YargsError("Must set externalSignerUrl with externalSignerPublicKeys");
}
if (!isValidHttpUrl(externalSignerUrl)) {
throw new YargsError(`Invalid external signer URL ${externalSignerUrl}`);
}
if (args["externalSigner.pubkeys"] && args["externalSigner.pubkeys"].length === 0) {
throw new YargsError("externalSignerPublicKeys is set to an empty list");
}

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

return pubkeys.map((pubkey) => ({type: SignerType.Remote, pubkey, url: externalSignerUrl}));
}
3 changes: 3 additions & 0 deletions packages/config/src/forkConfig/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,23 @@ export function createIForkConfig(config: IChainConfig): IForkConfig {
epoch: GENESIS_EPOCH,
version: config.GENESIS_FORK_VERSION,
// Will never be used
prevVersion: config.GENESIS_FORK_VERSION,
prevForkName: ForkName.phase0,
};
const altair = {
name: ForkName.altair,
seq: ForkSeq.altair,
epoch: config.ALTAIR_FORK_EPOCH,
version: config.ALTAIR_FORK_VERSION,
prevVersion: phase0.version,
prevForkName: ForkName.phase0,
};
const bellatrix = {
name: ForkName.bellatrix,
seq: ForkSeq.bellatrix,
epoch: config.BELLATRIX_FORK_EPOCH,
version: config.BELLATRIX_FORK_VERSION,
prevVersion: altair.prevVersion,
prevForkName: ForkName.altair,
};

Expand Down
1 change: 1 addition & 0 deletions packages/config/src/forkConfig/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export interface IForkInfo {
seq: ForkSeq;
epoch: Epoch;
version: Version;
prevVersion: Version;
prevForkName: ForkName;
}

Expand Down
2 changes: 2 additions & 0 deletions packages/config/src/genesisConfig/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export function createICachedGenesis(chainForkConfig: IChainForkConfig, genesisV
}

return {
genesisValidatorsRoot,

getDomain(stateSlot: Slot, domainType: DomainType, messageSlot?: Slot): Uint8Array {
// ```py
// def get_domain(state: BeaconState, domain_type: DomainType, epoch: Epoch=None) -> Domain:
Expand Down
4 changes: 3 additions & 1 deletion packages/config/src/genesisConfig/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {ForkName} from "@lodestar/params";
import {DomainType, ForkDigest, Slot} from "@lodestar/types";
import {DomainType, ForkDigest, Root, Slot} from "@lodestar/types";

export type ForkDigestHex = string;

Expand All @@ -17,4 +17,6 @@ export interface ICachedGenesis extends IForkDigestContext {
* Note: The configured fork schedule is always used rather than on-chain fork schedule.
*/
getDomain(stateSlot: Slot, domainType: DomainType, messageSlot?: Slot): Uint8Array;

readonly genesisValidatorsRoot: Root;
}
7 changes: 5 additions & 2 deletions packages/validator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
"pretest": "yarn run check-types",
"test:unit": "nyc --cache-dir .nyc_output/.cache -e .ts mocha 'test/unit/**/*.test.ts'",
"test": "yarn test:unit",
"test:e2e": "yarn run download-spec-tests && mocha 'test/spec/**/*.test.ts'",
"test:e2e:only": "mocha 'test/e2e/**/*.test.ts'",
"test:spec": "mocha 'test/spec/**/*.test.ts'",
"test:e2e": "yarn run download-spec-tests && yarn test:spec && yarn test:e2e:only",
"download-spec-tests": "node --loader=ts-node/esm test/spec/downloadTests.ts",
"coverage": "codecov -F lodestar-validator",
"check-readme": "typescript-docs-verifier"
Expand Down Expand Up @@ -61,6 +63,7 @@
"strict-event-emitter-types": "^2.0.0"
},
"devDependencies": {
"bigint-buffer": "^1.1.5"
"bigint-buffer": "^1.1.5",
"testcontainers": "^8.13.2"
}
}