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 13 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
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
141 changes: 127 additions & 14 deletions packages/validator/src/services/validatorStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import {BitArray, fromHexString, toHexString} from "@chainsafe/ssz";
import {routes} from "@lodestar/api";
import {ISlashingProtection} from "../slashingProtection/index.js";
import {PubkeyHex} from "../types.js";
import {externalSignerPostSignature} from "../util/externalSignerClient.js";
import {externalSignerPostSignature, Web3SignerForkInfo, SignableMessage} from "../util/externalSignerClient.js";
import {Metrics} from "../metrics.js";
import {IndicesService} from "./indices.js";
import {DoppelgangerService} from "./doppelgangerService.js";
Expand Down Expand Up @@ -90,7 +90,8 @@ export class ValidatorStore {
private readonly metrics: Metrics | null,
initialSigners: Signer[],
private readonly suggestedFeeRecipient: string,
private readonly gasLimit: number
private readonly gasLimit: number,
private readonly genesisValidatorRoot: Root
) {
for (const signer of initialSigners) {
this.addSigner(signer);
Expand Down Expand Up @@ -195,7 +196,19 @@ export class ValidatorStore {
this.metrics?.slashingProtectionBlockError.inc();
throw e;
}
const signature = await this.getSignature(pubkey, signingRoot);

const signableMessage: SignableMessage = {
singablePayload: {
type: "BLOCK_V2",
data: {
version: this.config.getForkInfo(blindedOrFull.slot).name,
block: blindedOrFull,
},
},
forkInfo: this.getForkInfo(currentSlot),
};

const signature = await this.getSignature(pubkey, signingRoot, signableMessage);

return {message: blindedOrFull, signature} as allForks.FullOrBlindedSignedBeaconBlock;
}
Expand All @@ -205,7 +218,17 @@ export class ValidatorStore {
const randaoDomain = this.config.getDomain(slot, DOMAIN_RANDAO);
const randaoSigningRoot = computeSigningRoot(ssz.Epoch, epoch, randaoDomain);

return await this.getSignature(pubkey, randaoSigningRoot);
const signableMessage: SignableMessage = {
singablePayload: {
type: "RANDAO_REVEAL",
data: {
epoch,
},
},
forkInfo: this.getForkInfo(slot),
};

return await this.getSignature(pubkey, randaoSigningRoot, signableMessage);
}

async signAttestation(
Expand Down Expand Up @@ -239,10 +262,18 @@ export class ValidatorStore {
throw e;
}

const signableMessage: SignableMessage = {
singablePayload: {
type: "ATTESTATION",
data: attestationData,
},
forkInfo: this.getForkInfo(attestationData.slot),
};

return {
aggregationBits: BitArray.fromSingleBit(duty.committeeLength, duty.validatorCommitteeIndex),
data: attestationData,
signature: await this.getSignature(duty.pubkey, signingRoot),
signature: await this.getSignature(duty.pubkey, signingRoot, signableMessage),
};
}

Expand All @@ -262,9 +293,17 @@ export class ValidatorStore {
const domain = this.config.getDomain(duty.slot, DOMAIN_AGGREGATE_AND_PROOF);
const signingRoot = computeSigningRoot(ssz.phase0.AggregateAndProof, aggregateAndProof, domain);

const signableMessage: SignableMessage = {
singablePayload: {
type: "AGGREGATE_AND_PROOF",
data: aggregateAndProof,
},
forkInfo: this.getForkInfo(aggregateAndProof.aggregate.data.slot),
};

return {
message: aggregateAndProof,
signature: await this.getSignature(duty.pubkey, signingRoot),
signature: await this.getSignature(duty.pubkey, signingRoot, signableMessage),
};
}

Expand All @@ -277,11 +316,22 @@ export class ValidatorStore {
const domain = this.config.getDomain(slot, DOMAIN_SYNC_COMMITTEE);
const signingRoot = computeSigningRoot(ssz.Root, beaconBlockRoot, domain);

const signableMessage: SignableMessage = {
singablePayload: {
type: "SYNC_COMMITTEE_MESSAGE",
data: {
beaconBlockRoot,
slot,
},
},
forkInfo: this.getForkInfo(slot),
};

return {
slot,
validatorIndex,
beaconBlockRoot,
signature: await this.getSignature(pubkey, signingRoot),
signature: await this.getSignature(pubkey, signingRoot, signableMessage),
};
}

Expand All @@ -299,17 +349,35 @@ export class ValidatorStore {
const domain = this.config.getDomain(contribution.slot, DOMAIN_CONTRIBUTION_AND_PROOF);
const signingRoot = computeSigningRoot(ssz.altair.ContributionAndProof, contributionAndProof, domain);

const signableMessage: SignableMessage = {
singablePayload: {
type: "SYNC_COMMITTEE_CONTRIBUTION_AND_PROOF",
data: contributionAndProof,
},
forkInfo: this.getForkInfo(contributionAndProof.contribution.slot),
};

return {
message: contributionAndProof,
signature: await this.getSignature(duty.pubkey, signingRoot),
signature: await this.getSignature(duty.pubkey, signingRoot, signableMessage),
};
}

async signAttestationSelectionProof(pubkey: BLSPubkeyMaybeHex, slot: Slot): Promise<BLSSignature> {
const domain = this.config.getDomain(slot, DOMAIN_SELECTION_PROOF);
const signingRoot = computeSigningRoot(ssz.Slot, slot, domain);

return await this.getSignature(pubkey, signingRoot);
const signableMessage: SignableMessage = {
Copy link
Contributor

Choose a reason for hiding this comment

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

If the web3signer is not enabled, all this objects are being created for nothing right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to only create the Singable messages when web3signer is enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dapplion Your recent changes now makes it such that the objects are being created even when web3signer is not enabled. Is that intentional/okay now?

singablePayload: {
type: "AGGREGATION_SLOT",
data: {
slot: String(slot),
},
},
forkInfo: this.getForkInfo(slot),
};

return await this.getSignature(pubkey, signingRoot, signableMessage);
}

async signSyncCommitteeSelectionProof(
Expand All @@ -325,7 +393,18 @@ export class ValidatorStore {

const signingRoot = computeSigningRoot(ssz.altair.SyncAggregatorSelectionData, signingData, domain);

return await this.getSignature(pubkey, signingRoot);
const singableRequest: SignableMessage = {
singablePayload: {
type: "SYNC_COMMITTEE_SELECTION_PROOF",
data: {
slot,
subcommitteeIndex: String(subcommitteeIndex),
},
},
forkInfo: this.getForkInfo(slot),
};

return await this.getSignature(pubkey, signingRoot, singableRequest);
}

async signVoluntaryExit(
Expand All @@ -338,9 +417,16 @@ export class ValidatorStore {
const voluntaryExit: phase0.VoluntaryExit = {epoch: exitEpoch, validatorIndex};
const signingRoot = computeSigningRoot(ssz.phase0.VoluntaryExit, voluntaryExit, domain);

const signableMessage: SignableMessage = {
singablePayload: {
type: "VOLUNTARY_EXIT",
data: voluntaryExit,
},
};

return {
message: voluntaryExit,
signature: await this.getSignature(pubkey, signingRoot),
signature: await this.getSignature(pubkey, signingRoot, signableMessage),
};
}

Expand All @@ -364,13 +450,23 @@ export class ValidatorStore {
};
const domain = computeDomain(DOMAIN_APPLICATION_BUILDER, this.config.GENESIS_FORK_VERSION, ZERO_HASH);
const signingRoot = computeSigningRoot(ssz.bellatrix.ValidatorRegistrationV1, validatorRegistation, domain);
const signableMessage: SignableMessage = {
singablePayload: {
type: "VALIDATOR_REGISTRATION",
data: validatorRegistation,
},
};
return {
message: validatorRegistation,
signature: await this.getSignature(pubkey, signingRoot),
signature: await this.getSignature(pubkey, signingRoot, signableMessage),
};
}

private async getSignature(pubkey: BLSPubkeyMaybeHex, signingRoot: Uint8Array): Promise<BLSSignature> {
private async getSignature(
pubkey: BLSPubkeyMaybeHex,
signingRoot: Uint8Array,
signableMessage: SignableMessage
): Promise<BLSSignature> {
// TODO: Refactor indexing to not have to run toHexString() on the pubkey every time
const pubkeyHex = typeof pubkey === "string" ? pubkey : toHexString(pubkey);

Expand All @@ -390,7 +486,12 @@ export class ValidatorStore {
case SignerType.Remote: {
const timer = this.metrics?.remoteSignTime.startTimer();
try {
const signatureHex = await externalSignerPostSignature(signer.url, pubkeyHex, toHexString(signingRoot));
const signatureHex = await externalSignerPostSignature(
signer.url,
pubkeyHex,
toHexString(signingRoot),
signableMessage
);
return fromHexString(signatureHex);
} catch (e) {
this.metrics?.remoteSignErrors.inc();
Expand Down Expand Up @@ -420,6 +521,18 @@ export class ValidatorStore {
throw new Error(`Doppelganger state for key ${pubkeyHex} is not safe`);
}
}

private getForkInfo(slot: Slot): Web3SignerForkInfo {
const forkInfo = this.config.getForkInfo(slot);
return {
fork: {
previousVersion: forkInfo.prevVersion,
currentVersion: forkInfo.version,
epoch: forkInfo.epoch,
},
genesisValidatorRoot: this.genesisValidatorRoot,
};
}
}

function getSignerPubkeyHex(signer: Signer): PubkeyHex {
Expand Down