fix(sdk-core): skip auto-sign for message-sign approvals

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

@@ -706,4 +706,54 @@ describe('Pending Approvals:', () => {
       });
     });
   });
+
+  describe('message-sign approvals (transactionRequestFull)', () => {
+    const buildMessageSignPendingApprovalData = (intentType: string): PendingApprovalData => ({
+      id: 'pa-msg',
+      info: {
+        type: Type.TRANSACTION_REQUEST_FULL,
+        transactionRequestFull: {
+          txRequestId: 'txreq-msg-1',
+          intent: { intentType },
+        },
+      } as PendingApprovalInfo,
+      state: State.PENDING,
+      creator: 'test',
+      txRequestId: 'txreq-msg-1',
+    });
+
+    ['signMessage', 'signTypedStructuredData'].forEach((intentType) => {
+      it(`should approve without the post-approve auto-sign for a ${intentType} approval`, async () => {
+        const ethBaseCoin = bitgo.coin('teth');
+        const paData = buildMessageSignPendingApprovalData(intentType);
+        const pendingApproval = new PendingApproval(bitgo, ethBaseCoin, paData, wallet);
+
+        const paScope = nock(bgUrl)
+          .put(`/api/v2/teth/pendingapprovals/pa-msg`, { state: 'approved', otp: undefined })
+          .reply(200, { ...paData, state: 'approved' });
+        const recreateTransactionTssStub = sandbox.stub(PendingApproval.prototype, 'recreateAndSignTSSTransaction');
+
+        await pendingApproval.approve({ walletPassphrase: 'pass' });
+
+        recreateTransactionTssStub.notCalled.should.be.true();
+        paScope.isDone().should.be.true();
+      });
+    });
+
+    it('should keep the post-approve auto-sign for transaction approvals', async () => {
+      const ethBaseCoin = bitgo.coin('teth');
+      const paData = buildMessageSignPendingApprovalData('payment');
+      const pendingApproval = new PendingApproval(bitgo, ethBaseCoin, paData, wallet);
+
+      const paScope = nock(bgUrl)
+        .put(`/api/v2/teth/pendingapprovals/pa-msg`, { state: 'approved', otp: undefined })
+        .reply(200, { ...paData, state: 'approved' });
+      const recreateTransactionTssStub = sandbox.stub(PendingApproval.prototype, 'recreateAndSignTSSTransaction');
+
+      await pendingApproval.approve({ walletPassphrase: 'pass' });
+
+      recreateTransactionTssStub.calledOnce.should.be.true();
+      paScope.isDone().should.be.true();
+    });
+  });
 });

modules/sdk-core/src/bitgo/pendingApproval/iPendingApproval.ts

@@ -48,8 +48,24 @@ export interface PendingApprovalInfo {
     };
     sourceWallet?: string;
   };
+  /**
+   * Present on transactionRequestFull pending approvals. intent.intentType is
+   * MESSAGE_SIGNING_INTENT_TYPES for message-sign approvals, which have no
+   * unsigned transaction.
+   */
+  transactionRequestFull?: {
+    txRequestId?: string;
+    intent?: {
+      intentType?: string;
+      [index: string]: unknown;
+    };
+    [index: string]: unknown;
+  };
 }
 
+/** Intent types whose txRequest carries a message to sign rather than a transaction. */
+export const MESSAGE_SIGNING_INTENT_TYPES: ReadonlySet<string> = new Set(['signMessage', 'signTypedStructuredData']);
+
 export interface PendingApprovalData {
   id: string;
   wallet?: string;

modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts

@@ -4,6 +4,7 @@
 import * as _ from 'lodash';
 import * as utxolib from '@bitgo/utxo-lib';
 import type { AttestationPayload } from '@bitgo/public-types';
+import { MESSAGE_SIGNING_INTENT_TYPES } from './iPendingApproval';
 import { IBaseCoin } from '../baseCoin';
 import { BitGoBase } from '../bitgoBase';
 import {
@@ -172,7 +173,10 @@ export class PendingApproval implements IPendingApproval {
   }
 
   /**
-   * Sets this PendingApproval to an approved state
+   * Sets this PendingApproval to an approved state.
+   * For transaction approvals on TSS wallets, this also attempts to recreate
+   * and sign the transaction. For message-sign approvals, signing is deferred:
+   * the message must be signed separately via signAndSendMessageTxRequest.
    */
   async approve(params: ApproveOptions = {}): Promise<any> {
     params.previewPendingTxs = true;
@@ -489,12 +493,23 @@ export class PendingApproval implements IPendingApproval {
               reqId
             );
           } else if (this.canRecreateTransaction(params) && this.baseCoin.supportsTss()) {
+            if (this.isMessageSigningRequest()) {
+              return;
+            }
             await this.recreateAndSignTSSTransaction(params, reqId);
           }
         }
     }
   }
 
+  /**
+   * True when the approval's intent is a message-sign intent.
+   */
+  private isMessageSigningRequest(): boolean {
+    const intentType = this.info()?.transactionRequestFull?.intent?.intentType;
+    return Boolean(intentType && MESSAGE_SIGNING_INTENT_TYPES.has(intentType));
+  }
+
   /**
    * Helper function to ensure that self.wallet is set
    */

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

@@ -590,11 +590,13 @@ export class EcdsaUtils extends BaseEcdsaUtils {
 
     if (requestType === RequestType.tx) {
       assert(
-        txRequest.transactions || (txRequest as TxRequest).unsignedTxs,
+        (txRequest.transactions && txRequest.transactions.length > 0) ||
+          (txRequest.unsignedTxs && txRequest.unsignedTxs.length > 0),
         'Unable to find transactions in txRequest'
       );
       const unsignedTx =
         txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
+      assert(unsignedTx, 'Missing unsignedTx in txRequest');
       derivationPath = unsignedTx.derivationPath;
     } else if (requestType === RequestType.message) {
       // TODO BG-67299 Message signing with derivation path
@@ -641,9 +643,14 @@ export class EcdsaUtils extends BaseEcdsaUtils {
     const txRequest: TxRequest = tssParams.txRequest;
     let signablePayload;
     if (requestType === RequestType.tx) {
-      assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
+      assert(
+        (txRequest.transactions && txRequest.transactions.length > 0) ||
+          (txRequest.unsignedTxs && txRequest.unsignedTxs.length > 0),
+        'Unable to find transactions in txRequest'
+      );
       const unsignedTx =
         txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
+      assert(unsignedTx, 'Missing unsignedTx in txRequest');
       signablePayload = Buffer.from(unsignedTx.signableHex, 'hex');
     } else if (requestType === RequestType.message) {
       signablePayload = (params.tssParams as TSSParamsForMessage).bufferToSign;
@@ -799,9 +806,14 @@ export class EcdsaUtils extends BaseEcdsaUtils {
     let derivationPath = '';
 
     if (requestType === RequestType.tx) {
-      assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
+      assert(
+        (txRequest.transactions && txRequest.transactions.length > 0) ||
+          (txRequest.unsignedTxs && txRequest.unsignedTxs.length > 0),
+        'Unable to find transactions in txRequest'
+      );
       const unsignedTx =
         txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
+      assert(unsignedTx, 'Missing unsignedTx in txRequest');
 
       if (
         'recipientSource' in params &&

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

@@ -962,9 +962,14 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
     }
 
     if (requestType === RequestType.tx) {
-      assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
+      assert(
+        (txRequest.transactions && txRequest.transactions.length > 0) ||
+          (txRequest.unsignedTxs && txRequest.unsignedTxs.length > 0),
+        'Unable to find transactions in txRequest'
+      );
       const unsignedTx =
         txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
+      assert(unsignedTx, 'Missing unsignedTx in txRequest');
 
       if (
         'recipientSource' in params &&

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

@@ -546,11 +546,16 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
       throw new Error('Invalid user key - missing backupYShare');
     }
 
-    assert(txRequestResolved.transactions || txRequestResolved.unsignedTxs, 'Unable to find transactions in txRequest');
+    assert(
+      (txRequestResolved.transactions && txRequestResolved.transactions.length > 0) ||
+        (txRequestResolved.unsignedTxs && txRequestResolved.unsignedTxs.length > 0),
+      'Unable to find transactions in txRequest'
+    );
     const unsignedTx =
       txRequestResolved.apiVersion === 'full'
         ? txRequestResolved.transactions![0].unsignedTx
         : txRequestResolved.unsignedTxs[0];
+    assert(unsignedTx, 'Missing unsignedTx in txRequest');
 
     const signingKey = MPC.keyDerive(
       userSigningMaterial.uShare,
@@ -634,11 +639,16 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
       throw new Error('Invalid user key - missing backupYShare');
     }
 
-    assert(txRequestResolved.transactions || txRequestResolved.unsignedTxs, 'Unable to find transactions in txRequest');
+    assert(
+      (txRequestResolved.transactions && txRequestResolved.transactions.length > 0) ||
+        (txRequestResolved.unsignedTxs && txRequestResolved.unsignedTxs.length > 0),
+      'Unable to find transactions in txRequest'
+    );
     const unsignedTx =
       txRequestResolved.apiVersion === 'full'
         ? txRequestResolved.transactions![0].unsignedTx
         : txRequestResolved.unsignedTxs[0];
+    assert(unsignedTx, 'Missing unsignedTx in txRequest');
 
     const signablePayload = Buffer.from(unsignedTx.signableHex, 'hex');
 
@@ -765,11 +775,13 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
     let unsignedTx: UnsignedTransactionTss;
     if (requestType === RequestType.tx) {
       assert(
-        txRequestResolved.transactions || txRequestResolved.unsignedTxs,
+        (txRequestResolved.transactions && txRequestResolved.transactions.length > 0) ||
+          (txRequestResolved.unsignedTxs && txRequestResolved.unsignedTxs.length > 0),
         'Unable to find transactions in txRequest'
       );
       unsignedTx =
         apiVersion === 'full' ? txRequestResolved.transactions![0].unsignedTx : txRequestResolved.unsignedTxs[0];
+      assert(unsignedTx, 'Missing unsignedTx in txRequest');
     } else if (requestType === RequestType.message) {
       assert(txRequestResolved.messages?.length, 'Unable to find messages in txRequest for message signing');
       const message = txRequestResolved.messages[0];

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

@@ -587,9 +587,14 @@ export class EddsaMPCv2Utils extends BaseEddsaUtils {
     assert(bitgoGpgPubKey, 'Missing BitGo GPG key for MPCv2');
 
     if (requestType === RequestType.tx) {
-      assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
+      assert(
+        (txRequest.transactions && txRequest.transactions.length > 0) ||
+          (txRequest.unsignedTxs && txRequest.unsignedTxs.length > 0),
+        'Unable to find transactions in txRequest'
+      );
       const unsignedTx =
         txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs![0];
+      assert(unsignedTx, 'Missing unsignedTx in txRequest');
       txOrMessageToSign = unsignedTx.signableHex;
       assert(txOrMessageToSign, 'Missing signableHex in unsignedTx');
       derivationPath = unsignedTx.derivationPath;

modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts

@@ -1489,3 +1489,32 @@ describe('EcdsaMPCv2Utils.signEcdsaMPCv2TssUsingExternalSigner attestation', ()
     }
   });
 });
+
+describe('EcdsaMPCv2Utils.signTxRequest assert', () => {
+  it('should throw a clear error when transactions is empty', async () => {
+    const mockBg = {} as BitGoBase;
+    mockBg.getEnv = sinon.stub().returns('test');
+    mockBg.setRequestTracer = sinon.stub();
+    const mockCoin = {} as IBaseCoin;
+    mockCoin.getHashFunction = sinon.stub().callsFake(() => createKeccakHash('keccak256') as Hash);
+
+    const utils = new EcdsaMPCv2Utils(mockBg, mockCoin);
+    sinon.stub(utils as any, 'pickBitgoPubGpgKeyForSigning').resolves(bitgoGpgKey.public);
+
+    const emptyTxRequest = {
+      txRequestId: 'assert-test',
+      apiVersion: 'full',
+      transactions: [],
+      messages: [],
+    } as unknown as TxRequest;
+
+    await assert.rejects(
+      utils.signTxRequest({
+        txRequest: emptyTxRequest,
+        prv: 'fake-prv',
+        reqId: { inc: sinon.stub(), toString: sinon.stub().returns('test-req') } as any,
+      } as any),
+      /Unable to find transactions in txRequest/
+    );
+  });
+});