Add a single-extend-per-transaction option for exported transactions - #113
Conversation
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Adds --single-extend-per-tx / singleExtendPerTransaction so exported transactions never grow a metadata or buffer account by more than REALLOC_LIMIT within one transaction, which is what the runtime enforces when the instructions run via CPI (Squads etc.). A custom MessagePackerInstructionPlan computes the remaining growth budget of the candidate message from the program-metadata instructions already in it (allocate = +96, extend = +length, write/setData = unknown → refuse) and sizes each extend accordingly. The dense path drops Kit's getReallocMessagePackerInstructionPlan for a local chunker to avoid the trailing 0-byte extend on exact multiples. REALLOC_LIMIT moves to utils.ts and is exported.
I checked the program's write processor to confirm it resizes to max(current, offset + len) — so the 'unknown' treatment is correct, and post-extend writes/setData are 0-growth, which is what makes "rest of the plan follows the last extend" safe. The packer mirrors Kit's packMessageToCapacity(message, { maxInstructions }) contract and 16-instruction default correctly, and the planner recovers from both error codes it throws. The plumbing through createMetadata / updateMetadata / writeMetadata / createBuffer and the four CLI commands is complete (top-level builders spread input, so the flag reaches the leaf builders). The --export guard in getClient covers every command.
One real gap worth closing
The extend plan is only inserted when dataLength > REALLOC_LIMIT, but under CPI the relevant quantity in the create paths is ACCOUNT_HEADER_LENGTH + dataLength, because allocate sits in the same transaction. For getCreateMetadataInstructionPlanUsingExistingBuffer (createMetadata.ts ~L188) with 10_144 < dataLength <= 10_240, no extend is emitted, and the single exported tx does allocate (+96) then write from the source buffer (+dataLength) — e.g. 10,296 bytes of growth → InvalidRealloc. That's precisely the README's Squads workflow (write idl --buffer … --export), so the option silently doesn't help in that 96-byte window. The same gate exists in …UsingNewBuffer (L137) and getPdaBufferInstructionPlan, though there writes are ~1KB chunks so at most one lands per tx and it isn't a practical problem.
Suggested fix: gate on dataLength + ACCOUNT_HEADER_LENGTH > REALLOC_LIMIT (at least when singleExtendPerTransaction is set — doing it unconditionally is also fine, it just adds one harmless extend to the same tx in dense mode). The packer already does the right thing from there: tx1 gets extend(10_144), tx2 gets extend(56) + write + initialize. A planning-level test at e.g. dataLength = REALLOC_LIMIT would pin it down. See inline comment.
Notes for other reviewers
- The e2e tests run top-level, so they can't observe the CPI limit — the real guard is the planning-level
getGrowthassertion inextendInstructionPlan.test.ts. Worth keeping that test honest if the packer changes. DEFAULT_MAX_INSTRUCTIONS_PER_TRANSACTION = 16duplicates Kit's default (correct as of Kit v7). It only matters when the planner doesn't passconfig.maxInstructions, which it does in practice, so drift risk is low.- Update paths are fine as-is: no
allocatein those transactions, sosizeDifference <= REALLOC_LIMITmeanssetDataalone grows by at most the limit. - The
budget <= 0branch builds a throwaway message just to populatenumBytesRequiredfor the error — cheap and only hit once per tx boundary, fine.
trevor-cortex
left a comment
There was a problem hiding this comment.
Re-review
The gap from my previous pass is closed. The new needsExtend(dataLength) helper gates on ACCOUNT_HEADER_LENGTH + dataLength > REALLOC_LIMIT and is used in all three allocate-based create paths (getCreateMetadataInstructionPlanUsingNewBuffer, …UsingExistingBuffer, getPdaBufferInstructionPlan). Plumbing is intact — the canonical/non-canonical buffer builders spread input into getPdaBufferInstructionPlan, so the flag reaches the extend plan. getCreateBufferInstructionPlan (keypair buffer via system createAccount) correctly stays untouched since it sizes the account up front and has no extend.
Coverage for the fix is solid:
- Planning-level:
accounts for the header when the data alone fits within the realloc limitpinsdataLength = REALLOC_LIMITwith the flag →[allocate, extend(10_144)]then[extend(96), write, initialize], with thegetGrowthassertion on every message. The dense counterpart asserts a singleextend(10_240)in one tx. - E2E: the new
creates a canonical metadata account using an existing buffer as large as the realloc limitcase runs both modes on-chain, which also confirms the now-unconditional extraextendin dense mode is harmless (writeresizes tomax(current, offset + len), so it's a no-op after the extend).
Small behavioural note for the changelog, not a blocker: dense mode now emits one extend for 10_144 < dataLength <= 10_240 where it previously emitted none. That's a tiny extra instruction in the same transaction and strictly safer, so I think unconditional is the right call over gating it on the flag.
Everything else from my first review stands (packer contract, 'unknown' handling, --export guard, update paths). Approving.
This PR adds a `--single-extend-per-tx` CLI option (and a matching `singleExtendPerTransaction` input on the JS client's plan builders) for transactions that are exported and later executed through a CPI, e.g. by a Squads multisig. In that setting the Solana runtime applies the 10,240-byte realloc limit to the whole top-level instruction rather than to each inner instruction, so the extend instructions the client packs into a single transaction to grow a metadata or buffer account beyond 10KB fail with `InvalidRealloc`. When the option is set, a dedicated message packer ensures an account never grows by more than the realloc limit within a single transaction. It sizes each `extend` from the growth budget the candidate transaction has left, accounting for program-metadata instructions already present in it, so the first extend shares its transaction with the rent transfer and the `allocate` that creates the 96-byte header, every following extend gets its own transaction, and the rest of the plan (writes, `initialize` or `setData`) follows the last one. The option only applies to exported transactions and errors out without `--export`, since transactions run by the CLI directly execute top-level and are not affected. The default (dense) packing now computes the extend chunk sizes locally instead of relying on Kit's `getReallocMessagePackerInstructionPlan`, which emits a final 0-byte extend when the total is an exact multiple of 10,240 and leaves the account one chunk short. This is a stopgap until the upstream fix ships. `REALLOC_LIMIT` is now exported from the client and reused by the tests instead of being redeclared. Tests cover both packing modes at the planning level (per-transaction growth, exact-multiple chunking, the allocate header adjustment) and end to end with 25KB payloads through the create and update-from-buffer paths, which previously had no coverage above the realloc limit. The README documents the new option and when the Squads workflow needs it.
Under CPI, the `allocate` and the first write share a transaction, so an account holding between 10,144 and 10,240 bytes of data would grow by more than the realloc limit despite `--single-extend-per-tx`, since no extend plan was emitted for it. Creation paths now gate the extend plan on `ACCOUNT_HEADER_LENGTH + dataLength` via a new `needsExtend` helper. The gate is unconditional: in dense mode it only adds one extend instruction to the same transaction for that 96-byte window.
5a15b66 to
f3d8674
Compare
febo
left a comment
There was a problem hiding this comment.
Looks great! I only have a few suggestions on the comments.
Adopts Febo's wording suggestions on the docblocks and README to say the realloc limit applies per top-level instruction and to spell out why an exported transaction cannot resize an account by more than 10KB.
Reworded the CLI help and two docblocks so they no longer suggest the runtime changes its realloc rule under CPI. The limit always applies per top-level instruction; when an exported transaction is executed through a CPI it runs as a single top-level instruction, so the whole transaction is subject to it.
This PR adds a
--single-extend-per-txCLI option (and a matchingsingleExtendPerTransactioninput on the JS client's plan builders) for transactions that are exported and later executed through a CPI, e.g. by a Squads multisig. In that setting the Solana runtime applies the 10,240-byte realloc limit to the whole top-level instruction rather than to each inner instruction, so the extend instructions the client packs into a single transaction to grow a metadata or buffer account beyond 10KB fail withInvalidRealloc.When the option is set, a dedicated message packer ensures an account never grows by more than the realloc limit within a single transaction. It sizes each
extendfrom the growth budget the candidate transaction has left, accounting for program-metadata instructions already present in it, so the first extend shares its transaction with the rent transfer and theallocatethat creates the 96-byte header, every following extend gets its own transaction, and the rest of the plan (writes,initializeorsetData) follows the last one. The option only applies to exported transactions and errors out without--export, since transactions run by the CLI directly execute top-level and are not affected.The default (dense) packing now computes the extend chunk sizes locally instead of relying on Kit's
getReallocMessagePackerInstructionPlan, which emits a final 0-byte extend when the total is an exact multiple of 10,240 and leaves the account one chunk short. This is a stopgap until the upstream fix ships.REALLOC_LIMITis now exported from the client and reused by the tests instead of being redeclared.Tests cover both packing modes at the planning level (per-transaction growth, exact-multiple chunking, the allocate header adjustment) and end to end with 25KB payloads through the create and update-from-buffer paths, which previously had no coverage above the realloc limit. The README documents the new option and when the Squads workflow needs it.