Skip to content

Commit

Permalink
fix: review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
guru-web3 committed Feb 19, 2024
1 parent 50d9fb3 commit 3d24247
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 59 deletions.
9 changes: 1 addition & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions packages/common-types/src/baseTypes/aggregateTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export interface IMetadata extends ISerializable {

nonce: number;

chainCode?: string;
accountSalt?: string;

getShareIndexesForPolynomial(polyID: PolynomialID): string[];
getLatestPublicPolynomial(): PublicPolynomial;
Expand All @@ -91,7 +91,7 @@ export interface IMetadata extends ISerializable {
factorEncs?: {
[factorPubID: string]: FactorEnc;
};
chainCode?: string;
accountSalt?: string;
}): void;
addPublicShare(polynomialID: PolynomialID, publicShare: PublicShare): void;
setGeneralStoreDomain(key: string, obj: unknown): void;
Expand Down
4 changes: 4 additions & 0 deletions packages/common-types/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,7 @@ export function generateID(): string {
// after the decimal.
return `${Math.random().toString(36).substr(2, 9)}`;
}

export function generateSalt() {
return generatePrivate().toString("hex").padStart(64, "0");
}
1 change: 0 additions & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
"@toruslabs/rss-client": "^1.5.0",
"@toruslabs/torus.js": "^11.0.6",
"bn.js": "^5.2.1",
"crypto": "^1.0.1",
"elliptic": "^6.5.4",
"json-stable-stringify": "^1.0.2"
},
Expand Down
54 changes: 24 additions & 30 deletions packages/core/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
FromJSONConstructor,
GenerateNewShareResult,
generatePrivateExcludingIndexes,
generateSalt,
getPubKeyECC,
getPubKeyPoint,
hexPoint,
Expand Down Expand Up @@ -226,19 +227,6 @@ class ThresholdKey implements ITKey {
throw CoreError.metadataUndefined();
}

computeNonce(index: number) {
// generation should occur during tkey.init, fails if chaincode is absent
const { chainCode } = this.metadata;
if (!chainCode) {
throw CoreError.default("chainCode is absent, required for nonce generation");
}
return index && index > 0 ? new BN(keccak256(Buffer.from(`${index}${chainCode}`)).slice(2), "hex").umod(ecCurve.curve.n) : new BN(0);
}

generateSalt() {
return generatePrivate().toString("hex");
}

async initialize(params?: {
withShare?: ShareStore;
importKey?: BN;
Expand Down Expand Up @@ -313,8 +301,8 @@ class ThresholdKey implements ITKey {
});
if (useTSS) {
const { factorEncs, factorPubs, tssPolyCommits } = await this._initializeNewTSSKey(this.tssTag, deviceTSSShare, factorPub, deviceTSSIndex);
const chainCode = this.generateSalt();
this.metadata.addTSSData({ tssTag: this.tssTag, tssNonce: 0, tssPolyCommits, factorPubs, factorEncs, chainCode });
const accountSalt = generateSalt();
this.metadata.addTSSData({ tssTag: this.tssTag, tssNonce: 0, tssPolyCommits, factorPubs, factorEncs, accountSalt });
}
return this.getKeyDetails();
}
Expand Down Expand Up @@ -432,7 +420,7 @@ class ThresholdKey implements ITKey {
}
if (tssSharePub.getX().cmp(_tssSharePub.getX()) === 0 && tssSharePub.getY().cmp(_tssSharePub.getY()) === 0) {
if (accountIndex && accountIndex > 0) {
const nonce = this.computeNonce(accountIndex);
const nonce = this.computeAccountNonce(accountIndex);
const derivedShare = userDec.add(nonce).umod(ecCurve.n);
return { tssIndex, tssShare: derivedShare };
}
Expand Down Expand Up @@ -463,15 +451,12 @@ class ThresholdKey implements ITKey {
for (let j = 0; j < tssIndex; j++) {
_tssSharePub = _tssSharePub.add(tssCommitA1);
}
if (accountIndex && accountIndex > 0) {
const nonce = this.computeNonce(accountIndex);
const derivedShare = tssShare.add(nonce).umod(ecCurve.n);
if (tssSharePub.getX().cmp(_tssSharePub.getX()) === 0 && tssSharePub.getY().cmp(_tssSharePub.getY()) === 0) {
if (tssSharePub.getX().cmp(_tssSharePub.getX()) === 0 && tssSharePub.getY().cmp(_tssSharePub.getY()) === 0) {
if (accountIndex && accountIndex > 0) {
const nonce = this.computeAccountNonce(accountIndex);
const derivedShare = tssShare.add(nonce).umod(ecCurve.n);
return { tssIndex, tssShare: derivedShare };
}
}

if (tssSharePub.getX().cmp(_tssSharePub.getX()) === 0 && tssSharePub.getY().cmp(_tssSharePub.getY()) === 0) {
return { tssIndex, tssShare };
}
}
Expand All @@ -488,17 +473,16 @@ class ThresholdKey implements ITKey {
}

getTSSPub(accountIndex?: number): Point {
const tssCommits = this.getTSSCommits();
if (accountIndex && accountIndex > 0) {
const nonce = this.computeNonce(accountIndex);
const nonce = this.computeAccountNonce(accountIndex);
// we need to add the pub key nonce to the tssPub
const noncePub = ecCurve.keyFromPrivate(nonce.toString("hex")).getPublic();
const pubKeyPoint = ecCurve
.keyFromPublic({ x: this.getTSSCommits()[0].x.toString("hex"), y: this.getTSSCommits()[0].y.toString("hex") })
.getPublic();
const pubKeyPoint = ecCurve.keyFromPublic({ x: tssCommits[0].x.toString("hex"), y: tssCommits[0].y.toString("hex") }).getPublic();
const dervicepubKeyPoint = pubKeyPoint.add(noncePub);
return new Point(dervicepubKeyPoint.getX().toString("hex"), dervicepubKeyPoint.getY().toString("hex"));
}
return this.getTSSCommits()[0];
return tssCommits[0];
}

/**
Expand Down Expand Up @@ -912,15 +896,15 @@ class ThresholdKey implements ITKey {
serverEncs: refreshResponse.serverFactorEncs,
};
}
const chainCode = this.generateSalt();
const accountSalt = generateSalt();

this.metadata.addTSSData({
tssTag: this.tssTag,
tssNonce: newTssNonce,
tssPolyCommits: newTSSCommits,
factorPubs,
factorEncs,
chainCode,
accountSalt,
});
await this._syncShareMetadata();
} catch (error) {
Expand Down Expand Up @@ -2007,6 +1991,16 @@ class ThresholdKey implements ITKey {
private async initializeModules() {
return Promise.all(Object.keys(this.modules).map((x) => this.modules[x].initialize()));
}

private computeAccountNonce(index: number) {
// generation should occur during tkey.init, fails if accountSalt is absent
const { accountSalt } = this.metadata;
if (!accountSalt) {
throw CoreError.accountSaltUndefined();
}
const accountHash = keccak256(Buffer.from(`${index}${accountSalt}`)).slice(2);
return index && index > 0 ? new BN(accountHash, "hex").umod(ecCurve.curve.n) : new BN(0);
}
}

export default ThresholdKey;
5 changes: 5 additions & 0 deletions packages/core/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class CoreError extends TkeyError {
1103: "setMetadata errored",
1104: "previouslyFetchedCloudMetadata provided in initialization is outdated",
1105: "previouslyFetchedCloudMetadata.nonce should never be higher than the latestShareDetails, please contact support",
1106: "Account Salt is absent, required for nonce generation",
// tkeystore
1201: "Invalid tkeyStore",
1202: "Encryption failed",
Expand Down Expand Up @@ -90,6 +91,10 @@ class CoreError extends TkeyError {
return CoreError.fromCode(1103, extraMessage);
}

public static accountSaltUndefined(extraMessage = ""): ITkeyError {
return CoreError.fromCode(1106, extraMessage);
}

// TkeyData
public static tkeyStoreInvalid(extraMessage = ""): ITkeyError {
return CoreError.fromCode(1201, extraMessage);
Expand Down
16 changes: 8 additions & 8 deletions packages/core/src/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class Metadata implements IMetadata {
};

// salt
chainCode?: string;
accountSalt?: string;

constructor(input: Point) {
this.tssPolyCommits = {};
Expand All @@ -87,7 +87,7 @@ class Metadata implements IMetadata {
}

static fromJSON(value: StringifiedType): Metadata {
const { pubKey, polyIDList, generalStore, tkeyStore, scopedStore, nonce, tssNonces, tssPolyCommits, factorPubs, factorEncs, chainCode } = value;
const { pubKey, polyIDList, generalStore, tkeyStore, scopedStore, nonce, tssNonces, tssPolyCommits, factorPubs, factorEncs, accountSalt } = value;
const point = Point.fromCompressedPub(pubKey);
const metadata = new Metadata(point);
const unserializedPolyIDList: PolyIDAndShares[] = [];
Expand All @@ -96,7 +96,7 @@ class Metadata implements IMetadata {
if (tkeyStore) metadata.tkeyStore = tkeyStore;
if (scopedStore) metadata.scopedStore = scopedStore;
if (nonce) metadata.nonce = nonce;
if (chainCode) metadata.chainCode = chainCode;
if (accountSalt) metadata.accountSalt = accountSalt;
if (tssPolyCommits) {
metadata.tssPolyCommits = {};
for (const key in tssPolyCommits) {
Expand Down Expand Up @@ -194,15 +194,15 @@ class Metadata implements IMetadata {
factorEncs?: {
[factorPubID: string]: FactorEnc;
};
chainCode?: string;
accountSalt?: string;
}): void {
const { tssTag, tssNonce, tssPolyCommits, factorPubs, factorEncs, chainCode } = tssData;
const { tssTag, tssNonce, tssPolyCommits, factorPubs, factorEncs, accountSalt } = tssData;
if (tssNonce !== undefined) this.tssNonces[tssTag] = tssNonce;
if (tssPolyCommits) this.tssPolyCommits[tssTag] = tssPolyCommits;
if (factorPubs) this.factorPubs[tssTag] = factorPubs;
if (factorEncs) this.factorEncs[tssTag] = factorEncs;
if (chainCode && !this.chainCode) {
this.chainCode = chainCode;
if (accountSalt && !this.accountSalt) {
this.accountSalt = accountSalt;
}
}

Expand Down Expand Up @@ -347,7 +347,7 @@ class Metadata implements IMetadata {
...(this.tssPolyCommits && { tssPolyCommits: this.tssPolyCommits }),
...(this.factorPubs && { factorPubs: this.factorPubs }),
...(this.factorEncs && { factorEncs: this.factorEncs }),
...(this.chainCode && { chainCode: this.chainCode }),
...(this.accountSalt && { accountSalt: this.accountSalt }),
};
}
}
Expand Down
20 changes: 10 additions & 10 deletions packages/default/test/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ export const sharedTestCases = (mode, torusSP, storageLayer) => {
const storageLayer = initStorageLayer({ hostUrl: metadataURL });
const tb1 = new ThresholdKey({ serviceProvider: sp, storageLayer, manualSync: mode });

// chainCode is absent, required for nonce generation
// accountSalt is absent, required for nonce generation
// can be only initialize with tkey.initialize();
rejects(async () => {
await tb1.computeNonce(1);
await tb1.computeAccountNonce(1);
});
// factor key needs to passed from outside of tKey
const factorKey = new BN(generatePrivate());
Expand All @@ -111,7 +111,7 @@ export const sharedTestCases = (mode, torusSP, storageLayer) => {
}
const { tssShare: retrievedTSS1, tssIndex: retrievedTSSIndex1 } = await tb1.getTSSShare(factorKey, { accountIndex: 1 });
const tssPrivKey1 = getLagrangeCoeffs([1, retrievedTSSIndex1], 1)
.mul(serverDKGPrivKeys[0].add(tb1.computeNonce(1)))
.mul(serverDKGPrivKeys[0].add(tb1.computeAccountNonce(1)))
.add(getLagrangeCoeffs([1, retrievedTSSIndex1], retrievedTSSIndex1).mul(retrievedTSS1))
.umod(ecCurve.n);
const tssPubKey1 = ecCurve.keyFromPrivate(tssPrivKey1).getPublic();
Expand Down Expand Up @@ -141,7 +141,7 @@ export const sharedTestCases = (mode, torusSP, storageLayer) => {

const { tssShare: retrievedTSS2, tssIndex: retrievedTSSIndex2 } = await tb2.getTSSShare(factorKey, { accountIndex: 2 });
const tssPrivKey2 = getLagrangeCoeffs([1, retrievedTSSIndex2], 1)
.mul(serverDKGPrivKeys[0].add(tb1.computeNonce(2)))
.mul(serverDKGPrivKeys[0].add(tb1.computeAccountNonce(2)))
.add(getLagrangeCoeffs([1, retrievedTSSIndex2], retrievedTSSIndex2).mul(retrievedTSS2))
.umod(ecCurve.n);

Expand Down Expand Up @@ -174,15 +174,15 @@ export const sharedTestCases = (mode, torusSP, storageLayer) => {
const tssSharePub = ecCurve.keyFromPrivate(retrievedTSS.toString("hex")).getPublic();
const { tssShare: retrievedTSS2 } = await tb2.getTSSShare(factorKey, { accountIndex: 1 });
const tssSharePub2 = ecCurve.keyFromPrivate(retrievedTSS2.toString("hex")).getPublic();
const nonce = tb1.computeNonce(1);
const nonce = tb1.computeAccountNonce(1);
const noncePub = ecCurve.keyFromPrivate(nonce.toString("hex")).getPublic();
const tssShareDerived = tssSharePub.add(noncePub);
strictEqual(tssShareDerived.getX().toString("hex"), tssSharePub2.getX().toString("hex"));
strictEqual(tssShareDerived.getY().toString("hex"), tssSharePub2.getY().toString("hex"));

const { tssShare: retrievedTSS3 } = await tb2.getTSSShare(factorKey, { accountIndex: 2 });
const tssSharePub3 = ecCurve.keyFromPrivate(retrievedTSS3.toString("hex")).getPublic();
const nonce2 = tb1.computeNonce(2);
const nonce2 = tb1.computeAccountNonce(2);
const noncePub2 = ecCurve.keyFromPrivate(nonce2.toString("hex")).getPublic();
const tssShareDerived2 = tssSharePub.add(noncePub2);
strictEqual(tssShareDerived2.getX().toString("hex"), tssSharePub3.getX().toString("hex"));
Expand All @@ -192,7 +192,7 @@ export const sharedTestCases = (mode, torusSP, storageLayer) => {
{
const { tssShare: newTSS, tssIndex } = await tb1.getTSSShare(factorKey, { accountIndex: 1 });
const newTSSPrivKey = getLagrangeCoeffs([1, 2], 1)
.mul(new BN(serverDKGPrivKeys[1], "hex").add(tb1.computeNonce(1)))
.mul(new BN(serverDKGPrivKeys[1], "hex").add(tb1.computeAccountNonce(1)))
.add(getLagrangeCoeffs([1, 2], 2).mul(newTSS))
.umod(ecCurve.n);
strictEqual(tssPrivKey1.toString(16, 64), newTSSPrivKey.toString(16, 64));
Expand All @@ -203,7 +203,7 @@ export const sharedTestCases = (mode, torusSP, storageLayer) => {
{
const { tssShare: newTSS2, tssIndex } = await tb2.getTSSShare(factorKey2, { accountIndex: 1 });
const newTSSPrivKey = getLagrangeCoeffs([1, 3], 1)
.mul(new BN(serverDKGPrivKeys[1], "hex").add(tb1.computeNonce(1)))
.mul(new BN(serverDKGPrivKeys[1], "hex").add(tb1.computeAccountNonce(1)))
.add(getLagrangeCoeffs([1, 3], 3).mul(newTSS2))
.umod(ecCurve.n);
strictEqual(tssPrivKey1.toString(16, 64), newTSSPrivKey.toString(16, 64));
Expand All @@ -214,7 +214,7 @@ export const sharedTestCases = (mode, torusSP, storageLayer) => {
{
const { tssShare: newTSS, tssIndex } = await tb2.getTSSShare(factorKey, { accountIndex: 2 });
const newTSSPrivKey = getLagrangeCoeffs([1, 2], 1)
.mul(new BN(serverDKGPrivKeys[1], "hex").add(tb2.computeNonce(2)))
.mul(new BN(serverDKGPrivKeys[1], "hex").add(tb2.computeAccountNonce(2)))
.add(getLagrangeCoeffs([1, 2], 2).mul(newTSS))
.umod(ecCurve.n);
strictEqual(tssPrivKey2.toString(16, 64), newTSSPrivKey.toString(16, 64));
Expand All @@ -225,7 +225,7 @@ export const sharedTestCases = (mode, torusSP, storageLayer) => {
{
const { tssShare: newTSS2, tssIndex } = await tb2.getTSSShare(factorKey2, { accountIndex: 2 });
const newTSSPrivKey = getLagrangeCoeffs([1, 3], 1)
.mul(new BN(serverDKGPrivKeys[1], "hex").add(tb1.computeNonce(2)))
.mul(new BN(serverDKGPrivKeys[1], "hex").add(tb1.computeAccountNonce(2)))
.add(getLagrangeCoeffs([1, 3], 3).mul(newTSS2))
.umod(ecCurve.n);
strictEqual(tssPrivKey2.toString(16, 64), newTSSPrivKey.toString(16, 64));
Expand Down

0 comments on commit 3d24247

Please sign in to comment.