fix(escrow): return closed offer and vault rent to the maker; verify system program, offer owner and canonical bump - #4
Open
SwineCoder101 wants to merge 2 commits into
Open
Conversation
… misses account checks - native + pinocchio: Take Offer must return the closed vault's and offer account's rent to the maker who funded them, and the taker's balance must be unchanged - native: take_offer/refund_offer with a substitute account in the system_program slot must be rejected, and the offer must remain program-owned - pinocchio: make_offer with a valid but non-canonical bump for an existing (maker, id) must be rejected
…system program, offer owner and canonical bump - native + pinocchio take_offer: the vault CloseAccount destination and the offer account's lamports now go to the maker who funded them in make_offer, not to the taker / the taker's chosen payer. maker is now writable in both TS clients, matching the anchor variant. - native take_offer/refund_offer: verify the system_program account before assigning the closed offer to it, and require the offer account to be owned by this program. - pinocchio take_offer/refund_offer: require the offer account to be owned by this program. - pinocchio make_offer: derive the offer PDA with find_program_address and reject any bump other than the canonical one, so only one offer can exist per (maker, id).
SwineCoder101
force-pushed
the
fix/escrow-rent-to-maker
branch
from
August 27, 2026 12:23
97cfb45 to
30ddb00
Compare
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
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.
Bug: take_offer pays the maker's rent to the taker (plus unchecked system program / non-canonical bump)
This is a bug, not a style change. In the
nativeandpinocchioescrow variants,take_offercloses the vault and the offer account but sends their rent (~0.0037 SOL per offer) to the taker and to apayeraccount the taker picks, even though both accounts were funded by the maker inmake_offer. Any taker skims that amount from every maker (medium). Two smaller checks were also missing:nativecloses the offer by assigning it to whatever account is passed in thesystem_programslot, letting a taker squat a maker's(maker, id)offer slot under their own program (low), andpinocchio'smake_offeraccepts any valid non-canonical bump, letting one maker hold several live offers under one(maker, id)(low). Theanchorvariant is correct (close = maker, vaultCloseAccountdestination is the maker, canonical bump enforced) and was used as the reference.Affected
tokens/escrow/native/program/src/instructions/take_offer.rstokens/escrow/native/program/src/instructions/refund_offer.rstokens/escrow/native/tests/instruction.ts(client:makeris now writable inbuildTakeOffer)tokens/escrow/pinocchio/program/src/instructions/take_offer.rstokens/escrow/pinocchio/program/src/instructions/refund_offer.rstokens/escrow/pinocchio/program/src/instructions/make_offer.rstokens/escrow/pinocchio/tests/instruction.ts(client:makeris now writable inbuildTakeOffer)Functionality
make_offercreates the offer PDA[b"offer", maker, id]and its vault ATA, both paid for by the maker, and moves the maker's token A into the vault.take_offersends the taker's token B to the maker, releases the vaulted token A to the taker, and closes the vault and the offer account.refund_offerreturns the vaulted token A to the maker and closes both accounts. Whoever funded the closed accounts (the maker) should get their rent back, and a closed offer must be handed back to the system program so the maker can reuse the(maker, id)slot.The bug
Rent to the wrong party (native
take_offer.rs:188-204before the fix, pinocchiotake_offer.rs:136-148): the vaultCloseAccountdestination wastaker, and the offer account's lamports were credited topayer- the take instruction's own payer, which the taker chooses. The pinocchio comment "returning its rent to the payer that funded it" was wrong: that payer istake_offer's, notmake_offer's. Net effect: every take transfers the maker'soffer + vaultrent (1,677,360 + 2,039,280 = 3,716,640 lamports on a default litesvm/mainnet rent schedule) to the taker.Unverified system program (native
take_offer.rs:212andrefund_offer.rs:114before the fix):offer_info.assign(system_program.key)uses the caller-supplied account key without checking it, and neither instruction checkedoffer_info.owner == program_id. A taker can pass their own program in thesystem_programslot (both receiving ATAs already exist, so nothing else in the instruction touches the account) and, in the same transaction, top the emptied account back up to rent exemption. The maker's offer PDA is then permanently owned by the taker's program, so the maker can never create an offer with thatidagain. Pinocchio'sclose()hardcodes the system program owner, so only the owner check was missing there.Non-canonical bump (pinocchio
make_offer.rs:45-57before the fix): the client-supplieddata[24]bump was checked withcreate_program_address, which accepts any bump that yields an off-curve address. Roughly half of all bumps do, so a maker can create several live offers under one(maker, id), whichtake_offer/refund_offerand the anchor/native variants treat as unique. Native usesfind_program_address; anchor enforces the canonical bump.Reproduce
Tests added (all fail against the unmodified programs):
Take Offer returns the vault and offer rent to the maker, not the taker,Take Offer rejects a bogus system program account,Refund Offer rejects a bogus system program accountTake Offer returns the vault and offer rent to the maker, not the taker,Make Offer rejects a non-canonical offer bumpFailing output before the fix (native; pinocchio's rent test fails identically):
Fix
take_offer: the vaultCloseAccountdestination and the offer account's lamports now go tomaker, which the instruction already binds tooffer.maker.makermust therefore be writable; both TS clients (buildTakeOffer) pass it asWRITABLEnow, matching the anchor variant's#[account(mut)] maker.take_offer/refund_offer:system_programis checked againstsolana_system_interface::program::check_idbefore being used as the close target (IncorrectProgramIdotherwise), and the offer account must be owned by this program (IllegalOwnerotherwise).take_offer/refund_offer: added the sameoffer_account.owned_by(program_id)check.make_offer: the offer PDA is derived withfind_program_address; the supplied account must match it and the supplied bump must equal the canonical bump (InvalidSeedsotherwise). The instruction data layout is unchanged.Verification