-
Notifications
You must be signed in to change notification settings - Fork 109
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
SIMD-0162: Remove Accounts is_executable
Flag Checks
#162
Merged
joncinque
merged 2 commits into
solana-foundation:main
from
Lichtso:remove-accounts-executable-flag-checks
Nov 25, 2024
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
139 changes: 139 additions & 0 deletions
139
proposals/0162-remove-accounts-executable-flag-checks.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
--- | ||
simd: '0162' | ||
title: Remove Accounts `is_executable` Flag Checks | ||
authors: | ||
- Alexander Meißner | ||
category: Standard | ||
type: Core | ||
status: Draft | ||
created: 2024-07-16 | ||
feature: TBD | ||
extends: 0093 | ||
--- | ||
|
||
## Summary | ||
|
||
Remove all checks of accounts `is_executable` flag which can throw errors in | ||
the runtime. | ||
|
||
## Motivation | ||
|
||
Currently, a program account which satisfies the following conditions can be | ||
invoked / executed: | ||
|
||
- Has the `is_executable` flag set | ||
- Is owned by a loader | ||
- Contains a successfully verified ELF, contains the address of a program data | ||
account which does or is a built-in program | ||
|
||
The second condition can be used as a performance optimization to filter out | ||
accounts which definitely do not contain programs and the third condition alone | ||
would be sufficient to guarantee correct execution. The first condition is | ||
however is not only useless, it is downright deceptive and entangled with the | ||
confusing proxy account workaround in loader-v3. | ||
|
||
Originally, the `is_executable` flag meant that a program was deployed and | ||
finalized, which effectively renders its program account read-only forever, | ||
as the `is_executable` can not be cleared / reset. | ||
|
||
With the introduction of the upgradeable loader (loader-v3), instead of | ||
removing the first condition back then, a workaround was created: The program | ||
account became a proxy account which has the `is_executable` flag set and then | ||
points to the actual program data account which does not have the | ||
`is_executable` flag set. Meaning the `is_executable` flag now neither does | ||
reliably indicate that an account contains a program (as the program data | ||
account does not have the flag set), nor does it indicate that a program is | ||
finalized (as the program account has it set while the program remains | ||
upgradeable). | ||
|
||
Removing the first condition not only removes some redundant error checks from | ||
the runtime and confusion for future maintainers, but more importantly enables | ||
us to deploy loader-v4 which is upgradeable like loader-v3 but without the need | ||
for proxy accounts. | ||
|
||
## New Terminology | ||
|
||
None. | ||
|
||
## Detailed Design | ||
|
||
This proposal aims to unblock loader-v4 with minimal impact on the ecosystem by | ||
only removing checks of the `is_executable` flag. A complete removal of the | ||
flag can be addressed in a subsequent proposal. Thus, the following must remain | ||
unaffected for now: | ||
|
||
- Setting of the `is_executable` flag during program deployment in loader-v3 | ||
- Calculation of the account hashes | ||
- Minimization of snapshots | ||
- Serialization of instruction accounts `is_executable` flag for dapps | ||
- CPI ignores changes made by the caller to instruction accounts which have | ||
the `is_executable` flag set | ||
|
||
These checks of the `is_executable` flag must be removed: | ||
|
||
- `ExecutableLamportChange` during execution (transaction succeeds instead) | ||
|
||
These checks of the `is_executable` flag do not influence whether transactions | ||
fail or succeed because they are all covered by other checks. Thus, only the | ||
thrown error codes will change, which does not affect consensus. Nevertheless, | ||
they should still be removed because all implementations should aim to produce | ||
the same error codes: | ||
|
||
- during transaction loading: | ||
- `InvalidProgramForExecution` (fallthrough to `UnsupportedProgramId` during | ||
execution) | ||
- `AccountNotExecutable` (fallthrough to `UnsupportedProgramId` during | ||
execution) | ||
- Meaning the transaction loading checks are effectively deferred until | ||
execution, which gives users more complete transaction logs. | ||
|
||
- in the `Upgrade` instruction of loader-v3: | ||
- `AccountNotExecutable` | ||
(fallthrough to `IncorrectProgramId` or `InvalidAccountData`, depending on | ||
the owner) | ||
|
||
- during execution: | ||
- `AccountNotExecutable` (fallthrough to `UnsupportedProgramId` during | ||
execution) | ||
- `IncorrectProgramId` (fallthrough to `UnsupportedProgramId` during | ||
execution) | ||
- `ExecutableDataModified` | ||
(fallthrough to `ExternalAccountDataModified` during execution) | ||
- `ExecutableModified` | ||
(is unreachable, as it is overshadowed by owner and writability checks) | ||
- `ModifiedProgramId` | ||
(is unreachable, as it is overshadowed by owner and writability checks) | ||
|
||
Similarly the following checks during execution, unrelated to the | ||
`is_executable` flag, but related to whether an account contains a program or | ||
not, should be changed to throw `UnsupportedProgramId` instead: | ||
|
||
- `InvalidAccountData` for programs which are closed, in visibility delay, | ||
failed verification or not owned by a loader | ||
- `IncorrectProgramId` for unrecognized built-in programs | ||
|
||
All in all, the following error messages related to invocation of program | ||
accounts will be coalesced into `UnsupportedProgramId`, but some of them will | ||
remain in use in other circumstances unrelated to program execution: | ||
|
||
- `InvalidProgramForExecution` | ||
- `AccountNotExecutable` | ||
- `InvalidAccountData` | ||
- `IncorrectProgramId` | ||
- `UnsupportedProgramId` | ||
|
||
## Alternatives Considered | ||
|
||
None. | ||
|
||
## Impact | ||
|
||
Error codes of these conditions, which are rarely triggered, will change. | ||
|
||
The only consensus relevant change is that it will become possible (for | ||
everybody) to donate funds to program accounts. That however is expected not to | ||
break any existing programs. | ||
|
||
## Security Considerations | ||
|
||
None. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this change open doors for user to withdraw funds from program accounts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, it seems that this change will open doors for modifying
data
on executable account, and resetexecutable
flag itself on executable account. Are there any concerns about this for consensus? Should we document the impact of these changes in the SIMD?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users will remain unable to: Withdraw funds from program accounts, reset the
is_executable
flag, or change the data directly without going through the loaders program managements instructions. That is because for that you need to be the owner, but programs are owned by a loader. Thus, the loader decides what happens to program accounts.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Make sense.