Skip to content

fix(soak): trader-roundtrip default rate band must fit bob's hardcoded deposit - #538

Closed
vrogojin wants to merge 1 commit into
mainfrom
fix/trader-soak-rate-band-coverage
Closed

vrogojin wants to merge 1 commit into
mainfrom
fix/trader-soak-rate-band-coverage

Conversation

@vrogojin

Copy link
Copy Markdown
Contributor

Summary

  • Narrow the trader-roundtrip soak's default TRADER_RATE_MAX_ETH_PER_UCT from 0.12 to 0.09 so the upper-bound exposure (rate_max × volume_max = 0.09 × 50 = 4.5 ETH) exactly matches bob's hardcoded 4.5 ETH deposit at §5.
  • Document the invariant in the env-contract docstring and inline next to the default so future operators don't reproduce the trap.
  • Update the predicted end-state in the script header (bob -~5 ETH-~4.25 ETH) to match the new midpoint (0.085 × 50 = 4.25).

Why

Surfaced while running the soak to validate sphere-sdk#535 (PR #536). With the prior defaults, bob's [0.08, 0.12] band promised to honor up to 0.12 × 50 = 6 ETH, but bob only had 4.5 ETH deposited. Trader-service's NP-0 matcher (trader-main.ts:355-364) picks the midpoint of the overlap, which for two identical [0.08, 0.12] bands is 0.10 → 5 ETH owed. Reservation correctly refused (4.5 < 5), and the deal went terminal with VOLUME_RESERVATION_FAILED. Settlement never started — verifyPayout (the code PR #536 changed) was never even reached.

Unit handling along the way is correct, per the project convention:

Intents work in human-readable decimal strings; actual swaps work in exact bigint smallest-units.

trader-main.ts:489-493 does Number(volume) × Number(rate) (decimal strings → JS number for the ratio) → toSmallestUnitsBigInt at the wallet-reservation boundary. The unit-conversion seam is in the right place.

The bug was purely that the soak made bob post an intent he couldn't honor at his own declared upper bound. This PR fixes that by trimming the band; defense-in-depth (the trader engine refusing to create the intent in the first place) is tracked separately as vrogojin/trader-service#29.

Test plan

  • npx tsc --noEmit — no code changes to TS, soak is a bash script
  • Empirical: rebuilt the trader image against this branch's sphere-sdk and re-ran the soak. With rate_max=0.09, alice's trader proposals fire at rate 0.08499999999999999 (vs the prior 0.10), so bob's required reservation is 0.085 × 50 = 4.25 ETH ≤ 4.5 ETH deposited — no VOLUME_RESERVATION_FAILED. Trace:
    alice-trader: np_deal_proposed { rate: "0.08499999999999999", volume: "50" }
    
    (Prior run on the same branch with rate_max=0.12: rate: "0.1", reservation aborts.)
  • End-to-end soak completion was blocked on the re-run by orphan BUY intents from the prior failed attempt still sitting on the market feed (expiry_ms ~40 min out). Alice's matcher locked onto a stale acceptor pubkey whose container was already gone, and her proposals timed out instead of being responded to. This is an unrelated market-feed isolation issue, not regressed by this PR.

Related

…d deposit

The default trader-roundtrip soak hung at §8 because bob couldn't fund
the negotiated price. Surfaced while running the soak to validate
sphere-sdk#535 (PR #536).

Math
----
Soak hardcodes bob's deposit at 4.5 ETH (§5, `sphere payments send
"@$BOB_TRADER_TAG" 4.5 ETH`). Bob's BUY intent declared rate band
`[0.08, 0.12]` and volume 50 UCT, meaning bob promised to honor any
negotiated rate in that band — so up to `0.12 × 50 = 6 ETH`. Trader-
service's NP-0 matcher (`trader-main.ts:355-364`) picks the midpoint of
the overlap, which for two identical `[0.08, 0.12]` bands is `0.10` →
bob owes 5 ETH. Reservation correctly refuses (4.5 < 5) and the deal
goes terminal with `VOLUME_RESERVATION_FAILED`.

Unit handling along the way is correct, per the project convention:

> Intents work in human-readable decimal strings; actual swaps work in
> exact bigint smallest-units.

`trader-main.ts:489-493` does `Number(volume) × Number(rate)` (decimal
strings → JS number for the ratio computation) → `toSmallestUnitsBigInt`
at the wallet-reservation boundary. The bug is purely that bob posted an
intent he couldn't honor at his own declared upper bound.

Fix
---
Narrow the default rate_max from `0.12` to `0.09` so the upper-bound
exposure `rate_max × volume_max = 0.09 × 50 = 4.5 ETH` exactly matches
bob's deposit. With overlap `[0.08, 0.09]` both sides, the midpoint is
`0.085`, so bob owes `4.25 ETH` and the reservation passes with a
0.25 ETH cushion.

Empirically verified by re-running the soak with this fix: alice's
trader proposals now fire at rate `0.08499999999999999` (vs the prior
`0.10`), with no `VOLUME_RESERVATION_FAILED`.

Documentation updates
---------------------
- Soak header: `bob -~5 ETH` → `-~4.25 ETH` (the new accurate expected
  outcome under the narrowed band).
- Env contract docstring: rate_max default `0.12` → `0.09`, with the
  full `rate_max × volume_max ≤ bob's deposit` invariant explained so
  future operators don't reproduce this trap.
- Inline comment at the new default points at trader-service#29, which
  tracks the upstream defensive fix (pre-flight check that refuses to
  create an intent the trader can't fund at rate_max).

Related
-------
- trader-service#29 — defensive pre-flight balance check (independent;
  this commit unblocks the soak now without waiting on the upstream
  fix).
- PR #536 / issue #535 — the original reason this soak was being run.
  The swap-CLI soak already validated PR #536 end-to-end; this fix
  unblocks the trader-CLI path too once stale market intents from
  prior failed runs expire (not addressed here; orthogonal market-feed
  isolation issue).
@vrogojin

Copy link
Copy Markdown
Contributor Author

Soak re-run — rate-band fix verified end-to-end through swap execution; only trailing failure is unrelated to this PR

Re-ran the soak this morning with the default rate band (0.08–0.09) after the 40-min orphan-intent TTL had long elapsed (>22 h since the prior attempt).

What landed cleanly

All §1–§7 asserts pass. Negotiation completes, both sides agree on the same deal:

{
  "deal_id": "a3252045e0263fb79026bdd05723f01f7f0392efff59eb143242e1f604bc96a4",
  "rate":   "0.08499999999999999",       // midpoint of overlap [0.08, 0.09]
  "volume": "50",                         // bob owes 0.085 × 50 = 4.25 ETH
  "state":  "ACCEPTED"
}

No VOLUME_RESERVATION_FAILED — bob's [0.08, 0.09] band keeps the worst-case demand at 4.5 ETH, exactly his deposit, and the midpoint at 4.25 ETH (≤ deposit). This PR's rate-band fix does what it claims.

Past the matcher, the swap actually executes — bob's trader log shows escrow payout completed at L3:

swap_id      = 6693a8df66f2fb8db78875fa7267a85cdbbca4d9c84a3e67472b3b83edc36939
invoice_id   = 0000255380924ef4...
invoice.state= CLOSED
isCovered    = true
covered      = 50000000000000000000  (50 UCT, exactly as agreed)

So bob's tenant did receive the 50 UCT payout on-chain.

Trailing failure (not a PR #538 issue)

After payout the trader's daemon-side verifyPayout loops:

[Swap] verifyPayout for 6693a8df66f2: L3 validation found 1 invalid token(s) covering this payout invoice — retry after wallet sync
swap_payout_verify_retry_failed { attempt: 25, remaining: 15 }   →   attempt: 26, remaining: 14 → ...

Note the log shape — "L3 validation found N invalid token(s)" — that's the pre-#534 payments.validate() code path. The v0.6 trader image was built ~3 h ago against file:../sphere-sdk, where the sibling sphere-sdk checkout (/home/vrogojin/sphere-sdk) is at 8c7d93d — i.e. PRE-#534. Without #534's payments.validate() JSON-string fix, every token in the wallet reports invalid, and the retry never resolves.

This is the daemon-vs-CLI divergent failure mode described in #545 — identical signature. Not introduced by this PR; the soak just happens to drive a long-running daemon all the way to the gate.

The deals stay at state: ACCEPTED, swap_id: null for the full 900s deal-deadline before §8 asserts ASSERT FAIL (deal-deadline).

Test plan box

I'd argue this PR's end-to-end test plan box can be checked even though §8 fails, because the failure point is entirely downstream of what this PR touches. The proper green-end soak run requires:

cd /home/vrogojin/sphere-sdk && git checkout 8b1955a9   # main HEAD with #534 + #536
\
  docker build -t ghcr.io/vrogojin/agentic-hosting/trader:v0.6 .
# then re-run the soak

That rebuild step is a trader-service concern, not a sphere-sdk concern. Soak workspace preserved at /tmp/trader-soak-538 for inspection.

vrogojin added a commit that referenced this pull request Jun 15, 2026
…ITIALIZED in save() chain + 2 soak fixes (#552)

Three commits squashed:

1. fix(communications)(sphere-sdk#551): swallow transient PROFILE_NOT_INITIALIZED in save() chain
2. fix(soak): trader-roundtrip default rate_max=0.09 to fit bob's hardcoded 4.5 ETH deposit
3. fix(soak): trader-roundtrip agreed-volume assert was comparing human-decimal field against smallest-units

The #551 catch lives at CommunicationsModule.save() — the actual fire-and-forget owner — rather than at the storage layer. Code review (sweep) surfaced that the storage-layer alternative silently breaks AutoReturnLedger.save(critical=true)'s write-first rollback contract; moving the catch up preserves strict storage-layer semantics while still protecting the fire-and-forget DM save chain (handleIncomingMessage, transport.onReadReceipt).

Verified:
- 2923 unit tests pass (3 new for #551 race + 2920 existing)
- Run 4 soak: 18/18 ASSERT OK end-to-end
- Runs 5-8: §4 ACP probe failures traced to testnet environmental degradation, not code (monotonic latency increase across runs with constant SDK code; isolation run 7 reproduced the failure with reverted trader image)

Closes #538 (subsumed by commit 2).
Follow-up filed for testnet relay degradation observed during verification.
@vrogojin

Copy link
Copy Markdown
Contributor Author

Closing as superseded by PR #552 (merged): commit 60be0dc was cherry-picked into PR #552 as part of the soak-fix bundle.

@vrogojin vrogojin closed this Jun 15, 2026
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.

1 participant