From 478bebb89f54413d484d5a2e7325872e0652b819 Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Tue, 5 Nov 2024 08:32:30 +0000 Subject: [PATCH] Add failing tests for bigints in RPC errors (#3518) ### 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. --- .../src/__tests__/instruction-error-test.ts | 135 ++++++++++-------- .../src/__tests__/json-rpc-error-test.ts | 5 + .../src/__tests__/transaction-error-test.ts | 29 ++++ 3 files changed, 112 insertions(+), 57 deletions(-) diff --git a/packages/errors/src/__tests__/instruction-error-test.ts b/packages/errors/src/__tests__/instruction-error-test.ts index c0c4bfc7e61c..2f036ffb9499 100644 --- a/packages/errors/src/__tests__/instruction-error-test.ts +++ b/packages/errors/src/__tests__/instruction-error-test.ts @@ -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( @@ -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( diff --git a/packages/errors/src/__tests__/json-rpc-error-test.ts b/packages/errors/src/__tests__/json-rpc-error-test.ts index f26a022357b9..d4ff4b70ba9c 100644 --- a/packages/errors/src/__tests__/json-rpc-error-test.ts +++ b/packages/errors/src/__tests__/json-rpc-error-test.ts @@ -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, diff --git a/packages/errors/src/__tests__/transaction-error-test.ts b/packages/errors/src/__tests__/transaction-error-test.ts index 14af609559bb..aeb0d566e073 100644 --- a/packages/errors/src/__tests__/transaction-error-test.ts +++ b/packages/errors/src/__tests__/transaction-error-test.ts @@ -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( @@ -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 }, @@ -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(