SIMD-0296: Larger Transaction Size#296
Conversation
|
Very excited for this! One point about this comes to mind for me upon a read-through: This SIMD explicitly leaves the fee structure for larger txs as an open question. Imo it would be better to settle this before merging this SIMD, as otherwise we run the risk of determining a fee structure that requires some extra changes to the proposed v1 tx format / lock ourselves into the v1 format prematurely. Also, iiuc the proposed |
mcintyre94
left a comment
There was a problem hiding this comment.
Definitely in favour of this SIMD! Just some comments about the proposed message format.
| For example bit 0 may mean "requested_cus", bit 1 may mean "microlamports per | ||
| cu" | ||
| NumStaticKeys (u8) | ||
| RecentBlockhash (hash) |
There was a problem hiding this comment.
Are we removing durable nonce support for v1 transactions?
There was a problem hiding this comment.
If not, then rename this to LifetimeSpecifier.
There was a problem hiding this comment.
As part of these discussions, it was proposed to get rid of nonce because it hits accountsdb, see https://github.com/solana-foundation/solana-improvement-documents/pull/296/files#r2129100914
There was a problem hiding this comment.
We can't let noncery block this proposal. Rename it to LifetimeSpecficier seems fine.
There was a problem hiding this comment.
I'm still in favor of removal of nonce support for v1 transactions. With v0 and legacy still coexisting with v1, this shouldn't be a blocker.
|
@jacobcreech I love this idea. Thank you for putting it together and for all your hard work on it. Simply because QUIC doesn't specify a maximum stream size does not give license to arbitrarily increase the size. I love the idea of increasing size and want it, but im curious if this SIMD is a rubber stamp of something the core team already wants to do or if this is an actual proposal. The Solana team an community are much more thorough in how proposals are brought and I would at minimum expect some kind of analysis on how shoving 4kb through a normally 1kb pipe will affect the network. Will it require new hardware or requirements for those not on rented setups? Will the network cards need to be tweaked? One thing I really fee strongly about is to USE THE TXN VERSIONING, which you have put forth. I have seen rumblings of some new versioning thing or not doing this under a version. Breaking compatibility just kneecaps solana adoption because so many developers will just get errors and users will suddenly have daaps not working. Yes these devs should update their apps, but the reality is that some of the biggest apps with dedicated teams still dont update things(UIs, Clis, Tooling) quickly "excuse me ...cough cough ...Raydium" or they are still on legacy txns with no priority fee (I cant even tell you who this is). Another problem will be that documentation will instantly be broken, and people will be building apps with LUTs and so forth while thinking that its the correct documentation, but it wont work unless there is a commitment to backward compatibility(which sucks, its painful). Lets call it a draw then, for the pain we were all put under having to use LUTs and the PDA model in the first place, core team can relent and feel the pain of managing another version, or perhaps having a rollout of deprecating legacy txns (that should get the big boys moving). Again thanks, and Im so excited for this. (Not written by AI) |
|
Can we get some more details on validation/invariants? For example, the sum of the 3 values in the legacy header must be no more than
Where do the offsets in the
|
|
Can we add a field for the fee that the transaction pays in lamports? I think we should take advantage of the opportunity this new format presents to switch from pricing in We've also talked about the idea of a failure fee (or an in-protocol extra fee for success). This could be a use of the ResourceRequestMask, but I don't know that it fits the name exactly. More broadly, we've talked about getting bundles in protocol. We should think about this as we develop this new format. |
| ResourceRequests [u64] -- Array of request values. (section size is popcount | ||
| ResourceRequestMask * 8). each value is a u64. | ||
| Ixs [(u16, u16, u16, u16)] -- Number matches NumInstructions. Values are | ||
| (num_accounts, accounts_offset, num_data_bytes, bytes_offset). |
There was a problem hiding this comment.
I presume that the accounts which are looked up in the TRAILING_DATA_SECTION are made up of (u8) indexes into the StaticKeys array above. If true, this design adds 2 bytes of data per account instruction over the existing design.
This SIMD (worst case):
- u8 num ’static keys‘
- [u8; 32] per ‘static key’
- u16 num accounts per instruction
- u16 num accounts offset per instruction
- u8 index of account in the trailing data section per address per instruction
Fixed cost: 1 byte (num ‘static keys’)
Per instruction cost: 4 bytes (num accounts + accounts offset)
Per address cost: (32 + num_instructions) bytes (key + entry in TRAILING_DATA_SECTION per instruction)
Previously (worst case):
- u8 num ‘static keys’
- [u8; 32] per ‘static key’
- compact-u16-array account indexes per address per instruction
Fixed cost: 1 byte (num ‘static keys’)
Per instruction cost: 2 bytes (compact-u16 index array length)
Per address cost: (32 + num_instructions) bytes (key + entry instruction's compact-u16 array)
There was a problem hiding this comment.
It's unclear to me where you're getting 2 extra bytes per account. The design is definitely less space efficient, that's fine from our view since it's paired with a tx-size increase. It's better have constant sized fields than deal with compressed u16s inherent branchiness.
There was a problem hiding this comment.
I said ‘per account’; meant ‘per instruction.’ Sorry.
The scheduler is not maintained via the SIMD process today and it is in validator's best interests to pack the blocks in a way that makes them the most money. This SIMD stays in line with this thought process and does not recommend a structure or tie any direct fee or CUs to it.
Yes, this would supercede the proposal in 287. |
@austbot This SIMD is an actual proposal that has been discussed at length with multiple Solana core engineers and contributors. More testing will follow shortly with public review, just need to hack together a new cluster with these changes. |
@ptaffet-jump is this to move away from the CU pricing and just have that be something the scheduler decides?
Could you elaborate on this idea a bit more? I am not privy to these discussions. How would this failure fee work? |
I was thinking more from the developer's point of view, as I'd imagine that ceteris paribus a scheduler would always choose a tx with a smaller size over a bigger one, so having a way to be able to tell the scheduler "I care about this large size tx landing and am willing to pay extra for it" is important. But yea, I suppose in the near term the existing CU price can sort of be used for this and if a dedicated system for this is added in the future it would probably be something along the lines of "lamports per tx byte" or a fixed fee like suggested by @ptaffet-jump, both of which would fit in the new ResourceRequest API. |
This comment was marked as spam.
This comment was marked as spam.
|
One note is that if we increase the max size of the transaction v1 but not for v0, on the network layer side we cannot distinguish these two versions. Because to know the version we need to deserialize. This will create a workflow when we increase the stream size for both, but later check the version and drop those transactions v0 which are too large. Alternatively, we can introduce two streamers on two ports -- one taking transactions v0 and the other transactions v1 which would require changes in gossip to advertise both. |
Might be harder on network side to do that for sure, but we don't need to fully deserialize. The first byte in the transaction will tell us if it is a v1 or one of the legacy/v0 formats. |
Sounds like cheap solution: we can discard chunks if accumulated size is larger than expected for this type of tx. Do you know by chance if this version byte already set to 0 for transaction v0? if we need to add this flag for transaction v0 format? |
It's a bit more complicated because of legacy txs (before versioned) For v0 it's set to 128 |
jstarry
left a comment
There was a problem hiding this comment.
Note that we still limit the serialized size of builtin program instructions to 1232 bytes despite the increase in tx size here. I think that's fine and we can update those restrictions separately if desired.
The only one that looks useful to extend is the BPF Loader "write" instruction which should be able to write larger chunks of program data but is currently limited to at most 1216 (1232 - 4 - 4 - 8) bytes per chunk.
| | signatures per transaction | 12 | 12 | 42 | | ||
| | num accounts | 64 | 64 | 96 | | ||
| | num instructions | 64 | 64 | 255 | | ||
| | accounts/instruction | 255 | 255 | 255 | |
There was a problem hiding this comment.
Note that this actually wasn't a prior sanitization limit (it was only enforced in the runtime), it's a new sanitization constraint and will need to be added as part of the implementation of this SIMD.
| If a configured value, such as priority-fee, needs more than 4-bytes a field can | ||
| use 2 bits in the mask. | ||
|
|
||
| Initially supported fields and assigned bits: |
There was a problem hiding this comment.
I think if any bits are set beyond the first 5, the transaction is invalid and cannot be included in blocks.
There was a problem hiding this comment.
agree - to be clear we should allow for additions to be made later but until they are the txs should be marked as invalid.
|
why is tx v1 not its own simd? this thing is nearly incomprehensible due to the interleave of "bigger tx" and "tx v1" |
txv1 is tied to larger size - practically they cannot be separated because without lookup tables many mnb txs would not be able to use the txv1, so a txv1 without size increase is kind of useless. I may be a minority opinion, but I don't see value in separation here. Title should probably be updated to reflect that it's ALSO a format though. |
|
I am also in favour of bundling the txn format change and the size increase into a single change. There is little benefit to changing the format without increasing the transaction size, and we will have to change the parsing and processing code anyway for either change, so it's less risky to change this code once, not twice - same goes for users transaction sending code. |
|
@t-nelson updated the title to address the feedback. Larger transactions are part of the v1 format, otherwise I'd love to separate the SIMD. |
huh? v1 format can support larger payload without relaxing the limit. that would focus topics and improve simd quality |
I have now made this current SIMD to focus on the larger transaction size issue instead of also proposing a new format to decouple and make a smaller SIMD
|
I have removed the tx v1 specification to be defined in #385 to focus on this SIMD only on larger transaction sizes. |
| an explicit maximum stream size, allowing for larger transactions to be | ||
| sent. The larger transaction size would only be supported by the v1 transaction | ||
| format as described in SIMD-0385. Clients will need to support accepting the | ||
| larger transaction sizes on their network ports and not rejest the larger size |
There was a problem hiding this comment.
reject -> reject.
Let's also just re-iterate this should only not be rejected for supported tx formats (i.e. not current ones).
bw-solana
left a comment
There was a problem hiding this comment.
Approving on behalf of Anza.
Now brace for old man yelling at clouds... There's nothing to implement for this SIMD. This is purely a signaling SIMD, which isn't really what the SIMD process is for. #385 is where all the technical details reside that are actually implementable.
Yes, I recognize the audacity given there was an explicit request to fork out Transaction V1 format from this SIMD. I'm grumbling anyways.
|
I agree with @bw-solana, there is no point in separating this SIMD from #385 because they are inherently tied together. One cannot exist without the other since AFAIK there is no intention to enable v1 transactions with the legacy size limit of 1232 bytes. |
Today's transaction size limit is capped at 1232 bytes for the payload, significantly limiting some of the usecases that developers on Solana can do.
This proposal raises that limit to 4kb, with potential for raising higher on a new transaction format.