Skip to content

fix(sdk-coin-sol): add early validation for transaction size limits #7789

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nvrakesh06 wants to merge 1 commit into
base: master
Choose a base branch
from
+247 −16
Open

fix(sdk-coin-sol): add early validation for transaction size limits #7789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions modules/sdk-coin-sol/src/lib/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
37 changes: 30 additions & 7 deletions modules/sdk-coin-sol/src/lib/tokenTransferBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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({
Expand All @@ -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,
Expand Down Expand Up @@ -117,6 +117,29 @@ export class TokenTransferBuilder extends TransactionBuilder {
/** @inheritdoc */
protected async buildImplementation(): Promise<Transaction> {
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<TokenTransfer> => {
const coin = getSolTokenFromTokenName(sendParams.tokenName);
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -193,7 +216,7 @@ export class TokenTransferBuilder extends TransactionBuilder {
})
);
const addPriorityFeeInstruction: SetPriorityFee = {
type: InstructionBuilderTypes.SetPriorityFee,
type: Constants.InstructionBuilderTypes.SetPriorityFee,
params: {
fee: this._priorityFee,
},
Expand Down
42 changes: 33 additions & 9 deletions modules/sdk-coin-sol/src/lib/transferBuilderV2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -42,22 +42,22 @@ 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({
address: transferInstruction.params.toAddress,
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,
Expand Down Expand Up @@ -130,6 +130,30 @@ export class TransferBuilderV2 extends TransactionBuilder {
/** @inheritdoc */
protected async buildImplementation(): Promise<Transaction> {
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<Transfer | TokenTransfer> => {
if (sendParams.tokenName) {
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -205,7 +229,7 @@ export class TransferBuilderV2 extends TransactionBuilder {
programId
);
return {
type: InstructionBuilderTypes.CreateAssociatedTokenAccount,
type: Constants.InstructionBuilderTypes.CreateAssociatedTokenAccount,
params: {
ownerAddress: recipient.ownerAddress,
tokenName: tokenName,
Expand All @@ -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,
},
Expand Down
84 changes: 84 additions & 0 deletions modules/sdk-coin-sol/test/unit/transactionBuilder/tokenTransferBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
88 changes: 88 additions & 0 deletions modules/sdk-coin-sol/test/unit/transactionBuilder/transferBuilderV2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});