Skip to content

fix(swap): keep a verified deposit's claim window from lapsing#593

Merged
anderdc merged 2 commits into
testfrom
fix/claim-relay-runway
Jul 24, 2026
Merged

fix(swap): keep a verified deposit's claim window from lapsing#593
anderdc merged 2 commits into
testfrom
fix/claim-relay-runway

Conversation

@anderdc

@anderdc anderdc commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

The failure

A taker on the manual path sends source funds, then relays them with alw swap post-tx. submit_swap_claim requires reserved_until >= now. If the reservation lapses in between:

no claim → no Swap → no timeout → no refund. The deposit is gone, sitting in the miner's wallet, silently. There is no recovery instruction in the contract — I checked all 24.

Three TAO sends (~1.5 TAO) were lost exactly this way today by an external agent.

There are two independent gaps, one on each side of the send. Both are small; keeping them together because they only make sense as a pair.

1. Validator — extend before claiming

confirm_deposit slides reserved_until forward when a verified deposit arrives with little runway left, then submits the claim.

Evidence-gated on purpose. The deposit has already matched the pinned reservation by the time this runs, so we only ever extend for a taker who demonstrably sent funds — never on request. That's the same justification the crank extends under (_extend_reservation_action), one step earlier in the lifecycle: the crank can only help once a Swap exists, and here there is no Swap precisely because the claim hasn't landed. That's the hole.

Best-effort — a failed extension still attempts the claim. The reservation may have just enough runway, and a claim that lands is worth more than a clean error path.

CLAIM_RELAY_MARGIN_SECS = 90 is deliberately much shorter than the CLI's 180s pre-send margin: at this point only the claim tx remains, the send and relay are done. Once the Swap exists the crank owns every later extension.

2. CLI — state the deadline

_SEND_MARGIN_SECS is checked once, at reservation time. On the manual path the send and the relay then happen outside this process, and nothing re-checks it. That's why the existing guard didn't save the incident above — it passed, swap now exited, and the send went out minutes later via btcli.

So the manual path now prints the deadline as a wall-clock instant plus the raw reserved_until, and says plainly that missing it is unrecoverable. A bare countdown is stale the moment it prints and useless in an agent's log. When --send wasn't used it also points at alw swap now --send, which keeps send + relay + watch in-process where the guard still applies.

Extracted to _deadline_lines so the wording is testable rather than buried in a console.print.

Limits — stated plainly

  • Neither change recovers an already-expired reservation. The contract forbids extending one (extend_reservation.rs:43), and that's correct.
  • Neither helps a taker who sends with no reservation at all.
  • This closes the honest race — deposit verified, runway nearly gone — not every way to lose funds here.

The structural gap remains: source funds go to the miner before a Swap exists, with no escrow. Worth a separate discussion; I didn't want to bundle a design change into a fix.

On raising reservation_ttl_secs instead

It was raised as a simpler alternative, and it is one instruction (set_reservation_ttl, already admin-settable — no code needed). I'd argue against it:

  • The TTL is an anti-griefing lever, not a confirmation budget. Doubling it halves the effective cost of locking a miner busy at the same 0.02 SOL fee.
  • It only moves the cliff. A relay that runs long at TTL+1s loses the funds exactly the same way.

The evidence-gated lever is strictly better here: it costs nothing when unused, can't be griefed (you must produce a real deposit), and is bounded by max_extend_at, which is frozen at creation and separately admin-tunable via set_max_total_extension if we want more headroom.

Tests

pytest tests/804 passed. ruff check + format clean.

New coverage:

  • extends when runway is short, and still claims
  • does not extend when runway is ample (don't burn ceiling budget)
  • does not extend past max_extend_at, but still attempts the claim
  • claims even when the extension throws (best-effort)
  • deadline notice: wall-clock instant, parseable reserved_until, runway from the injected now
  • the --send hint appears only on the unguarded path
  • runway never renders negative

A taker on the manual path sends source funds, then relays them with
`alw swap post-tx`. submit_swap_claim requires reserved_until >= now, and
if the reservation lapses in between there is no claim, no Swap, no
timeout and no refund — the deposit is simply gone, to the miner, in
silence. Three TAO sends were lost this way today.

Two independent gaps, one on each side of the send.

Validator: confirm_deposit now slides reserved_until forward when a
verified deposit arrives with little runway left, before submitting the
claim. Evidence-gated — the deposit has already matched the pinned
reservation, so we only extend for a taker who demonstrably sent funds,
never on request. This is the same justification the crank extends under,
one step earlier in the lifecycle: the crank can only act once a Swap
exists, and here there is no Swap precisely because the claim has not
landed. Best-effort; a failed extension still attempts the claim, since
the reservation may have just enough runway and a claim that lands beats
a clean error path.

CLI: the manual path now prints the deadline as a wall-clock instant plus
the raw reserved_until, and says plainly that missing it is unrecoverable.
_SEND_MARGIN_SECS is checked once, at reservation time; after that the
send and the relay happen outside this process and nothing re-checks it.
A bare countdown is stale the moment it prints and useless in an agent's
log, hence the absolute time. When --send was not used it also points at
`alw swap now --send`, which keeps send + relay + watch in-process where
the guard still applies.

Limits, stated plainly: neither change recovers an already-expired
reservation (the contract forbids extending one), and neither helps a
taker who sends with no reservation at all. This closes the honest race —
deposit verified, runway nearly gone — not every way to lose funds here.
The structural gap remains that source funds go to the miner before a
Swap exists, with no escrow.

@LandynDev LandynDev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments, one worth fixing before merge. Everything else looks solid — the guards mirror the contract exactly (strictly-later, ceiling, live-only), the extension confirms before the claim, and the test coverage is good.

Comment thread allways/validator/reserve_engine.py Outdated
CLAIM_RELAY_MARGIN_SECS = 90


def _extend_for_claim(client, miner_pk, reservation, now: int) -> None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now here is the one stamped in confirm_deposit before verify_transaction runs — and that's a source-chain RPC that can take seconds on BTC. By the time this executes, the clock is stale in the wrong direction: the runway check can see "ample" when real runway is already under 90s, and when it does extend, now + CLAIM_RELAY_MARGIN_SECS buys less than 90s of real time. That's the same race this PR is closing, just narrower.

Suggest recapturing inside the helper and dropping the param:

def _extend_for_claim(client, miner_pk, reservation) -> None:
    now = int(time.time())

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in e720255, exactly as suggested: now = int(time.time()) inside the helper, param dropped.

Worth spelling out that it cut both ways, since the second half is the nastier one: with a stale clock the runway check reads "ample" on a window that is already short (so no extension at all), and when it does extend, now + CLAIM_RELAY_MARGIN_SECS is measured from a point already seconds in the past — the extension buys less than 90s of real runway. Both failure modes are the race this PR is closing.

Added test_confirm_measures_runway_after_the_source_rpc: it hangs verify_transaction for 60s on a fake clock against a reservation with 120s left, then asserts the extension fired and targeted now_after_rpc + margin. Off the pre-RPC clock that reservation looks ample and never extends — I verified the test fails on the old signature before restoring the fix.

f'([cyan]{max(0, reserved_until - now)}s[/cyan] from now, reserved_until={reserved_until}). '
'Miss it and the deposit is unrecoverable — there is no refund.'
]
if not want_send:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small gap: with --send, if _auto_send_wizard falls back to manual (missing creds, wrong wallet), we still land on this notice — but want_send=True suppresses the warning, even though that user is now sending outside alw too. That's arguably the audience that needs it most: an agent whose auto-send silently degraded.

Suggest splitting the line: always print the "forfeits the pre-send safety check" warning when the manual notice is reached; only gate the alw swap now --send hint on want_send (it's redundant for someone who just tried it). Non-blocking.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and taken — also in e720255. Split exactly as you proposed:

  • unconditional: Sending outside alw forfeits the pre-send safety check.
  • gated on not want_send: `alw swap now --send` does the send, the relay and the watch in one step.

You are right that reaching this notice at all means the send is happening outside the process — _auto_send_wizard returning False (no source provider, wallet does not control the pinned from_addr, declined confirm, coldkey unlock failed) is the only way past it with want_send=True, and none of those leave the guard applying. The old gate keyed off intent when the only thing that matters is outcome.

Test split to match: test_deadline_notice_always_warns_that_the_guard_is_forfeit (both values of want_send) and test_deadline_notice_hints_at_send_only_when_it_was_not_asked_for.

…n manual send

Review follow-ups.

1. `_extend_for_claim` took `now` from `confirm_deposit`, stamped *before*
   `verify_transaction` — a source-chain RPC that can burn seconds on BTC. The
   stale clock cut both ways: the runway check could read "ample" while the real
   window was already under the margin, and an extension computed off it bought
   less than 90s of real time. It now re-reads the clock itself; the param is gone.

2. On the manual path the "forfeits the pre-send safety check" warning was gated
   on `want_send`, so a `--send` run that fell back to manual (missing creds,
   wrong wallet) never saw it — the audience that needs it most. The warning is
   now unconditional; only the `alw swap now --send` hint stays gated, since it's
   redundant for someone who just asked for it.

Tests: the new runway test hangs the source RPC on a fake clock and asserts the
extension fires — it fails against the pre-fix signature. Deadline-notice tests
split to cover warning-always / hint-gated.

804 -> 806 passed. ruff check + format clean.
@anderdc
anderdc merged commit e601231 into test Jul 24, 2026
3 checks passed
@anderdc
anderdc deleted the fix/claim-relay-runway branch July 24, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants