Skip to content
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.

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

Change IInstruction to use ReadonlyUint8Array for data #3632

Merged
merged 2 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .changeset/spicy-parrots-raise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@solana/transaction-messages': patch
'@solana/instructions': patch
'@solana/errors': patch
Comment on lines +2 to +4
Copy link
Collaborator Author

@mcintyre94 mcintyre94 Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is debatable. I'm widening the type of IInstruction, so any code that creates one of those is unaffected, but any code that treats its data as mutable (relying on the existing narrower type) is affected. In our code nothing relies on it being mutable, but I needed to update some type params. Downstream consumers like Codama clients need to be updated to avoid any code changes for their dependencies. Happy to change this to whatever we think is best.

---

Change `data` field of transaction message instructions to use `ReadonlyUint8Array`
16 changes: 14 additions & 2 deletions examples/deserialize-transaction/src/example.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,13 @@ if (identifiedInstruction === SystemInstruction.TransferSol) {
log.info('[step 4] The first instruction calls the TransferSol instruction of the system program');
// Narrow the type again, the instruction must have accounts to be parsed as a transfer SOL instruction
assertIsInstructionWithAccounts(firstInstruction);
const parsedFirstInstruction = parseTransferSolInstruction(firstInstruction);

// TODO: This can just be `parseTransferSolInstruction(firstInstruction)` when the client is updated
// with the `@solana/web3.js` version that changes the instruction data type to `ReadonlyUint8Array`
const parsedFirstInstruction = parseTransferSolInstruction({
...firstInstruction,
data: firstInstruction.data as unknown as Uint8Array,
});
Comment on lines +385 to +390
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to create a new build of the Codama JS clients with this change to make this go away.

This was actually where I first hit this issue. I was using an API that returned instruction data as base58, and then trying to convert its output into an IInstruction that I could parse with Codama. Since instruction data is Uint8Array this required me to cast my ReadonlyUint8Array from encoding the API's base58 data to a Uint8Array like this. I figured it makes sense to just widen the type on IInstruction as in this PR

log.info(parsedFirstInstruction, '[step 4] The parsed Transfer SOL instruction');

// This gives us an understanding of what exactly is happening in the instruction
Expand All @@ -406,7 +412,13 @@ if (secondInstruction.programAddress === MEMO_PROGRAM_ADDRESS) {
// We know it's always an addMemo instruction

assertIsInstructionWithData(secondInstruction);
const parsedSecondInstruction = parseAddMemoInstruction(secondInstruction);

// TODO: This can just be `parseAddMemoInstruction(secondInstruction)` when the client is updated
// with the `@solana/web3.js` version that changes the instruction data type to `ReadonlyUint8Array`
const parsedSecondInstruction = parseAddMemoInstruction({
...secondInstruction,
data: secondInstruction.data as unknown as Uint8Array,
});
log.info(parsedSecondInstruction, '[step 4] The parsed Add Memo instruction');
log.info(`[step 4] The second instruction adds a memo with message "${parsedSecondInstruction.data.memo}"`);

Expand Down
2 changes: 1 addition & 1 deletion packages/errors/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ export type SolanaErrorContext = DefaultUnspecifiedErrorContextToUndefined<
instructionErrorContext?: unknown;
};
[SOLANA_ERROR__INSTRUCTION__EXPECTED_TO_HAVE_ACCOUNTS]: {
data?: Uint8Array;
data?: ReadonlyUint8Array;
programAddress: string;
};
[SOLANA_ERROR__INSTRUCTION__EXPECTED_TO_HAVE_DATA]: {
Expand Down
1 change: 1 addition & 0 deletions packages/instructions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"maintained node versions"
],
"dependencies": {
"@solana/codecs-core": "workspace:*",
"@solana/errors": "workspace:*"
},
"devDependencies": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Address } from '@solana/addresses';
import { ReadonlyUint8Array } from '@solana/codecs-core';

import { IAccountLookupMeta, IAccountMeta } from '../accounts';
import {
Expand All @@ -21,12 +22,12 @@ import {
instruction satisfies IInstructionWithAccounts<readonly (IAccountLookupMeta | IAccountMeta)[]>;

// @ts-expect-error instruction might not have data
instruction satisfies IInstructionWithData<Uint8Array>;
instruction satisfies IInstructionWithData<ReadonlyUint8Array>;

if (isInstructionWithAccounts(instruction) && isInstructionWithData(instruction)) {
instruction satisfies IInstruction &
IInstructionWithAccounts<readonly (IAccountLookupMeta | IAccountMeta)[]> &
IInstructionWithData<Uint8Array>;
IInstructionWithData<ReadonlyUint8Array>;
}
}

Expand All @@ -38,15 +39,15 @@ import {
instruction satisfies IInstructionWithAccounts<readonly (IAccountLookupMeta | IAccountMeta)[]>;

// @ts-expect-error instruction might not have data
instruction satisfies IInstructionWithData<Uint8Array>;
instruction satisfies IInstructionWithData<ReadonlyUint8Array>;

assertIsInstructionWithAccounts(instruction);
instruction satisfies IInstruction & IInstructionWithAccounts<readonly (IAccountLookupMeta | IAccountMeta)[]>;

assertIsInstructionWithData(instruction);
instruction satisfies IInstruction &
IInstructionWithAccounts<readonly (IAccountLookupMeta | IAccountMeta)[]> &
IInstructionWithData<Uint8Array>;
IInstructionWithData<ReadonlyUint8Array>;
}

// narrowing by program address
Expand Down
9 changes: 5 additions & 4 deletions packages/instructions/src/instruction.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Address } from '@solana/addresses';
import { ReadonlyUint8Array } from '@solana/codecs-core';
import {
SOLANA_ERROR__INSTRUCTION__EXPECTED_TO_HAVE_ACCOUNTS,
SOLANA_ERROR__INSTRUCTION__EXPECTED_TO_HAVE_DATA,
Expand All @@ -13,7 +14,7 @@ export interface IInstruction<
TAccounts extends readonly (IAccountLookupMeta | IAccountMeta)[] = readonly (IAccountLookupMeta | IAccountMeta)[],
> {
readonly accounts?: TAccounts;
readonly data?: Uint8Array;
readonly data?: ReadonlyUint8Array;
readonly programAddress: Address<TProgramAddress>;
}

Expand Down Expand Up @@ -60,19 +61,19 @@ export function assertIsInstructionWithAccounts<
}
}

export interface IInstructionWithData<TData extends Uint8Array> extends IInstruction {
export interface IInstructionWithData<TData extends ReadonlyUint8Array> extends IInstruction {
readonly data: TData;
}

export function isInstructionWithData<
TData extends Uint8Array = Uint8Array,
TData extends ReadonlyUint8Array = ReadonlyUint8Array,
TInstruction extends IInstruction = IInstruction,
>(instruction: TInstruction): instruction is IInstructionWithData<TData> & TInstruction {
return instruction.data !== undefined;
}

export function assertIsInstructionWithData<
TData extends Uint8Array = Uint8Array,
TData extends ReadonlyUint8Array = ReadonlyUint8Array,
TInstruction extends IInstruction = IInstruction,
>(instruction: TInstruction): asserts instruction is IInstructionWithData<TData> & TInstruction {
if (instruction.data === undefined) {
Expand Down
3 changes: 2 additions & 1 deletion packages/transaction-messages/src/compile/instructions.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { Address } from '@solana/addresses';
import { ReadonlyUint8Array } from '@solana/codecs-core';
import { IInstruction } from '@solana/instructions';

import { OrderedAccounts } from './accounts';

type CompiledInstruction = Readonly<{
accountIndices?: number[];
data?: Uint8Array;
data?: ReadonlyUint8Array;
programAddressIndex: number;
}>;

Expand Down
3 changes: 2 additions & 1 deletion packages/transaction-messages/src/durable-nonce.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Address } from '@solana/addresses';
import { ReadonlyUint8Array } from '@solana/codecs-core';
import { SOLANA_ERROR__TRANSACTION__EXPECTED_NONCE_LIFETIME, SolanaError } from '@solana/errors';
import {
AccountRole,
Expand Down Expand Up @@ -111,7 +112,7 @@ export function isAdvanceNonceAccountInstruction(
);
}

function isAdvanceNonceAccountInstructionData(data: Uint8Array): data is AdvanceNonceAccountInstructionData {
function isAdvanceNonceAccountInstructionData(data: ReadonlyUint8Array): data is AdvanceNonceAccountInstructionData {
// AdvanceNonceAccount is the fifth instruction in the System Program (index 4)
return data.byteLength === 4 && data[0] === 4 && data[1] === 0 && data[2] === 0 && data[3] === 0;
}
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.