From 42261519168e32603a09033644460747e6daab40 Mon Sep 17 00:00:00 2001 From: shivam-bitgo Date: Fri, 10 Oct 2025 11:43:56 +0530 Subject: [PATCH] refactor(sdk-coin-apt): changes to convert argument by abi method Ticket: COIN-5799 --- .../src/lib/transaction/customTransaction.ts | 175 ++++++----- modules/sdk-coin-apt/src/lib/utils.ts | 18 ++ .../test/unit/customTransaction.test.ts | 277 ++++++++++++++++++ 3 files changed, 399 insertions(+), 71 deletions(-) create mode 100644 modules/sdk-coin-apt/test/unit/customTransaction.test.ts diff --git a/modules/sdk-coin-apt/src/lib/transaction/customTransaction.ts b/modules/sdk-coin-apt/src/lib/transaction/customTransaction.ts index 937130bc2b..ff328a3f04 100644 --- a/modules/sdk-coin-apt/src/lib/transaction/customTransaction.ts +++ b/modules/sdk-coin-apt/src/lib/transaction/customTransaction.ts @@ -2,13 +2,13 @@ import { Transaction } from './transaction'; import { BaseCoin as CoinConfig } from '@bitgo/statics'; import { InvalidTransactionError, TransactionType } from '@bitgo/sdk-core'; import { + AccountAddress, EntryFunctionABI, EntryFunctionArgumentTypes, SimpleEntryFunctionArgumentTypes, InputGenerateTransactionPayloadData, TransactionPayload, TransactionPayloadEntryFunction, - AccountAddress, TypeTagAddress, TypeTagBool, TypeTagU8, @@ -136,89 +136,119 @@ export class CustomTransaction extends Transaction { * Convert argument based on ABI type information */ private convertArgumentByABI(arg: any, paramType: any): any { - // Helper function to convert bytes to hex string - const bytesToHex = (bytes: number[]): string => { - return '0x' + bytes.map((b) => b.toString(16).padStart(2, '0')).join(''); - }; - - // Helper function to try converting a hex string to an AccountAddress - const tryToAddress = (hexStr: string): any => { - try { - return AccountAddress.fromString(hexStr); - } catch { - return hexStr; - } - }; - // Handle primitive values (string, number, boolean) if (typeof arg === 'string' || typeof arg === 'number' || typeof arg === 'boolean') { - // Address conversion for hex strings - if (paramType instanceof TypeTagAddress && typeof arg === 'string' && arg.startsWith('0x')) { - return tryToAddress(arg); - } - - // Type conversions based on parameter type - if (paramType instanceof TypeTagBool) return Boolean(arg); - if (paramType instanceof TypeTagU8 || paramType instanceof TypeTagU16 || paramType instanceof TypeTagU32) - return Number(arg); - if (paramType instanceof TypeTagU64 || paramType instanceof TypeTagU128 || paramType instanceof TypeTagU256) - return String(arg); - - return arg; + return this.convertPrimitiveArgument(arg, paramType); } // Handle BCS-encoded data with 'data' property if (arg && typeof arg === 'object' && 'data' in arg && arg.data) { - const bytes = Array.from(arg.data) as number[]; - const hexString = bytesToHex(bytes); - - return paramType instanceof TypeTagAddress ? tryToAddress(hexString) : hexString; + return this.convertBcsDataArgument(arg, paramType); } // Handle nested BCS structures with 'value' property if (arg && typeof arg === 'object' && 'value' in arg && arg.value) { - // Check if inner value is a Uint8Array (common for U64 arguments) - if (arg.value.value && arg.value.value instanceof Uint8Array) { - const bytes = Array.from(arg.value.value) as number[]; - if (this.isNumericType(paramType)) { - return this.convertNumericArgument(bytes, paramType); - } - // For non-numeric types, convert to hex string - return bytesToHex(bytes); - } + return this.convertNestedBcsArgument(arg, paramType); + } - // Simple value wrapper - if (!('value' in arg.value) || typeof arg.value.value !== 'object') { - return this.convertArgumentByABI(arg.value, paramType); - } + // For anything else, return as-is + return arg; + } + + /** + * Convert primitive argument values based on parameter type + */ + private convertPrimitiveArgument( + arg: string | number | boolean, + paramType: any + ): string | number | boolean | AccountAddress { + // Address conversion for hex strings + if (paramType instanceof TypeTagAddress && typeof arg === 'string' && arg.startsWith('0x')) { + return utils.tryParseAccountAddress(arg); + } + + // Type conversions based on parameter type + if (paramType instanceof TypeTagBool) return Boolean(arg); + + // Use unified numeric type handling + if (this.isNumericType(paramType)) { + return this.convertPrimitiveNumericArgument(arg); + } - // Double nested structure with numeric keys - const bytesObj = arg.value.value; - const keys = Object.keys(bytesObj) - .filter((k) => !isNaN(parseInt(k, 10))) - .sort((a, b) => parseInt(a, 10) - parseInt(b, 10)); + return arg; + } - if (keys.length === 0) return arg; + /** + * Convert primitive numeric arguments to string + * - Big numbers break JavaScript (precision loss) + * - String safer for large U64/U128/U256 values + * - Aptos SDK converts string to correct type automatically + */ + private convertPrimitiveNumericArgument(arg: string | number | boolean): string { + // Always string - safer for big numbers + return String(arg); + } - const bytes = keys.map((k) => bytesObj[k]); - let extractedValue: any; + /** + * Convert BCS data argument with 'data' property + */ + private convertBcsDataArgument(arg: any, paramType: any): string | AccountAddress { + const bytes = Array.from(arg.data) as number[]; + const hexString = utils.bytesToHex(bytes); - // Convert bytes based on parameter type using unified approach + return paramType instanceof TypeTagAddress ? utils.tryParseAccountAddress(hexString) : hexString; + } + + /** + * Convert nested BCS argument with 'value' property + */ + private convertNestedBcsArgument(arg: any, paramType: any): any { + // Check if inner value is a Uint8Array (common for U64 arguments) + if (arg.value.value && arg.value.value instanceof Uint8Array) { + const bytes = Array.from(arg.value.value) as number[]; if (this.isNumericType(paramType)) { - extractedValue = this.convertNumericArgument(bytes, paramType); - } else if (paramType instanceof TypeTagAddress) { - extractedValue = bytesToHex(bytes); - } else if (paramType instanceof TypeTagBool) { - extractedValue = bytes[0] === 1; - } else { - extractedValue = bytesToHex(bytes); + return this.convertNumericArgument(bytes, paramType); } + // For non-numeric types, convert to hex string + return utils.bytesToHex(bytes); + } - return this.convertArgumentByABI(extractedValue, paramType); + // Simple value wrapper - fix the bug in original implementation + if (typeof arg.value !== 'object' || !('value' in arg.value)) { + return this.convertArgumentByABI(arg.value, paramType); } - // For anything else, return as-is - return arg; + // Double nested structure with numeric keys + const bytes = this.extractBytesFromBcsObject(arg.value.value); + if (bytes.length === 0) return arg; + + let extractedValue: any; + + // Convert bytes based on parameter type using unified approach + if (this.isNumericType(paramType)) { + extractedValue = this.convertNumericArgument(bytes, paramType); + } else if (paramType instanceof TypeTagAddress) { + extractedValue = utils.bytesToHex(bytes); + } else if (paramType instanceof TypeTagBool) { + extractedValue = bytes[0] === 1; + } else { + extractedValue = utils.bytesToHex(bytes); + } + + return this.convertArgumentByABI(extractedValue, paramType); + } + + /** + * Extract bytes from BCS object with numeric keys + */ + private extractBytesFromBcsObject(bcsObject: any): number[] { + const keys = Object.keys(bcsObject) + .filter((k) => !isNaN(parseInt(k, 10))) + .sort((a, b) => parseInt(a, 10) - parseInt(b, 10)); + + if (keys.length === 0) return []; + + return keys.map((k) => bcsObject[k]); } /** @@ -282,24 +312,27 @@ export class CustomTransaction extends Transaction { } /** - * Convert numeric argument using consistent little-endian byte handling + * Convert byte arrays to string numbers + * - Small types (U8/U16/U32): 1-4 bytes, parse manually + * - Large types (U64/U128/U256): 8+ bytes, use existing utility + * - Both return string for consistency */ - private convertNumericArgument(bytes: number[], paramType: any): any { + private convertNumericArgument(bytes: number[], paramType: any): string { if (paramType instanceof TypeTagU8 || paramType instanceof TypeTagU16 || paramType instanceof TypeTagU32) { - // Small integers: use Number for compatibility + // Small types: parse bytes manually (1-4 bytes) let result = 0; for (let i = bytes.length - 1; i >= 0; i--) { result = result * 256 + bytes[i]; } - return result; + return result.toString(); } if (paramType instanceof TypeTagU64 || paramType instanceof TypeTagU128 || paramType instanceof TypeTagU256) { - // Large integers: reuse the existing utility method + // Large types: use existing method (needs 8+ bytes) return utils.getAmountFromPayloadArgs(new Uint8Array(bytes)); } - // Fallback for unexpected numeric types - return '0x' + bytes.map((b) => b.toString(16).padStart(2, '0')).join(''); + // Unknown type: convert to hex + return utils.bytesToHex(bytes); } } diff --git a/modules/sdk-coin-apt/src/lib/utils.ts b/modules/sdk-coin-apt/src/lib/utils.ts index d3025ba9ea..cb4afb908a 100644 --- a/modules/sdk-coin-apt/src/lib/utils.ts +++ b/modules/sdk-coin-apt/src/lib/utils.ts @@ -199,6 +199,24 @@ export class Utils implements BaseUtils { getTxnExpirationTimestamp(): number { return Math.floor(Date.now() / 1e3) + SECONDS_PER_WEEK; } + + /** + * Convert bytes array to hex string with 0x prefix + */ + bytesToHex(bytes: number[]): string { + return '0x' + bytes.map((b) => b.toString(16).padStart(2, '0')).join(''); + } + + /** + * Try to convert hex string to AccountAddress, return original string if conversion fails + */ + tryParseAccountAddress(hexStr: string): AccountAddress | string { + try { + return AccountAddress.fromString(hexStr); + } catch { + return hexStr; + } + } } const utils = new Utils(); diff --git a/modules/sdk-coin-apt/test/unit/customTransaction.test.ts b/modules/sdk-coin-apt/test/unit/customTransaction.test.ts new file mode 100644 index 0000000000..797bc78b24 --- /dev/null +++ b/modules/sdk-coin-apt/test/unit/customTransaction.test.ts @@ -0,0 +1,277 @@ +import { coins } from '@bitgo/statics'; +import { CustomTransaction } from '../../src/lib/transaction/customTransaction'; +import { TypeTagAddress, TypeTagBool, TypeTagU8, TypeTagU64, TypeTagU128 } from '@aptos-labs/ts-sdk'; +import should from 'should'; +import * as testData from '../resources/apt'; + +describe('CustomTransaction - Argument Conversion', () => { + let customTransaction: CustomTransaction; + + beforeEach(() => { + customTransaction = new CustomTransaction(coins.get('tapt')); + }); + + // Helper function to access private method for testing + const callConvertArgumentByABI = (transaction: CustomTransaction, arg: any, paramType: any): any => { + return (transaction as any).convertArgumentByABI(arg, paramType); + }; + + describe('Real Transaction Scenarios', () => { + describe('Common Primitive Arguments', () => { + it('should handle address strings from test data', () => { + // Test with actual addresses from test resources + const result1 = callConvertArgumentByABI(customTransaction, testData.sender.address, new TypeTagAddress()); + const result2 = callConvertArgumentByABI( + customTransaction, + testData.recipients[0].address, + new TypeTagAddress() + ); + + should.exist(result1); + should.exist(result2); + + // Results should be AccountAddress objects or original strings + if (typeof result1 === 'object') { + should.exist(result1.toString); + } + if (typeof result2 === 'object') { + should.exist(result2.toString); + } + }); + + it('should handle amount strings from test data', () => { + // Test with actual amounts from test resources + const result1 = callConvertArgumentByABI(customTransaction, testData.AMOUNT.toString(), new TypeTagU64()); + const result2 = callConvertArgumentByABI(customTransaction, testData.recipients[0].amount, new TypeTagU64()); + + should.equal(result1, testData.AMOUNT.toString()); + should.equal(result2, testData.recipients[0].amount); + }); + + it('should handle numeric amounts as numbers for small types', () => { + const result1 = callConvertArgumentByABI(customTransaction, 255, new TypeTagU8()); + const result2 = callConvertArgumentByABI(customTransaction, testData.AMOUNT, new TypeTagU64()); + + should.equal(result1, 255); + should.equal(result2, testData.AMOUNT.toString()); // Large numbers become strings + }); + + it('should handle boolean values correctly', () => { + const result1 = callConvertArgumentByABI(customTransaction, true, new TypeTagBool()); + const result2 = callConvertArgumentByABI(customTransaction, false, new TypeTagBool()); + + should.equal(result1, true); + should.equal(result2, false); + }); + }); + + describe('BCS Encoded Arguments from Real Transactions', () => { + it('should handle BCS data with data property (from serialized tx)', () => { + // This simulates BCS-encoded data structure found in real transactions + const bcsData = { + data: new Uint8Array([0xe8, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]), // 1000 in little-endian + }; + + const result = callConvertArgumentByABI(customTransaction, bcsData, new TypeTagU64()); + should.equal(result, '0xe803000000000000'); + }); + + it('should handle BCS address data', () => { + // Create proper 32-byte address data as found in real transactions + const addressBytes = new Array(32).fill(0); + addressBytes[31] = 0x01; // Standard address format + + const bcsData = { + data: new Uint8Array(addressBytes), + }; + + const result = callConvertArgumentByABI(customTransaction, bcsData, new TypeTagAddress()); + should.exist(result); + + if (typeof result === 'string') { + result.should.match(/^0x[0-9a-fA-F]+$/); + } + }); + }); + + describe('Nested BCS Structures (Real Transaction Parsing)', () => { + it('should handle U64 amount in nested structure (typical from parsed tx)', () => { + // This structure is common when parsing amounts from real Aptos transactions + const nestedAmount = { + value: { + value: new Uint8Array([0xe8, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]), // 1000 + }, + }; + + const result = callConvertArgumentByABI(customTransaction, nestedAmount, new TypeTagU64()); + should.equal(result, '1000'); + }); + + it('should handle double nested numeric structure from BCS parsing', () => { + // This represents the typical structure when deserializing BCS-encoded numeric values + const nestedData = { + value: { + value: { + 0: 0xe8, // 1000 in little-endian format + 1: 0x03, + 2: 0x00, + 3: 0x00, + 4: 0x00, + 5: 0x00, + 6: 0x00, + 7: 0x00, + }, + }, + }; + + const result = callConvertArgumentByABI(customTransaction, nestedData, new TypeTagU64()); + should.equal(result, '1000'); + }); + + it('should handle nested structure for boolean values', () => { + // Boolean values in BCS format + const booleanTrue = { + value: { + value: { + 0: 1, + }, + }, + }; + + const booleanFalse = { + value: { + value: { + 0: 0, + }, + }, + }; + + const result1 = callConvertArgumentByABI(customTransaction, booleanTrue, new TypeTagBool()); + const result2 = callConvertArgumentByABI(customTransaction, booleanFalse, new TypeTagBool()); + + should.equal(result1, true); + should.equal(result2, false); + }); + + it('should handle simple nested value (fix for original bug)', () => { + // This tests the bug fix where primitive values in nested structures failed + const nestedPrimitive = { + value: '1000', + }; + + const result = callConvertArgumentByABI(customTransaction, nestedPrimitive, new TypeTagU64()); + should.equal(result, '1000'); // Should extract and convert the primitive value + }); + }); + + describe('Large Integer Types (Common in DeFi)', () => { + it('should handle U128 amounts (common for high-precision tokens)', () => { + const largeAmount = '340282366920938463463374607431768211455'; // Near max U128 + const result = callConvertArgumentByABI(customTransaction, largeAmount, new TypeTagU128()); + + should.equal(result, largeAmount); + }); + + it('should handle U128 in BCS format', () => { + // 16-byte array for U128 (example: value 1000) + const bytes = new Array(16).fill(0); + bytes[0] = 0xe8; + bytes[1] = 0x03; + + const nestedData = { + value: { + value: Object.fromEntries(bytes.map((byte, index) => [index.toString(), byte])), + }, + }; + + const result = callConvertArgumentByABI(customTransaction, nestedData, new TypeTagU128()); + should.equal(result, '1000'); + }); + }); + + describe('Error Handling for Real Edge Cases', () => { + it('should handle empty BCS structures gracefully', () => { + const emptyStructure = { + value: { + value: {}, + }, + }; + + const result = callConvertArgumentByABI(customTransaction, emptyStructure, new TypeTagU64()); + should.equal(result, emptyStructure); // Should return original when no valid bytes found + }); + + it('should handle non-BCS objects', () => { + const regularObject = { someProperty: 'value' }; + const result = callConvertArgumentByABI(customTransaction, regularObject, new TypeTagU64()); + + should.equal(result, regularObject); // Should return as-is for non-BCS objects + }); + + it('should handle null and undefined (defensive)', () => { + const result1 = callConvertArgumentByABI(customTransaction, null, new TypeTagU64()); + const result2 = callConvertArgumentByABI(customTransaction, undefined, new TypeTagU64()); + + should.equal(result1, null); + should.equal(result2, undefined); + }); + }); + + describe('Integration with Utility Functions', () => { + it('should use bytesToHex utility for non-numeric BCS data', () => { + const bcsData = { + data: new Uint8Array([0x12, 0x34, 0x56, 0x78]), + }; + + const result = callConvertArgumentByABI(customTransaction, bcsData, new TypeTagAddress()); + // Should either be converted to AccountAddress or remain as hex string + should.exist(result); + }); + + it('should use tryParseAccountAddress utility for address strings', () => { + const validAddress = '0x0000000000000000000000000000000000000000000000000000000000000001'; + const result = callConvertArgumentByABI(customTransaction, validAddress, new TypeTagAddress()); + + should.exist(result); + // Should either be AccountAddress object or original string + if (typeof result === 'object') { + should.exist(result.toString); + } + }); + + it('should use convertNumericArgument utility for byte arrays', () => { + const uint8Data = new Uint8Array([0xe8, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]); // 1000 + const nestedData = { + value: { + value: uint8Data, + }, + }; + + const result = callConvertArgumentByABI(customTransaction, nestedData, new TypeTagU64()); + should.equal(result, '1000'); // Should use existing numeric conversion logic + }); + }); + }); + + describe('Round-trip Compatibility', () => { + it('should maintain compatibility with existing transaction parsing', () => { + // This ensures our refactored method works with the existing transaction builder tests + const testArguments = [ + { arg: testData.sender.address, type: new TypeTagAddress() }, + { arg: testData.AMOUNT.toString(), type: new TypeTagU64() }, + { arg: true, type: new TypeTagBool() }, + { arg: 255, type: new TypeTagU8() }, + ]; + + testArguments.forEach(({ arg, type }) => { + const result = callConvertArgumentByABI(customTransaction, arg, type); + should.exist(result); + + // Result should be compatible with existing transaction building logic + if (typeof result === 'string' || typeof result === 'number' || typeof result === 'boolean') { + should.exist(result.toString); + } + }); + }); + }); +});