From e2347049f7edd7407cafdbfc41dd2a1ec4d9cf78 Mon Sep 17 00:00:00 2001 From: Sreeraj S Date: Mon, 31 Aug 2026 08:10:19 +0000 Subject: [PATCH] fix(sdk-coin-sui): preserve custom transaction amount precision Aggregate custom transaction recipient amounts with BigNumber instead of converting external u64 values to JavaScript numbers. Add a regression test and document the cross-coin audit findings because rounded Sui amounts can produce incorrect transaction explanations and input totals. Ticket: CSHLD-1018 Session-Id: c3c753a4-4f1a-4c00-a472-9e69452eb9da Task-Id: 9ebc0ba0-6ecb-47d2-854b-bad260bb0691 --- docs/javascript-precision-audit-CSHLD-1018.md | 29 +++++++++++++++++++ .../sdk-coin-sui/src/lib/customTransaction.ts | 13 +++++++-- .../customTransactionBuilder.ts | 17 ++++++++++- 3 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 docs/javascript-precision-audit-CSHLD-1018.md diff --git a/docs/javascript-precision-audit-CSHLD-1018.md b/docs/javascript-precision-audit-CSHLD-1018.md new file mode 100644 index 0000000000..3a0126b3fc --- /dev/null +++ b/docs/javascript-precision-audit-CSHLD-1018.md @@ -0,0 +1,29 @@ +# JavaScript numeric precision audit + +Ticket: CSHLD-1018 + +## Method + +Audited TypeScript under `modules/sdk-coin-*`, `modules/abstract-*`, and +shared transaction code for `Number`, `parseInt`, `parseFloat`, and big-number +`toNumber()` conversions. Values were classified as metadata (indexes, +versions, timestamps, opcodes) or blockchain quantities (amounts, balances, +fees, and serialized integer fields). + +## Findings and disposition + +| Area | Risk | Disposition | +| --- | --- | --- | +| Sui custom transaction recipient aggregation | Recipient amounts are external decimal strings and can be u64 values. Converting each amount with `Number()` rounded values above `Number.MAX_SAFE_INTEGER` before summing and explaining a transaction. | Fixed in `sdk-coin-sui/src/lib/customTransaction.ts` by aggregating with `BigNumber` and returning a fixed-point decimal string. | +| Sui transfer, token-transfer, staking, and Walrus paths | Existing code uses `BigNumber` for amount aggregation and `BigInt`/string-preserving paths for serialized u64 values. A few `Number()` conversions are limited to protocol metadata or SDK APIs that require JavaScript numbers; these require separate API-compatible changes before replacement. | Confirmed safe for blockchain quantities in the audited paths, except the custom transaction path fixed above. | +| Solana SPL token transfer and mint/burn paths | Amounts are passed to SPL APIs as `BigInt` and existing large-amount tests cover values beyond the safe integer range. | Confirmed safe. Native System/Stake APIs still expose number-only parameters and need a dedicated compatibility change rather than an unsafe cast. | +| EVM nonce/sequence, Cosmos/Substrate metadata, UTXO indexes, and timestamps | Conversions are bounded protocol metadata or array/index values rather than coin amounts. | Confirmed safe for this audit scope. | +| Coin fee/balance explanation fields | Several coins intentionally expose legacy number fields while retaining string fields. These are compatibility surfaces and need coin-specific follow-up tickets where the underlying chain permits u64/u128 values. | Triaged for follow-up; no broad type change made in this ticket. | + +## Follow-up triage + +The remaining number-returning compatibility surfaces should be addressed per +coin, with API changes that preserve existing string fields and add boundary +tests before changing public types. In particular, review Solana native +System/Stake instruction APIs and fee/balance explanation interfaces for coins +that serialize u64/u128 quantities. diff --git a/modules/sdk-coin-sui/src/lib/customTransaction.ts b/modules/sdk-coin-sui/src/lib/customTransaction.ts index 0075a3e3a6..dad486624d 100644 --- a/modules/sdk-coin-sui/src/lib/customTransaction.ts +++ b/modules/sdk-coin-sui/src/lib/customTransaction.ts @@ -9,6 +9,7 @@ import { Transaction } from './transaction'; import { BaseCoin as CoinConfig } from '@bitgo/statics'; import utils from './utils'; import { BaseKey, InvalidTransactionError, Recipient, TransactionRecipient, TransactionType } from '@bitgo/sdk-core'; +import BigNumber from 'bignumber.js'; import { UNAVAILABLE_TEXT } from './constants'; export class CustomTransaction extends Transaction { @@ -77,7 +78,10 @@ export class CustomTransaction extends Transaction accumulator + Number(current.amount), 0); + const totalAmount = this._recipients.reduce( + (accumulator, current) => accumulator.plus(current.amount), + new BigNumber(0) + ); this._inputs = [ { @@ -169,11 +173,14 @@ export class CustomTransaction extends Transaction recipient); - const outputAmount = recipients.reduce((accumulator, current) => accumulator + Number(current.amount), 0); + const outputAmount = recipients.reduce( + (accumulator, current) => accumulator.plus(current.amount), + new BigNumber(0) + ); return { ...explanationResult, outputs, - outputAmount, + outputAmount: outputAmount.toFixed(), }; } } diff --git a/modules/sdk-coin-sui/test/unit/transactionBuilder/customTransactionBuilder.ts b/modules/sdk-coin-sui/test/unit/transactionBuilder/customTransactionBuilder.ts index c21b1685f3..eee8bc1a27 100644 --- a/modules/sdk-coin-sui/test/unit/transactionBuilder/customTransactionBuilder.ts +++ b/modules/sdk-coin-sui/test/unit/transactionBuilder/customTransactionBuilder.ts @@ -107,7 +107,22 @@ describe('Sui Custom Transaction Builder', () => { deserialized.toBroadcastFormat().should.equal(rawTx); }); - it('should reject a custom tx with unsupported txn type', async function () { + it('preserves precision when aggregating amounts above Number.MAX_SAFE_INTEGER', async function () { + const tx = new CustomTransaction(coins.get('tsui')); + tx.fromRawTransaction(CUSTOM_TX_PUBLIC_TRANSFER); + + const largeAmount = '9007199254740993'; + (tx as unknown as { _recipients: { address: string; amount: string }[] })._recipients = [ + { address: tx.recipients[0].address, amount: largeAmount }, + { address: tx.recipients[1].address, amount: '1' }, + ]; + tx.loadInputsAndOutputs(); + + should.equal(tx.inputs[0].value, '9007199254740994'); + should.equal(tx.explainTransaction().outputAmount, '9007199254740994'); + }); + + should(() => factory.from(UNSUPPORTED_TX)).throwError( 'unsupported target method 0000000000000000000000000000000000000000000000000000000000000003::staking_pool::split_staked_sui' );