From b2b0c4eb4124324000eb782ebbc1be7a74e032b8 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 6 Nov 2024 09:21:38 -0600 Subject: [PATCH 1/7] Initial draft --- ...enable-transaction-loading-failure-fees.md | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 proposals/0191-enable-transaction-loading-failure-fees.md diff --git a/proposals/0191-enable-transaction-loading-failure-fees.md b/proposals/0191-enable-transaction-loading-failure-fees.md new file mode 100644 index 000000000..79c9431b9 --- /dev/null +++ b/proposals/0191-enable-transaction-loading-failure-fees.md @@ -0,0 +1,106 @@ +--- +simd: '0191' +title: Relax Transaction Loading Constraints +authors: + - Andrew Fitzgerald (Anza) +category: Standard +type: Core +status: Review +created: 2024-11-06 +feature: PaymEPK2oqwT9TXAVfadjztH2H6KfLEB9Hhd5Q5frvP +supersedes: +superseded-by: +extends: +--- + +## Summary + +This proposal aims to relax certain transaction errors related to loading +transaction accounts, from protocol violations to runtime errors. +Specifically, if a transaction fails to load a valid program account or +exceeds the requested maximum loaded account data size, the transaction +may be included in a block, and the transaction fee will be charged. + +## Motivation + +The current transaction constraints are overly restrictive and adds complexity +in determining whether a block is valid or not. +This proposal aims to relax these loading constraints to simplify the protocol, +and give block-producers more flexibility in determining which transactions +may be included in a block. +The goal is to remove this reliance on account-state in order to validate a +block. + +## New Terminology + +These terms are used elsewhere, but are defined here for clarity: + +- Protocol Violating Transaction Error: A transaction error that violates the + protocol. This class of errors must result in the entire block being rejected + by the network. +- Runtime Transaction Error: A transaction error that results in a failed + transaction, and may be included in the block. These transactions still + incur transaction fees, and nonce advancements. + +## Detailed Design + +Among others, a transaction that fails to load due to violating one of the +following constraints is considered a protocol violation error: + +1. The total loaded data size of the transaction must not exceed + `requested_loaded_accounts_data_size_limit`, or the default limit (64MiB). +2. Any account used as a program in a top-level instruction must: + - be the native loader: `NativeLoader1111111111111111111111111111111` + - OR + - exist + - be executable + - be owned by the native loader: `NativeLoader1111111111111111111111111111111` + - OR + - exist + - be executable + - the owner account be owned by the native loader: `NativeLoader1111111111111111111111111111111` + - the owner account must be executable + +This proposal moves these errors from protocol violations to runtime errors. +A transaction that fails to load due to violating either one of these +constraints may be included in a block, so long as it is otherwise valid. +The transaction must pay transaction fees, and if present, the nonce must be +advanced. + +## Alternatives Considered + +- Do nothing + - This is the simplest option, as we could leave the protocol as is. + However, this leaves the protocol more complex than it needs to be. +- Relax additional constraints: + - SIMD-0082 sought to relax additional constraints, but has not been + accepted. This proposal is a subset of SIMD-0082, intended to make the + review process simpler and faster. Therefore, we have decided to keep + this proposal focused specifically on certain loading failures. + +## Impact + +- Transactions that would previously have been dropped with a protocol + violation error can now be included and will be charged fees. + - Users must be more careful when constructing transactions to ensure they + are executable if they do not want to waste fees. +- Block-production is simplified as it can be done without needing to load + large program accounts for the initial decision to include a transaction. + +## Security Considerations + +None + +## Drawbacks + +- Users must be more careful about what they sign, as they will be charged fees + for transactions that are included in a block, even if they are not executed. +- This will likely break a lot of tooling, such as explorers, which may expect + all transactions to attempt execution. + +## Backwards Compatibility + +This proposal is backwards compatible with the current protocol, since it only +relaxes constraints, and does not add any new constraints. All previously valid +blocks would still be valid. However, new blocks may not be valid under the old +protocol. From 0e3b852b0ee8bf9679513170d8854ccb05f5c2ed Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 6 Nov 2024 10:32:33 -0600 Subject: [PATCH 2/7] link to feature --- proposals/0191-enable-transaction-loading-failure-fees.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/0191-enable-transaction-loading-failure-fees.md b/proposals/0191-enable-transaction-loading-failure-fees.md index 79c9431b9..1848e45e5 100644 --- a/proposals/0191-enable-transaction-loading-failure-fees.md +++ b/proposals/0191-enable-transaction-loading-failure-fees.md @@ -7,7 +7,7 @@ category: Standard type: Core status: Review created: 2024-11-06 -feature: PaymEPK2oqwT9TXAVfadjztH2H6KfLEB9Hhd5Q5frvP +feature: PaymEPK2oqwT9TXAVfadjztH2H6KfLEB9Hhd5Q5frvP (https://github.com/anza-xyz/agave/issues/3244) supersedes: superseded-by: extends: From cc73e72160586cf529f7d7ddb753819b3abc5103 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Thu, 14 Nov 2024 13:12:17 -0600 Subject: [PATCH 3/7] specify when to check, error variants --- ...enable-transaction-loading-failure-fees.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/proposals/0191-enable-transaction-loading-failure-fees.md b/proposals/0191-enable-transaction-loading-failure-fees.md index 1848e45e5..8a73ecc25 100644 --- a/proposals/0191-enable-transaction-loading-failure-fees.md +++ b/proposals/0191-enable-transaction-loading-failure-fees.md @@ -67,6 +67,30 @@ constraints may be included in a block, so long as it is otherwise valid. The transaction must pay transaction fees, and if present, the nonce must be advanced. +Constraints must be checked before committing transactions and voting. +It is reccomended that the constraints are checked before transaction +execution, in order to avoid unnecessary computation. + +The `TransactionError` variants do not need to change from their current +values. This proposal only changes how the validator handles these errors. +For completeness, the following error variants are affected: + +- `MaxLoadedAccountDataSizeExceeded` - breaking constraint 1: + - Loaded acount data exceeds the specified or default limit. + - There are proposed changes to the evaluation of this constraint: + [SIMD-0186](https://github.com/solana-foundation/solana-improvement-documents/pull/186) +- `ProgramAccountNotFound`, `InvalidProgramForExecution` - breaking constraint 2. + Currently checks are performed in this order: + - (`ProgramAccountNotFound`) Transaction-level instruction invokes an + account that does not exist + - (`InvalidProgramForExecution`) Transaction-level instruction invokes an + account that is not executable + - (`InvalidProgramForExecution`) Transaction-level instruction invokes an + account which is not owned by an account which is owned by the native + loader (only relevant after [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162)) + - (``ProgramAccountNotFound`) Transaction-level instruction invokes an + account whose owner does not exist (only relevant after [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162)) + ## Alternatives Considered - Do nothing From 41d47a95ffe9c128974a5fda995f40ba1b09d4cc Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 15 Nov 2024 09:20:03 -0600 Subject: [PATCH 4/7] reccomendation of check time --- proposals/0191-enable-transaction-loading-failure-fees.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proposals/0191-enable-transaction-loading-failure-fees.md b/proposals/0191-enable-transaction-loading-failure-fees.md index 8a73ecc25..52f2765d2 100644 --- a/proposals/0191-enable-transaction-loading-failure-fees.md +++ b/proposals/0191-enable-transaction-loading-failure-fees.md @@ -67,9 +67,9 @@ constraints may be included in a block, so long as it is otherwise valid. The transaction must pay transaction fees, and if present, the nonce must be advanced. -Constraints must be checked before committing transactions and voting. -It is reccomended that the constraints are checked before transaction -execution, in order to avoid unnecessary computation. +Constraints SHOULD be checked for each transaction before execution to avoid +unnecessary computation. If a constraint violating transaction is executed, the +constraints MUST be checked BEFORE committing transaction changes. The `TransactionError` variants do not need to change from their current values. This proposal only changes how the validator handles these errors. From 30f2741e0964686558ade055f0a1370c56e08770 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 15 Nov 2024 14:20:16 -0600 Subject: [PATCH 5/7] order --- ...enable-transaction-loading-failure-fees.md | 47 ++++++++++++------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/proposals/0191-enable-transaction-loading-failure-fees.md b/proposals/0191-enable-transaction-loading-failure-fees.md index 52f2765d2..63e3743e8 100644 --- a/proposals/0191-enable-transaction-loading-failure-fees.md +++ b/proposals/0191-enable-transaction-loading-failure-fees.md @@ -73,23 +73,36 @@ constraints MUST be checked BEFORE committing transaction changes. The `TransactionError` variants do not need to change from their current values. This proposal only changes how the validator handles these errors. -For completeness, the following error variants are affected: - -- `MaxLoadedAccountDataSizeExceeded` - breaking constraint 1: - - Loaded acount data exceeds the specified or default limit. - - There are proposed changes to the evaluation of this constraint: - [SIMD-0186](https://github.com/solana-foundation/solana-improvement-documents/pull/186) -- `ProgramAccountNotFound`, `InvalidProgramForExecution` - breaking constraint 2. - Currently checks are performed in this order: - - (`ProgramAccountNotFound`) Transaction-level instruction invokes an - account that does not exist - - (`InvalidProgramForExecution`) Transaction-level instruction invokes an - account that is not executable - - (`InvalidProgramForExecution`) Transaction-level instruction invokes an - account which is not owned by an account which is owned by the native - loader (only relevant after [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162)) - - (``ProgramAccountNotFound`) Transaction-level instruction invokes an - account whose owner does not exist (only relevant after [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162)) + +For each `Pubkey` included in the transaction message, or loaded from an +address lookup table, the following checks MUST be performed, and SHOULD be +performed in this order for error-consistency: + +- Check if the account exists. If not, assume default account state (empty). +- Accumulate account's `data` field `len`. If the total exceeds the + `requested_loaded_accounts_data_size_limit` (or default if unspecified), + return `MaxLoadedAccountDataSizeExceeded`. + +For each transction-level instruction in the transaction the following +checks MUST be performed, and SHOULD be performed in this order for +error-consistency: + +- If the program account is `native_loader`, continue to next + instruction. +- If the program account does not exist, return `ProgramAccountNotFound` +- If the program account is not executable, return ``InvalidProgramForExecution` + - This only applies until [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162) is activated +- If the program account's owner is the native_loader, continue to next + instruction. +- If the program account's owner does not exist, return `ProgramAccountNotFound` +- If the program account's owner is not the native_loader, return ``InvalidProgramForExecution` +- If the program account's owner is not executable, return ``InvalidProgramForExecution` + - This only applies until [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162) is activated +- Accumulate the owner account's `data` field `len` and check if the total + exceeds the `requested_loaded_accounts_data_size_limit` (or default if + unspecified), return `MaxLoadedAccountDataSizeExceeded`. + - The owner's data size MUST only be accumulated on the first instruction that uses the + program account. ## Alternatives Considered From 89ccac3159fecdff9e4f4f0473e47b88a4afdb1a Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 15 Nov 2024 14:26:06 -0600 Subject: [PATCH 6/7] clarifying following --- proposals/0191-enable-transaction-loading-failure-fees.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proposals/0191-enable-transaction-loading-failure-fees.md b/proposals/0191-enable-transaction-loading-failure-fees.md index 63e3743e8..5ed764db1 100644 --- a/proposals/0191-enable-transaction-loading-failure-fees.md +++ b/proposals/0191-enable-transaction-loading-failure-fees.md @@ -74,6 +74,9 @@ constraints MUST be checked BEFORE committing transaction changes. The `TransactionError` variants do not need to change from their current values. This proposal only changes how the validator handles these errors. +`agave` currently performs the relevant checks in the following order. +This order is not necessary for consensus, and is only provided for clarity. + For each `Pubkey` included in the transaction message, or loaded from an address lookup table, the following checks MUST be performed, and SHOULD be performed in this order for error-consistency: From ebed71e291c41d8791b6e92f27a416847bbacb16 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Mon, 18 Nov 2024 07:23:17 -0600 Subject: [PATCH 7/7] linting --- .../0191-enable-transaction-loading-failure-fees.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/proposals/0191-enable-transaction-loading-failure-fees.md b/proposals/0191-enable-transaction-loading-failure-fees.md index 5ed764db1..512979917 100644 --- a/proposals/0191-enable-transaction-loading-failure-fees.md +++ b/proposals/0191-enable-transaction-loading-failure-fees.md @@ -94,18 +94,22 @@ error-consistency: instruction. - If the program account does not exist, return `ProgramAccountNotFound` - If the program account is not executable, return ``InvalidProgramForExecution` - - This only applies until [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162) is activated + - This only applies until + [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162) + is activated - If the program account's owner is the native_loader, continue to next instruction. - If the program account's owner does not exist, return `ProgramAccountNotFound` - If the program account's owner is not the native_loader, return ``InvalidProgramForExecution` - If the program account's owner is not executable, return ``InvalidProgramForExecution` - - This only applies until [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162) is activated + - This only applies until + [SIMD-0162](https://github.com/solana-foundation/solana-improvement-documents/pull/162) + is activated - Accumulate the owner account's `data` field `len` and check if the total exceeds the `requested_loaded_accounts_data_size_limit` (or default if unspecified), return `MaxLoadedAccountDataSizeExceeded`. - - The owner's data size MUST only be accumulated on the first instruction that uses the - program account. + - The owner's data size MUST only be accumulated on the first instruction + that uses the program account. ## Alternatives Considered