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.

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

Jump to bottom

Convert errors in @solana/transactions to coded exceptions #2207

Merged
merged 1 commit into from
Feb 29, 2024
Merged

Convert errors in @solana/transactions to coded exceptions #2207

merged 1 commit into from
Feb 29, 2024

Conversation

steveluscher
Copy link
Collaborator

@steveluscher steveluscher commented Feb 27, 2024 •
edited

Addresses #2118.

@steveluscher
Copy link
Collaborator Author

steveluscher commented Feb 27, 2024 •
edited

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

This was referenced Feb 27, 2024
Merged
Merged
@steveluscher steveluscher force-pushed the 02-27-Convert_errors_in_solana/transactions_to_coded_exceptions branch from 1579936 to bfa481d Compare February 27, 2024 23:23
steveluscher
steveluscher commented Feb 27, 2024
View reviewed changes
packages/errors/src/messages.ts Outdated
Comment on lines 47 to 49
[SOLANA_ERROR__TRANSACTION_INVOKED_PROGRAMS_CANNOT_PAY_FEES]:
'This transaction includes an address (`$programAddress`) which is both ' +
'invoked and set as the fee payer. Program addresses may not pay fees',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not 100% sure if this error should be this, or:

Suggested change
[SOLANA_ERROR__TRANSACTION_INVOKED_PROGRAMS_CANNOT_PAY_FEES]:
'This transaction includes an address (`$programAddress`) which is both ' +
'invoked and set as the fee payer. Program addresses may not pay fees',
[SOLANA_ERROR__TRANSACTION_PROGRAMS_CANNOT_PAY_FEES]:
'This transaction includes a program (`$programAddress`) which is set as ' +
'the fee payer. Program addresses may not pay fees',

ie. does it matter that it's invoked, or is it that programs can't pay fees, period? cc/ @joncinque

packages/transactions/src/decompile-transaction.ts
Comment on lines +97 to +98
highestKnownIndex: addresses.length - 1,
highestRequestedIndex: highestIndex,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Modified this to include the highest index available.

@steveluscher steveluscher force-pushed the 02-27-Convert_errors_in_solana/transactions_to_coded_exceptions branch from bfa481d to 49bb70a Compare February 27, 2024 23:29
@steveluscher steveluscher force-pushed the 02-27-Remove_coded_error_TODO_from_solana/webcrypto-ed25519-polyfill_ branch from cf80a05 to 95a3903 Compare February 27, 2024 23:32
@steveluscher steveluscher force-pushed the 02-27-Convert_errors_in_solana/transactions_to_coded_exceptions branch from 49bb70a to 46e2e72 Compare February 27, 2024 23:32
steveluscher
steveluscher commented Feb 27, 2024
View reviewed changes
packages/errors/src/messages.ts
[SOLANA_ERROR__TRANSACTION_EXPECTED_BLOCKHASH_LIFETIME]: 'Transaction does not have a blockhash lifetime',
[SOLANA_ERROR__TRANSACTION_EXPECTED_NONCE_LIFETIME]: 'Transaction is not a durable nonce transaction',
[SOLANA_ERROR__TRANSACTION_FAILED_TO_DECOMPILE_ADDRESS_LOOKUP_TABLE_CONTENTS_MISSING]:
'Contents of these address lookup tables unknown: $lookupTableAddresses',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed this.

@steveluscher steveluscher changed the base branch from 02-27-Remove_coded_error_TODO_from_solana/webcrypto-ed25519-polyfill_ to master February 27, 2024 23:36
@steveluscher steveluscher force-pushed the 02-27-Convert_errors_in_solana/transactions_to_coded_exceptions branch from 46e2e72 to cfb0332 Compare February 27, 2024 23:36
lorisleiva
lorisleiva reviewed Feb 29, 2024
View reviewed changes
packages/errors/src/codes.ts Outdated
Comment on lines 16 to 24
export const SOLANA_ERROR__TRANSACTION_INVOKED_PROGRAMS_CANNOT_PAY_FEES = 99901 as const;
export const SOLANA_ERROR__TRANSACTION_INVOKED_PROGRAMS_MUST_NOT_BE_WRITABLE = 99902 as const;
export const SOLANA_ERROR__TRANSACTION_EXPECTED_BLOCKHASH_LIFETIME = 99903 as const;
export const SOLANA_ERROR__TRANSACTION_EXPECTED_NONCE_LIFETIME = 99904 as const;
export const SOLANA_ERROR__TRANSACTION_VERSION_NUMBER_OUT_OF_RANGE = 99905 as const;
export const SOLANA_ERROR__TRANSACTION_FAILED_TO_DECOMPILE_ADDRESS_LOOKUP_TABLE_CONTENTS_MISSING = 99906 as const;
export const SOLANA_ERROR__TRANSACTION_FAILED_TO_DECOMPILE_ADDRESS_LOOKUP_TABLE_INDEX_OUT_OF_RANGE = 99907 as const;
export const SOLANA_ERROR__TRANSACTION_FAILED_TO_DECOMPILE_INSTRUCTION_PROGRAM_ADDRESS_NOT_FOUND = 99908 as const;
export const SOLANA_ERROR__TRANSACTION_FAILED_TO_DECOMPILE_FEE_PAYER_MISSING = 99909 as const;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't feel too strongly about this but how do you guys feel about double-underscoring after the prefix so it's clear that we are creating a "group" of error codes?

Cc @buffalojoec @steveluscher @mcintyre94

export const SOLANA_ERROR__TRANSACTION__INVOKED_PROGRAMS_CANNOT_PAY_FEES = 99901 as const;
export const SOLANA_ERROR__TRANSACTION__INVOKED_PROGRAMS_MUST_NOT_BE_WRITABLE = 99902 as const;
export const SOLANA_ERROR__TRANSACTION__EXPECTED_BLOCKHASH_LIFETIME = 99903 as const;
export const SOLANA_ERROR__TRANSACTION__EXPECTED_NONCE_LIFETIME = 99904 as const;
export const SOLANA_ERROR__TRANSACTION__VERSION_NUMBER_OUT_OF_RANGE = 99905 as const;
export const SOLANA_ERROR__TRANSACTION__FAILED_TO_DECOMPILE_ADDRESS_LOOKUP_TABLE_CONTENTS_MISSING = 99906 as const;
export const SOLANA_ERROR__TRANSACTION__FAILED_TO_DECOMPILE_ADDRESS_LOOKUP_TABLE_INDEX_OUT_OF_RANGE = 99907 as const;
export const SOLANA_ERROR__TRANSACTION__FAILED_TO_DECOMPILE_INSTRUCTION_PROGRAM_ADDRESS_NOT_FOUND = 99908 as const;
export const SOLANA_ERROR__TRANSACTION__FAILED_TO_DECOMPILE_FEE_PAYER_MISSING = 99909 as const;

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also don't feel too strongly here but the double underscore is a bit more readable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like Next.js' build system inlines the codes just fine, so however long we want to make these names is fine by me!

image.png

@steveluscher steveluscher force-pushed the 02-27-Convert_errors_in_solana/transactions_to_coded_exceptions branch from cfb0332 to 3468945 Compare February 29, 2024 18:48
@steveluscher steveluscher merged commit 75a18e3 into master Feb 29, 2024
4 of 6 checks passed
@steveluscher steveluscher deleted the 02-27-Convert_errors_in_solana/transactions_to_coded_exceptions branch February 29, 2024 18:49
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 2, 2024

🎉 This PR is included in version 1.90.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions GitHub Actions
Copy link
Contributor

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lorisleiva lorisleiva lorisleiva approved these changes

@buffalojoec buffalojoec Awaiting requested review from buffalojoec

@mcintyre94 mcintyre94 Awaiting requested review from mcintyre94

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@steveluscher @lorisleiva @buffalojoec