Merge pull request #7485 from BitGo/WP-6717-construct-pending-approval-tx-type-validation

fix(express): construct pending approval type codec

Author: lokesh-bitgo

modules/express/src/typedRoutes/api/v1/constructPendingApprovalTx.ts

@@ -14,50 +14,79 @@ export const ConstructPendingApprovalTxRequestParams = {
  * Request body for constructing a pending approval transaction
  */
 export const ConstructPendingApprovalTxRequestBody = {
-  /** The wallet passphrase to decrypt the user key (either walletPassphrase or xprv must be provided for transactionRequest type) */
+  /**
+   * The wallet passphrase to decrypt the user key. Required for transactionRequest type approvals
+   * if xprv is not provided. Use this to sign the transaction with the wallet's encrypted key.
+   */
   walletPassphrase: optional(t.string),
-  /** The extended private key (alternative to walletPassphrase) */
+  /**
+   * The extended private key in BIP32 format (xprv...). Alternative to walletPassphrase.
+   * Required for transactionRequest type approvals if walletPassphrase is not provided.
+   */
   xprv: optional(t.string),
-  /** Whether to use the original fee from the transaction request (cannot be used with fee, feeRate, or feeTxConfirmTarget) */
+  /**
+   * Whether to use the original fee from the transaction request. Applies to transactionRequest
+   * type approvals only. Cannot be used with fee, feeRate, or feeTxConfirmTarget.
+   */
   useOriginalFee: optional(t.boolean),
-  /** Custom fee amount in satoshis (cannot be used with useOriginalFee) */
+  /**
+   * Custom fee amount in satoshis to use for the transaction. Cannot be used with useOriginalFee.
+   */
   fee: optional(t.number),
-  /** Custom fee rate in satoshis per kilobyte (cannot be used with useOriginalFee) */
+  /**
+   * Custom fee rate in satoshis per kilobyte. Cannot be used with useOriginalFee.
+   */
   feeRate: optional(t.number),
-  /** Custom fee confirmation target in blocks (cannot be used with useOriginalFee) */
+  /**
+   * Target number of blocks for fee estimation. The fee will be calculated to confirm the
+   * transaction within this many blocks. Cannot be used with useOriginalFee.
+   */
   feeTxConfirmTarget: optional(t.number),
 };
 
 /**
  * Response for constructing a pending approval transaction
  */
 export const ConstructPendingApprovalTxResponse = t.type({
-  /** The signed transaction hex */
+  /** The signed transaction in hex-encoded format, ready for broadcast to the network */
   tx: t.string,
-  /** The fee amount in satoshis */
+  /** The total fee amount in satoshis paid for this transaction (optional) */
   fee: optional(t.number),
-  /** The fee rate in satoshis per kilobyte */
+  /** The fee rate in satoshis per kilobyte used for this transaction (optional) */
   feeRate: optional(t.number),
-  /** Whether the transaction is instant */
+  /** Whether this is an instant (RBF-disabled) transaction that cannot be fee-bumped (optional) */
   instant: optional(t.boolean),
-  /** The BitGo fee amount */
+  /** BitGo service fee information including amount and address (optional) */
   bitgoFee: optional(t.unknown),
-  /** Travel information */
+  /** Travel Rule compliance information for regulated transactions (optional) */
   travelInfos: optional(t.unknown),
-  /** Estimated transaction size in bytes */
+  /** Estimated size of the transaction in bytes (optional) */
   estimatedSize: optional(t.number),
-  /** Unspent transaction outputs used */
+  /** Array of unspent transaction outputs (UTXOs) used as inputs in this transaction (optional) */
   unspents: optional(t.array(t.unknown)),
 });
 
 /**
  * Construct a pending approval transaction
  *
- * This endpoint constructs and signs a transaction for a pending approval, returning the transaction hex
- * but not sending it to the network. This is useful for reviewing the transaction before approving it.
+ * Constructs and signs a transaction for a pending approval without broadcasting it to the network.
+ * This endpoint allows you to preview and validate the transaction details before final approval.
  *
- * For transaction request type approvals, either a wallet passphrase or xprv must be provided to sign the transaction.
- * You can optionally specify fee-related parameters to customize the transaction fee.
+ * **Authentication Requirements:**
+ * - For transactionRequest type approvals, you must provide either walletPassphrase or xprv to sign the transaction
+ * - The user must have permission to approve the pending approval
+ *
+ * **Fee Customization:**
+ * - Use `useOriginalFee: true` to preserve the fee from the original transaction request
+ * - Alternatively, specify custom fee parameters (fee, feeRate, or feeTxConfirmTarget)
+ * - Fee parameters cannot be combined with useOriginalFee
+ *
+ * **Workflow:**
+ * 1. Retrieves the pending approval by ID
+ * 2. Constructs the transaction according to the approval parameters
+ * 3. Signs the transaction (for transactionRequest approvals)
+ * 4. Returns the signed transaction hex and metadata
+ * 5. Transaction is NOT broadcast to the network
  *
  * @operationId express.v1.pendingapproval.constructTx
  * @tag express
@@ -70,9 +99,16 @@ export const PutConstructPendingApprovalTx = httpRoute({
     body: ConstructPendingApprovalTxRequestBody,
   }),
   response: {
-    /** Successfully constructed transaction */
+    /** Successfully constructed and signed the transaction. Returns transaction hex and metadata. */
     200: ConstructPendingApprovalTxResponse,
-    /** Invalid request or construction fails */
+    /**
+     * Bad request. Possible reasons:
+     * - Invalid pending approval ID
+     * - Missing required authentication (walletPassphrase or xprv for transactionRequest)
+     * - Invalid fee parameters (e.g., combining useOriginalFee with other fee options)
+     * - Incorrect wallet passphrase or xprv
+     * - Transaction construction failed
+     */
     400: BitgoExpressError,
   },
 });

modules/express/test/unit/typedRoutes/constructPendingApprovalTx.ts

@@ -192,13 +192,20 @@ describe('ConstructPendingApprovalTx codec tests', function () {
   });
 
   describe('ConstructPendingApprovalTxResponse', function () {
-    it('should validate response with required tx field', function () {
+    it('should validate response with only required tx field', function () {
       const validResponse = {
         tx: '0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
       };
 
       const decoded = assertDecode(ConstructPendingApprovalTxResponse, validResponse);
       assert.strictEqual(decoded.tx, validResponse.tx);
+      assert.strictEqual(decoded.fee, undefined);
+      assert.strictEqual(decoded.feeRate, undefined);
+      assert.strictEqual(decoded.instant, undefined);
+      assert.strictEqual(decoded.bitgoFee, undefined);
+      assert.strictEqual(decoded.travelInfos, undefined);
+      assert.strictEqual(decoded.estimatedSize, undefined);
+      assert.strictEqual(decoded.unspents, undefined);
     });
 
     it('should validate response with all fields', function () {
@@ -244,7 +251,7 @@ describe('ConstructPendingApprovalTx codec tests', function () {
       });
     });
 
-    it('should reject response with non-number fee', function () {
+    it('should reject response with non-number fee (optional field, but must be number if provided)', function () {
       const invalidResponse = {
         tx: '0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
         fee: '10000', // string instead of number
@@ -255,7 +262,7 @@ describe('ConstructPendingApprovalTx codec tests', function () {
       });
     });
 
-    it('should reject response with non-number feeRate', function () {
+    it('should reject response with non-number feeRate (optional field, but must be number if provided)', function () {
       const invalidResponse = {
         tx: '0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
         feeRate: '20000', // string instead of number
@@ -266,7 +273,7 @@ describe('ConstructPendingApprovalTx codec tests', function () {
       });
     });
 
-    it('should reject response with non-boolean instant', function () {
+    it('should reject response with non-boolean instant (optional field, but must be boolean if provided)', function () {
       const invalidResponse = {
         tx: '0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
         instant: 'false', // string instead of boolean
@@ -276,7 +283,8 @@ describe('ConstructPendingApprovalTx codec tests', function () {
         assertDecode(ConstructPendingApprovalTxResponse, invalidResponse);
       });
     });
-    it('should reject response with non-number estimatedSize', function () {
+
+    it('should reject response with non-number estimatedSize (optional field, but must be number if provided)', function () {
       const invalidResponse = {
         tx: '0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
         estimatedSize: '256', // string instead of number
@@ -386,6 +394,8 @@ describe('ConstructPendingApprovalTx codec tests', function () {
       fee: 10000,
       feeRate: 20000,
       instant: false,
+      estimatedSize: 256,
+      unspents: [{ id: 'unspent1', value: 1000000 }],
     };
 
     afterEach(function () {
@@ -523,6 +533,46 @@ describe('ConstructPendingApprovalTx codec tests', function () {
       assert.strictEqual(decodedResponse.fee, 15000);
     });
 
+    it('should successfully construct a pending approval transaction with minimal response (only tx)', async function () {
+      const requestBody = {
+        walletPassphrase: 'mySecurePassphrase',
+      };
+
+      const minimalMockResponse = {
+        tx: '0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
+      };
+
+      const mockPendingApproval = {
+        constructApprovalTx: sinon.stub().resolves(minimalMockResponse),
+      };
+
+      const mockPendingApprovals = {
+        get: sinon.stub().resolves(mockPendingApproval),
+      };
+
+      sinon.stub(BitGo.prototype, 'pendingApprovals').returns(mockPendingApprovals as any);
+
+      const result = await agent
+        .put('/api/v1/pendingapprovals/test-approval-id-123/constructTx')
+        .set('Authorization', 'Bearer test_access_token_12345')
+        .set('Content-Type', 'application/json')
+        .send(requestBody);
+
+      assert.strictEqual(result.status, 200);
+      result.body.should.have.property('tx');
+      assert.strictEqual(result.body.tx, minimalMockResponse.tx);
+
+      const decodedResponse = assertDecode(ConstructPendingApprovalTxResponse, result.body);
+      assert.strictEqual(decodedResponse.tx, minimalMockResponse.tx);
+      assert.strictEqual(decodedResponse.fee, undefined);
+      assert.strictEqual(decodedResponse.feeRate, undefined);
+      assert.strictEqual(decodedResponse.instant, undefined);
+      assert.strictEqual(decodedResponse.bitgoFee, undefined);
+      assert.strictEqual(decodedResponse.travelInfos, undefined);
+      assert.strictEqual(decodedResponse.estimatedSize, undefined);
+      assert.strictEqual(decodedResponse.unspents, undefined);
+    });
+
     it('should successfully construct a pending approval transaction with all optional response fields', async function () {
       const requestBody = {
         walletPassphrase: 'mySecurePassphrase',
@@ -866,7 +916,7 @@ describe('ConstructPendingApprovalTx codec tests', function () {
       });
     });
 
-    it('should reject response with wrong type for fee', async function () {
+    it('should reject response with wrong type for fee (optional but must be number if provided)', async function () {
       const requestBody = {
         walletPassphrase: 'mySecurePassphrase',
       };