From e2c71f56a0dd33392a34017dd670870239fe5a2b Mon Sep 17 00:00:00 2001 From: N V Rakesh Reddy Date: Mon, 22 Dec 2025 18:36:22 +0530 Subject: [PATCH] fix(sdk-coin-sol): add early validation for transaction size limits TICKET: WIN-8401 --- modules/sdk-coin-sol/src/lib/constants.ts | 12 +++ .../src/lib/tokenTransferBuilder.ts | 37 ++++++-- .../sdk-coin-sol/src/lib/transferBuilderV2.ts | 42 +++++++-- .../tokenTransferBuilder.ts | 84 ++++++++++++++++++ .../transactionBuilder/transferBuilderV2.ts | 88 +++++++++++++++++++ 5 files changed, 247 insertions(+), 16 deletions(-) diff --git a/modules/sdk-coin-sol/src/lib/constants.ts b/modules/sdk-coin-sol/src/lib/constants.ts index 33026c874e..8379beaa6b 100644 --- a/modules/sdk-coin-sol/src/lib/constants.ts +++ b/modules/sdk-coin-sol/src/lib/constants.ts @@ -28,6 +28,18 @@ export const UNAVAILABLE_TEXT = 'UNAVAILABLE'; */ export const SOLANA_TRANSACTION_MAX_SIZE = 1232; +/** + * Maximum safe recipient limits for Solana token transfers + * + * These limits are based on empirical testing to stay within SOLANA_TRANSACTION_MAX_SIZE (1232 bytes). + * Source: modules/sdk-coin-sol/scripts/transaction-size-benchmark-results.json + * + * With ATA Creation: Includes Associated Token Account initialization instructions + * Without ATA Creation: Recipients already have token accounts + */ +export const MAX_RECIPIENTS_WITH_ATA_CREATION = 9; +export const MAX_RECIPIENTS_WITHOUT_ATA_CREATION = 19; + export const JITO_STAKE_POOL_ADDRESS = 'Jito4APyf642JPZPx3hGc6WWJ8zPKtRbRs4P815Awbb'; export const JITOSOL_MINT_ADDRESS = 'J1toso1uCk3RLmjorhTtrVwY9HJ7X8V9yYac6Y7kGCPn'; export const JITO_STAKE_POOL_RESERVE_ACCOUNT = 'BgKUXdS29YcHCFrPm5M8oLHiTzZaMDjsebggjoaQ6KFL'; diff --git a/modules/sdk-coin-sol/src/lib/tokenTransferBuilder.ts b/modules/sdk-coin-sol/src/lib/tokenTransferBuilder.ts index 97c2a1c2f7..9849dc290e 100644 --- a/modules/sdk-coin-sol/src/lib/tokenTransferBuilder.ts +++ b/modules/sdk-coin-sol/src/lib/tokenTransferBuilder.ts @@ -9,11 +9,11 @@ import { validateMintAddress, validateOwnerAddress, } from './utils'; -import { InstructionBuilderTypes } from './constants'; +import * as Constants from './constants'; import { AtaInit, TokenAssociateRecipient, TokenTransfer, SetPriorityFee } from './iface'; import assert from 'assert'; import { TransactionBuilder } from './transactionBuilder'; -import _ from 'lodash'; +import * as _ from 'lodash'; export interface SendParams { address: string; @@ -43,7 +43,7 @@ export class TokenTransferBuilder extends TransactionBuilder { super.initBuilder(tx); for (const instruction of this._instructionsData) { - if (instruction.type === InstructionBuilderTypes.TokenTransfer) { + if (instruction.type === Constants.InstructionBuilderTypes.TokenTransfer) { const transferInstruction: TokenTransfer = instruction; this.sender(transferInstruction.params.fromAddress); this.send({ @@ -55,7 +55,7 @@ export class TokenTransferBuilder extends TransactionBuilder { decimalPlaces: transferInstruction.params.decimalPlaces, }); } - if (instruction.type === InstructionBuilderTypes.CreateAssociatedTokenAccount) { + if (instruction.type === Constants.InstructionBuilderTypes.CreateAssociatedTokenAccount) { const ataInitInstruction: AtaInit = instruction; this._createAtaParams.push({ ownerAddress: ataInitInstruction.params.ownerAddress, @@ -117,6 +117,29 @@ export class TokenTransferBuilder extends TransactionBuilder { /** @inheritdoc */ protected async buildImplementation(): Promise { assert(this._sender, 'Sender must be set before building the transaction'); + + const uniqueAtaCount = _.uniqBy(this._createAtaParams, (recipient: TokenAssociateRecipient) => { + return recipient.ownerAddress + recipient.tokenName; + }).length; + + if (uniqueAtaCount > 0 && this._sendParams.length > Constants.MAX_RECIPIENTS_WITH_ATA_CREATION) { + throw new BuildTransactionError( + `Transaction too large: ${this._sendParams.length} recipients with ${uniqueAtaCount} ATA creations. ` + + `Solana legacy transactions are limited to ${Constants.SOLANA_TRANSACTION_MAX_SIZE} bytes ` + + `(maximum ${Constants.MAX_RECIPIENTS_WITH_ATA_CREATION} recipients with ATA creation). ` + + `Please split into multiple transactions with max ${Constants.MAX_RECIPIENTS_WITH_ATA_CREATION} recipients each.` + ); + } + + if (uniqueAtaCount === 0 && this._sendParams.length > Constants.MAX_RECIPIENTS_WITHOUT_ATA_CREATION) { + throw new BuildTransactionError( + `Transaction too large: ${this._sendParams.length} recipients. ` + + `Solana legacy transactions are limited to ${Constants.SOLANA_TRANSACTION_MAX_SIZE} bytes ` + + `(maximum ${Constants.MAX_RECIPIENTS_WITHOUT_ATA_CREATION} recipients without ATA creation). ` + + `Please split into multiple transactions with max ${Constants.MAX_RECIPIENTS_WITHOUT_ATA_CREATION} recipients each.` + ); + } + const sendInstructions = await Promise.all( this._sendParams.map(async (sendParams: SendParams): Promise => { const coin = getSolTokenFromTokenName(sendParams.tokenName); @@ -139,7 +162,7 @@ export class TokenTransferBuilder extends TransactionBuilder { } const sourceAddress = await getAssociatedTokenAccountAddress(tokenAddress, this._sender, false, programId); return { - type: InstructionBuilderTypes.TokenTransfer, + type: Constants.InstructionBuilderTypes.TokenTransfer, params: { fromAddress: this._sender, toAddress: sendParams.address, @@ -180,7 +203,7 @@ export class TokenTransferBuilder extends TransactionBuilder { ataAddress = await getAssociatedTokenAccountAddress(tokenAddress, recipient.ownerAddress, false, programId); } return { - type: InstructionBuilderTypes.CreateAssociatedTokenAccount, + type: Constants.InstructionBuilderTypes.CreateAssociatedTokenAccount, params: { ownerAddress: recipient.ownerAddress, mintAddress: tokenAddress, @@ -193,7 +216,7 @@ export class TokenTransferBuilder extends TransactionBuilder { }) ); const addPriorityFeeInstruction: SetPriorityFee = { - type: InstructionBuilderTypes.SetPriorityFee, + type: Constants.InstructionBuilderTypes.SetPriorityFee, params: { fee: this._priorityFee, }, diff --git a/modules/sdk-coin-sol/src/lib/transferBuilderV2.ts b/modules/sdk-coin-sol/src/lib/transferBuilderV2.ts index 40e1bdc0dc..046ea6531a 100644 --- a/modules/sdk-coin-sol/src/lib/transferBuilderV2.ts +++ b/modules/sdk-coin-sol/src/lib/transferBuilderV2.ts @@ -12,7 +12,7 @@ import { import { BaseCoin as CoinConfig } from '@bitgo/statics'; import assert from 'assert'; import { AtaInit, TokenAssociateRecipient, TokenTransfer, Transfer, SetPriorityFee } from './iface'; -import { InstructionBuilderTypes } from './constants'; +import * as Constants from './constants'; import _ from 'lodash'; export interface SendParams { @@ -42,14 +42,14 @@ export class TransferBuilderV2 extends TransactionBuilder { super.initBuilder(tx); for (const instruction of this._instructionsData) { - if (instruction.type === InstructionBuilderTypes.Transfer) { + if (instruction.type === Constants.InstructionBuilderTypes.Transfer) { const transferInstruction: Transfer = instruction; this.sender(transferInstruction.params.fromAddress); this.send({ address: transferInstruction.params.toAddress, amount: transferInstruction.params.amount, }); - } else if (instruction.type === InstructionBuilderTypes.TokenTransfer) { + } else if (instruction.type === Constants.InstructionBuilderTypes.TokenTransfer) { const transferInstruction: TokenTransfer = instruction; this.sender(transferInstruction.params.fromAddress); this.send({ @@ -57,7 +57,7 @@ export class TransferBuilderV2 extends TransactionBuilder { amount: transferInstruction.params.amount, tokenName: transferInstruction.params.tokenName, }); - } else if (instruction.type === InstructionBuilderTypes.CreateAssociatedTokenAccount) { + } else if (instruction.type === Constants.InstructionBuilderTypes.CreateAssociatedTokenAccount) { const ataInitInstruction: AtaInit = instruction; this._createAtaParams.push({ ownerAddress: ataInitInstruction.params.ownerAddress, @@ -130,6 +130,30 @@ export class TransferBuilderV2 extends TransactionBuilder { /** @inheritdoc */ protected async buildImplementation(): Promise { assert(this._sender, 'Sender must be set before building the transaction'); + + // Validate transaction size limits + const uniqueAtaCount = _.uniqBy(this._createAtaParams, (recipient: TokenAssociateRecipient) => { + return recipient.ownerAddress + recipient.tokenName; + }).length; + + if (uniqueAtaCount > 0 && this._sendParams.length > Constants.MAX_RECIPIENTS_WITH_ATA_CREATION) { + throw new BuildTransactionError( + `Transaction too large: ${this._sendParams.length} recipients with ${uniqueAtaCount} ATA creations. ` + + `Solana legacy transactions are limited to ${Constants.SOLANA_TRANSACTION_MAX_SIZE} bytes ` + + `(maximum ${Constants.MAX_RECIPIENTS_WITH_ATA_CREATION} recipients with ATA creation). ` + + `Please split into multiple transactions with max ${Constants.MAX_RECIPIENTS_WITH_ATA_CREATION} recipients each.` + ); + } + + if (uniqueAtaCount === 0 && this._sendParams.length > Constants.MAX_RECIPIENTS_WITHOUT_ATA_CREATION) { + throw new BuildTransactionError( + `Transaction too large: ${this._sendParams.length} recipients. ` + + `Solana legacy transactions are limited to ${Constants.SOLANA_TRANSACTION_MAX_SIZE} bytes ` + + `(maximum ${Constants.MAX_RECIPIENTS_WITHOUT_ATA_CREATION} recipients without ATA creation). ` + + `Please split into multiple transactions with max ${Constants.MAX_RECIPIENTS_WITHOUT_ATA_CREATION} recipients each.` + ); + } + const sendInstructions = await Promise.all( this._sendParams.map(async (sendParams: SendParams): Promise => { if (sendParams.tokenName) { @@ -154,7 +178,7 @@ export class TransferBuilderV2 extends TransactionBuilder { const sourceAddress = await getAssociatedTokenAccountAddress(tokenAddress, this._sender, false, programId); return { - type: InstructionBuilderTypes.TokenTransfer, + type: Constants.InstructionBuilderTypes.TokenTransfer, params: { fromAddress: this._sender, toAddress: sendParams.address, @@ -168,7 +192,7 @@ export class TransferBuilderV2 extends TransactionBuilder { }; } else { return { - type: InstructionBuilderTypes.Transfer, + type: Constants.InstructionBuilderTypes.Transfer, params: { fromAddress: this._sender, toAddress: sendParams.address, @@ -205,7 +229,7 @@ export class TransferBuilderV2 extends TransactionBuilder { programId ); return { - type: InstructionBuilderTypes.CreateAssociatedTokenAccount, + type: Constants.InstructionBuilderTypes.CreateAssociatedTokenAccount, params: { ownerAddress: recipient.ownerAddress, tokenName: tokenName, @@ -224,10 +248,10 @@ export class TransferBuilderV2 extends TransactionBuilder { this._instructionsData = [...createAtaInstructions, ...sendInstructions]; } else if ( createAtaInstructions.length !== 0 || - sendInstructions.some((instruction) => instruction.type === InstructionBuilderTypes.TokenTransfer) + sendInstructions.some((instruction) => instruction.type === Constants.InstructionBuilderTypes.TokenTransfer) ) { addPriorityFeeInstruction = { - type: InstructionBuilderTypes.SetPriorityFee, + type: Constants.InstructionBuilderTypes.SetPriorityFee, params: { fee: this._priorityFee, }, diff --git a/modules/sdk-coin-sol/test/unit/transactionBuilder/tokenTransferBuilder.ts b/modules/sdk-coin-sol/test/unit/transactionBuilder/tokenTransferBuilder.ts index b555b330b0..a5c36e523e 100644 --- a/modules/sdk-coin-sol/test/unit/transactionBuilder/tokenTransferBuilder.ts +++ b/modules/sdk-coin-sol/test/unit/transactionBuilder/tokenTransferBuilder.ts @@ -796,5 +796,89 @@ describe('Sol Token Transfer Builder', () => { }) ).throwError('Invalid token name, got: ' + invalidTokenName); }); + + it('should fail with more than 9 recipients with ATA creation', async () => { + const txBuilder = factory.getTokenTransferBuilder(); + txBuilder.nonce(recentBlockHash); + txBuilder.sender(authAccount.pub); + + // Generate 10 unique recipients for ATA creation + const recipients: string[] = []; + for (let i = 0; i < 10; i++) { + const keypair = new KeyPair(); + recipients.push(keypair.getKeys().pub); + } + + for (const address of recipients) { + txBuilder.send({ address, amount, tokenName: nameUSDC }); + txBuilder.createAssociatedTokenAccount({ + ownerAddress: address, + tokenName: nameUSDC, + }); + } + + await txBuilder + .build() + .should.be.rejectedWith( + /Transaction too large: 10 recipients with 10 ATA creations.*maximum 9 recipients with ATA creation/ + ); + }); + + it('should fail with more than 19 recipients without ATA creation', async () => { + const txBuilder = factory.getTokenTransferBuilder(); + txBuilder.nonce(recentBlockHash); + txBuilder.sender(authAccount.pub); + + // Add 20 recipients without ATA creation (reusing addresses is fine without ATA) + for (let i = 0; i < 20; i++) { + // Only use first 3 addresses to avoid invalid address at index 3 + const address = testData.addresses.validAddresses[i % 3]; + txBuilder.send({ address, amount, tokenName: nameUSDC }); + } + + await txBuilder + .build() + .should.be.rejectedWith(/Transaction too large: 20 recipients.*maximum 19 recipients without ATA creation/); + }); + + it('should succeed with 9 recipients with ATA creation', async () => { + const txBuilder = factory.getTokenTransferBuilder(); + txBuilder.nonce(recentBlockHash); + txBuilder.sender(authAccount.pub); + + // Generate exactly 9 unique recipients for ATA creation + const recipients: string[] = []; + for (let i = 0; i < 9; i++) { + const keypair = new KeyPair(); + recipients.push(keypair.getKeys().pub); + } + + for (const address of recipients) { + txBuilder.send({ address, amount, tokenName: nameUSDC }); + txBuilder.createAssociatedTokenAccount({ + ownerAddress: address, + tokenName: nameUSDC, + }); + } + + const tx = await txBuilder.build(); + tx.should.be.ok(); + }); + + it('should succeed with 19 recipients without ATA creation', async () => { + const txBuilder = factory.getTokenTransferBuilder(); + txBuilder.nonce(recentBlockHash); + txBuilder.sender(authAccount.pub); + + // Add exactly 19 recipients without ATA creation (reusing addresses is fine without ATA) + for (let i = 0; i < 19; i++) { + // Only use first 3 addresses to avoid invalid address at index 3 + const address = testData.addresses.validAddresses[i % 3]; + txBuilder.send({ address, amount, tokenName: nameUSDC }); + } + + const tx = await txBuilder.build(); + tx.should.be.ok(); + }); }); }); diff --git a/modules/sdk-coin-sol/test/unit/transactionBuilder/transferBuilderV2.ts b/modules/sdk-coin-sol/test/unit/transactionBuilder/transferBuilderV2.ts index 60cd1ebbf8..5725d01236 100644 --- a/modules/sdk-coin-sol/test/unit/transactionBuilder/transferBuilderV2.ts +++ b/modules/sdk-coin-sol/test/unit/transactionBuilder/transferBuilderV2.ts @@ -872,5 +872,93 @@ describe('Sol Transfer Builder V2', () => { `input amount ${excessiveAmount} exceeds max safe int 9007199254740991` ); }); + + it('should fail with more than 9 recipients with ATA creation', async () => { + const txBuilder = factory.getTransferBuilderV2(); + txBuilder.nonce(recentBlockHash); + txBuilder.sender(authAccount.pub); + txBuilder.feePayer(feePayerAccount.pub); + + // Generate 10 unique recipients for ATA creation + const recipients: string[] = []; + for (let i = 0; i < 10; i++) { + const keypair = new KeyPair(); + recipients.push(keypair.getKeys().pub); + } + + for (const address of recipients) { + txBuilder.send({ address, amount, tokenName: nameUSDC }); + txBuilder.createAssociatedTokenAccount({ + ownerAddress: address, + tokenName: nameUSDC, + }); + } + + await txBuilder + .build() + .should.be.rejectedWith( + /Transaction too large: 10 recipients with 10 ATA creations.*maximum 9 recipients with ATA creation/ + ); + }); + + it('should fail with more than 19 token recipients without ATA creation', async () => { + const txBuilder = factory.getTransferBuilderV2(); + txBuilder.nonce(recentBlockHash); + txBuilder.sender(authAccount.pub); + txBuilder.feePayer(feePayerAccount.pub); + + // Add 20 token recipients without ATA creation (reusing addresses is fine without ATA) + for (let i = 0; i < 20; i++) { + // Only use first 3 addresses to avoid invalid address at index 3 + const address = testData.addresses.validAddresses[i % 3]; + txBuilder.send({ address, amount, tokenName: nameUSDC }); + } + + await txBuilder + .build() + .should.be.rejectedWith(/Transaction too large: 20 recipients.*maximum 19 recipients without ATA creation/); + }); + + it('should succeed with 9 recipients with ATA creation', async () => { + const txBuilder = factory.getTransferBuilderV2(); + txBuilder.nonce(recentBlockHash); + txBuilder.sender(authAccount.pub); + txBuilder.feePayer(feePayerAccount.pub); + + // Generate exactly 9 unique recipients for ATA creation + const recipients: string[] = []; + for (let i = 0; i < 9; i++) { + const keypair = new KeyPair(); + recipients.push(keypair.getKeys().pub); + } + + for (const address of recipients) { + txBuilder.send({ address, amount, tokenName: nameUSDC }); + txBuilder.createAssociatedTokenAccount({ + ownerAddress: address, + tokenName: nameUSDC, + }); + } + + const tx = await txBuilder.build(); + tx.should.be.ok(); + }); + + it('should succeed with 19 token recipients without ATA creation', async () => { + const txBuilder = factory.getTransferBuilderV2(); + txBuilder.nonce(recentBlockHash); + txBuilder.sender(authAccount.pub); + txBuilder.feePayer(feePayerAccount.pub); + + // Add exactly 19 token recipients without ATA creation (reusing addresses is fine without ATA) + for (let i = 0; i < 19; i++) { + // Only use first 3 addresses to avoid invalid address at index 3 + const address = testData.addresses.validAddresses[i % 3]; + txBuilder.send({ address, amount, tokenName: nameUSDC }); + } + + const tx = await txBuilder.build(); + tx.should.be.ok(); + }); }); });