-
Notifications
You must be signed in to change notification settings - Fork 240
SIMD-0385: Transaction V1 Format #385
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| - 'v1 transaction' - A new transaction format that is designed to enable larger | ||
| transactions sizes while not having the address lookup table features | ||
| introduced in v0 transactions. |
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 mention v1 does not need (or support) compute budget instructions?
| - `num_readonly_unsigned_accounts` addresses for which the transaction does | ||
| not contain signatures and are loaded as readonly | ||
|
|
||
| Any section with 0 addresses is skipped. |
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.
nit - the sentence is a bit ambiguous since both "section" and "skipped" aren’t clearly defined. It may be clearer to just remove it.
|
|
||
| - If bits [0, 1] are not set, the priority-fee is 0. | ||
| - If bit 2 is not set, the requested compute-unit-limit is 0. | ||
| - If bit 3 is not set the requested accounts data size limit is 0. |
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.
note - in v0/legacy, the default of this is "MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES". Setting it to 0 in v1 may be intentional, just want to pointing it out.
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.
it's intentional but yes it is different so probably best to call that out
| the bits is set, the transaction is invalid and cannot be included in blocks. | ||
|
|
||
| For TxV1 transactions, any ComputeBudgetProgram instructions are ignored for | ||
| configuration, even if they are invalid. |
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.
Curious — why ignore the ComputeBudget instruction rather than reject the transaction outright?
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.
then i'd still have to scan over the ixs to validate 😄
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.
makes sense. it could be a runtime error if txv1 includes ComputeBudget program. This can happen when CB program is deprecated.
| ### Transaction Constraints | ||
|
|
||
| Any transaction violating these constraints will be considered invalid and will | ||
| not be included in the chain. Violations are considered sanitization failures: |
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.
nit - “invalid” and “sanitization failures” are mentioned in several places. It might help to consolidate all validation criteria into a single section for easier reference.
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.
Compiled the sanitization rules below, do I miss anything?
- Version must be 129.
- NumInstructions <= 64.
- NumAddresses <= 64 and >= num_required_signatures + num_readonly_unsigned_accounts.
- Signature count matches num_required_signatures and is capped (<= 12).
- No duplicate addresses.
- Each instruction account index must be < num_addresses.
- No trailing data after signatures.
- Transaction total size cap (<= 4096).
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.
I think technically the duplicate address check is performed following sanitization and it also checks the lock limit, but I don't see why this couldn't be done earlier.
Legacy also requires the fee-payer to not be a program index and the fee-payer be RW.
Maybe beyond the scope of this proposal.... Would it be possible to demote additional executable-account write-locks by requiring the tx to statically provide indices of program keys only invoked in CPI and not top-level instructions? It would eliminate needless locking and JIT compilation.
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.
We definitely need to revisit write lock demotion but I'm not sure this list would make a big difference. I think it's usually a safe assumption to load the program cache for all programs included in a transaction because they'll probably be invoked.
|
While the idea of increasing the transaction size to 4kB is great, in the absence of ALTs the limit of 64 accounts is way too low. Current limit w/o ALTs is 35, why bump the tx size 4x but the account number by less than 2x? This is especially important for DEX aggregation. A single DEX swap on average touches 10+ accounts (this varies between DEX-es, some are at 8, some at 15+) and the current way of doing things is to shove all pool-specific accounts (token accounts, accounts holding DEX state, configs, token programs etc.) into one ALT and greatly reduce the amount of data and accounts used. If I understand correctly, current cap is at 256 accounts touched this way (https://docs.anza.xyz/proposals/versioned-transactions/?utm_source=chatgpt.com#limitations) and with other limitations -- 1.4M CU and 1232 bytes tx size -- this allows for ~10 swaps per transaction. Having a limit at 64 greatly reduces possible route space, realistically to 4-5 swaps per tx. This will result in worse prices for all people swapping on Solana. I suggest bump the account limit to 128. This is still not great, but way less limiting than a hard cap at 64. |
There is feature gate to bump limit to 128. |
|
i haven't even read this thing but can we reserve some header space for things in the future? success/failure fees, for example. we should reserve 12 or 16 bytes in the header |
Not opposed to it if expect to use it soon. If the need isn’t immediate, we could also keep things simpler for now and introduce it later as part of a “v2”. |
if u promise v2 can be done quickly, let's just wait |
| LifetimeSpecifier is the same as the Recent Blockhash field in prior | ||
| transaction formats. It has been renamed to clarify its use without changing | ||
| its meaning. |
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.
does this mean that it is in fact a recent blockhash, or that it can also be a durable nonce? we have been debating how to improve or replace nonce transactions, ideally a way to offer similar functionality but require they be normal blockhash transactions from the pov of the runtime
if no one objects, i would suggest officially not supporting nonce transactions for this transaction format. we can always change our mind and support it, but its very hard to change our mind and unsupport it
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.
I support this SIMD to be explicit on not supporting Nonce transaction and compute budget instructions.
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.
For nonce transactions I am going to make a different SIMD addressing that. Nonce transactions are too controversial not to be in a separate SIMD, but also should not hold up transaction v1
This is the point of the resources mask it's done. Please read, I'm very confident you know how. |
I have separated the Transaction V1 format from the larger transaction size SIMD #296. Both of these SIMDs would eventually have the same feature gate if they are accepted.