feat(svm): enable V5 destination fills

[Reinis-FRP]

Summary

• enable the reserved Gateway-authenticated V5 Fill adapter branch with strict committed-recipient, output-mint, minimum-amount, exact-witness, exclusivity, deadline, and canonical relay-hash validation
• create the shared relay fill-status PDA from the submitter-scoped payer float, rejecting sibling fills and preserving permissionless post-expiry rent reclaim to that float
• pull exactly the JIT output amount from the canonical Gateway vault into the committed recipient ATA through the static v5_fill_delegate, accepting sufficient allowance
• support canonical Gateway-vault self-delivery without approval or self-transfer after authenticating the live balance, leaving the continuing atomic tape responsible for consumption
• route legacy and V5 fills through a shared _fill core that owns validation, normalized token delivery, fill-type/event resolution, and canonical FilledRelay construction while retaining only their distinct status-account lifecycle and callback mechanics
• emit the standard FilledRelay event with the supplied repayment fields, original witness hash, and an empty updated-message hash
• add validator coverage for external delivery, sibling competition/replay, commitment failures, exclusivity/deadline checks, paused fills, account/allowance failures, downstream rollback, payer reclaim, self-delivery, and final-empty-vault behavior
• update the V5 adapter specification and entrypoint documentation now that destination fills are enabled

Part of ACP-184. This is Step 4 of the implementation plan and is stacked on #1537.

Validation

• cargo +nightly fmt --all -- --check
• cargo check -p svm-spoke --features test
• cargo check -p mock-gateway --features test
• cargo test -p svm-spoke --lib --features test (15 passed, including shared-fill parity, guard, and in-place delivery coverage)
• cargo clippy -p svm-spoke --features test -- -A unexpected-cfgs (passed with pre-existing warnings)
• cargo clippy -p mock-gateway --features test -- -A unexpected-cfgs (passed with pre-existing warnings)
• pinned Anchor 0.31.1 test-IDL generation for both svm_spoke and mock_gateway
• pinned Anchor 0.31.1 production- and test-feature svm_spoke builds
• yarn build-ts
• focused Prettier check for SvmSpoke.V5Fill.ts
• focused validator coverage in SvmSpoke.V5Fill.ts before the shared-delivery follow-up (6 passed)
• repository pre-commit hook

The current-head local validator rerun is not valid evidence: the locally built svm_spoke artifact aborts during entrypoint dispatch under the installed Agave 4.1.2 (410 CU, access violation at address 0x32) for every instruction, including initialization, before any fill logic executes. The pinned production SBF build passes; verified-SVM CI is the runtime authority for the shared-delivery follow-up. A legacy-fill validator smoke test under the local Anchor 1.1.2 SBF build also remains blocked by the existing generated FillRelay::try_accounts frame (5,056 bytes versus the 4,096-byte SBF limit).

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[linear]

ACP-184

[Reinis-FRP]

@droplet-rl please recheck this PR. I moved the create-only fill-status account helper refactor into the Step 3 base PR (#1537) and rebased this PR on top. The final Step 4 tree is unchanged; v5_fill_status.rs now differs here only by removing #[allow(dead_code)] when the production call site is enabled.

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[droplet-rl]

Recheck on cf4475c2

Re-verified locally on a fresh clone. cargo check -p svm-spoke --features test clean, cargo test -p svm-spoke --lib --features test → 15 passed, matching your Validation section.

On your three claims

Two hold exactly; one needs a correction.

Claim	Verdict
Helper refactor moved into #1537	✅ Old Step-4 delta touched v5_fill_status.rs with 35 changed lines; it is now 1 deletion.
v5_fill_status.rs differs only by dropping #[allow(dead_code)]	✅ Verified — that is the entire diff vs. c7fe69fe.
"The final Step 4 tree is unchanged"	⚠️ Not as stated.

The rebase itself was tree-neutral — 066c68f6 (old tip) and 29fd3dbf (its rebased twin) have identical trees, so that part was clean. But the same force-push also carried three new commits (27b621c3, 32a1ce4f, cf4475c2) touching fill.rs, deposit.rs, and transfer_utils.rs. I checked all three and they are semantics-preserving, so no harm done:

• require!(fill_deadline >= current_time) → if fill_deadline < current_time { return err!(…) } — exact negation.
• hash_non_empty_message moved after the status write — pure function, no observable reordering.
• transfer_from_with_delegate → transfer_from, deleting the old 7-arg wrapper. I confirmed via git grep at 066c68f6 that the wrapper had zero call sites, so this is dead-code removal plus a rename, not a signature change on a live path.

Flagging it only because "tree is unchanged" is what tells a reviewer they can skip re-reading — and here three files did move. Worth splitting the rebase from the follow-up commits next time.

The one thing I want to resolve before this merges

v5_adapter.rs:184-191 — the self-delivery balance check is a read, not a debit.

let fill_delegate_info = if gateway_vault == recipient {
    require!(source.amount >= output_amount, V5Error::InsufficientVaultBalance);
    None
} else { /* delegate + allowance, pulls exactly output_amount */ };

Because load_token_account asserts owner == vault_authority on the source and owner == fill.recipient on the destination, gateway_vault == recipient is reachable precisely when fill.recipient == vault_authority — which the V5Fill.ts self-delivery test confirms (relay.recipient = vaultAuthority). In that branch _fill takes the delegate: None path, so no tokens move at all; require_keys_eq!(from, recipient) is the only gate.

The check therefore establishes "the shared vault currently holds ≥ output_amount" but reserves nothing. Contrast the deposit side, which relies on the same shared vault but actually transfers out to the spoke vault (deposit.rs:147) — that makes it self-limiting, since a second draw fails for insufficient funds. The fill self-delivery path has no such backstop, and the spec is explicit that the surrounding invariant is unenforced (line 57): "the continuing tape must leave no residual balance. Gateway does not currently enforce this net-zero settlement invariant."

Concretely, the chain I can't rule out from this repo:

1. Vaults are shared per mint, and dst_step_id is chosen by the depositor at origin, so two genuine V5 relays can commit to two different steps of one tape, both with recipient = vault_authority.
2. Fund the vault once with output_amount. Both fills pass source.amount >= output_amount — neither debits, so the second sees the same balance.
3. Per the spec's own note, residual vault balance is "already movable by a later committed Gateway TRANSFER", so a trailing step returns that balance to the submitter.
4. Two FilledRelay events → two refunds of input_amount, against one output_amount of liquidity that was never actually surrendered.

Self-dealing (attacker as both depositor and relayer) nets flat, so that isn't the concern. The concern is a submitter doing this against other users' legitimate vault-recipient deposits: the victim's origin funds are released to the submitter and the victim's tape receives nothing.

This turns residual vault balance from a custody question (which the spec already accepts) into a fill-authentication question, which I don't think the existing caveat covers. It's new surface in Step 4.

Three things would each close it, cheapest first:

• Reject recipient == vault_authority and require route builders to deliver into a distinct tape-owned ATA. Removes the branch entirely.
• Deliver into a per-relay-hash escrow the tape must consume, so the fill debits something.
• Keep it, but state and enforce the Gateway-side invariant — e.g. the vault is provably zero at step entry, or the Gateway reserves per-step. If enforcement lives in the Gateway program, please point me at it; it's out of this repo and I can't confirm it, and the spec currently says the opposite.

Test-wise, the happy path passes consume: outputAmount. There is no case with consume: 0 (fill recorded, funds still sitting there) and none with two self-delivery fills against one balance. Both are cheap additions and the second is the one that would actually demonstrate the property.

Carried forward from #1537 — resolved

The item I flagged as most likely to slip into Step 4 is genuinely closed, and structurally rather than contractually, which is the stronger of the two options I offered:

• execute_v5_fill derives the hash on-chain via get_relay_hash(relay, state.chain_id) (v5_adapter.rs:123), so fill_deadline and relay_hash come from the same RelayData by construction — they cannot diverge.
• The expiry precondition is honored: _fill rejects fill_deadline < current_time at line 165, before the create_v5_fill_status_account call at line 181. So no expired-then-closed status can be recreated.

Also verified: fill.message.is_empty() is enforced at v5.rs:149, so the "empty updated-message hash" claim in the body is accurate — good, since the V5 branch never calls invoke_handler, and a non-empty value there would have advertised a message that was never executed.

Smaller notes, non-blocking

• create_v5_fill_status_account still does two find_program_address calls (v5_fill_status.rs:38-39). I raised this in feat(svm): add V5 fill-status payer float #1537 as a Step-4 concern and it now is the hot path, inside an atomic tape alongside get_relay_hash. Passing both bumps in and using create_program_address stays safe — a non-canonical bump derives a different address and trips the require_keys_eq! below. Worth recovering the budget before this is CU-constrained in a longer tape.
• load_v5_fill_accounts reads source.amount before _fill runs its pause/exclusivity/deadline guards. Harmless today (nothing mutates it in between), but it means the balance authenticating the fill is sampled at instruction entry, further from the fill decision than it looks.
• find_v5_account returns the first key match and does not check uniqueness. Fine as used, since the aliasing case is handled explicitly, but it's load-bearing on that explicit handling.
• V5Source.ts correctly drops the UnsupportedMode assertion now that Fill is live.

Verdict

Structurally this is good work — the shared _fill core genuinely unifies pause, exclusivity, deadline, delivery, and event construction while keeping the status-lifecycle and callback differences at the edges, and the fill-status binding I was worried about came out stronger than I asked for.

I'm not approving yet, purely on the self-delivery item. If the Gateway does enforce a per-step reservation or a zero-balance-at-entry invariant, point me at it and I'll re-read and approve — the code is fine under that assumption. If it doesn't, I'd rather the adapter not depend on it. Happy to push the consume: 0 and double-fill tests, or the recipient == vault_authority rejection, if you want either; I've left the tree untouched so as not to race your agent.

[droplet-rl]

🔎 View trace

[droplet-rl]

Assessment — d7079a64 (base 7de27b68)

Re-verified locally: cargo check -p svm-spoke --features test clean, cargo test -p svm-spoke --lib --features test → 15 passed, matching the Validation section.

Since my recheck on cf4475c2

The net tree change is four files, and all of it is naming and extraction — I read each and none of it is semantic:

• filler → submitter, status_relayer → rent_recipient, FillStatusMode → FillStatusInput, fill → fill_input. These are real clarity wins, not churn: rent_recipient in particular stops the FillStatusAccount.relayer field from reading as "the relayer" when V5 stores the payer PDA there, and the state/fill.rs field comment now says so explicitly.
• write_v5_fill_status extraction moved into the base PR (9d74419b) and is now shared by the production path and the test entrypoint, replacing the duplicated try_serialize. Good — this also brings back the helper whose removal I flagged in #1537.
• Step 4's own delta to v5_fill_status.rs is now exactly two allow(dead_code) removals. Your earlier claim about that file holds on this head.

Resolved

The item I flagged in #1537 as most likely to slip into Step 4 is closed, and closed structurally rather than contractually:

• execute_v5_fill derives the hash on-chain via get_relay_hash(relay, state.chain_id), so fill_deadline and relay_hash come from the same RelayData by construction.
• _fill rejects fill_deadline < current_time (fill.rs:165) before create_v5_fill_status_account (fill.rs:181), so no expired-then-closed status can be recreated.
• The # Safety block now additionally requires every successful path to call write_v5_fill_status with that committed unexpired deadline. That's a tighter contract than I asked for.

Also confirmed on this head: fill.message.is_empty() is enforced at v5.rs:149, so the "empty updated-message hash" claim is accurate — which matters because the V5 branch never calls invoke_handler.

Why I'm still requesting changes

One item, and it's the same one: the self-delivery branch records a fill on the strength of an invariant this program neither enforces nor attributes. The response to my last review was a two-line comment asserting the invariant rather than pointing at what enforces it — and that assertion now contradicts the spec's own text 96 lines earlier in the same PR. Details inline on v5_adapter.rs:191-194 and V5_ADAPTER_SPEC.md:153.

To be clear about the strength of my claim: I am not asserting a demonstrated exploit. If the continuing path is genuinely mandatory, atomic, and sized to output_amount, the double-count self-corrects and the branch is sound. My objection is that this program takes an action with fund-loss consequences — recording a fill, which is what triggers repayment — on an assumption that is unverifiable from this repo and is stated inconsistently within it.

Any one of these closes it and I'll approve:

1. Name the enforcing component (Gateway PR, program, or spec section) so a future reader can check it, and reconcile the wording with spec line 57.
2. Add the consume: 0 negative test so the behaviour when the tape doesn't consume is pinned rather than assumed.
3. Reject recipient == vault_authority and require delivery into a distinct tape-owned ATA, removing the branch.

Everything else below is non-blocking. Structurally this is good work — the shared _fill core genuinely unifies pause, exclusivity, deadline, delivery, and event construction while keeping the status-lifecycle and callback differences at the edges, and the naming pass on top of it makes the V5/legacy split much easier to read than it was two revisions ago.

[droplet-rl]

This is the item from my last review, and I don't think the comment closes it.

Because load_token_account asserts owner == vault_authority on the source and owner == fill_input.recipient on the destination, gateway_vault == recipient is reachable precisely when fill_input.recipient == vault_authority — which the V5Fill.ts self-delivery test confirms (relay.recipient = vaultAuthority). In that branch _fill takes the delegate: None path, so no tokens move at all; require_keys_eq!(from, recipient) at fill.rs:197 is the only remaining gate.

So source.amount >= output_amount establishes "the shared vault currently holds at least this much" but reserves nothing. Contrast the deposit side, which leans on the same shared vault but actually transfers out to the spoke vault (deposit.rs:147) — that makes it self-limiting, because a second draw fails for insufficient funds. This path has no such backstop.

I'll grant that the first clause of your comment addresses one shape (two source deposits mapped onto one step). It doesn't cover two distinct steps in one tape, each with its own genuine deposit and its own step_id, both with recipient = vault_authority: each fill independently reads the same undebited balance and passes.

What actually closes that hole is the second clause — the continuing path consuming output_amount per fill, so two fills require 2× balance. That clause is the one this program doesn't enforce and doesn't attribute. "Independently enforced" names no enforcer, and the mock gateway takes consume_amount as a free parameter including 0, so nothing on this side constrains it.

Concretely, please either point me at the Gateway-side component that makes consumption mandatory and correctly sized, or drop the dependency. Related: source.amount is sampled here in load_v5_fill_accounts, before _fill runs its pause, exclusivity, and deadline guards — harmless today since nothing mutates it in between, but it does mean the balance authenticating the fill is read further from the fill decision than it appears.

[Reinis-FRP]

Agreed—the previous comment overstated what is enforced. There is no current Gateway-side reservation or net-zero check in this iteration; the in-place branch only asserts the live balance and does not debit it.

ceaa8016 now attributes the dependency precisely:

• the code calls this a non-debiting balance assertion;
• step-root reuse alone is allowed;
• external route builders must either permit at most one in-place fill before a post-fill floor and full-balance consumption, or enforce a cumulative floor covering every fill recorded before that consumption;
• the spec states that a fixed minimum for one fill does not prove aggregate delivery.

Correction to my earlier wording: ACP-184 Step 5 does not implement the production builder/order-assembly boundary, because that code belongs to separate off-chain repositories. Step 5 owns actual-Gateway integration tests, reference/test encoders, cross-VM fixtures, and documentation of the contract that those builders must satisfy. The production builders must implement corresponding fail-closed validation under separate blocking work in their owning repositories before the SVM V5 route is enabled.

So there is no hidden on-chain enforcer, nor an off-chain enforcer delivered by this PR or Step 5. This PR remains a draft primitive; Step 5 proves the expected path shapes, while production enablement depends separately on the owning off-chain builders conforming to them. Neither svm_spoke nor the generic Gateway inspects an Across-specific downstream tape.

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[droplet-rl]

This paragraph and line 57 now say opposite things about the same mechanism, in the same PR.

Line 57, on the deposit side:

the continuing tape must leave no residual balance. Gateway does not currently enforce this net-zero settlement invariant.

And the new comment at v5_adapter.rs:193, on the fill side:

must consume this in-place balance through an independently enforced atomic delivery.

One of these is wrong, or they're describing different guarantees that need distinguishing by name. This matters more than a docs nit because line 57 is the honest disclosure that made the deposit-side shared-vault reliance acceptable to me in #1537 — it says "we depend on this and it isn't enforced," which is a reviewable claim. The fill-side wording asserts enforcement instead, which reads as if the risk is retired.

If enforcement genuinely exists now, line 57 should be updated too and both should cite it. If it doesn't, this paragraph should carry the same explicit "not currently enforced" caveat, and the fill-side consequence is worth spelling out: an unconsumed in-place delivery leaves a recorded fill, and a recorded fill is what triggers repayment.

[Reinis-FRP]

Fixed in ceaa8016.

Agreed—these were describing the same unenforced dependency, and the fill-side wording was misleading. The spec and code comment now consistently state that:

• Gateway does not enforce net-zero settlement;
• the in-place branch only asserts the live balance; it does not debit it;
• step-root reuse is allowed, but the route builder must either permit at most one in-place fill before a post-fill floor and full-balance consumption, or enforce a cumulative floor covering every recorded fill before that consumption; and
• a fixed minimum covering only one fill is insufficient for an aggregated route.

The consequence is also explicit in the revised text: this branch records the fill without transferring funds, and only a later failing command rolls that state back atomically. A malformed continuing path that succeeds without the required aggregate delivery would therefore leave a recorded fill eligible for repayment. ACP-184 Step 5 owns actual-Gateway conformance tests and specification in this repo; fail-closed production builder enforcement is separate blocking work in the owning off-chain repositories before route enablement.

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[droplet-rl]

The self-delivery happy path passes consume: outputAmount, so it only demonstrates the case where the invariant holds. Two gaps:

1. No consume: 0 case. The mock already takes consume_amount as a free parameter, so this is a couple of lines: execute with consume: 0n and assert what actually results — fill status Filled, vault still holding outputAmount, nothing delivered. Right now the behaviour when the tape doesn't consume is unasserted, which is exactly the assumption v5_adapter.rs:193 rests on.
2. No two-fills-one-balance case. Two relays with distinct stepIds, both recipient = vaultAuthority, vault funded once, no consumption. If it passes, that pins the double-count precisely; if it fails, that's the enforcement I'm asking about and the test documents it.

The negative case at line 278 is good and does pin the check itself (outputAmount + 1 against outputAmount → InsufficientVaultBalance). It just can't speak to what happens after the fill is recorded.

[Reinis-FRP]

Added both boundary cases in c6119ef1.

• consume: 0n now asserts Filled, an unchanged Gateway-vault balance, and zero balance in the downstream consumption account.
• A second test executes two distinct relays with distinct stepIds against one once-funded vault, consumes neither, and asserts that both fill-status accounts are Filled while the same vault balance remains available.

These tests intentionally pin the low-level double-count behavior; they do not claim to enforce the production route invariant. Actual-Gateway conformance remains Step 5 scope, with fail-closed production route construction owned by the relevant off-chain repositories. The focused V5 destination-fill suite passes 8/8.

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[droplet-rl]

Non-blocking, but this is the CU point from my #1537 review now made concrete, and it's worse than I estimated there because the derivations are duplicated rather than merely present.

Here the adapter derives payer (202) and fill_status (204) to locate the accounts. _fill then calls create_v5_fill_status_account, which re-derives both (v5_fill_status.rs:39-40) purely to require_keys_eq! them against what you just passed in. That's four find_program_address calls for two PDAs on the fill hot path, inside an atomic tape.

Passing the derived keys and bumps through — and switching the helper's validation to create_program_address with the supplied bump — keeps the safety property intact, since a non-canonical bump derives a different address and still trips the existing require_keys_eq!. Worth doing before a longer tape makes this budget-relevant, though I'd understand deferring it to a follow-up since the helper signature lives in the base PR.

[Reinis-FRP]

Addressed across the stacked PRs.

Step 3 d4e59080 now owns a private-field V5FillStatusPdas bundle whose only constructor performs the two canonical find_program_address derivations and retains the authenticated submitter and relay-hash seed references. Account validation, invoke_signed seeds, and the serialized rent recipient all read from that same bundle; no raw bump, loose seed, or loose payer value can diverge.

Step 4 1cf0df33 derives the bundle once in load_v5_fill_accounts, uses its keys to locate both accounts, and passes it through _fill for creation and finalization. The hot path therefore performs two PDA derivations total rather than four, with no additional create_program_address calls.

Validation passed on the final stack: production/test Rust checks, 15/15 Step 4 Rust tests, SBF build, and 8/8 focused V5 destination-fill validator tests. Step 3 separately passes 11/11 Rust and 3/3 focused fill-status validator tests.

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[Reinis-FRP]

Commit-ID correction after a DCO-only history rewrite: the final Step 3 implementation is b9edf074, and the final Step 4 plumbing is d10f2ec5. Both trees are byte-for-byte identical to the versions described above, and all 20 Step 4 commits range-diff exactly; only commit ancestry changed to replace an unsigned one-line base commit.

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[Reinis-FRP]

Final implementation update after Droplet’s last Step 3 pass: 5a8a5350 replaces the loose account-plus-bundle serializer inputs with a private-field, must_use pending handle returned by account creation. This binds the exact validated account and canonical bundle without adding a duplicate require_keys_eq! on the hot path. Step 4 d0f213c1 carries that handle across token delivery and consumes it for finalization. The obsolete standalone dead-code-removal commit dropped during rebase because the free serializer no longer exists; every other Step 4 commit range-diffs unchanged.

Final validation: production/test Rust checks; 11/11 Step 3 and 15/15 Step 4 Rust tests; both SBF builds; 3/3 fill-status and 8/8 destination-fill validator tests.

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[droplet-rl]

Confirming this is correctly unreachable from the legacy path, since it's the one place where a fill can complete without moving tokens: FillAccounts::from(&FillRelay) hardcodes delegate: Some(...) (fill.rs:129), so fill_relay always transfers. shared_fill_core_only_skips_authenticated_in_place_delivery pins that, and the guard fails closed with InvalidTokenAccount when from != recipient.

One readability note: V5Error::InvalidTokenAccount on a shared core that legacy also calls is slightly odd, but since the branch is V5-only in practice it's the right error, and renaming would churn the error space for no gain.

The Option<AccountInfo> encoding of "no transfer needed" is doing a lot of work for a single None. The doc comment on FillAccounts::delegate carries the whole explanation, which is fine, but it means a future caller constructing FillAccounts with delegate: None silently opts out of token delivery. Worth considering a named type (Delivery::InPlace / Delivery::Delegated(..)) so the opt-out is explicit at the construction site rather than inferred from a None.

[Reinis-FRP]

Addressed in f15a54b6.

FillAccounts now carries explicit FillDelivery::{Delegated(AccountInfo), InPlace} instead of Option. The legacy FillAccounts::from implementation always selects Delegated, while the V5 loader selects InPlace only for the authenticated source/recipient alias and otherwise selects Delegated. The shared _fill match retains the from == recipient check as the fail-closed contract for InPlace, so a future caller cannot silently omit delivery by passing None.

This preserves legacy transfer behavior and the existing transfer/status/callback/event ordering.

Validation passed: formatting and production/test Rust checks, 15/15 Rust tests, production and test-feature SBF builds, and 8/8 focused V5 destination-fill validator tests.

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[droplet-rl]

🔎 View trace

[droplet-rl]

Approving — f15a54b6 (base 11d8df8e)

All five threads from my last review are resolved, and I verified each independently rather than taking the replies on faith. Locally on this head:

cargo check -p svm-spoke --features test    # exit 0
cargo check -p svm-spoke                    # exit 0, no dead-code warnings
cargo test -p svm-spoke --lib --features test    # 15 passed

Thread	Resolution	Verified
In-place branch depends on unattributed enforcement	Comment rewritten to "This check is not a debit", builder obligation stated	✅
Spec contradicts itself on net-zero enforcement	Both sites now attribute to route builders; neither claims enforcement	✅
No consume: 0 / two-fill coverage	Both tests added	✅
Four find_program_address for two PDAs	Now two, via V5FillStatusPdas	✅
Option<AccountInfo> silently opts out of delivery	Replaced with FillDelivery::{Delegated, InPlace}	✅

On the blocking item

You did the harder and more useful thing than what I asked for. I offered "name the enforcing component" as an option; the honest answer turned out to be that there is no enforcer, and rather than leaving the ambiguity you retracted the earlier wording and made both spec sites say the same unenforced thing. Spec line 56 now reads "route builders must make the continuing tape leave no residual balance. Gateway does not currently enforce this net-zero settlement invariant," and the fill-side section says its safety "depends on the proportional or aggregate continuing-path rule above." Those are consistent, and the builder rule is stated precisely enough to implement against — including the part that's easy to get wrong, that a fixed single-fill minimum doesn't prove aggregate delivery.

The two new tests are what actually make me comfortable, and specifically the second one. records two distinct in-place fills against one unconsumed Gateway-vault balance executes two relays with distinct stepIds against a once-funded vault and asserts both statuses are Filled with the balance still sitting there. That is the exact double-count I described, now pinned as an asserted property rather than an argued one. Anything that later changes this behaviour has to change that test, which is a far more durable guard than a comment.

I want to be explicit about what I'm approving, since the residual risk is real and doesn't disappear because it's documented: this program can record a repayment-eligible fill without moving funds, and the only thing preventing that is off-chain route-builder discipline living in other repositories. I'm accepting it for three reasons. It's the same shared-vault, explicitly-unenforced dependency I already accepted on the deposit side in #1537, and the fill side now carries strictly more protection than the deposit side does — disclosure, a written builder contract, and behavioural tests. The Gateway tape model inherently delegates composition safety to the composer, so pushing it on-chain here would mean rejecting legitimate tape-continuation routing. And you've scoped production enablement behind separate blocking work.

One non-blocking follow-up in that last area, inline on v5_adapter.rs.

Two things worth calling out as better than requested

V5FillStatusPdas is a stronger answer than the create_program_address plumbing I suggested. Private fields with a single constructor that performs both derivations and retains the seed references means the invoke_signed seeds, the account validation, and the serialized rent recipient all read from one bundle — no raw bump or loose payer can diverge, and it lands at two derivations without adding a second derivation path to audit. PendingV5FillStatus being #[must_use] and sourcing the rent recipient from pdas.payer() closes the loose-parameter hole I hadn't flagged.

Separately: there are now zero allow(dead_code) attributes left in svm-spoke, and the production profile builds clean without them. That's a genuine improvement over the earlier stack state and independently confirms every V5 fill-status helper is reachable from production code rather than merely from tests. It also retires the concern from my #1537 review about refactor commits quietly removing things — the compiler now enforces it.

Nice work on the naming pass across the whole stack; submitter / rent_recipient / FillStatusInput / FillDelivery make the V5-vs-legacy split much easier to follow than three revisions ago.

[droplet-rl]

Resolved, and the wording is now accurate: "This check is not a debit" plus the aggregate-floor obligation, with no claim of enforcement. Cross-checked against spec lines 56 and 107-113 — all three sites are consistent now.

One non-blocking follow-up, and it's the only part of this I can't verify from the repo. The safety of this branch rests on "separate blocking work in the owning off-chain repositories before route enablement." That's a process guarantee spanning repo boundaries, and those are the ones that get lost — not because anyone forgets the risk, but because the person who eventually flips the route on is usually not the person who read this comment.

Would you file a tracked blocker (Linear, linked from ACP-184) that names the builder-side validation as a prerequisite for SVM V5 route enablement, and reference it from the spec paragraph? That turns "separate blocking work exists" into something a future reviewer or release engineer can actually check. Cheap, and it's the difference between a documented dependency and an auditable gate.

Also still true, and now harmless given the honest framing: source.amount is sampled up here in load_v5_fill_accounts, before _fill runs its pause, exclusivity, and deadline guards. Nothing mutates it in between, so there's no bug — just noting the read sits further from the fill decision than it reads.

[droplet-rl]

Verified both new cases do what your reply says.

consume: 0n (line 279) asserts the vault still holds outputAmount, the consumption account holds 0, and the status is Filled — so the unconsumed path is now pinned rather than assumed.

This one is the valuable one. Distinct stepId and depositId give distinct relay hashes and therefore distinct fill-status PDAs, both fills execute against a single funding with no consumption, and both statuses come out Filled with the balance intact. That's precisely the double-count I described in my last review, now asserted as behaviour.

I'd suggest one small addition whenever this file is next touched: a comment on this test saying why it exists — that it deliberately pins accepted low-level behaviour whose mitigation lives in off-chain route builders, not a property the program guarantees. As written, a future reader could reasonably mistake it for a test asserting that two fills against one balance is fine, and "delete this weird test" is an easy conclusion to reach. The intent deserves to survive you.

Good call keeping the InsufficientVaultBalance case at line 330 — together the three now cover the check passing, the check failing, and the check passing but delivery never happening.

[Reinis-FRP]

Added in 79c0b5d3. The test now explicitly says it pins accepted low-level behavior and that off-chain route builders must prevent aggregate underdelivery, so its purpose is not mistaken for a program-level safety guarantee.

This is a comment-only change; Prettier and diff checks pass.

---

Sent from Reinis Martinsons's Codex CLI Agent using gpt-5.6-sol 🤖

[droplet-rl]

This is exactly the shape I was hoping for, and it resolves the concern properly rather than cosmetically.

Verified the three properties: legacy From<&FillRelay> hardcodes FillDelivery::Delegated (line 136), the V5 loader selects InPlace only inside the authenticated gateway_vault == recipient branch (v5_adapter.rs:193-197), and the InPlace arm retains require_keys_eq!(from, recipient) as the fail-closed contract (line 203). So skipping delivery is now an explicit named choice at the construction site, and a caller can no longer opt out by defaulting a field.

The doc comment on the InPlace variant carrying the invariant is the right place for it — it travels with the variant instead of with a field whose None case a reader has to go look up.

[droplet-rl]

🔎 View trace

[fusmanii]

logic makes sense, main comments around re-enforcing InPlace validation

[fusmanii]

Since this is very load baring now, I would also check here that both from and recipient are Gateway vault here as well

[fusmanii]

might be an over-kill and not sure what it would really protect, but what do you think about checking that from has the desired balance at here, wdyt?

[Reinis-FRP]

In this PR, _fill is shared by legacy and V5, so the InPlace arm deliberately enforces only the path-agnostic invariant that from == recipient. The V5 loader already proves both accounts are the canonical Gateway vault and checks its balance, with no intervening token CPI that could change it. Repeating those V5-specific checks in _fill would duplicate validation and couple the shared helper to V5.

In the stacked V5-only refactor in #1564, _fill is removed and V5FillAccounts::load becomes the sole private constructor, so even the equality check becomes redundant I'm considering to remove it there.

[fusmanii]

should we also have load_token_account check if the account is not AccountState::Frozen? this would ensure that if for some reason the GW vault is not able to release funds we are not allowing orders to be filled

[Reinis-FRP]

The delegated paths already reject them through transfer_checked and the only behavioral difference is InPlace, where the Gateway vault is also the recipient and no token CPI occurs.

For that branch to observe a frozen vault with sufficient balance, funds must have been left/prefunded while it was unfrozen and then frozen by the mint’s freeze authority. A supported tape's terminal delivery/consumption would still fail on the frozen account and atomically roll back the fill. A tape without that consumption is already unsafe against any pre-existing shared-vault balance, regardless of whether the vault is frozen.

Therefore, an explicit frozen-state check would only provide SPL self-transfer parity, but it does not close a distinct reachable attack under the supported flow.

[fusmanii]

load_v5_fill_accounts and DepositAccounts::load_v5 both find the committed mint, take the token program from the mint owner and allowlist it, run validate_v5_mint, then derive and validate the canonical Gateway vault ATA.

Suggestion: extract a shared load_v5_token_env(remaining_accounts, token) helper returning the mint, token program + id, decimals, the Gateway vault account, and the deserialized vault TokenAccount, and have both loaders call it before doing their branch-specific work.

[Reinis-FRP]

Good suggestion, the mint/program/Gateway-vault validation is genuinely shared. But I think it might be a better place to do this in the stacked V5-only refactor in #1564 while keeping this PR focused on implementing v5 fills.