Merge pull request #8494 from BitGo/cecho-697-fix

feat(sdk-coin-tempo): enhance transaction validation and error handling

Author: nayandas190

modules/sdk-coin-tempo/src/lib/transactionBuilder.ts

@@ -322,7 +322,15 @@ export class Tip20TransactionBuilder extends AbstractTransactionBuilder {
       throw new BuildTransactionError(`Invalid contract address: ${call.to}`);
     }
     if (!isValidHexData(call.data)) {
-      throw new BuildTransactionError(`Invalid calldata: must be a non-empty 0x-prefixed hex string`);
+      throw new BuildTransactionError(`Invalid calldata: must be a non-empty 0x-prefixed hex string with even length`);
+    }
+    if (call.value !== undefined) {
+      try {
+        const v = BigInt(call.value);
+        if (v < 0n) throw new Error();
+      } catch {
+        throw new BuildTransactionError(`Invalid value: must be a non-negative integer string`);
+      }
     }
     this.rawCalls.push(call);
     return this;

modules/sdk-coin-tempo/src/lib/utils.ts

@@ -154,7 +154,7 @@ export function isValidMemoId(memoId: string): boolean {
  * Used to validate pre-encoded calldata for raw contract calls
  */
 export function isValidHexData(data: string): boolean {
-  return typeof data === 'string' && /^0x[0-9a-fA-F]+$/.test(data) && data.length > 2;
+  return typeof data === 'string' && /^0x[0-9a-fA-F]+$/.test(data) && data.length > 2 && data.length % 2 === 0;
 }
 
 const utils = {

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

@@ -258,43 +258,41 @@ export class Tempo extends AbstractEthLikeNewCoins {
     const operations = tx.getOperations();
     const rawCalls = tx.getRawCalls();
 
-    // If the caller specified explicit recipients, verify they match operations and raw calls 1-to-1
+    // If the caller specified explicit recipients, verify they match the transaction.
+    // A transaction is either all token transfer operations OR a single raw contract call — never mixed.
     const recipients = txParams?.recipients;
     if (recipients && recipients.length > 0) {
-      const totalCallCount = operations.length + rawCalls.length;
-      if (totalCallCount !== recipients.length) {
-        throw new Error(
-          `Transaction has ${totalCallCount} call(s) but ${recipients.length} recipient(s) were requested`
-        );
-      }
-
-      let opIndex = 0;
-      let rawIndex = 0;
-      for (let i = 0; i < recipients.length; i++) {
-        const recipient = recipients[i];
-        if (recipient.data) {
-          // Contract call recipient — verify against rawCalls
-          const rawCall = rawCalls[rawIndex++];
-          if (!rawCall) {
-            throw new Error(`Missing raw call for recipient ${i}`);
-          }
+      if (rawCalls.length > 0) {
+        // Contract call transaction — single raw call, single recipient with data
+        if (rawCalls.length !== recipients.length) {
+          throw new Error(
+            `Transaction has ${rawCalls.length} call(s) but ${recipients.length} recipient(s) were requested`
+          );
+        }
+        for (let i = 0; i < rawCalls.length; i++) {
+          const rawCall = rawCalls[i];
+          const recipient = recipients[i];
           if (rawCall.to.toLowerCase() !== recipient.address.split('?')[0].toLowerCase()) {
             throw new Error(`Raw call ${i} address mismatch: expected ${recipient.address}, got ${rawCall.to}`);
           }
-          if (rawCall.data !== recipient.data) {
+          if (!recipient.data || rawCall.data.toLowerCase() !== recipient.data.toLowerCase()) {
             throw new Error(`Raw call ${i} calldata mismatch`);
           }
-        } else {
-          // Token transfer recipient — verify against operations
-          const op = operations[opIndex++];
-          if (!op) {
-            throw new Error(`Missing operation for recipient ${i}`);
-          }
+        }
+      } else {
+        // Token transfer transaction — operations matched 1-to-1 against recipients
+        if (operations.length !== recipients.length) {
+          throw new Error(
+            `Transaction has ${operations.length} operation(s) but ${recipients.length} recipient(s) were requested`
+          );
+        }
+        for (let i = 0; i < operations.length; i++) {
+          const op = operations[i];
+          const recipient = recipients[i];
           const recipientBaseAddress = recipient.address.split('?')[0];
           if (op.to.toLowerCase() !== recipientBaseAddress.toLowerCase()) {
             throw new Error(`Operation ${i} recipient mismatch: expected ${recipient.address}, got ${op.to}`);
           }
-          // Compare amounts in base units (smallest denomination)
           const opAmountBaseUnits = amountToTip20Units(op.amount).toString();
           if (opAmountBaseUnits !== recipient.amount.toString()) {
             throw new Error(`Operation ${i} amount mismatch: expected ${recipient.amount}, got ${opAmountBaseUnits}`);

modules/sdk-coin-tempo/test/resources/tempo.ts

@@ -77,7 +77,7 @@ export const MEMO_TEST_CASES = {
 // ============================================================================
 
 export const ERROR_MESSAGES = {
-  noOperations: /At least one operation is required/,
+  noOperations: /At least one operation or raw call is required/,
   missingNonce: /Nonce is required/,
   missingGas: /Gas limit is required/,
   missingMaxFeePerGas: /maxFeePerGas is required/,

modules/sdk-coin-tempo/test/unit/transactionBuilder.ts

@@ -677,7 +677,7 @@ describe('Tempo coin - parseTransaction / verifyTransaction', () => {
               ],
             },
           }),
-        /call\(s\)/
+        /operation\(s\)/
       );
     });
 
@@ -819,6 +819,21 @@ describe('Raw Contract Call Builder', () => {
       assert.throws(() => builder.addRawCall({ to: mockContract, data: 'a9059cbb' }), /Invalid calldata/);
     });
 
+    it('should throw for odd-length hex calldata', () => {
+      const builder = new Tip20TransactionBuilder(mockCoinConfig);
+      assert.throws(() => builder.addRawCall({ to: mockContract, data: '0xabc' }), /Invalid calldata/);
+    });
+
+    it('should throw for invalid value string', () => {
+      const builder = new Tip20TransactionBuilder(mockCoinConfig);
+      assert.throws(() => builder.addRawCall({ to: mockContract, data: mockCalldata, value: '1e18' }), /Invalid value/);
+    });
+
+    it('should throw for negative value', () => {
+      const builder = new Tip20TransactionBuilder(mockCoinConfig);
+      assert.throws(() => builder.addRawCall({ to: mockContract, data: mockCalldata, value: '-1' }), /Invalid value/);
+    });
+
     it('should allow raw-call-only transaction (no operations)', async () => {
       const builder = new Tip20TransactionBuilder(mockCoinConfig);
       builder
@@ -906,61 +921,19 @@ describe('Raw Contract Call Builder', () => {
     });
   });
 
-  describe('Mixed: operations + raw calls', () => {
-    it('should build and round-trip a transaction with both operations and raw calls', async () => {
-      const tokenAddress = ethers.utils.getAddress(TESTNET_TOKENS.alphaUSD.address);
-      const recipientAddress = ethers.utils.getAddress(TEST_RECIPIENT_ADDRESS);
-
+  describe('Raw call toJson and outputs', () => {
+    it('should expose raw call contract address as output', async () => {
       const builder = new Tip20TransactionBuilder(mockCoinConfig);
       builder
-        .addOperation({ token: tokenAddress, to: recipientAddress, amount: '10.0', memo: '1' })
-        .addRawCall({ to: mockContract, data: mockCalldata })
-        .nonce(1)
-        .gas(200000n)
-        .maxFeePerGas(TX_PARAMS.defaultMaxFeePerGas)
-        .maxPriorityFeePerGas(TX_PARAMS.defaultMaxPriorityFeePerGas);
-
-      const originalTx = (await builder.build()) as Tip20Transaction;
-      assert.strictEqual(originalTx.getOperations().length, 1);
-      assert.strictEqual(originalTx.getRawCalls().length, 1);
-      assert.strictEqual(originalTx.getOperationCount(), 2);
-
-      const serialized = await originalTx.serialize();
-
-      const builder2 = new Tip20TransactionBuilder(mockCoinConfig);
-      builder2.from(serialized);
-      const restoredTx = (await builder2.build()) as Tip20Transaction;
-
-      assert.strictEqual(restoredTx.getOperations().length, 1);
-      assert.strictEqual(restoredTx.getRawCalls().length, 1);
-      assert.strictEqual(restoredTx.getOperationCount(), 2);
-
-      const ops = restoredTx.getOperations();
-      assert.strictEqual(ops[0].to.toLowerCase(), recipientAddress.toLowerCase());
-      assert.strictEqual(ops[0].amount, '10.0');
-
-      const rawCalls = restoredTx.getRawCalls();
-      assert.strictEqual(rawCalls[0].to.toLowerCase(), mockContract.toLowerCase());
-      assert.strictEqual(rawCalls[0].data.toLowerCase(), mockCalldata.toLowerCase());
-    });
-
-    it('should expose outputs for both operations and raw calls', async () => {
-      const tokenAddress = ethers.utils.getAddress(TESTNET_TOKENS.alphaUSD.address);
-      const recipientAddress = ethers.utils.getAddress(TEST_RECIPIENT_ADDRESS);
-
-      const builder = new Tip20TransactionBuilder(mockCoinConfig);
-      builder
-        .addOperation({ token: tokenAddress, to: recipientAddress, amount: '5.0' })
         .addRawCall({ to: mockContract, data: mockCalldata, value: '0' })
         .nonce(2)
         .gas(200000n)
         .maxFeePerGas(TX_PARAMS.defaultMaxFeePerGas)
         .maxPriorityFeePerGas(TX_PARAMS.defaultMaxPriorityFeePerGas);
 
       const tx = (await builder.build()) as Tip20Transaction;
-      assert.strictEqual(tx.outputs.length, 2);
-      assert.strictEqual(tx.outputs[0].address.toLowerCase(), recipientAddress.toLowerCase());
-      assert.strictEqual(tx.outputs[1].address.toLowerCase(), mockContract.toLowerCase());
+      assert.strictEqual(tx.outputs.length, 1);
+      assert.strictEqual(tx.outputs[0].address.toLowerCase(), mockContract.toLowerCase());
     });
 
     it('should include rawCalls in toJson() output', async () => {