Skip to content

Commit 79f4736

Browse files
joncinquejoncinque
authored
js-legacy: Fix variable-size encoding of COptionPubkey (#71)
#### Problem The JS library encodes and decodes `COption<Pubkey>` improperly. During encoding, it overallocates the instruction data buffer to always include the additional 32 bytes, even when the value is `null`. During decoding, it assumes the worst-case size for the instruction data length. #### Summary of changes buffer-layout actually contains the way to do this right, by properly implementing `getSpan` to return an error if it's ever used without a buffer: https://github.com/solana-labs/buffer-layout/blob/ee6cdb8074265b04c884485bb46738cdbc077e41/src/Layout.ts#L282 So this properly fixes the encoding and decoding at the same time: * during encoding, overallocate to the worst case, then truncate down * correctly return an error during `getSpan` * during decoding, use the input buffer with `getSpan` to dynamically calculate the expected length * test the behavior for transfer fee encoding / decoding
1 parent c0c98ee commit 79f4736

File tree

7 files changed

+49
-21
lines changed

7 files changed

+49
-21
lines changed

‎clients/js-legacy/src/extensions/transferFee/instructions.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export function createInitializeTransferFeeConfigInstruction(
7070
}
7171
const keys = [{ pubkey: mint, isSigner: false, isWritable: true }];
7272

73-
const data = Buffer.alloc(initializeTransferFeeConfigInstructionData.span);
73+
const data = Buffer.alloc(78); // worst-case size
7474
initializeTransferFeeConfigInstructionData.encode(
7575
{
7676
instruction: TokenInstruction.TransferFeeExtension,
@@ -83,7 +83,11 @@ export function createInitializeTransferFeeConfigInstruction(
8383
data,
8484
);
8585

86-
return new TransactionInstruction({ keys, programId, data });
86+
return new TransactionInstruction({
87+
keys,
88+
programId,
89+
data: data.subarray(0, initializeTransferFeeConfigInstructionData.getSpan(data)),
90+
});
8791
}
8892

8993
/** A decoded, valid InitializeTransferFeeConfig instruction */
@@ -115,7 +119,7 @@ export function decodeInitializeTransferFeeConfigInstruction(
115119
programId: PublicKey,
116120
): DecodedInitializeTransferFeeConfigInstruction {
117121
if (!instruction.programId.equals(programId)) throw new TokenInvalidInstructionProgramError();
118-
if (instruction.data.length !== initializeTransferFeeConfigInstructionData.span)
122+
if (instruction.data.length !== initializeTransferFeeConfigInstructionData.getSpan(instruction.data))
119123
throw new TokenInvalidInstructionDataError();
120124

121125
const {

‎clients/js-legacy/src/instructions/initializeMint.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export function createInitializeMintInstruction(
5151
{ pubkey: SYSVAR_RENT_PUBKEY, isSigner: false, isWritable: false },
5252
];
5353

54-
const data = Buffer.alloc(initializeMintInstructionData.span);
54+
const data = Buffer.alloc(67); // worst-case size
5555
initializeMintInstructionData.encode(
5656
{
5757
instruction: TokenInstruction.InitializeMint,
@@ -62,7 +62,11 @@ export function createInitializeMintInstruction(
6262
data,
6363
);
6464

65-
return new TransactionInstruction({ keys, programId, data });
65+
return new TransactionInstruction({
66+
keys,
67+
programId,
68+
data: data.subarray(0, initializeMintInstructionData.getSpan(data)),
69+
});
6670
}
6771

6872
/** A decoded, valid InitializeMint instruction */
@@ -93,7 +97,8 @@ export function decodeInitializeMintInstruction(
9397
programId = TOKEN_PROGRAM_ID,
9498
): DecodedInitializeMintInstruction {
9599
if (!instruction.programId.equals(programId)) throw new TokenInvalidInstructionProgramError();
96-
if (instruction.data.length !== initializeMintInstructionData.span) throw new TokenInvalidInstructionDataError();
100+
if (instruction.data.length !== initializeMintInstructionData.getSpan(instruction.data))
101+
throw new TokenInvalidInstructionDataError();
97102

98103
const {
99104
keys: { mint, rent },

‎clients/js-legacy/src/instructions/initializeMint2.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export function createInitializeMint2Instruction(
4848
): TransactionInstruction {
4949
const keys = [{ pubkey: mint, isSigner: false, isWritable: true }];
5050

51-
const data = Buffer.alloc(initializeMint2InstructionData.span);
51+
const data = Buffer.alloc(67); // worst-case size
5252
initializeMint2InstructionData.encode(
5353
{
5454
instruction: TokenInstruction.InitializeMint2,
@@ -59,7 +59,11 @@ export function createInitializeMint2Instruction(
5959
data,
6060
);
6161

62-
return new TransactionInstruction({ keys, programId, data });
62+
return new TransactionInstruction({
63+
keys,
64+
programId,
65+
data: data.subarray(0, initializeMint2InstructionData.getSpan(data)),
66+
});
6367
}
6468

6569
/** A decoded, valid InitializeMint2 instruction */
@@ -89,7 +93,8 @@ export function decodeInitializeMint2Instruction(
8993
programId = TOKEN_PROGRAM_ID,
9094
): DecodedInitializeMint2Instruction {
9195
if (!instruction.programId.equals(programId)) throw new TokenInvalidInstructionProgramError();
92-
if (instruction.data.length !== initializeMint2InstructionData.span) throw new TokenInvalidInstructionDataError();
96+
if (instruction.data.length !== initializeMint2InstructionData.getSpan(instruction.data))
97+
throw new TokenInvalidInstructionDataError();
9398

9499
const {
95100
keys: { mint },

‎clients/js-legacy/src/instructions/initializeMintCloseAuthority.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export function createInitializeMintCloseAuthorityInstruction(
4343
}
4444
const keys = [{ pubkey: mint, isSigner: false, isWritable: true }];
4545

46-
const data = Buffer.alloc(initializeMintCloseAuthorityInstructionData.span);
46+
const data = Buffer.alloc(34); // worst-case size
4747
initializeMintCloseAuthorityInstructionData.encode(
4848
{
4949
instruction: TokenInstruction.InitializeMintCloseAuthority,
@@ -52,7 +52,11 @@ export function createInitializeMintCloseAuthorityInstruction(
5252
data,
5353
);
5454

55-
return new TransactionInstruction({ keys, programId, data });
55+
return new TransactionInstruction({
56+
keys,
57+
programId,
58+
data: data.subarray(0, initializeMintCloseAuthorityInstructionData.getSpan(data)),
59+
});
5660
}
5761

5862
/** A decoded, valid InitializeMintCloseAuthority instruction */
@@ -80,7 +84,7 @@ export function decodeInitializeMintCloseAuthorityInstruction(
8084
programId: PublicKey,
8185
): DecodedInitializeMintCloseAuthorityInstruction {
8286
if (!instruction.programId.equals(programId)) throw new TokenInvalidInstructionProgramError();
83-
if (instruction.data.length !== initializeMintCloseAuthorityInstructionData.span)
87+
if (instruction.data.length !== initializeMintCloseAuthorityInstructionData.getSpan(instruction.data))
8488
throw new TokenInvalidInstructionDataError();
8589

8690
const {

‎clients/js-legacy/src/instructions/setAuthority.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export function createSetAuthorityInstruction(
6969
): TransactionInstruction {
7070
const keys = addSigners([{ pubkey: account, isSigner: false, isWritable: true }], currentAuthority, multiSigners);
7171

72-
const data = Buffer.alloc(setAuthorityInstructionData.span);
72+
const data = Buffer.alloc(35); // worst-case
7373
setAuthorityInstructionData.encode(
7474
{
7575
instruction: TokenInstruction.SetAuthority,
@@ -79,7 +79,11 @@ export function createSetAuthorityInstruction(
7979
data,
8080
);
8181

82-
return new TransactionInstruction({ keys, programId, data });
82+
return new TransactionInstruction({
83+
keys,
84+
programId,
85+
data: data.subarray(0, setAuthorityInstructionData.getSpan(data)),
86+
});
8387
}
8488

8589
/** A decoded, valid SetAuthority instruction */
@@ -110,7 +114,8 @@ export function decodeSetAuthorityInstruction(
110114
programId = TOKEN_PROGRAM_ID,
111115
): DecodedSetAuthorityInstruction {
112116
if (!instruction.programId.equals(programId)) throw new TokenInvalidInstructionProgramError();
113-
if (instruction.data.length !== setAuthorityInstructionData.span) throw new TokenInvalidInstructionDataError();
117+
if (instruction.data.length !== setAuthorityInstructionData.getSpan(instruction.data))
118+
throw new TokenInvalidInstructionDataError();
114119

115120
const {
116121
keys: { account, currentAuthority, multiSigners },

‎clients/js-legacy/src/serialization.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,6 @@ export class COptionPublicKeyLayout extends Layout<PublicKey | null> {
3434
const option = buffer[offset];
3535
return option === 0 ? 1 : 1 + this.publicKeyLayout.span;
3636
}
37-
return 1 + this.publicKeyLayout.span;
37+
throw new RangeError('Buffer must be provided');
3838
}
3939
}

‎clients/js-legacy/test/unit/transferfee.test.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import {
33
calculateEpochFee,
44
ONE_IN_BASIS_POINTS,
55
createInitializeTransferFeeConfigInstruction,
6-
decodeInitializeTransferFeeConfigInstructionUnchecked,
6+
decodeInitializeTransferFeeConfigInstruction,
7+
TOKEN_2022_PROGRAM_ID,
78
} from '../../src';
89
import { expect } from 'chai';
910
import { Keypair, PublicKey } from '@solana/web3.js';
@@ -21,7 +22,8 @@ describe('transferFee', () => {
2122
100,
2223
100n,
2324
);
24-
const decoded = decodeInitializeTransferFeeConfigInstructionUnchecked(instruction);
25+
expect(instruction.data.length).to.eql(78);
26+
const decoded = decodeInitializeTransferFeeConfigInstruction(instruction, TOKEN_2022_PROGRAM_ID);
2527
expect(decoded.data.transferFeeConfigAuthority).to.eql(transferFeeConfigAuthority);
2628
expect(decoded.data.withdrawWithheldAuthority).to.eql(withdrawWithheldAuthority);
2729
expect(decoded.data.transferFeeBasisPoints).to.eql(100);
@@ -37,7 +39,8 @@ describe('transferFee', () => {
3739
100,
3840
100n,
3941
);
40-
const decoded = decodeInitializeTransferFeeConfigInstructionUnchecked(instruction);
42+
expect(instruction.data.length).to.eql(46);
43+
const decoded = decodeInitializeTransferFeeConfigInstruction(instruction, TOKEN_2022_PROGRAM_ID);
4144
expect(decoded.data.transferFeeConfigAuthority).to.eql(null);
4245
expect(decoded.data.withdrawWithheldAuthority).to.eql(withdrawWithheldAuthority);
4346
expect(decoded.data.transferFeeBasisPoints).to.eql(100);
@@ -53,7 +56,8 @@ describe('transferFee', () => {
5356
100,
5457
100n,
5558
);
56-
const decoded = decodeInitializeTransferFeeConfigInstructionUnchecked(instruction);
59+
expect(instruction.data.length).to.eql(46);
60+
const decoded = decodeInitializeTransferFeeConfigInstruction(instruction, TOKEN_2022_PROGRAM_ID);
5761
expect(decoded.data.transferFeeConfigAuthority).to.eql(transferFeeConfigAuthority);
5862
expect(decoded.data.withdrawWithheldAuthority).to.eql(null);
5963
expect(decoded.data.transferFeeBasisPoints).to.eql(100);
@@ -62,7 +66,8 @@ describe('transferFee', () => {
6266
it('should encode and decode with no authorities', () => {
6367
const mint = Keypair.generate().publicKey;
6468
const instruction = createInitializeTransferFeeConfigInstruction(mint, null, null, 100, 100n);
65-
const decoded = decodeInitializeTransferFeeConfigInstructionUnchecked(instruction);
69+
expect(instruction.data.length).to.eql(14);
70+
const decoded = decodeInitializeTransferFeeConfigInstruction(instruction, TOKEN_2022_PROGRAM_ID);
6671
expect(decoded.data.transferFeeConfigAuthority).to.eql(null);
6772
expect(decoded.data.withdrawWithheldAuthority).to.eql(null);
6873
expect(decoded.data.transferFeeBasisPoints).to.eql(100);

0 commit comments

Comments
 (0)