fix(sdk-coin-eth): fixes to the sign and verify functions for eth tss

Ticket: bg-58807
BitGo · Oct 3, 2022 · ce79269 · ce79269
1 parent 9aed51e
commit ce79269
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 11 deletions.
diff --git a/modules/account-lib/test/unit/mpc/tss/ecdsa/ecdsa.ts b/modules/account-lib/test/unit/mpc/tss/ecdsa/ecdsa.ts
@@ -121,7 +121,7 @@ describe('TSS ECDSA TESTS', function () {
   });
 
   describe('ECDSA Signing', async function () {
-    let config: { signerOne: ECDSA.KeyCombined; signerTwo: ECDSA.KeyCombined; hash?: Hash }[];
+    let config: { signerOne: ECDSA.KeyCombined; signerTwo: ECDSA.KeyCombined; hash?: string; shouldHash?: boolean }[];
 
     before(async () => {
       const [A, B, C, D, E, F] = keyShares;
@@ -137,11 +137,14 @@ describe('TSS ECDSA TESTS', function () {
         { signerOne: F, signerTwo: D },
 
         // Checks with specific hashing algorithm
-        { signerOne: A, signerTwo: B, hash: createKeccakHash('keccak256') },
+        { signerOne: A, signerTwo: B, hash: 'keccak256' },
+
+        // checks with no hashing
+        { signerOne: A, signerTwo: B, shouldHash: false },
       ];
     });
 
-    for (let index = 0; index < 3; index++) {
+    for (let index = 0; index < 8; index++) {
       it(`should properly sign the message case ${index}`, async function () {
         // Step One
         // signerOne, signerTwo have decided to sign the message
@@ -213,9 +216,24 @@ describe('TSS ECDSA TESTS', function () {
         // and finally signs the message using their private OShare
         // and delta share received from the other signer
 
+        const hashGenerator = (hashType?: string): Hash | undefined => {
+          return hashType === 'keccak256' ? createKeccakHash('keccak256') : undefined;
+        };
         const [signA, signB] = [
-          MPC.sign(MESSAGE, signCombineOne.oShare, signCombineTwo.dShare, config[index].hash),
-          MPC.sign(MESSAGE, signCombineTwo.oShare, signCombineOne.dShare, config[index].hash),
+          MPC.sign(
+            MESSAGE,
+            signCombineOne.oShare,
+            signCombineTwo.dShare,
+            hashGenerator(config[index].hash),
+            config[index].shouldHash,
+          ),
+          MPC.sign(
+            MESSAGE,
+            signCombineTwo.oShare,
+            signCombineOne.dShare,
+            hashGenerator(config[index].hash),
+            config[index].shouldHash,
+          ),
         ];
 
         // Step Eight
@@ -226,7 +244,7 @@ describe('TSS ECDSA TESTS', function () {
         // Step Nine
         // Verify signature
 
-        const isValid = MPC.verify(MESSAGE, signature, config[index].hash);
+        const isValid = MPC.verify(MESSAGE, signature, hashGenerator(config[index].hash), config[index].shouldHash);
         isValid.should.equal(true);
       });
     }

diff --git a/modules/sdk-core/src/account-lib/mpc/tss/ecdsa/ecdsa.ts b/modules/sdk-core/src/account-lib/mpc/tss/ecdsa/ecdsa.ts
@@ -313,10 +313,11 @@ export default class Ecdsa {
    * @param {OShare} oShare private omicron share of current participant
    * @param {DShare} dShare delta share received from the other participant
    * @param {Hash} hash hashing algorithm implementing Node`s standard crypto hash interface
+   * @param {boolean} shouldHash if true, we hash the provided buffer before signing
    * @returns {SShare}
    */
-  sign(M: Buffer, oShare: OShare, dShare: DShare, hash?: Hash): SShare {
-    const m = (hash || createHash('sha256')).update(M).digest();
+  sign(M: Buffer, oShare: OShare, dShare: DShare, hash?: Hash, shouldHash = true): SShare {
+    const m = shouldHash ? (hash || createHash('sha256')).update(M).digest() : M;
 
     const delta = Ecdsa.curve.scalarAdd(hexToBigInt(oShare.delta), hexToBigInt(dShare.delta));
 
@@ -370,11 +371,13 @@ export default class Ecdsa {
    * @param {Buffer} message
    * @param {Signature } signature
    * @param {Hash} hash hashing algorithm implementing Node`s standard crypto hash interface
+   * @param {boolean} shouldHash if true, we hash the provided buffer before verifying
    * @returns {boolean} True if signature is valid; False otherwise
    */
-  verify(message: Buffer, signature: Signature, hash?: Hash): boolean {
+  verify(message: Buffer, signature: Signature, hash?: Hash, shouldHash = true): boolean {
+    const messageToVerify = shouldHash ? (hash || createHash('sha256')).update(message).digest() : message;
     return Ecdsa.curve.verify(
-      (hash || createHash('sha256')).update(message).digest(),
+      messageToVerify,
       Buffer.concat([
         Buffer.from([signature['recid']]),
         bigIntToBufferBE(hexToBigInt(signature['r']), 32),

diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts
@@ -309,7 +309,7 @@ export class EcdsaUtils extends baseTSSUtils<KeyShare> {
     let signablePayload;
 
     if (requestType === RequestType.tx) {
-      signablePayload = Buffer.from(txRequestResolved.transactions[0].unsignedTx.serializedTxHex, 'hex');
+      signablePayload = Buffer.from(txRequestResolved.transactions[0].unsignedTx.signableHex, 'hex');
     } else if (requestType === RequestType.message) {
       assert(txRequestResolved.unsignedMessages?.[0]);
       signablePayload = Buffer.from(txRequestResolved.unsignedMessages[0].message, 'hex');