Repository navigation
Conversation
A settlement paying out exactly the native SOL buffer's whole balance was accepted by the runtime, which then deleted the account, leaving every later native SOL payout failing with ExternalAccountLamportSpend and no instruction able to recreate it. FinalizeSettle now rejects payouts leaving the buffer below its rent-exempt minimum with NativeSolBufferBelowRent. Fixes SC-552 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
The runtime already rejects a transaction leaving the buffer between zero and its rent-exempt minimum, so zero is the only balance FinalizeSettle needs to refuse. Dropping the Rent sysvar read brings a native payout's overhead from ~130 CU down to ~5. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kaze-cow
changed the title
kaze-cow
marked this pull request as ready for review
October 8, 2026 16:22
fedgiac
reviewed
Oct 8, 2026
fedgiac
left a comment
fedgiac
left a comment
Contributor
There was a problem hiding this comment.
Great catch!
Can you also add a test where two orders sum to the total balance?
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
…l-buffer-can-be-deleted-by-a-settlement-paying-out
Contributor
Author
The test 941a0d2 Perhaps it may be worth deleting the simpler version of the test which only has one order? Or is it worth keeping both? lmk if I should change it. |
…-by-a-settlement-paying-out
fedgiac
approved these changes
Oct 9, 2026
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
kaze-cow
deleted the
kaze/sc-552-native-sol-buffer-can-be-deleted-by-a-settlement-paying-out
branch
October 9, 2026 12:22
kaze-cow
added a commit
that referenced
this pull request
Oct 12, 2026
# Description Adds a `WithdrawNativeSol` instruction (discriminator 12) that moves a given `amount` of lamports out of the native SOL buffer to a recipient of the caller's choosing. Only the settlement-owned-order authority can call it. ## Motivation Fees paid out in native SOL build up in `NATIVE_SOL_BUFFER_PDA`, and right now nothing can get them out. The other buffers are emptied by selling their contents through `CreateSettlementOwnedOrder`, but SOL isn't a token, so it can't be the sell side of an order. Because the buffer is owned by the program, we can debit it directly, which makes this a small instruction. See SC-403. ## Considerations ### Which authority It's gated by `SettlementOwnedOrderAuthority`, since that role is already the one trusted with withdrawing fees from the buffers. Adding a new role would have changed the state layout, `Initialize` args, the IDL and the CLI just to hold a key that would most likely be the same one anyway. The role's doc comment, `DESIGN.md` and `SECURITY_DESIGN.md` now say it can do this too. ### Protecting the buffer's rent This uses the same approach as #192: Solana itself rejects a withdrawal that leaves the buffer partly below rent (`InsufficientFundsForRent`), so the only case the program has to catch is an empty buffer, which it rejects with the existing `NativeSolBufferEmptied`. I widened that error's doc to cover both instructions instead of adding a near-duplicate. The only new errors are `UnauthorizedNativeSolWithdrawal` (46) and `NativeSolBufferMismatch` (47). ### Explicit amount vs. sweep The caller passes an explicit `u64` instead of "everything above rent". That way partial withdrawals are possible, and the instruction doesn't have to read the `Rent` sysvar to work out the sweepable amount. ### Cost `withdraw_native_sol/happy_path_withdraws_to_a_recipient_chosen_by_the_authority`: 374 CU. The extra dispatch arm moves 15 other rows by 1–3 CU (all down). ### Tests The processor has unit tests for every rejection path and the happy path. Integration tests in `tests/withdraw_native_sol.rs` cover: - withdrawing to a third party - withdrawing to the authority itself - the other three roles being refused - dipping into rent - emptying the buffer - passing a different program-owned account as the buffer ## How to test `just test`. For just this instruction: `cargo test --test withdraw_native_sol` (after `just build-program`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
FinalizeSettlenow rejects a settlement whose native SOL payouts would leave the native SOL buffer with a zero balance, with a newNativeSolBufferEmptiederror.Motivation
Native SOL payouts are taken out of
NATIVE_SOL_BUFFER_PDAwithmove_lamports, which has no rent floor. The runtime already rejects a payout that leaves the buffer partially under-funded (InsufficientFundsForRent), but a payout of exactly the whole balance is accepted, and the runtime then deletes the account at the end of the transaction.Once deleted, the address goes back to being a system-owned account, so every later native SOL payout fails with
ExternalAccountLamportSpend(even if the buffer is topped up first). Nothing in the program can recreate it, sinceInitializecan't run twice andCreateBufferonly makes token accounts. Any SOL sent to the address afterwards would be stuck too. It takes a solver to do this (bug or malice), but recovering would need a program upgrade.This was confirmed with a litesvm test on
mainbefore applying the fix. See SC-552.Considerations
Zero check vs. a rent floor
The first version of this PR compared the remaining balance against the rent-exempt minimum from the
Rentsysvar. But the runtime only lets an account end a transaction either rent-exempt or at exactly zero, so anything in between already reverts withInsufficientFundsForRent(covered byrejects_a_push_spending_the_native_sol_buffers_rent). Zero is the only gap, and checking for it skips the sysvar read, which was nearly all of the cost.Making the buffer recreatable instead
The alternative was to let
CreateBuffer(or a new instruction) recreate the native buffer idempotently.CanonicalPda::create_idempotentalready handles a pre-funded address, so this wouldn't be much code. But it only repairs the damage after native settlements have already started failing, whereas this check means it can't happen in the first place.Cost
A settlement with a native payout costs 5 CU more (
happy_path_sell_tokens_for_native_sol: 4658 → 4663), every other settlement 1 CU more. The rent floor version was +131 CU.Tests
Two tests are added for the edges: paying out everything but the rent succeeds, and paying out the whole balance is rejected with
NativeSolBufferEmptied.How to test
just test. To see the original issue, revert thefinalize_settle.rschange and runcargo test --test settle_native_sol whole_native_sol_buffer: the draining settlement goes through.🤖 Generated with Claude Code