fix: multiple issues with message signing

Ticket: BG-61620

Author: jeanlarrieux

modules/bitgo/test/v2/unit/wallet.ts

@@ -17,6 +17,9 @@ import {
   TssUtils,
   TxRequest,
   Wallet,
+  SignatureShareType,
+  Ecdsa,
+  Keychains,
 } from '@bitgo/sdk-core';
 
 import { TestBitGo } from '@bitgo/sdk-test';
@@ -2034,6 +2037,7 @@ describe('V2 Wallet:', function () {
     });
 
     describe('Message Signing', function () {
+      const txHash = '0xrrrsss1b';
       const txRequestForMessageSigning: TxRequest = {
         txRequestId: 'id',
         transactions: [],
@@ -2050,6 +2054,12 @@ describe('V2 Wallet:', function () {
         walletId: 'walletId',
         unsignedTxs: [],
         unsignedMessages: [],
+        messages: [{
+          state: 'signed',
+          signatureShares: [{ from: SignatureShareType.USER, to: SignatureShareType.USER, share: '' }],
+          combineSigShare: '0:rrr:sss:3',
+          txHash,
+        }],
       };
       let signTxRequestForMessage;
       const messageSigningCoins = ['teth', 'tpolygon'];
@@ -2058,10 +2068,13 @@ describe('V2 Wallet:', function () {
       beforeEach(async function () {
         signTxRequestForMessage = sandbox.stub(ECDSAUtils.EcdsaUtils.prototype, 'signTxRequestForMessage');
         signTxRequestForMessage.resolves(txRequestForMessageSigning);
+        sandbox.stub(Keychains.prototype, 'getKeysForSigning').resolves([{ commonKeychain: 'test', id: '', pub: '' }]);
+        sinon.stub(Ecdsa.prototype, 'verify').resolves(true);
       });
 
       afterEach(async function () {
         sinon.restore();
+        nock.cleanAll();
       });
 
       it('should throw error for unsupported coins', async function () {
@@ -2088,7 +2101,7 @@ describe('V2 Wallet:', function () {
             message: { messageRaw, txRequestId },
             prv: 'secretKey',
           });
-          signMessage.should.deepEqual({ txRequestId } );
+          signMessage.should.deepEqual(txHash);
           const actualArg = signMessageTssSpy.getCalls()[0].args[0];
           actualArg.message.messageEncoded.should.equal(`\u0019Ethereum Signed Message:\n${messageRaw.length}${messageRaw}`);
         });
@@ -2105,7 +2118,7 @@ describe('V2 Wallet:', function () {
             message: { messageRaw },
             prv: 'secretKey',
           });
-          signMessage.should.deepEqual({ txRequestId } );
+          signMessage.should.deepEqual(txHash);
           const actualArg = signMessageTssSpy.getCalls()[0].args[0];
           actualArg.message.messageEncoded.should.equal(`\u0019Ethereum Signed Message:\n${messageRaw.length}${messageRaw}`);
         });
@@ -2121,7 +2134,7 @@ describe('V2 Wallet:', function () {
             message: { messageRaw },
             prv: 'secretKey',
           });
-          signMessage.should.deepEqual({ txRequestId } );
+          signMessage.should.deepEqual(txHash);
           const actualArg = signMessageTssSpy.getCalls()[0].args[0];
           actualArg.message.messageEncoded.should.equal(`\u0019Ethereum Signed Message:\n${messageRaw.length}${messageRaw}`);
         });
@@ -2131,7 +2144,7 @@ describe('V2 Wallet:', function () {
             reqId,
             message: { messageRaw, txRequestId },
             prv: '',
-          }).should.be.rejectedWith('prv required to sign message with TSS');
+          }).should.be.rejectedWith('keychain does not have property encryptedPrv');
         });
       });
 

modules/sdk-coin-eth/src/eth.ts

@@ -41,12 +41,14 @@ import {
   HalfSignedTransaction,
   FullySignedTransaction,
   MPCAlgorithm,
+  ECDSAMethods,
 } from '@bitgo/sdk-core';
 
 import { BaseCoin as StaticsBaseCoin, EthereumNetwork, ethGasConfigs } from '@bitgo/statics';
 import type * as EthTxLib from '@ethereumjs/tx';
 import type * as EthCommon from '@ethereumjs/common';
 import { calculateForwarderV1Address, getProxyInitcode } from './lib';
+import { intToHex, stripHexPrefix, addHexPrefix } from 'ethereumjs-util';
 
 export { Recipient, HalfSignedTransaction, FullySignedTransaction };
 
@@ -1752,6 +1754,25 @@ export class Eth extends BaseCoin {
     return prefix.concat(message);
   }
 
+  /**
+   *  Create the final signed message hash from the combine share
+   * @param combineShare
+   */
+  constructFinalSignedMessageHash(combineShare: string): string {
+    const combineSigArray = combineShare.split(ECDSAMethods.delimeter);
+    const signature = {
+      recid: parseInt(combineSigArray[0], 10),
+      r: combineSigArray[1],
+      s: combineSigArray[2],
+      y: combineSigArray[3],
+    };
+    const v = 27 + signature.recid;
+    const vHex = intToHex(v);
+    const vStr = stripHexPrefix(vHex);
+    const finalSig = addHexPrefix(signature.r.concat(signature.s, vStr));
+    return finalSig;
+  }
+
   private isETHAddress(address: string): boolean {
     return !!address.match(/0x[a-fA-F0-9]{40}/);
   }

modules/sdk-core/src/bitgo/features/constructFinalSignedMessageHash.ts

@@ -0,0 +1,7 @@
+export interface CoinThatConstructFinalSignedMessageHash {
+  constructFinalSignedMessageHash(combineShare: string): string;
+}
+
+export function isCoinThatConstructFinalSignedMessageHash(c: unknown): c is CoinThatConstructFinalSignedMessageHash {
+  return typeof (c as any).constructFinalSignedMessageHash === 'function';
+}

modules/sdk-core/src/bitgo/tss/ecdsa/ecdsa.ts

@@ -208,7 +208,13 @@ export async function sendShareToBitgo(
       const kShare = share as KShare;
       signatureShare = convertKShare(kShare);
       await sendSignatureShare(bitgo, walletId, txRequestId, signatureShare, requestType, signerShare, 'ecdsa');
-      responseFromBitgo = await getBitgoToUserLatestShare(bitgo, walletId, txRequestId, ReceivedShareType.AShare);
+      responseFromBitgo = await getBitgoToUserLatestShare(
+        bitgo,
+        walletId,
+        txRequestId,
+        ReceivedShareType.AShare,
+        requestType
+      );
       break;
     case SendShareType.MUShare:
       const shareToSend = share as MuDShare;
@@ -220,7 +226,13 @@ export async function sendShareToBitgo(
         share: `${muShareRecord.share}${secondaryDelimeter}${dShareRecord.share}`,
       };
       await sendSignatureShare(bitgo, walletId, txRequestId, signatureShare, requestType, signerShare, 'ecdsa');
-      responseFromBitgo = await getBitgoToUserLatestShare(bitgo, walletId, txRequestId, ReceivedShareType.DShare);
+      responseFromBitgo = await getBitgoToUserLatestShare(
+        bitgo,
+        walletId,
+        txRequestId,
+        ReceivedShareType.DShare,
+        requestType
+      );
       break;
     case SendShareType.SShare:
       const sShare = share as SShare;
@@ -247,11 +259,22 @@ export async function getBitgoToUserLatestShare(
   bitgo: BitGoBase,
   walletId: string,
   txRequestId: string,
-  shareType: ReceivedShareType
+  shareType: ReceivedShareType,
+  requestType: RequestType
 ): Promise<SendShareToBitgoRT> {
   let responseFromBitgo: SendShareToBitgoRT;
   const txRequest = await getTxRequest(bitgo, walletId, txRequestId);
-  const userShares = txRequest.transactions[0].signatureShares;
+  let userShares;
+  switch (requestType) {
+    case RequestType.tx:
+      assert(txRequest.transactions, 'transactions required as part of txRequest');
+      userShares = txRequest.transactions[0].signatureShares;
+      break;
+    case RequestType.message:
+      assert(txRequest.messages, 'messages required as part of txRequest');
+      userShares = txRequest.messages[0].signatureShares;
+      break;
+  }
   if (!userShares || !userShares.length) {
     throw new Error('user share is not present');
   }

modules/sdk-core/src/bitgo/tss/eddsa/eddsa.ts

@@ -20,6 +20,7 @@ import { BaseTransaction } from './../../../account-lib/baseCoin/baseTransaction
 import { Ed25519BIP32 } from './../../../account-lib/mpc/hdTree';
 import _ = require('lodash');
 import { getTxRequest, sendSignatureShare } from '../common';
+import assert from 'assert';
 
 export { getTxRequest, sendSignatureShare };
 
@@ -228,8 +229,13 @@ export async function getBitgoToUserRShare(
   txRequestId: string
 ): Promise<SignatureShareRecord> {
   const txRequest = await getTxRequest(bitgo, walletId, txRequestId);
-  const signatureShares =
-    txRequest.apiVersion === 'full' ? txRequest.transactions[0].signatureShares : txRequest.signatureShares;
+  let signatureShares;
+  if (txRequest.apiVersion === 'full') {
+    assert(txRequest.transactions, 'transactions required as part of txRequest');
+    signatureShares = txRequest.transactions[0].signatureShares;
+  } else {
+    signatureShares = txRequest.signatureShares;
+  }
   if (_.isNil(signatureShares) || _.isEmpty(signatureShares)) {
     throw new Error(`No signatures shares found for id: ${txRequestId}`);
   }

modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts

@@ -18,8 +18,8 @@ import {
   TxRequest,
   TxRequestVersion,
   BackupKeyShare,
-  IntentOptionsBase,
-  PopulatedIntentBase,
+  IntentOptionsForMessage,
+  PopulatedIntentForMessageSigning,
 } from './baseTypes';
 import { SignShare, YShare, GShare } from '../../../account-lib/mpc/tss/eddsa/types';
 
@@ -204,16 +204,18 @@ export default class BaseTssUtils<KeyShare> extends MpcUtils implements ITssUtil
    * @param preview
    */
   async createTxRequestWithIntentForMessageSigning(
-    params: IntentOptionsBase,
+    params: IntentOptionsForMessage,
     apiVersion: TxRequestVersion = 'full',
     preview?: boolean
   ): Promise<TxRequest> {
-    const intentOptions: PopulatedIntentBase = {
+    const intentOptions: PopulatedIntentForMessageSigning = {
       intentType: params.intentType,
       sequenceId: params.sequenceId,
       comment: params.comment,
       memo: params.memo?.value,
       isTss: params.isTss,
+      messageRaw: params.messageRaw,
+      messageEncoded: params.messageEncoded ?? '',
     };
     const whitelistedParams = {
       intent: {

modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts

@@ -56,7 +56,7 @@ export interface TokenTransferRecipientParams {
   tokenId?: string;
   decimalPlaces?: number;
 }
-export interface IntentOptionsBase {
+interface IntentOptionsBase {
   reqId: IRequestTracer;
   intentType: string;
   sequenceId?: string;
@@ -67,6 +67,11 @@ export interface IntentOptionsBase {
   custodianMessageId?: string;
 }
 
+export interface IntentOptionsForMessage extends IntentOptionsBase {
+  messageRaw: string;
+  messageEncoded?: string;
+}
+
 export interface PrebuildTransactionWithIntentOptions extends IntentOptionsBase {
   recipients?: {
     address?: string;
@@ -95,14 +100,19 @@ export interface IntentRecipient {
   data?: string;
   tokenData?: TokenTransferRecipientParams;
 }
-export interface PopulatedIntentBase {
+interface PopulatedIntentBase {
   intentType: string;
   sequenceId?: string;
   comment?: string;
   memo?: string;
   isTss?: boolean;
 }
 
+export interface PopulatedIntentForMessageSigning extends PopulatedIntentBase {
+  messageRaw: string;
+  messageEncoded: string;
+}
+
 export interface PopulatedIntent extends PopulatedIntentBase {
   recipients?: IntentRecipient[];
   nonce?: string;
@@ -185,11 +195,17 @@ export type TxRequest = {
   // Only available in 'lite' version
   unsignedTxs: UnsignedTransactionTss[]; // Should override with blockchain / sig scheme specific unsigned tx
   // Only available in 'full' version
-  transactions: {
+  transactions?: {
     state: TransactionState;
     unsignedTx: UnsignedTransactionTss; // Should override with blockchain / sig specific unsigned tx
     signatureShares: SignatureShareRecord[];
   }[];
+  messages?: {
+    state: TransactionState;
+    signatureShares: SignatureShareRecord[];
+    combineSigShare?: string;
+    txHash?: string;
+  }[];
   apiVersion?: TxRequestVersion;
   latest: boolean;
 };
@@ -213,7 +229,8 @@ export type TSSParams = {
 };
 
 export type TSSParamsForMessage = TSSParams & {
-  finalMessage: string;
+  messageRaw: string;
+  messageEncoded?: string;
 };
 
 export interface BitgoHeldBackupKeyShare {

modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts

@@ -587,13 +587,13 @@ export class EcdsaUtils extends baseTSSUtils<KeyShare> {
     let signablePayload;
 
     if (requestType === RequestType.tx) {
+      assert(txRequestResolved.transactions, 'Unable to find transactions in txRequest');
       signablePayload = Buffer.from(txRequestResolved.transactions[0].unsignedTx.signableHex, 'hex');
     } else if (requestType === RequestType.message) {
-      const finalMessage = (params as TSSParamsForMessage).finalMessage;
-      assert(finalMessage);
-      signablePayload = Buffer.from(finalMessage, 'hex');
+      const finalMessage = (params as TSSParamsForMessage).messageEncoded;
+      assert(finalMessage, 'finalMessage is required');
+      signablePayload = Buffer.from(finalMessage);
     }
-
     const userSShare = await ECDSAMethods.createUserSignatureShare(
       userOmicronAndDeltaShare.oShare,
       bitgoToUserDShare,
@@ -630,8 +630,8 @@ export class EcdsaUtils extends baseTSSUtils<KeyShare> {
    * @returns {Promise<TxRequest>} fully signed TxRequest object
    */
   async signTxRequestForMessage(params: TSSParamsForMessage): Promise<TxRequest> {
-    if (!params.finalMessage) {
-      throw new Error('finalMessage required to sign message');
+    if (!params.messageRaw) {
+      throw new Error('Raw message required to sign message');
     }
     return this.signRequestBase(params, RequestType.message);
   }

modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts

@@ -26,6 +26,7 @@ import {
 import { KeyShare, YShare } from './types';
 import baseTSSUtils from '../baseTSSUtils';
 import { KeychainsTriplet } from '../../../baseCoin';
+import assert from 'assert';
 
 /**
  * Utility functions for TSS work flows.
@@ -284,6 +285,7 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
       throw new Error('Invalid user key - missing backupYShare');
     }
 
+    assert(txRequestResolved.transactions, 'Unable to find transactions in txRequest');
     const unsignedTx =
       txRequestResolved.apiVersion === 'full'
         ? txRequestResolved.transactions[0].unsignedTx
@@ -323,6 +325,7 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
       throw new Error('Invalid user key - missing backupYShare');
     }
 
+    assert(txRequestResolved.transactions, 'Unable to find transactions in txRequest');
     const unsignedTx =
       txRequestResolved.apiVersion === 'full'
         ? txRequestResolved.transactions[0].unsignedTx
@@ -403,6 +406,7 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
       throw new Error('Invalid user key - missing backupYShare');
     }
 
+    assert(txRequestResolved.transactions, 'Unable to find transactions in txRequest');
     const unsignedTx =
       txRequestResolved.apiVersion === 'full'
         ? txRequestResolved.transactions[0].unsignedTx