fix: make threshold in option optional

packages/core/src/core.ts

@@ -70,6 +70,7 @@ import {
 import Metadata from "./metadata";
 // TODO: handle errors for get and set with retries
 export const TSS_MODULE = "tssModule";
+export const ACCOUNTSALT = "accountSalt";
 
 class ThresholdKey implements ITKey {
   modules: ModuleMap;
@@ -304,12 +305,14 @@ class ThresholdKey implements ITKey {
         });
         if (useTSS) {
           const { factorEncs, factorPubs, tssPolyCommits } = await this._initializeNewTSSKey(this.tssTag, deviceTSSShare, factorPub, deviceTSSIndex);
+
+          this.metadata.addTSSData({ tssTag: this.tssTag, tssNonce: 0, tssPolyCommits, factorPubs, factorEncs });
           const accountSalt = generateSalt();
           await this._setTKeyStoreItem(TSS_MODULE, {
             id: "accountSalt",
             value: accountSalt,
           });
-          this.metadata.addTSSData({ tssTag: this.tssTag, tssNonce: 0, tssPolyCommits, factorPubs, factorEncs });
+          this._accountSalt = accountSalt;
         }
         return this.getKeyDetails();
       }
@@ -394,7 +397,7 @@ class ThresholdKey implements ITKey {
    * getTSSShare accepts a factorKey and returns the TSS share based on the factor encrypted TSS shares in the metadata
    * @param factorKey - factor key
    */
-  async getTSSShare(factorKey: BN, opts?: { threshold: number; accountIndex?: number }): Promise<{ tssIndex: number; tssShare: BN }> {
+  async getTSSShare(factorKey: BN, opts?: { threshold?: number; accountIndex?: number }): Promise<{ tssIndex: number; tssShare: BN }> {
     if (!this.privKey) throw CoreError.default("tss share cannot be returned until you've reconstructed tkey");
     const factorPub = getPubKeyPoint(factorKey);
     const factorEncs = this.getFactorEncs(factorPub);
@@ -625,7 +628,7 @@ class ThresholdKey implements ITKey {
     // assign account salt from tKey store if it exists
     if (Object.keys(this.metadata.tssPolyCommits).length > 0) {
       const accountSalt = await this.getTKeyStoreItem(TSS_MODULE, "accountSalt");
-      if (accountSalt && accountSalt?.value) {
+      if (accountSalt && accountSalt.value) {
         this._accountSalt = accountSalt.value;
       } else {
         const newSalt = generateSalt();
@@ -634,6 +637,11 @@ class ThresholdKey implements ITKey {
           value: newSalt,
         });
         this._accountSalt = newSalt;
+        // this is very specific case where exisiting user do not have salt.
+        // sync metadata to cloud to ensure salt is stored incase of manual sync mode
+        // new user or importKey should not hit this cases
+        // NOTE this is not mistake, we force sync for this case
+        if (this.manualSync) await this.syncLocalMetadataTransitions();
       }
     }
 
@@ -837,6 +845,8 @@ class ThresholdKey implements ITKey {
       authSignatures: string[];
     }
   ): Promise<void> {
+    if (!this.privKey) throw CoreError.privateKeyUnavailable();
+
     const oldTag = this.tssTag;
     try {
       const { importKey, factorPub, newTSSIndex, tag } = params;
@@ -920,18 +930,24 @@ class ThresholdKey implements ITKey {
           serverEncs: refreshResponse.serverFactorEncs,
         };
       }
-      const accountSalt = generateSalt();
       this.metadata.addTSSData({
         tssTag: this.tssTag,
         tssNonce: newTssNonce,
         tssPolyCommits: newTSSCommits,
         factorPubs,
         factorEncs,
       });
-      await this._setTKeyStoreItem(TSS_MODULE, {
-        id: "accountSalt",
-        value: accountSalt,
-      });
+      if (!this._accountSalt) {
+        const accountSalt = generateSalt();
+        // tkeyStoreItem already syncMetadata
+        await this._setTKeyStoreItem(TSS_MODULE, {
+          id: "accountSalt",
+          value: accountSalt,
+        });
+        this._accountSalt = accountSalt;
+      } else {
+        await this._syncShareMetadata();
+      }
     } catch (error) {
       this.tssTag = oldTag;
       throw error;
@@ -1819,7 +1835,7 @@ class ThresholdKey implements ITKey {
     return decrypt(toPrivKeyECC(this.privKey), encryptedMessage);
   }
 
-  async _setTKeyStoreItem(moduleName: string, data: TkeyStoreItemType): Promise<void> {
+  async _setTKeyStoreItem(moduleName: string, data: TkeyStoreItemType, updateMetadata: boolean = true): Promise<void> {
     if (!this.metadata) {
       throw CoreError.metadataUndefined();
     }
@@ -1840,7 +1856,7 @@ class ThresholdKey implements ITKey {
 
     // update metadataStore
     this.metadata.setTkeyStoreDomain(moduleName, rawTkeyStoreItems);
-    await this._syncShareMetadata();
+    if (updateMetadata) await this._syncShareMetadata();
   }
 
   async _deleteTKeyStoreItem(moduleName: string, id: string): Promise<void> {

packages/default/test/RefreshAndAccountIndex.js

@@ -1,20 +1,104 @@
 import { ecCurve, getPubKeyPoint } from "@tkey-mpc/common-types";
 // eslint-disable-next-line import/no-extraneous-dependencies
 import { generatePrivate } from "@toruslabs/eccrypto";
-import { fail, notEqual, rejects, strictEqual } from "assert";
+import assert, { fail, notEqual, rejects, strictEqual } from "assert";
 import BN from "bn.js";
 
 import ThresholdKey from "../src/index";
 import { assignTssDkgKeys, computeIndexedPrivateKey, fetchPostboxKeyAndSigs, getMetadataUrl, initStorageLayer } from "./helpers";
 
-const TSS_MODULE = "tssModule";
 const metadataURL = getMetadataUrl();
-
+const TSS_MODULE = "tssModule";
 // eslint-disable-next-line mocha/no-exports
-export const refreshAndAccountIndex = (customSP, manualSync, accountIndexBackwardCompatible) => {
+export const refreshAndAccountIndex = (customSP, manualSync, resetAccountSalt) => {
   const mode = manualSync;
   const { useTSS } = customSP;
-  describe(`RefreshAndAccountIndex : useTss ${useTSS}, manualSync ${manualSync}, bcAccountIndex ${accountIndexBackwardCompatible}`, function () {
+  describe(`RefreshAndAccountIndex : useTss ${useTSS}, manualSync ${manualSync}, bcAccountIndex ${resetAccountSalt}`, function () {
+    it("#should be able to get same key each login using differnt accountIndex", async function () {
+      const sp = customSP;
+      if (!sp.useTSS) this.skip();
+
+      const deviceTSSShare = new BN(generatePrivate());
+      const deviceTSSIndex = 2;
+
+      sp.verifierName = "torus-test-health";
+      sp.verifierId = "[email protected]";
+      const testId = sp.getVerifierNameVerifierId();
+      const { postboxkey } = await fetchPostboxKeyAndSigs({
+        serviceProvider: sp,
+        verifierName: sp.verifierName,
+        verifierId: sp.verifierId,
+      });
+      sp.postboxKey = postboxkey;
+
+      const storageLayer = initStorageLayer({ hostUrl: metadataURL });
+      const tb0 = new ThresholdKey({ serviceProvider: sp, storageLayer, manualSync: mode });
+
+      // accountSalt is absent, required for nonce generation
+      //  can be only initialize with tkey.initialize();
+      rejects(async () => {
+        tb0.computeAccountNonce(1);
+      });
+      // factor key needs to passed from outside of tKey
+      const factorKey = new BN(generatePrivate());
+      const factorPub = getPubKeyPoint(factorKey);
+
+      await tb0.initialize({ useTSS: true, factorPub, deviceTSSShare, deviceTSSIndex });
+      await tb0.reconstructKey();
+      // Test for backward compatibility ( accountSalt is absent for old account )
+
+      const tss0ShareIndex0 = await tb0.getTSSShare(factorKey);
+      const tss0ShareIndex1 = await tb0.getTSSShare(factorKey, { accountIndex: 1 });
+      const tss0ShareIndex2 = await tb0.getTSSShare(factorKey, { accountIndex: 2 });
+      const tss0ShareIndex99 = await tb0.getTSSShare(factorKey, { accountIndex: 99 });
+      if (resetAccountSalt) {
+        // tb0.metadata.encryptedSalt = {};
+        await tb0._deleteTKeyStoreItem(TSS_MODULE, "accountSalt");
+      }
+
+      const newShare = await tb0.generateNewShare();
+      await tb0.syncLocalMetadataTransitions();
+
+      const tb1 = new ThresholdKey({ serviceProvider: sp, storageLayer, manualSync: mode });
+      await tb1.initialize({ useTSS: true, factorPub, deviceTSSShare, deviceTSSIndex });
+      await tb1.inputShareStoreSafe(newShare.newShareStores[newShare.newShareIndex.toString("hex")]);
+      await tb1.reconstructKey();
+
+      const tss1ShareIndex0 = await tb1.getTSSShare(factorKey);
+      const tss1ShareIndex1 = await tb1.getTSSShare(factorKey, { accountIndex: 1 });
+      const tss1ShareIndex2 = await tb1.getTSSShare(factorKey, { accountIndex: 2 });
+      const tss1ShareIndex99 = await tb1.getTSSShare(factorKey, { accountIndex: 99 });
+
+      const tb2 = new ThresholdKey({ serviceProvider: sp, storageLayer, manualSync: mode });
+      await tb2.initialize({ useTSS: true, factorPub, deviceTSSShare, deviceTSSIndex });
+      await tb2.inputShareStoreSafe(newShare.newShareStores[newShare.newShareIndex.toString("hex")]);
+      await tb2.reconstructKey();
+
+      const tss2ShareIndex0 = await tb2.getTSSShare(factorKey);
+      const tss2ShareIndex1 = await tb2.getTSSShare(factorKey, { accountIndex: 1 });
+      const tss2ShareIndex2 = await tb2.getTSSShare(factorKey, { accountIndex: 2 });
+      const tss2ShareIndex99 = await tb2.getTSSShare(factorKey, { accountIndex: 99 });
+
+      strictEqual(tss0ShareIndex0.tssShare.toString("hex"), tss1ShareIndex0.tssShare.toString("hex"));
+
+      // expect different account address when accountSalt is reseted ( to support existing account that do not have this features)
+      if (resetAccountSalt) {
+        notEqual(tss0ShareIndex1.tssShare.toString("hex"), tss1ShareIndex1.tssShare.toString("hex"));
+        notEqual(tss0ShareIndex2.tssShare.toString("hex"), tss1ShareIndex2.tssShare.toString("hex"));
+        notEqual(tss0ShareIndex99.tssShare.toString("hex"), tss1ShareIndex99.tssShare.toString("hex"));
+      } else {
+        strictEqual(tss0ShareIndex1.tssShare.toString("hex"), tss1ShareIndex1.tssShare.toString("hex"));
+        strictEqual(tss0ShareIndex2.tssShare.toString("hex"), tss1ShareIndex2.tssShare.toString("hex"));
+        strictEqual(tss0ShareIndex99.tssShare.toString("hex"), tss1ShareIndex99.tssShare.toString("hex"));
+      }
+
+      // expect same account address after relogin
+      strictEqual(tss1ShareIndex0.tssShare.toString("hex"), tss2ShareIndex0.tssShare.toString("hex"));
+      strictEqual(tss1ShareIndex1.tssShare.toString("hex"), tss2ShareIndex1.tssShare.toString("hex"));
+      strictEqual(tss1ShareIndex2.tssShare.toString("hex"), tss2ShareIndex2.tssShare.toString("hex"));
+      strictEqual(tss1ShareIndex99.tssShare.toString("hex"), tss2ShareIndex99.tssShare.toString("hex"));
+    });
+
     it("#should be able to refresh tss shares", async function () {
       const sp = customSP;
       if (!sp.useTSS) this.skip();
@@ -52,10 +136,11 @@ export const refreshAndAccountIndex = (customSP, manualSync, accountIndexBackwar
 
       await tb0.initialize({ useTSS: true, factorPub, deviceTSSShare, deviceTSSIndex });
       await tb0.reconstructKey();
-
       // Test for backward compatibility ( accountSalt is absent for old account )
-      if (accountIndexBackwardCompatible) {
-        await tb0._deleteTKeyStoreItem(TSS_MODULE, "accountSalt");
+
+      if (resetAccountSalt) {
+        // eslint-disable-next-line require-atomic-updates
+        tb0.metadata.encryptedSalt = {};
       }
 
       const newShare = await tb0.generateNewShare();
@@ -92,8 +177,9 @@ export const refreshAndAccountIndex = (customSP, manualSync, accountIndexBackwar
         serverPubKeys,
         authSignatures: signatures,
       });
-      await tb1.syncLocalMetadataTransitions();
-
+      if (manualSync) {
+        await tb1.syncLocalMetadataTransitions();
+      }
       const tssPrivKeyIndex1 = await computeIndexedPrivateKey(tb1, factorKey, serverDKGPrivKeys[1], 1);
       const tssPrivKeyIndex2 = await computeIndexedPrivateKey(tb1, factorKey, serverDKGPrivKeys[1], 2);
 
@@ -131,6 +217,22 @@ export const refreshAndAccountIndex = (customSP, manualSync, accountIndexBackwar
         authSignatures: signatures,
       });
 
+      {
+        // ensure tssShare is the master share
+        const { tssShare } = await tb2.getTSSShare(factorKey);
+        const { tssShare: tssShareIndex1 } = await tb2.getTSSShare(factorKey, { accountIndex: 1 });
+        const { tssShare: tssShareIndex2 } = await tb2.getTSSShare(factorKey, { accountIndex: 2 });
+        const { tssShare: tssShareIndex0 } = await tb2.getTSSShare(factorKey, { accountIndex: 0 });
+
+        const nonce1 = tb2.computeAccountNonce(1);
+        const nonce2 = tb2.computeAccountNonce(2);
+
+        assert(tb2.computeAccountNonce(0).eq(new BN(0)), "nonce for account 0 should be 0");
+        assert(tssShare.eq(tssShareIndex0), "tssShareIndex0 should be equal to tssShare");
+        assert(tssShare.add(nonce1).umod(ecCurve.n).eq(tssShareIndex1), "tssShareIndex1 should be equal to tssShare + nonce1");
+        assert(tssShare.add(nonce2).umod(ecCurve.n).eq(tssShareIndex2), "tssShareIndex2 should be equal to tssShare + nonce2");
+        assert(!tssShareIndex1.add(nonce1).umod(ecCurve.n).eq(tssShareIndex2), "tssShareIndex2 should not be equal to tssShareIndex1 + nonce1");
+      }
       // test case to ensure nonce mechanism
       {
         // make sure derived pub key is different from the index 0 key