Skip to content

Commit

Permalink
Add failing tests for bigints in RPC errors (#3518)
Browse files Browse the repository at this point in the history
### Problem

I've encountered a new issue with the latest RC when updating program clients.

Functions like `isProgramError` that check the custom error code for the program are no longer working because the relevant `SolanaError` context now contains `bigint` error codes or instruction indices instead of the `number` type defined in their context.

Unfortunately, we cannot rely on numeric keypaths for these errors because these only transform successful responses.

### Solution

Since we already have error factories that, given some RPC error message returns a proper `SolanaError`, we can adjust the logic of these functions such that they return a consistent output no matter if they receive `bigint` or `number` codes or indices.

These methods are:
- `getSolanaErrorFromJsonRpcError`: The main RPC error entry point. Which may delegate to:
- `getSolanaErrorFromTransactionError`: The function that resolves transaction errors. Which may delegate to:
- `getSolanaErrorFromInstructionError`: The function that resolves instruction errors.

The benefits of doing this is, these methods are used in both RPC calls and RPC subscriptions. Additionally, we can clean up [some custom logic](https://github.com/solana-labs/solana-web3.js/blob/33286743f6818e809a39d6332b435b56548b7907/packages/rpc-transport-http/src/http-transport-for-solana-rpc.ts#L21-L24) we're doing for RPC calls only.

### Plan

- First, in this PR, we add some failing tests to show that the current RPC error factories are not able to handle error messages that contain `bigint` error code or indices.
- We then make these tests pass in the next PR by adjusting the RPC error factories. That PR also adds a changeset.
  • Loading branch information
lorisleiva authored Nov 5, 2024
1 parent d83eaaf commit 478bebb
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 57 deletions.
135 changes: 78 additions & 57 deletions packages/errors/src/__tests__/instruction-error-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,64 +7,76 @@ import {
import { SolanaError } from '../error';
import { getSolanaErrorFromInstructionError } from '../instruction-error';

const EXPECTED_ERROR_CODES = [
['GenericError', 4615001],
['InvalidArgument', 4615002],
['InvalidInstructionData', 4615003],
['InvalidAccountData', 4615004],
['AccountDataTooSmall', 4615005],
['InsufficientFunds', 4615006],
['IncorrectProgramId', 4615007],
['MissingRequiredSignature', 4615008],
['AccountAlreadyInitialized', 4615009],
['UninitializedAccount', 4615010],
['UnbalancedInstruction', 4615011],
['ModifiedProgramId', 4615012],
['ExternalAccountLamportSpend', 4615013],
['ExternalAccountDataModified', 4615014],
['ReadonlyLamportChange', 4615015],
['ReadonlyDataModified', 4615016],
['DuplicateAccountIndex', 4615017],
['ExecutableModified', 4615018],
['RentEpochModified', 4615019],
['NotEnoughAccountKeys', 4615020],
['AccountDataSizeChanged', 4615021],
['AccountNotExecutable', 4615022],
['AccountBorrowFailed', 4615023],
['AccountBorrowOutstanding', 4615024],
['DuplicateAccountOutOfSync', 4615025],
['InvalidError', 4615027],
['ExecutableDataModified', 4615028],
['ExecutableLamportChange', 4615029],
['ExecutableAccountNotRentExempt', 4615030],
['UnsupportedProgramId', 4615031],
['CallDepth', 4615032],
['MissingAccount', 4615033],
['ReentrancyNotAllowed', 4615034],
['MaxSeedLengthExceeded', 4615035],
['InvalidSeeds', 4615036],
['InvalidRealloc', 4615037],
['ComputationalBudgetExceeded', 4615038],
['PrivilegeEscalation', 4615039],
['ProgramEnvironmentSetupFailure', 4615040],
['ProgramFailedToComplete', 4615041],
['ProgramFailedToCompile', 4615042],
['Immutable', 4615043],
['IncorrectAuthority', 4615044],
['AccountNotRentExempt', 4615046],
['InvalidAccountOwner', 4615047],
['ArithmeticOverflow', 4615048],
['UnsupportedSysvar', 4615049],
['IllegalOwner', 4615050],
['MaxAccountsDataAllocationsExceeded', 4615051],
['MaxAccountsExceeded', 4615052],
['MaxInstructionTraceLengthExceeded', 4615053],
['BuiltinProgramsMustConsumeComputeUnits', 4615054],
] as const;

describe('getSolanaErrorFromInstructionError', () => {
it.each([
['GenericError', 4615001],
['InvalidArgument', 4615002],
['InvalidInstructionData', 4615003],
['InvalidAccountData', 4615004],
['AccountDataTooSmall', 4615005],
['InsufficientFunds', 4615006],
['IncorrectProgramId', 4615007],
['MissingRequiredSignature', 4615008],
['AccountAlreadyInitialized', 4615009],
['UninitializedAccount', 4615010],
['UnbalancedInstruction', 4615011],
['ModifiedProgramId', 4615012],
['ExternalAccountLamportSpend', 4615013],
['ExternalAccountDataModified', 4615014],
['ReadonlyLamportChange', 4615015],
['ReadonlyDataModified', 4615016],
['DuplicateAccountIndex', 4615017],
['ExecutableModified', 4615018],
['RentEpochModified', 4615019],
['NotEnoughAccountKeys', 4615020],
['AccountDataSizeChanged', 4615021],
['AccountNotExecutable', 4615022],
['AccountBorrowFailed', 4615023],
['AccountBorrowOutstanding', 4615024],
['DuplicateAccountOutOfSync', 4615025],
['InvalidError', 4615027],
['ExecutableDataModified', 4615028],
['ExecutableLamportChange', 4615029],
['ExecutableAccountNotRentExempt', 4615030],
['UnsupportedProgramId', 4615031],
['CallDepth', 4615032],
['MissingAccount', 4615033],
['ReentrancyNotAllowed', 4615034],
['MaxSeedLengthExceeded', 4615035],
['InvalidSeeds', 4615036],
['InvalidRealloc', 4615037],
['ComputationalBudgetExceeded', 4615038],
['PrivilegeEscalation', 4615039],
['ProgramEnvironmentSetupFailure', 4615040],
['ProgramFailedToComplete', 4615041],
['ProgramFailedToCompile', 4615042],
['Immutable', 4615043],
['IncorrectAuthority', 4615044],
['AccountNotRentExempt', 4615046],
['InvalidAccountOwner', 4615047],
['ArithmeticOverflow', 4615048],
['UnsupportedSysvar', 4615049],
['IllegalOwner', 4615050],
['MaxAccountsDataAllocationsExceeded', 4615051],
['MaxAccountsExceeded', 4615052],
['MaxInstructionTraceLengthExceeded', 4615053],
['BuiltinProgramsMustConsumeComputeUnits', 4615054],
])('produces the correct `SolanaError` for a `%s` error', (transactionError, expectedCode) => {
const error = getSolanaErrorFromInstructionError(123, transactionError);
expect(error).toEqual(new SolanaError(expectedCode as SolanaErrorCode, { index: 123 }));
});
it.each(EXPECTED_ERROR_CODES)(
'produces the correct `SolanaError` for a `%s` error',
(transactionError, expectedCode) => {
const error = getSolanaErrorFromInstructionError(123, transactionError);
expect(error).toEqual(new SolanaError(expectedCode as SolanaErrorCode, { index: 123 }));
},
);
it.failing.each(EXPECTED_ERROR_CODES)(
'produces the correct `SolanaError` for a `%s` error with a bigint index',
(transactionError, expectedCode) => {
const error = getSolanaErrorFromInstructionError(123n as unknown as number, transactionError);
expect(error).toEqual(new SolanaError(expectedCode as SolanaErrorCode, { index: 123 }));
},
);
it('produces the correct `SolanaError` for a `Custom` error', () => {
const error = getSolanaErrorFromInstructionError(123, { Custom: 789 });
expect(error).toEqual(
Expand All @@ -74,6 +86,15 @@ describe('getSolanaErrorFromInstructionError', () => {
}),
);
});
it.failing('produces the correct `SolanaError` for a `Custom` error with a bigint code', () => {
const error = getSolanaErrorFromInstructionError(123, { Custom: 789n });
expect(error).toEqual(
new SolanaError(SOLANA_ERROR__INSTRUCTION_ERROR__CUSTOM, {
code: 789,
index: 123,
}),
);
});
it('produces the correct `SolanaError` for a `BorshIoError` error', () => {
const error = getSolanaErrorFromInstructionError(123, { BorshIoError: 'abc' });
expect(error).toEqual(
Expand Down
5 changes: 5 additions & 0 deletions packages/errors/src/__tests__/json-rpc-error-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ describe('getSolanaErrorFromJsonRpcError', () => {
const error = getSolanaErrorFromJsonRpcError({ code, message: 'o no' });
expect(error).toHaveProperty('context.__code', 123);
});
it.failing('converts bigint codes to numbers', () => {
const code = 123n;
const error = getSolanaErrorFromJsonRpcError({ code: code as unknown as number, message: 'o no' });
expect(error).toHaveProperty('context.__code', 123);
});
describe.each([
SOLANA_ERROR__JSON_RPC__SERVER_ERROR_MIN_CONTEXT_SLOT_NOT_REACHED,
SOLANA_ERROR__JSON_RPC__SERVER_ERROR_NODE_UNHEALTHY,
Expand Down
29 changes: 29 additions & 0 deletions packages/errors/src/__tests__/transaction-error-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ describe('getSolanaErrorFromTransactionError', () => {
}),
);
});
it.failing('produces the correct `SolanaError` for a `DuplicateInstruction` error with a bigint index', () => {
const error = getSolanaErrorFromTransactionError({ DuplicateInstruction: 1n });
expect(error).toEqual(
new SolanaError(SOLANA_ERROR__TRANSACTION_ERROR__DUPLICATE_INSTRUCTION, {
index: 1,
}),
);
});
it('produces the correct `SolanaError` for a `InsufficientFundsForRent` error', () => {
const error = getSolanaErrorFromTransactionError({ InsufficientFundsForRent: { account_index: 1 } });
expect(error).toEqual(
Expand All @@ -66,6 +74,14 @@ describe('getSolanaErrorFromTransactionError', () => {
}),
);
});
it.failing('produces the correct `SolanaError` for a `InsufficientFundsForRent` error with a bigint index', () => {
const error = getSolanaErrorFromTransactionError({ InsufficientFundsForRent: { account_index: 1n } });
expect(error).toEqual(
new SolanaError(SOLANA_ERROR__TRANSACTION_ERROR__INSUFFICIENT_FUNDS_FOR_RENT, {
accountIndex: 1,
}),
);
});
it('produces the correct `SolanaError` for a `ProgramExecutionTemporarilyRestricted` error', () => {
const error = getSolanaErrorFromTransactionError({
ProgramExecutionTemporarilyRestricted: { account_index: 1 },
Expand All @@ -76,6 +92,19 @@ describe('getSolanaErrorFromTransactionError', () => {
}),
);
});
it.failing(
'produces the correct `SolanaError` for a `ProgramExecutionTemporarilyRestricted` error with a bigint index',
() => {
const error = getSolanaErrorFromTransactionError({
ProgramExecutionTemporarilyRestricted: { account_index: 1n },
});
expect(error).toEqual(
new SolanaError(SOLANA_ERROR__TRANSACTION_ERROR__PROGRAM_EXECUTION_TEMPORARILY_RESTRICTED, {
accountIndex: 1,
}),
);
},
);
it("returns the unknown error when encountering an enum name that's missing from the map", () => {
const error = getSolanaErrorFromTransactionError('ThisDoesNotExist');
expect(error).toEqual(
Expand Down

0 comments on commit 478bebb

Please sign in to comment.