diff --git a/docs/DEMO-PLAYBOOK-TRADER-ROUNDTRIP.md b/docs/DEMO-PLAYBOOK-TRADER-ROUNDTRIP.md new file mode 100644 index 00000000..ecf9a87c --- /dev/null +++ b/docs/DEMO-PLAYBOOK-TRADER-ROUNDTRIP.md @@ -0,0 +1,869 @@ +# Sphere CLI Demo Playbook — Trader Round-Trip + +A presenter-friendly run-through of **autonomous AI agents trading on Unicity testnet**. Two trader tenants — `alice-trader` and `bob-trader` — are each spawned by their controller on a **per-user local Host Manager** (one HM per developer, scoped to that wallet's controller pubkey), then given a one-line trading intent and left to negotiate, match, and execute a token swap entirely on their own. No human approvals, no orchestrator, no shared backend. No shared HM, either — each peer brings its own. Just two daemons watching a market and talking over Nostr DMs. + +The twist that makes this demo land (versus the swap playbook, which exercises the same escrow but with humans driving every state transition): **after §6, the controllers go quiet.** The audience watches two AI tenants find each other on the market, negotiate a price inside both their bands, deposit into the same escrow service, and verify the payout — peer-to-peer, no human in the loop. The §3 "spin up two autonomous agents" beat and the §7 "watch them negotiate" beat are the load-bearing audience moments. + +The bot's controller surface is just `sphere trader create-intent`. Everything else — the strategy engine, the market scan loop, the negotiation protocol (NP-0), the escrow handshake, the swap settlement — happens inside the trader-agent container. + +This is the companion to the soak script [`manual-test-trader-roundtrip.sh`](../manual-test-trader-roundtrip.sh) — that script asserts the same thing programmatically; this playbook walks the same flow live in front of an audience. + +--- + +## At a glance + +``` +SETUP alice + bob controller wallets on testnet + alice faucet 100 UCT, bob faucet 10 ETH + +SPAWN sphere trader spawn --name alice-trader-$SUFFIX + --trusted-escrows @escrow-test-02 + sphere trader spawn --name bob-trader-$SUFFIX + --trusted-escrows @escrow-test-02 + (each command brings up a per-user local Host Manager + the + trader tenant; no shared HM, no controller-pubkey whitelist) + +FUND sphere payments send --recipient @alice-trader --amount 50 UCT + sphere payments send --recipient @bob-trader --amount 4.5 ETH + +INTENTS sphere trader create-intent --tenant @alice-trader + --direction sell --base UCT --quote ETH + --rate-min 0.08 --rate-max 0.12 + --volume-min 50 --volume-max 50 + sphere trader create-intent --tenant @bob-trader + --direction buy --base UCT --quote ETH + --rate-min 0.08 --rate-max 0.12 + --volume-min 50 --volume-max 50 + → controllers go quiet ← + +WATCH tenants discover each other on market-api, + negotiate via NIP-17 DMs (NP-0 protocol), + execute via @escrow-test-02 SwapModule + +VERIFY sphere trader portfolio --tenant @alice-trader → -50 UCT, +5 ETH + sphere trader portfolio --tenant @bob-trader → +50 UCT, -5 ETH + sphere trader list-deals --state completed → matching deal_id +``` + +Total run time: ~25 min on a healthy testnet (the trader scan interval defaults to 30s, and escrow finalization dominates the back half). + +The "controller is just `sphere trader create-intent`; the rest happens autonomously" beat is the whole point — once §6 lands, you stop typing and start narrating. + +--- + +## §0 Before you start + +### Prerequisites + +- `sphere` CLI on `PATH` (`which sphere` should resolve), built from a checkout that includes the `sphere trader spawn` / `sphere trader stop` wrapper ([unicity-sphere/sphere-cli#49](https://github.com/unicity-sphere/sphere-cli/pull/49) or later). The wrapper brings up a **per-user local Host Manager** scoped to the active wallet's controller pubkey, then spawns the trader tenant against it — no shared HM required. +- **Docker** available on the demo machine. The wrapper drives docker to start the local HM container. +- The **trader-agent template** registered in the wrapper's local templates registry. The container image is `ghcr.io/vrogojin/agentic-hosting/trader:v0.1` (see [Trader image staleness](#trader-image-staleness) below). +- Outbound HTTPS to: `faucet.unicity.network`, `goggregator-test.unicity.network`, `market-api.unicity.network`, Unicity IPFS gateways. +- Outbound WSS to: `wss://nostr-relay.testnet.unicity.network`. +- An escrow service reachable on the same testnet relay set. Default `@escrow-test-02`; pass via `--trusted-escrows` on `sphere trader spawn` or via `sphere trader set-strategy` if you've stood up your own. +- A clean workspace (the script wipes its own scratch dir on exit unless `KEEP=1`). + +### Pre-flight sanity checks + +Before you start typing for an audience, run all three of these and confirm the network is up: + +```bash +# 1. Docker is up (the wrapper needs it to start the per-user HM container). +docker info >/dev/null && echo "docker OK" + +# 2. Escrow is reachable and signing. +sphere swap ping @escrow-test-02 + +# 3. Market-api is reachable. Searching for an unlikely term should +# return an empty-array response, not a connection error. +curl -fsS "https://market-api.unicity.network/intents?base_asset=UCT"e_asset=ETH" | head -c 200; echo +``` + +If any of these fail, fix it before the demo — none of them recover gracefully under audience pressure. + +### Versions to confirm + +```bash +sphere --help | head -3 +sphere trader --help | head -3 # should list spawn / stop / create-intent / cancel-intent / list-intents / list-deals / portfolio / set-strategy +node --version # >= 18 +docker --version # any recent stable +``` + +### Suggested terminal layout + +- **T1** — alice's controller wallet. All `--tenant @alice-trader` commands. +- **T2** — bob's controller wallet. All `--tenant @bob-trader` commands. +- **T3** — log tail. After §6 starts, this is where you show `docker logs -f sphere-trader--` and `sphere trader list-deals --tenant @` running on both sides. + +### Workspace bootstrap + +```bash +ROOT="/tmp/demo-trader-$$" +mkdir -p "$ROOT/peer-alice" "$ROOT/peer-bob" +SUFFIX="$(date +%s | tail -c 5)$(printf '%04x' $((RANDOM % 65536)))" +ALICE_TAG="alice-$SUFFIX" +BOB_TAG="bob-$SUFFIX" +ALICE_TRADER_TAG="alice-trader-$SUFFIX" +BOB_TRADER_TAG="bob-trader-$SUFFIX" +# Instance names passed to `sphere trader spawn --name` in §3. We use +# the same slug as the nametag suffix for symmetry; they're separate +# identifiers (the instance name is the wrapper's local registry key, +# the nametag is the on-network identity). +ALICE_TRADER_INSTANCE="alice-trader-$SUFFIX" +BOB_TRADER_INSTANCE="bob-trader-$SUFFIX" +echo "ALICE_TAG=$ALICE_TAG" +echo "BOB_TAG=$BOB_TAG" +echo "ALICE_TRADER_TAG=$ALICE_TRADER_TAG" +echo "BOB_TRADER_TAG=$BOB_TRADER_TAG" + +# Escrow used by both tenants. The trader image bakes @escrow-test-02 +# as the default `trusted_escrows[0]` — if you need a different escrow, +# pass it via SET_STRATEGY (§3.5 sidebar) before posting intents. +ESCROW="${ESCROW:-@escrow-test-02}" +echo "ESCROW=$ESCROW" + +# CLI emits mnemonic on stdout in non-TTY when --no-encrypt-mnemonic is +# implied. Allowing this makes the live walkthrough scriptable. +export SPHERE_ALLOW_MNEMONIC_NON_TTY=1 +``` + +### Known gotchas (read before demoing) + +#### Pre-flight: which form does the CLI accept? (float vs bigint) + +The intended UX — and what the rest of this playbook is written against — is **human-friendly floats**: you type `--rate-min 0.08 --rate-max 0.12 --volume-min 50 --volume-max 50` and the CLI converts to smallest-unit bigints internally by looking up each asset's decimals in the token registry (UCT and ETH are both 18-decimal on testnet). + +**However**, today's `sphere trader create-intent --help` may still declare `--rate-min ` (string-encoded smallest-unit integer). The CLI float-conversion is a #474 follow-up; before it lands, the deployed CLI accepts only the bigint form. **Run this one quick check before going live:** + +```bash +sphere trader create-intent --help | grep -E -- '--rate-min|--volume-min' +``` + +- If the help output says `` or `` (or omits the type) — you're on the post-fix CLI. The float values in §5 / §6 below work as written. +- If the help output says `` — you're on the pre-fix CLI. **Use the smallest-unit form** instead: + + | Quantity (intended) | Smallest-unit bigint (UCT/ETH 18-decimal) | + |-----------------------|-----------------------------------------------| + | rate `0.08` ETH/UCT | `80000000000000000` (`8 × 10^16`) | + | rate `0.12` ETH/UCT | `120000000000000000` (`1.2 × 10^17`) | + | rate `0.10` ETH/UCT | `100000000000000000` (`1 × 10^17`) | + | volume `50` UCT | `50000000000000000000` (`5 × 10^19`) | + | volume `1` UCT | `1000000000000000000` (`1 × 10^18`) | + + Either substitute the bigint form into every `create-intent` call in §5 / §6, or set up bash aliases at the top of T1 / T2: + + ```bash + RATE_MIN=80000000000000000 + RATE_MAX=120000000000000000 + VOLUME_MIN=50000000000000000000 + VOLUME_MAX=50000000000000000000 + ``` + + …and then write `--rate-min "$RATE_MIN"` etc. in the §5 / §6 commands. + +**Verification recipe — run on a throwaway tenant before the live demo regardless of which form the CLI accepts.** This is the canonical way to confirm the deployed image's internal rate convention round-trips correctly: + +```bash +# Post a tiny test intent; read it back; confirm rate / volume round-trip. +sphere trader create-intent --tenant @ \ + --direction sell --base UCT --quote ETH \ + --rate-min 0.10 --rate-max 0.10 \ + --volume-min 1 --volume-max 1 \ + --expiry-ms 600000 --json +sphere trader list-intents --tenant @ --json | jq '.intents[] | {rate_min, rate_max, volume_min, volume_max}' +sphere trader cancel-intent --tenant @ --intent-id +``` + +If the read-back values differ from what you expect by some factor of `10^N`, the deployed image normalizes rate to a different unit than you assumed. See [§11 — rate unit recompute](#11-what-to-do-if-a-section-fails-live) for the recompute helper. + +#### Cross-process DM flakiness (issue #473) + +Controller CLI commands exit between calls. The tenant authenticates each incoming ACP request against the controller's pubkey, and DM delivery between a short-lived controller process and a long-running tenant is occasionally flaky (tracked in [sphere-sdk#473](https://github.com/unicity-sphere/sphere-sdk/issues/473)). The current workaround is a retry loop: + +```bash +# Helper: retry the next sphere trader call up to 3 times with 5s backoff. +trader_retry() { + local n=0 + while [ $n -lt 3 ]; do + "$@" && return 0 + n=$((n+1)) + echo " (retry $n/3 after 5s)" >&2 + sleep 5 + done + return 1 +} +# Usage: +trader_retry sphere trader portfolio --tenant "@$ALICE_TRADER_TAG" --json +``` + +The `list-deals` / `list-intents` polls in §7 and §8 already build this in; you only need the helper for one-shot calls. + +#### Trader image staleness + +`ghcr.io/vrogojin/agentic-hosting/trader:v0.1` was tagged before several recent sphere-sdk changes that affect the negotiation→escrow path: + +- `DEFAULT_ESCROW_ADDRESS = @escrow-test-02` ([sphere-sdk#468](https://github.com/unicity-sphere/sphere-sdk/pull/468)) +- counterparty transport pubkey fail-fast ([sphere-sdk#459](https://github.com/unicity-sphere/sphere-sdk/pull/459)) +- MuxAdapter await on async handler completion ([sphere-sdk#465](https://github.com/unicity-sphere/sphere-sdk/pull/465)) + +If §7 hangs on `proposal_received` or §8 shows `EXECUTING` with no progress for more than 5 minutes, the v0.1 image is the most likely culprit. Rebuild the trader image against sphere-sdk `main` and republish before retrying. + +#### Trader scan interval + +Default `TRADER_SCAN_INTERVAL_MS=30000` (30s). The first match round can take **up to a minute** after the second intent lands — the strategy engine scans the market at its own cadence, not on demand. During §7, tell the audience "give it a minute" and don't refresh every 5s. If you want a faster demo cycle, set `--env TRADER_SCAN_INTERVAL_MS=10000` in §3 when you spawn the tenants. We use the default in this playbook so the spawned tenants match the production image's behaviour. + +--- + +## §1 Create the two controller wallets + +These are the **humans'** wallets — alice and bob each have a sphere wallet that holds their primary funds, bootstraps their per-user local Host Manager (via `sphere trader spawn`), and authenticates as the controller on each tenant. + +### Alice — T1 + +```bash +cd "$ROOT/peer-alice" +sphere wallet create alice +sphere wallet use alice +sphere init --network testnet --nametag "$ALICE_TAG" +``` + +Capture alice's pubkey for the talk-track — the per-user local HM that `sphere trader spawn` brings up in §3 scopes ACP authorization to this pubkey automatically, so you don't pass it explicitly, but it's still useful to show the audience: + +```bash +ALICE_PUBKEY=$(sphere identity show --json | jq -r '.chainPubkey') +echo "ALICE_PUBKEY=$ALICE_PUBKEY" +``` + +### Bob — T2 + +```bash +cd "$ROOT/peer-bob" +sphere wallet create bob +sphere wallet use bob +sphere init --network testnet --nametag "$BOB_TAG" + +BOB_PUBKEY=$(sphere identity show --json | jq -r '.chainPubkey') +echo "BOB_PUBKEY=$BOB_PUBKEY" +``` + +**Talk track:** "These are the human wallets. Alice and Bob each hold their primary funds in their controller wallet and use it to govern their respective trader tenants. The tenant has its own separate keypair — controller and tenant are different identities, and the tenant only accepts ACP commands signed by the controller's pubkey we just captured." + +--- + +## §2 Faucet — asymmetric so the demo can catch cross-talk + +### T1 — alice gets UCT only + +```bash +cd "$ROOT/peer-alice" +sphere wallet use alice +sphere faucet 100 UCT +sphere payments sync +sphere payments receive --finalize +sphere balance +``` + +### T2 — bob gets ETH only + +```bash +cd "$ROOT/peer-bob" +sphere wallet use bob +sphere faucet 10 ETH +sphere payments sync +sphere payments receive --finalize +sphere balance +``` + +Expected: +- alice: `UCT: 100 (1 token)` and no ETH row. +- bob: `ETH: 10 (1 token)` and no UCT row. + +**Snapshot now.** This is the "before" state. The asymmetric faucet is intentional — every net-delta in §9 has to come out of the trade, not an existing pool of both coins. + +**Talk track:** "Same trick as the swap demo. Each controller has only the coin it's giving up. The only way alice's portfolio ends up with ETH (and bob's with UCT) is for the two autonomous tenants to actually negotiate and settle a swap. There's no fallback liquidity to mask a bug." + +--- + +## §3 Spawn the two trader tenants ← BIG MOMENT + +This is where the demo earns its title. The next two commands turn over the keys to two AI agents. + +Each peer runs its OWN local Host Manager — there's no shared backend, no whitelist to apply for, no operator to coordinate with. `sphere trader spawn` brings up a per-user HM container scoped to the current wallet's controller pubkey, then launches the trader tenant against it. One command per peer, no environment plumbing. + +### T1 — spawn alice's trader + +```bash +cd "$ROOT/peer-alice" +sphere wallet use alice +sphere trader spawn \ + --name "$ALICE_TRADER_INSTANCE" \ + --trusted-escrows "$ESCROW" \ + --json +``` + +Expected JSON excerpt (the wrapper streams progress lines then a final JSON document): + +```json +{ + "instance_name": "alice-trader-XXXXX", + "instance_id": "t_01HX...", + "tenant_direct_address": "DIRECT://0000...", + "hm_container": "sphere-hm-alice-...", + "hm_manager_address": "DIRECT://0000..." +} +``` + +### T2 — spawn bob's trader + +```bash +cd "$ROOT/peer-bob" +sphere wallet use bob +sphere trader spawn \ + --name "$BOB_TRADER_INSTANCE" \ + --trusted-escrows "$ESCROW" \ + --json +``` + +### Probe each tenant via ACP (proves Nostr transport is live + primes `since` cursor) + +`sphere trader spawn` already blocks until the trader container reports ready (via `--ready-timeout-ms`). One ACP smoke call doubles as a transport-layer liveness probe and primes the tenant's Nostr `since` cursor for subsequent DMs (workaround for [sphere-sdk#473](https://github.com/unicity-sphere/sphere-sdk/issues/473)): + +```bash +# T1 — first portfolio call doubles as a transport-layer liveness probe. +trader_retry sphere trader portfolio --tenant "@$ALICE_TRADER_TAG" +# T2 +trader_retry sphere trader portfolio --tenant "@$BOB_TRADER_TAG" +``` + +Expected — an empty portfolio at this point (the tenant has its own wallet but no balance yet): + +```json +{ + "balances": {}, + "address": "DIRECT://0000..." +} +``` + +**Talk track:** "These two containers are now autonomous. They have their own secp256k1 keypairs, their own subscriptions to the testnet Nostr relays, and a strategy engine that scans market-api every 30 seconds. Each peer runs its own local Host Manager — no shared HM, no whitelist to negotiate. The HM is just a launcher: once the tenant is RUNNING, the manager is out of the data path. Alice's only relationship with her trader is that the tenant accepts ACP commands signed by her controller pubkey — the same pubkey the local HM was bootstrapped against. Everything else, the tenant decides for itself." + +### §3.5 — Optional sidebar: tune strategy before funding + +If your demo image's default strategy is too aggressive (or too conservative), bring it in line before §4: + +```bash +sphere trader set-strategy --tenant "@$ALICE_TRADER_TAG" \ + --rate-strategy moderate --max-concurrent 1 +sphere trader set-strategy --tenant "@$BOB_TRADER_TAG" \ + --rate-strategy moderate --max-concurrent 1 +``` + +`moderate` aims for the midpoint of the overlap band; `aggressive` skews to the band edge that maximizes the bot's take; `conservative` skews the other way. `max-concurrent 1` ensures the bot won't try to start a second deal mid-demo if a stray intent appears. Both can be skipped on a clean testnet. + +--- + +## §4 Fund the tenants (controllers seed working capital) + +The tenant's wallet was created empty in §3. The controller now sends in the working capital. The tenant cannot post an intent it can't reserve — the strategy engine pre-flights every intent against current balance. + +### T1 — alice seeds 50 UCT into her tenant + +```bash +cd "$ROOT/peer-alice" +sphere wallet use alice +sphere payments send \ + --recipient "@$ALICE_TRADER_TAG" \ + --amount 50 \ + --coinId UCT \ + --memo "trader seed" +``` + +(`sphere payments send --amount` uses the human-friendly float form; `50` here means 50 UCT, which the CLI converts to `5 × 10^19` smallest-unit internally.) + +### T2 — bob seeds 4.5 ETH into his tenant + +```bash +cd "$ROOT/peer-bob" +sphere wallet use bob +sphere payments send \ + --recipient "@$BOB_TRADER_TAG" \ + --amount 4.5 \ + --coinId ETH \ + --memo "trader seed" +``` + +(4.5 ETH at 18 decimals = `4.5 × 10^18` smallest-unit internally.) + +### Poll until the seed lands + +```bash +# T1 +for i in {1..40}; do + bal=$(sphere trader portfolio --tenant "@$ALICE_TRADER_TAG" --json 2>/dev/null \ + | jq -r '.balances.UCT // "0"') + echo " alice-trader UCT balance: $bal" + [ "$bal" != "0" ] && [ -n "$bal" ] && break + sleep 3 +done + +# T2 +for i in {1..40}; do + bal=$(sphere trader portfolio --tenant "@$BOB_TRADER_TAG" --json 2>/dev/null \ + | jq -r '.balances.ETH // "0"') + echo " bob-trader ETH balance: $bal" + [ "$bal" != "0" ] && [ -n "$bal" ] && break + sleep 3 +done +``` + +Expected (after both polls settle): + +```text +alice-trader UCT balance: 50000000000000000000 +bob-trader ETH balance: 4500000000000000000 +``` + +**Talk track:** "The controller funded the tenant with a regular L3 payment — same wire format as any wallet-to-wallet send. The tenant received it, finalized the token, and updated its internal portfolio. Now the strategy engine sees there's working capital and will accept an intent up to that balance — anything more would fail the precommit reservation when the strategy engine maps the intent to a budget." + +--- + +## §5 Alice posts her SELL intent + +The controller's only job in the trading lifecycle is this command. Everything from here until §8 is the tenant. + +### T1 + +```bash +sphere wallet use alice +trader_retry sphere trader create-intent \ + --tenant "@$ALICE_TRADER_TAG" \ + --direction sell \ + --base UCT \ + --quote ETH \ + --rate-min 0.08 \ + --rate-max 0.12 \ + --volume-min 50 \ + --volume-max 50 \ + --expiry-ms 3600000 \ + --json +``` + +Rate band, read out loud: +- `rate-min` = `0.08` ETH per UCT +- `rate-max` = `0.12` ETH per UCT +- midpoint = `0.10` ETH per UCT — the price alice and bob's tenants should converge on under the `moderate` strategy. + +Volume: alice will sell exactly 50 UCT (`volume-min == volume-max`); at the midpoint rate that earns her 5 ETH. + +> **Pre-#474 CLI?** Substitute the bigint values from the [Pre-flight table](#pre-flight-which-form-does-the-cli-accept-float-vs-bigint) in §0 — `0.08` → `80000000000000000`, `0.12` → `120000000000000000`, `50` → `50000000000000000000`. + +Expected JSON response excerpt (the wire is bigint regardless of CLI input form): + +```json +{ + "ok": true, + "result": { + "intent_id": "i_01HX...", + "market_intent_id": "mi_XXXX...", + "state": "active", + "direction": "sell", + "base_asset": "UCT", + "quote_asset": "ETH", + "rate_min": "80000000000000000", + "rate_max": "120000000000000000", + "volume_min": "50000000000000000000", + "volume_max": "50000000000000000000", + "expires_at": "..." + } +} +``` + +Cross-verify the intent landed on the market-api: + +```bash +curl -fsS "https://market-api.unicity.network/intents?base_asset=UCT"e_asset=ETH&direction=sell" | jq '.intents[] | select(.market_intent_id == "")' +``` + +**Talk track:** "Alice's tenant has now told the market 'I will sell 50 UCT for any rate between 0.08 and 0.12 ETH per UCT.' The intent is signed with the *tenant's* own secp256k1 key — not alice's controller key — and posted to market-api with secp256k1 auth headers. From market-api's perspective, the tenant is a first-class trader: market-api doesn't know or care that there's a controller behind it." + +--- + +## §6 Bob posts the matching BUY intent ← THE TRIGGER + +### T2 + +```bash +sphere wallet use bob +trader_retry sphere trader create-intent \ + --tenant "@$BOB_TRADER_TAG" \ + --direction buy \ + --base UCT \ + --quote ETH \ + --rate-min 0.08 \ + --rate-max 0.12 \ + --volume-min 50 \ + --volume-max 50 \ + --expiry-ms 3600000 \ + --json +``` + +Same rate band as alice — `0.08` to `0.12` ETH per UCT. Same volume — 50 UCT. Opposite direction (buy vs sell). **Overlap is the whole band**; the midpoint `0.10` ETH/UCT is the price both bots will converge on under the `moderate` strategy. + +> **Pre-#474 CLI?** Same substitution as §5 — switch the float values for their bigint equivalents. + +```bash +BOB_INTENT_ID=$(jq -r '.result.intent_id' <<< "$LAST_JSON") # or just copy from above +echo "BOB_INTENT_ID=$BOB_INTENT_ID" +``` + +**Now stop typing.** Tell the audience: "From this point on, no human touches the keyboard until §8 verification. The next thing that happens is one tenant's strategy engine wakes up on its 30-second scan, queries market-api, sees a matching intent on the opposite side, and starts a negotiation. Watch." + +**Talk track:** "Two intents are now on the market — one to sell 50 UCT for ETH, one to buy 50 UCT for ETH, both with overlapping rate bands. From the bots' perspective, this is a perfect match: same pair, same volume, overlapping rates. The strategy engine on whichever tenant scans first will pick up the counterparty's intent, decide it's a viable deal, and initiate NP-0 (the negotiation protocol)." + +--- + +## §7 The negotiation (audience watches the bots talk) + +This is the demo's payoff. Tail both tenants' state continuously in T3 while you narrate. + +### T3 — watch both sides progress + +```bash +# In one terminal, two background pollers: +watch -n 5 ' +echo "=== alice-trader intents ==="; +sphere trader list-intents --tenant "@'"$ALICE_TRADER_TAG"'" --json 2>/dev/null | jq ".intents[] | {intent_id, state}"; +echo "=== alice-trader deals ==="; +sphere trader list-deals --tenant "@'"$ALICE_TRADER_TAG"'" --json 2>/dev/null | jq ".deals[] | {deal_id, state, base_volume, rate}"; +echo "=== bob-trader intents ==="; +sphere trader list-intents --tenant "@'"$BOB_TRADER_TAG"'" --json 2>/dev/null | jq ".intents[] | {intent_id, state}"; +echo "=== bob-trader deals ==="; +sphere trader list-deals --tenant "@'"$BOB_TRADER_TAG"'" --json 2>/dev/null | jq ".deals[] | {deal_id, state, base_volume, rate}"; +' +``` + +You should see the following sequence over the next 60–120 seconds: + +``` +t=0s both intents: state: active deals: (none) +t=30s one side fires the scan → match found + that side's intent: state: matching deals: [{state: NEGOTIATING}] +t=35s NP-0 propose_deal DM → counterparty's tenant + counterparty: state: matching deals: [{state: NEGOTIATING}] +t=45s NP-0 accept_deal DM → both transition to EXECUTING + both tenants: deals: [{state: EXECUTING, rate: "10000...", volume: "50..."}] +t=60s SwapModule.proposeSwap → escrow handshake begins +t=90s Both sides deposit, escrow validates, payouts emitted +t=120s Both tenants: deals: [{state: COMPLETED, ...}] + intents: state: filled (or partially filled) +``` + +If after 2 minutes no tenant has flipped to `NEGOTIATING`, see [§11 — Negotiation timeout](#11-what-to-do-if-a-section-fails-live). + +**Talk track (THE PAYOFF):** "Notice nobody touched a keyboard for 90 seconds. The agents found each other on market-api, ran an NP-0 negotiation over NIP-17 gift-wrapped DMs — that's the same encrypted DM channel sphere wallets use for everything else — agreed on a rate and volume inside both their bands, and then handed off to the SwapModule to actually settle on the escrow. The controller — that's alice, the human — is asleep. She'll wake up tomorrow with 5 ETH in her portfolio and a settled deal on the books." + +### §7.5 — Optional sidebar: tail tenant logs + +If the audience wants to see the bots talking in real time, tail each trader container's logs directly via docker: + +```bash +# T1 (alice's machine) +docker logs -f --tail 50 sphere-trader-alice-$SUFFIX +# T2 (bob's machine) +docker logs -f --tail 50 sphere-trader-bob-$SUFFIX +``` + +(Container names follow the `sphere-trader--` pattern that `sphere trader spawn` uses; `docker ps | grep sphere-trader` will show the exact name if your wallet name differs.) + +Look for: +- `intent_matched market_intent_id=mi_… counterparty=DIRECT://…` (the scanner woke up) +- `np.propose_deal sent` / `np.proposal_received` (the protocol handshake) +- `np.accept_deal sent` (deal agreed) +- `SwapModule.proposeSwap → swap_id=…` (handoff to swap settlement) +- `swap:deposit_confirmed` / `swap:completed` (escrow done) + +--- + +## §8 Verify deal completion + +Once `watch` shows both sides at `COMPLETED`, snapshot the deal record on each side. + +### T1 — alice's view of the deal + +```bash +cd "$ROOT/peer-alice" +sphere wallet use alice +sphere trader list-deals --tenant "@$ALICE_TRADER_TAG" --state completed --json | jq '.deals[0]' +``` + +### T2 — bob's view of the deal + +```bash +cd "$ROOT/peer-bob" +sphere wallet use bob +sphere trader list-deals --tenant "@$BOB_TRADER_TAG" --state completed --json | jq '.deals[0]' +``` + +Both sides should agree on: +- the same `deal_id` (it's content-addressed over the negotiation manifest), +- the same agreed `rate` (e.g. `100000000000000000` for `0.10` ETH/UCT under `moderate` strategy), +- the same agreed `base_volume` (`50000000000000000000`), +- both `state: COMPLETED`, +- both reference the same underlying `swap_id` (the escrow swap that backed the deal). + +**Talk track:** "Both tenants independently recorded the same deal — same id, same agreed rate, same volume. The deal record on each side is the bot's local commitment ledger; the underlying swap on the escrow is the cryptographic anchor. If the bots disagreed about any of these fields, the escrow would have refused to release the payouts — atomic settlement enforces consensus." + +--- + +## §9 Verify balances (portfolio view) + +### T1 — alice's tenant portfolio + +```bash +sphere trader portfolio --tenant "@$ALICE_TRADER_TAG" --json | jq '.balances' +``` + +Expected: + +```json +{ + "UCT": "0", + "ETH": "5000000000000000000" +} +``` + +### T2 — bob's tenant portfolio + +```bash +sphere trader portfolio --tenant "@$BOB_TRADER_TAG" --json | jq '.balances' +``` + +Expected: + +```json +{ + "UCT": "50000000000000000000", + "ETH": "0" +} +``` + +(Bob deposited 4.5 ETH but pays 5 ETH at the midpoint rate. If you want a clean `0` remainder, fund bob with exactly the midpoint payment — `5000000000000000000` — instead of `4500000000000000000`. The default in this playbook leaves a `−500000000000000000` shortfall: the bot will only execute at rates it can cover, so under `moderate` strategy with a `[0.08, 0.12]` band it will skew toward the lower edge to fit, or refuse if no feasible rate covers its budget. For a guaranteed clean outcome, fund bob with at least `6000000000000000000` (6 ETH) and accept that the residue will be left in the tenant.) + +### Net delta from baseline (§2) + +| Wallet | Baseline (§2) | After §9 | Δ | +|-------------------|---------------|---------------------|-------------------------| +| alice (controller)| 100 UCT, 0 ETH| 50 UCT, 0 ETH | −50 UCT (seeded to trader) | +| alice-trader | 0, 0 | 0 UCT, 5 ETH | **+5 ETH from trade** | +| bob (controller) | 0 UCT, 10 ETH | 0 UCT, 5.5 ETH | −4.5 ETH (seeded to trader) | +| bob-trader | 0, 0 | 50 UCT, 0 ETH | **+50 UCT from trade** | + +Roll up alice's controller + alice's tenant: she's `−50 UCT, +5 ETH` net. Roll up bob's: he's `+50 UCT, −4.5 ETH` net (or `−5 ETH` if you topped him up to 6 ETH). Both ledgers balance against the escrow's internal accounting. + +**Talk track:** "Atomic — both moved or neither. The rate the bots agreed on splits the overlap band evenly under the `moderate` strategy, so neither side feels squeezed. Notice the trader tenant holds the bought asset, not the controller — the controller would have to send a `payments send` from the tenant back to its own wallet to consolidate. In a long-running trading setup, you'd leave the proceeds in the tenant so it can roll them into the next intent. Alice and Bob can now go to bed; the daemons handle the next round." + +--- + +## §10 Cleanup + +### Cancel any leftover intents + +If either intent was partially filled or somehow lingered: + +```bash +# T1 +ALICE_OPEN=$(sphere trader list-intents --tenant "@$ALICE_TRADER_TAG" --state active --json | jq -r '.intents[]?.intent_id') +for id in $ALICE_OPEN; do + sphere trader cancel-intent --tenant "@$ALICE_TRADER_TAG" --intent-id "$id" +done + +# T2 +BOB_OPEN=$(sphere trader list-intents --tenant "@$BOB_TRADER_TAG" --state active --json | jq -r '.intents[]?.intent_id') +for id in $BOB_OPEN; do + sphere trader cancel-intent --tenant "@$BOB_TRADER_TAG" --intent-id "$id" +done +``` + +### Stop the tenants (or `--keep-hm` for Q&A) + +```bash +# T1 +cd "$ROOT/peer-alice" && sphere wallet use alice +sphere trader stop --name "$ALICE_TRADER_INSTANCE" + +# T2 +cd "$ROOT/peer-bob" && sphere wallet use bob +sphere trader stop --name "$BOB_TRADER_INSTANCE" +``` + +`sphere trader stop` stops the trader tenant and — when the last tenant attached to a given per-user local HM stops — also tears down the HM container. If you want to leave the local HMs running for Q&A so the audience can ask follow-up questions via `sphere trader portfolio` / `list-intents` / `list-deals`, pass `--keep-hm`: + +```bash +sphere trader stop --name "$ALICE_TRADER_INSTANCE" --keep-hm +sphere trader stop --name "$BOB_TRADER_INSTANCE" --keep-hm +``` + +The tenant processes still stop (the wrapper's local registry is the source of truth — leaving an unregistered tenant alive would orphan it), but the HMs remain so you can `sphere trader spawn` a fresh tenant against them without re-paying the HM bootstrap cost. + +### Wipe workspace + +```bash +rm -rf "$ROOT" +``` + +If you want to inspect afterwards: leave `$ROOT` in place; the controller wallet stores are in `$ROOT/peer-alice/.sphere-cli-alice/` and `$ROOT/peer-bob/.sphere-cli-bob/`. The tenant's state lives in the per-user local HM's docker volume — `docker ps` will show the `sphere-hm--*` and `sphere-trader-*` containers and `docker inspect` will show the volume mounts. + +--- + +## §11 What to do if a section fails live + +| Symptom | What it means | Demo recovery | +|---|---|---| +| `sphere trader spawn` exits 1 before the JSON document | The wrapper couldn't bring up the local HM (docker daemon down, port collision, template missing). | Verify `docker info` works. Check `docker ps -a | grep sphere-hm` for a stuck container from a previous run — `docker rm -f` it and retry. Confirm the wrapper's templates registry includes `trader-agent`. | +| `sphere trader spawn` ready-timeout exceeded | The trader image started but didn't reach ready before the wrapper's `--ready-timeout-ms` budget. | Tail the trader container with `docker logs -f sphere-trader--` and check for image-pull or first-boot errors. As a workaround, re-run with `--ready-timeout-ms 240000` (4 min) for slow IPFS warmups. | +| Tenant doesn't respond to `sphere trader portfolio` (TimeoutError after 30s) | Either: (a) the trader container died after `spawn` reported ready (check `docker logs`), or (b) the cross-process DM flakiness from [sphere-sdk#473](https://github.com/unicity-sphere/sphere-sdk/issues/473). | Re-run `trader_retry sphere trader portfolio --tenant @`. If retries also fail, check `docker ps` for the `sphere-trader--` container — if it's gone, re-spawn. | +| `sphere trader create-intent` returns `ok: false` with `INSUFFICIENT_BALANCE` | The strategy engine pre-flighted the intent against current tenant balance and it doesn't fit. | Re-check `sphere trader portfolio` and confirm §4 seed actually landed. If yes, the rate-unit ambiguity may have made the intent volume larger than expected — recompute (see below). | +| `sphere trader create-intent` returns `INVALID_PARAM rate_min must be a non-negative integer string` (or `volume_min …`) | The CLI is on the pre-#474 bigint surface but you passed float values like `0.08`. The float→bigint conversion isn't wired yet, so the CLI sent `"0.08"` verbatim and the trader rejected it. | Switch the demo values from float form to the smallest-unit bigint form using the [Pre-flight table](#pre-flight-which-form-does-the-cli-accept-float-vs-bigint) in §0 (`0.08` → `80000000000000000`, etc.). Re-run create-intent. | +| `sphere trader create-intent` returns `INVALID_PARAM` referencing `rate_min` / `rate_max` for any other reason | Rate-unit ambiguity — the value you sent decodes to something the trader rejects (wrong scale, out-of-range, min > max). | Verify locally with `sphere trader list-intents --tenant @` on a tiny test intent first. **Recompute rate:** if you want rate `R` (quote per base, as a decimal), and both base and quote have 18 decimals, encode rate as `floor(R × 10^18)`. Example: `0.10 ETH/UCT` → `10^17` → `100000000000000000`. | +| Intent appears on market-api but the other tenant never picks it up after 2 min | Either the other tenant's scanner isn't running, or it's filtering out this counterparty (trusted-escrow mismatch). | Tail logs (§7.5) on the other tenant; look for `scan tick` or `match_skipped reason=…` lines. If `trusted_escrows` mismatches, fix with `sphere trader set-strategy --tenant @ --trusted-escrows @escrow-test-02`. | +| Tenant log shows `RATE_UNACCEPTABLE` after a match | The negotiated rate ended up outside one party's band — usually a rate-unit interpretation bug between the two tenants. | Cancel both intents, recompute rates as above, re-post. | +| Negotiation timeout (>2 min, no `DEAL_PROPOSED` exchanged) | Most likely cause is the trader v0.1 image staleness — [DEFAULT_ESCROW_ADDRESS rotation (#468)](https://github.com/unicity-sphere/sphere-sdk/pull/468), [counterparty transport pubkey fail-fast (#459)](https://github.com/unicity-sphere/sphere-sdk/pull/459), or [MuxAdapter await fix (#465)](https://github.com/unicity-sphere/sphere-sdk/pull/465) all changed behaviour. | Rebuild the trader image against sphere-sdk `main` and re-run. As a last-resort live demo recovery: cancel both intents, drop to the **swap playbook** for the back half — the SwapModule layer underneath is the same. | +| Both tenants RUNNING but neither intent appears on market-api after 30 s | market-api unreachable from the tenant, or the tenant's auth signature is being rejected. | Check market-api directly with `curl https://market-api.unicity.network/health`. If reachable, check tenant logs for `market_api: 401` or similar auth errors — that usually means a relay-clock-skew issue or a misconfigured base URL. | +| Deal stuck at `EXECUTING` for >5 min | The escrow handshake is blocked — escrow unreachable, escrow rejected the manifest, or one tenant failed to deposit. | `sphere swap ping @escrow-test-02` first. If escrow is up, ask the tenant for its swap_id and run `sphere swap status --query-escrow` from a peer wallet — the escrow's view tells you which deposit leg is missing. | +| Trader negotiation falls into an `AGENT_BUSY` loop | Two tenants matched simultaneously and both initiated NP-0 — the protocol's symmetry-breaker resolves by lexicographic pubkey comparison; one will back off. | Wait one full scan interval (~30 s). The loser will release the lock and the deal proceeds. If after 60 s nothing has moved, manually cancel the busier intent and re-post. | +| `[Nostr] [AT-LEAST-ONCE] TOKEN_TRANSFER … not durable — leaving 'since' at ` | Background durability verifier couldn't confirm a previous event landed durably on the relay. Independent of trader flow. | Continue the demo. | + +### Rate unit recompute helper + +If `list-intents --json` shows your `rate_min` / `rate_max` round-tripping to values larger or smaller than expected by a factor of `10^N`: + +```bash +# You wanted 0.10 ETH/UCT, but the tenant stored 1e35. That's a 10^18 scale-up. +# Re-encode at 10^17 instead of 10^35 (i.e. divide by 10^18): +echo "DESIRED_RATE × 10^17" | bc # 0.10 → 10000000000000000 + +# Or, for arbitrary precision and decimals: +python3 -c 'import sys; R, decimals = float(sys.argv[1]), int(sys.argv[2]); print(int(R * 10**decimals))' 0.10 17 +# → 10000000000000000 +``` + +The `decimals` value in the helper above is the **exponent the deployed image expects** — confirm with the verification recipe in §0 before adjusting. + +--- + +## §12 Optional — the automated soak + +Everything in this playbook is the script [`manual-test-trader-roundtrip.sh`](../manual-test-trader-roundtrip.sh) in the SDK repo: + +```bash +cd +bash manual-test-trader-roundtrip.sh # default scenario +KEEP=1 bash manual-test-trader-roundtrip.sh # preserve workspace +KEEP_TENANTS=1 bash manual-test-trader-roundtrip.sh # leave per-user HMs running (--keep-hm) +ESCROW=@my-escrow bash manual-test-trader-roundtrip.sh # override escrow +SUFFIX=demo01 bash manual-test-trader-roundtrip.sh # deterministic tags +``` + +A green run prints `ALL GREEN — trader round-trip soak succeeded` and exits 0. + +The soak script: +- Builds the same alice/bob controller wallets. +- Spawns the same `trader-agent` tenants with the same env. +- Funds each tenant. +- Posts both intents. +- **Polls** `list-deals --state completed` on both sides (up to a configurable budget). +- Asserts the `deal_id` matches across sides, the agreed rate sits inside the overlap band, and the portfolios reflect the agreed flow. +- Cleans up unless `KEEP=1` / `KEEP_TENANTS=1`. + +Use the script for CI / nightly soaks; use this playbook for human demos. + +--- + +## Presenter cheat sheet + +```text + §0 $ROOT, $ALICE_TAG, $BOB_TAG, $ALICE_TRADER_TAG, $BOB_TRADER_TAG, $ESCROW + SPHERE_ALLOW_MNEMONIC_NON_TTY=1 + trader_retry() helper for #473 flakiness + Pre-flight: docker info, sphere swap ping @escrow-test-02, market-api curl + + §1 sphere wallet create / use / init --nametag ×2 controller wallets + capture ALICE_PUBKEY, BOB_PUBKEY (chainPubkey) + + §2 sphere faucet 100 UCT (alice controller) + sphere faucet 10 ETH (bob controller) ← asymmetric on purpose + + §3 sphere trader spawn --name alice-trader-$SUFFIX + --trusted-escrows @escrow-test-02 --json + (and the same for bob — each peer brings up its OWN local HM) + Wrapper blocks until trader image reports ready. + First successful sphere trader portfolio = transport-layer liveness proof. + ← BIG MOMENT: "autonomous agents are now alive" + + §4 sphere payments send → @alice-trader --amount 50 --coinId UCT + sphere payments send → @bob-trader --amount 4.5 --coinId ETH + Poll sphere trader portfolio --json until seed lands. + + §5 sphere trader create-intent --tenant @alice-trader + --direction sell --base UCT --quote ETH + --rate-min 0.08 --rate-max 0.12 + --volume-min 50 --volume-max 50 + --expiry-ms 3600000 + (pre-#474 CLI: substitute bigint form per §0) + + §6 sphere trader create-intent --tenant @bob-trader + --direction buy --base UCT --quote ETH + --rate-min 0.08 --rate-max 0.12 + --volume-min 50 --volume-max 50 + --expiry-ms 3600000 + ← TRIGGER: stop typing now + + §7 Tail both sides with watch -n 5 'list-intents + list-deals' + Expected: state: active → matching → NEGOTIATING → EXECUTING → COMPLETED + Talk track: "nobody is touching a keyboard" + + §8 sphere trader list-deals --tenant @alice-trader --state completed + sphere trader list-deals --tenant @bob-trader --state completed + Same deal_id, same rate, same volume, both COMPLETED. + + §9 sphere trader portfolio --tenant @alice-trader → UCT 0, ETH ~5e18 + sphere trader portfolio --tenant @bob-trader → UCT 5e19, ETH residue + "Alice and Bob can now go to bed; the daemons handle the next round." + + §10 cancel-intent leftovers, sphere trader stop --name both tenants, rm -rf $ROOT + (or sphere trader stop --keep-hm for Q&A) +``` + +### Command quick reference + +| When you want to… | Run | +|---|---| +| Spawn a trader tenant | `sphere trader spawn --name --trusted-escrows @ [--scan-interval-ms ] [--ready-timeout-ms ] [--json]` (brings up a per-user local HM + trader tenant) | +| Probe tenant liveness | `sphere trader portfolio --tenant @` (ACP — first successful call doubles as a transport-layer liveness probe) | +| Post a trading intent | `sphere trader create-intent --tenant @ --direction --base --quote --rate-min --rate-max --volume-min --volume-max ` (post-#474 UX; on pre-fix CLI, see [Pre-flight table](#pre-flight-which-form-does-the-cli-accept-float-vs-bigint)) | +| List the tenant's intents | `sphere trader list-intents --tenant @ [--state active\|filled\|cancelled\|expired]` | +| Cancel an intent | `sphere trader cancel-intent --tenant @ --intent-id ` | +| List the tenant's deals | `sphere trader list-deals --tenant @ [--state active\|completed\|failed]` | +| Show tenant balance | `sphere trader portfolio --tenant @` | +| Tune trader strategy | `sphere trader set-strategy --tenant @ [--rate-strategy aggressive\|moderate\|conservative] [--max-concurrent ] [--trusted-escrows @e1,@e2]` | +| Stop a tenant | `sphere trader stop --name [--keep-hm]` (auto-tears down the per-user HM when the last tenant stops; `--keep-hm` leaves it running for Q&A) | +| Pre-flight escrow liveness | `sphere swap ping @escrow-test-02` | + +### Exit codes that matter + +| Command | Exit | Meaning | +|---|---|---| +| `sphere trader spawn` | 0 | per-user HM up + tenant ready (final JSON document emitted) | +| `sphere trader spawn` | 1 | docker unavailable, port collision, template missing, or ready-timeout exceeded | +| `sphere trader stop` | 0 | tenant stopped (HM auto-torn-down unless `--keep-hm`) | +| `sphere trader stop` | 1 | name not found in wrapper's local registry, or docker error | +| `sphere trader create-intent` | 0 | intent accepted; `result.intent_id` returned | +| `sphere trader create-intent` | 1 | rejected (`INVALID_PARAM`, `INSUFFICIENT_BALANCE`, transport timeout) | +| `sphere trader cancel-intent` | 0 | cancelled; tenant flipped state to `cancelled` | +| `sphere trader cancel-intent` | 1 | not found, already terminal, or transport error | +| `sphere trader list-deals` | 0 | one or more matching deals returned (possibly empty array under `--state`) | +| `sphere trader list-deals` | 1 | transport error / tenant unreachable / `--limit` invalid | +| `sphere trader portfolio` | 0 | balances returned (possibly empty) | +| `sphere trader portfolio` | 1 | transport error / tenant unreachable | +| `sphere trader set-strategy` | 0 | strategy updated | +| `sphere trader set-strategy` | 1 | no fields provided, invalid value, or transport error | diff --git a/docs/uxf/ISSUE-473-INVESTIGATION.md b/docs/uxf/ISSUE-473-INVESTIGATION.md new file mode 100644 index 00000000..dc650822 --- /dev/null +++ b/docs/uxf/ISSUE-473-INVESTIGATION.md @@ -0,0 +1,596 @@ +# Issue #473 — Cross-process Nostr DM flakiness: root cause + defensive fix + +**Status:** Investigation report. No source files were modified; no fix has been applied. + +**Scope:** Hard dependency for #474 (trader-roundtrip soak). Controller-CLI ↔ tenant DM hops are cross-process; a single missed swap-proposal DM breaks the whole soak. + +--- + +## TL;DR + +The Mux's chat-side `since` cursor advances to **wall-clock-now at unwrap time**, BEFORE the async DM handler (SwapModule's `handleIncomingDM`) has run to completion — and BEFORE the storage write is flushed. The chat filter's `-NIP17_TIMESTAMP_RANDOMIZATION` (172800s) buffer compensates for ±2-day NIP-17 timestamp randomization, but NOT for the residual case where the receiver's CLI exits (or crashes, or is killed in the soak loop's 3 s budget) between the cursor advance and the handler's swap-persistence. The next CLI boot then re-subscribes with `since = lastDmTs - 172800` — and because `lastDmTs` was advanced past alice's *publish* time while alice's *randomized* `created_at` can be up to 172800 s earlier than that, alice's gift-wrap is filtered out by the relay for the rest of the soak. **Single smallest fix: in `MultiAddressTransportMux.routeGiftWrap`, move the `updateLastDmEventTimestamp` call from line 1173 to AFTER `await entry.adapter.dispatchMessage(...)` — and widen the look-back buffer used at subscription time from `NIP17_TIMESTAMP_RANDOMIZATION` to `2 * NIP17_TIMESTAMP_RANDOMIZATION` to belt-and-brace against the worst-case wall-clock-vs-randomization window.** + +--- + +## Mechanism rankings + +### M1: `since` cursor advances past the DM's `created_at` — **BLOCKING** + +This is the root cause. The Mux advances the persisted chat-side `since` cursor (`lastDmEventTs`) using **wall-clock time at unwrap**, NOT the event's `created_at`. The reason given in the code comment (line 1171-1172) is that NIP-17 randomization can place `created_at` in the future, but the side effect is that `lastDmTs` is **always strictly greater than the actual publish time** of the event that advanced it. A subsequent subscription opens with `since = lastDmTs - NIP17_TIMESTAMP_RANDOMIZATION`. Worst-case math: + +- Alice publishes gift-wrap at `T_pub`. NIP-17 randomization (`TIMESTAMP_RANDOMIZATION = 172800 s`, line 110 in `NostrTransportProvider.ts`; alias `NIP17_TIMESTAMP_RANDOMIZATION` line 79 in the Mux) places `event.created_at` uniformly in `[T_pub - 172800, T_pub + 172800]`. Worst-case low: `event.created_at = T_pub - 172800`. +- Bob's wallet processes (unwraps) the event at `T_proc`. `T_proc > T_pub` (causality). The cursor advances to `lastDmTs = T_proc` (wall-clock). +- Next subscription opens with `since = T_proc - 172800`. +- The relay returns `event.created_at >= since`. For alice's worst-case event: `T_pub - 172800 >= T_proc - 172800` ⇔ `T_pub >= T_proc`. **False** (T_proc > T_pub by construction). +- Alice's event is filtered out by the relay for every subsequent boot. + +The buffer only catches the **publish-time-to-cursor-advance gap** of 172800 s. If the cursor was advanced *before the handler completed* and the handler then failed to persist the swap (CLI exited, process killed, handler threw), the event is **permanently invisible** to bob until alice republishes — which the soak never does. + +#### Evidence + +`transport/MultiAddressTransportMux.ts` lines 1162-1310 (`routeGiftWrap`): + +```typescript +private async routeGiftWrap(event: NostrEvent): Promise { + for (const entry of this.addresses.values()) { + try { + const pm = NIP17.unwrap(event as any, entry.keyManager); + + // Successfully decrypted — route to this address. + // Persist DM timestamp after successful unwrap so failed decryptions + // do not advance the since filter and permanently skip events. + // Use real wall-clock time, NOT event.created_at — NIP-17 gift wraps + // randomize created_at by ±2 days for privacy, so it can be in the future. + this.updateLastDmEventTimestamp(entry, Math.floor(Date.now() / 1000)); // <-- LINE 1173 + // ... long async chain follows: dispatch read receipts, composing + // indicators, handler calls (entry.adapter.dispatchMessage), etc. + // All `await`s, but the cursor is already advanced. +``` + +`transport/MultiAddressTransportMux.ts` lines 1706-1716: + +```typescript +private updateLastDmEventTimestamp(entry: AddressEntry, createdAt: number): void { + if (!this.storage) return; + if (createdAt <= entry.lastDmEventTs) return; + + entry.lastDmEventTs = createdAt; + const storageKey = `${STORAGE_KEYS_GLOBAL.LAST_DM_EVENT_TS}_${entry.nostrPubkey.slice(0, 16)}`; + + this.storage.set(storageKey, createdAt.toString()).catch(err => { + logger.debug('Mux', 'Failed to save last DM event timestamp:', err); + }); +} +``` + +`transport/MultiAddressTransportMux.ts` lines 983-990 (`updateSubscriptions` chat filter): + +```typescript +const chatFilter = new Filter(); +chatFilter.kinds = [EventKinds.GIFT_WRAP]; +chatFilter['#p'] = allPubkeys; +// NIP-17 gift wraps have created_at randomized ±2 days for privacy. +// Without this offset, ~50% of messages are silently dropped by the relay +// because their randomized timestamp lands before the `since` filter. +// Math.max(0, ...) prevents negative timestamps when globalDmSince is small. +chatFilter.since = Math.max(0, globalDmSince - NIP17_TIMESTAMP_RANDOMIZATION); +``` + +`transport/MultiAddressTransportMux.ts` lines 1718-1750 (`getAddressDmSince` — reads stored cursor on boot): + +```typescript +private async getAddressDmSince(entry: AddressEntry): Promise { + if (this.storage) { + const storageKey = `${STORAGE_KEYS_GLOBAL.LAST_DM_EVENT_TS}_${entry.nostrPubkey.slice(0, 16)}`; + try { + const stored = await this.storage.get(storageKey); + const parsed = stored ? parseInt(stored, 10) : NaN; + if (Number.isFinite(parsed)) { + entry.lastDmEventTs = parsed; + entry.fallbackDmSince = null; + return parsed; + } + // ... fallthrough to fallback or wall-clock now +``` + +The chat handler chain after line 1173 includes (per `routeGiftWrap` body): +- NIP-17 unwrap of `pm.content` (CPU only). +- `entry.adapter.dispatchMessage(message)` — **awaits** SwapModule's `handleIncomingDM`, which itself awaits **multiple `deps.resolve(counterpartyAddress)` calls** (see `modules/swap/SwapModule.ts` lines 2789, 2847, 2888). Each `resolve` issues a `queryEvents` against the relay for nametag binding events; the default query timeout is **60 seconds** (`NostrTransportProvider.DEFAULT_QUERY_TIMEOUT_MS = 60000`, line 2831). + +`modules/swap/SwapModule.ts` line 2789: + +```typescript +const counterpartyAddress = isPartyA ? manifest.party_b_address : manifest.party_a_address; +try { + const counterpartyPeer = await deps.resolve(counterpartyAddress); + if (counterpartyPeer) { + if (!counterpartyPeer.transportPubkey || counterpartyPeer.transportPubkey !== dm.senderPubkey) { + // ... reject silently + return; + } + } +``` + +The CLI flow in `sphere-cli-work/sphere-cli/src/legacy/legacy-cli.ts` line 885-927 (`ensureSync`): + +```typescript +async function ensureSync(sphere: Sphere, mode: 'nostr' | 'full'): Promise { + console.log('Syncing...'); + try { + await sphere.fetchPendingEvents(); + // Allow async DM handlers (swap proposal processing, invoice import, etc.) + // to complete before reading in-memory state. + await new Promise(resolve => setTimeout(resolve, 500)); + } catch { /* ... */ } + // ... +} +``` + +The CLI grants async handlers **only 500 ms** of grace. With nametag-resolve queries taking 200 ms - 7 s on a healthy relay (the same window cited in `NostrTransportProvider.ts` comments line 442 and in `DEFAULT_QUERY_TIMEOUT_MS`'s 60s setting), 500 ms is **insufficient** for the handler chain to persist the swap before `swap list` reads `swapModule.getSwaps()`. So in any iteration where the soak's 3 s loop kills the CLI before the handler completes, **the cursor is already advanced but the swap is never persisted**. + +#### Verdict: BLOCKING + +The cursor-advance-before-handler-completes ordering plus worst-case NIP-17 randomization is sufficient to permanently lose alice's event after any handler-incomplete iteration. The 172800 s buffer is exactly cancelled by the worst-case `created_at = T_pub - 172800` shift, leaving zero net safety margin. + +--- + +### M2: NIP-17 ±2-day randomization for wallet events — **UNLIKELY** + +The Mux subscribes with `chatFilter.kinds = [GIFT_WRAP]` for NIP-17 wraps (with the `-NIP17_TIMESTAMP_RANDOMIZATION` compensation) and with a separate `walletFilter.kinds = [DIRECT_MESSAGE, TOKEN_TRANSFER, PAYMENT_REQUEST, PAYMENT_REQUEST_RESPONSE]` for non-NIP-17 events (with NO compensation). The non-NIP-17 wallet events use the real publish time as `created_at`, so no buffer is needed. + +#### Evidence + +`transport/MultiAddressTransportMux.ts` lines 949-958: + +```typescript +const walletFilter = new Filter(); +walletFilter.kinds = [ + EVENT_KINDS.DIRECT_MESSAGE, + EVENT_KINDS.TOKEN_TRANSFER, + EVENT_KINDS.PAYMENT_REQUEST, + EVENT_KINDS.PAYMENT_REQUEST_RESPONSE, +]; +walletFilter['#p'] = allPubkeys; +walletFilter.since = globalSince; // <-- no -RANDOMIZATION offset +``` + +Swap proposals are routed through `CommunicationsModule.sendDM` → `transport.sendMessage` → `NIP17.createGiftWrap` (see `transport/NostrTransportProvider.ts` lines 789-820), which **always** writes kind 1059 GIFT_WRAP — so they flow through the chat filter, not the wallet filter. The wallet-filter asymmetry is appropriate for its intended kinds. + +#### Verdict: UNLIKELY + +The asymmetry exists but is correct: DIRECT_MESSAGE (kind 4) is documented as deprecated for DMs (see `transport/MultiAddressTransportMux.ts` line 1321 and `NostrTransportProvider.ts` line 2142), and TOKEN_TRANSFER / PAYMENT_REQUEST events are plain (non-randomized). M2 is the hypothesis in #473's body that I was specifically asked to walk; it does not apply to the swap-proposal path. + +--- + +### M3: Subscription armed AFTER `now`, race-window event lost — **UNLIKELY** + +The Mux opens its persistent subscription with `since = lastDmTs - 172800` (or `now - 172800` for a fresh wallet), NOT with `since = now`. The relay returns all stored events with `created_at >= since`, including those published in the window `[since, now_at_sub_open]`. As long as the relay persists the event durably (which #473 confirms — "same soak retried within the next minute passes cleanly" — the event IS on the relay), the open subscription's `since` filter will return it on the next REQ. + +#### Evidence + +`transport/MultiAddressTransportMux.ts` lines 887-1029 (`updateSubscriptions`). The `since` value is computed from persisted state, not from `Date.now()` at subscription open. The only race that M3 could describe is a missed event between `mux.connect()` (which DOES `updateSubscriptions` if the gate isn't suppressed) and a later `armSubscriptions()` call. But Sphere bootstrap explicitly **suppresses subscriptions before connect** (`Sphere.ts` line 4369) and **arms after modules load** (`Sphere.ts` line 7781), so the gate ensures the live subscription is open by the time the CLI calls `fetchPendingEvents`. + +The auto-arm in the `onMessage` registration path (line 832 in `NostrTransportProvider.ts`) handles backward-compat with consumers that don't call `armSubscriptions` explicitly. + +#### Verdict: UNLIKELY + +The relay's stored-event replay model means a subscription opened at time T sees events with `created_at >= since` regardless of when T is. The persistence of the event (confirmed by #473's "same soak retried passes cleanly") rules out lost events from this mechanism. + +--- + +### M4: EOSE delivered before relay finishes streaming backlog — **UNLIKELY in isolation** + +The Nostr protocol guarantees the relay sends all stored matching events BEFORE the `EOSE` notice. The SDK's one-shot fetch path (`NostrTransportProvider.fetchPendingEvents` line 2724, Mux `fetchPendingEvents` line 734) collects events into an array and only iterates after `settle()` fires on EOSE. So a "premature EOSE" would have to be a relay protocol violation. + +However, M4 has a **derivative** flavor that DOES contribute: **the LIVE chat subscription's `onEvent` callbacks are fire-and-forget from the NostrClient's perspective** — the Mux's `handleEvent` is `async`, but the client just calls it and continues. So when the *one-shot* subscription's EOSE fires (which is what `fetchPendingEvents` waits for), the *live* chat subscription may have already received the backlog event but its async `handleEvent` chain (NIP-17 unwrap → `routeGiftWrap` → `dispatchMessage` → SwapModule's network resolves) is **still running**. The CLI's 500 ms grace (`ensureSync` line 897) is far less than the worst-case handler chain time. + +This isn't strictly M4 — it's a **handler-completion race**. EOSE is not a handler-completion barrier. The fix in PR #465 made `dispatchMessage` await async handlers per the at-least-once invariant, but that only helps callers that **await `handleEvent` themselves** (which the one-shot `fetchPendingEvents` path does because it iterates events sequentially and awaits each `handleEvent`). The live subscription doesn't await its own callbacks. + +#### Evidence + +`transport/MultiAddressTransportMux.ts` line 962-981 (live wallet subscription `onEvent` is a plain callback wrapping `this.handleEvent(event)` — the Promise is discarded): + +```typescript +this.walletSubscriptionId = this.nostrClient.subscribe(walletFilter, { + onEvent: (event) => { + this.handleEvent({ // <-- async, NOT awaited by the client + id: event.id, ... + }); + }, + // ... +}); +``` + +Same pattern at line 992-1017 for the chat subscription. + +#### Verdict: UNLIKELY in pure-EOSE form; **contributes** in handler-completion form + +Combined with M1, the handler-completion race is what makes the "intermittent" character match: when alice's randomization happens to land low AND the receiver's CLI exits before the handler completes, the cursor moves but the swap doesn't. + +--- + +### M5: Relay state divergence between writes and reads — **INCONCLUSIVE** + +A Nostr relay can in principle accept an event on one connection but fail to serve it on another — quorum issues, transient indexing lag, replication delay. #473 reports that "same soak retried within the next minute passes cleanly," which suggests the relay DOES store the event durably (consistent with M1, where the event is on-relay but the *filter* excludes it). But this doesn't fully rule out relay-side issues — a brief indexing gap immediately after publish could cause the receiver's first subscription to miss the event, and the cursor-advance from any unrelated DM in that window would then permanently exclude it. + +#### Evidence + +No code-level evidence available. Would need relay-side logs to confirm or deny. + +#### Verdict: INCONCLUSIVE + +Cannot rule out from the SDK side; the M1-shaped fix below is robust against this case too, because the widened look-back buffer covers transient indexing lag. + +--- + +## Recommended fix (THE LOAD-BEARING SECTION) + +The single smallest fix that closes the #473 symptom is **two surgical changes inside `transport/MultiAddressTransportMux.ts`**, no API changes, no impact on other call sites. + +### Change 1: defer the chat cursor advance until after the handler completes + +**File:** `transport/MultiAddressTransportMux.ts` +**Function:** `routeGiftWrap` +**Lines:** 1162-1310 (the relevant edits cluster around line 1173 and at every `return` / fall-through point in the function body) + +**Current code (line 1162-1175):** + +```typescript + private async routeGiftWrap(event: NostrEvent): Promise { + for (const entry of this.addresses.values()) { + try { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const pm = NIP17.unwrap(event as any, entry.keyManager); + + // Successfully decrypted — route to this address. + // Persist DM timestamp after successful unwrap so failed decryptions + // do not advance the since filter and permanently skip events. + // Use real wall-clock time, NOT event.created_at — NIP-17 gift wraps + // randomize created_at by ±2 days for privacy, so it can be in the future. + this.updateLastDmEventTimestamp(entry, Math.floor(Date.now() / 1000)); + logger.debug('Mux', `Gift wrap decrypted by address ${entry.index}, sender: ${pm.senderPubkey?.slice(0, 16)}`); +``` + +**Proposed diff (conceptual — capture the timestamp once but defer the persist):** + +```diff +@@ transport/MultiAddressTransportMux.ts:1162-1175 @@ + private async routeGiftWrap(event: NostrEvent): Promise { + for (const entry of this.addresses.values()) { + try { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const pm = NIP17.unwrap(event as any, entry.keyManager); + +- // Successfully decrypted — route to this address. +- // Persist DM timestamp after successful unwrap so failed decryptions +- // do not advance the since filter and permanently skip events. +- // Use real wall-clock time, NOT event.created_at — NIP-17 gift wraps +- // randomize created_at by ±2 days for privacy, so it can be in the future. +- this.updateLastDmEventTimestamp(entry, Math.floor(Date.now() / 1000)); ++ // Issue #473 — capture the cursor candidate but DO NOT persist yet. ++ // We must not advance `lastDmEventTs` past `event.created_at`'s ++ // worst-case randomization shift (-172800 s) until the handler has ++ // observed the event, otherwise CLI processes that exit between ++ // unwrap and handler-completion permanently lose the event on the ++ // next boot's `since = lastDmTs - 172800` subscription filter. ++ const cursorCandidate = Math.floor(Date.now() / 1000); ++ // Track whether dispatch completed so we can advance the cursor in ++ // a single place at the end of the unwrap-success branch. ++ let dispatched = false; + logger.debug('Mux', `Gift wrap decrypted by address ${entry.index}, sender: ${pm.senderPubkey?.slice(0, 16)}`); +``` + +Then at the END of the unwrap-success branch — after `await entry.adapter.dispatchMessage(...)` (lines ~1211, ~1284, ~1304) — and before each `return`, add: + +```diff + await entry.adapter.dispatchMessage(message); ++ dispatched = true; + return; // Successfully routed, stop trying other addresses +``` + +Finally, at the bottom of the per-entry `try` block (just before the `} catch { continue; }` at line 1306-1309), centralize the cursor-advance: + +```diff + await entry.adapter.dispatchMessage(message); ++ dispatched = true; + return; // Successfully routed, stop trying other addresses + } catch { + // Decryption failed for this address — try next + continue; ++ } finally { ++ // Issue #473 — only advance the cursor if the dispatch completed. ++ // If the handler threw or the process is in shutdown, we leave ++ // `lastDmTs` unchanged so the next boot's chat subscription's ++ // `since` filter still includes alice's event. ++ if (dispatched) { ++ this.updateLastDmEventTimestamp(entry, cursorCandidate); ++ } + } + } +``` + +Note: a `try/finally` inside the `for (entry of …)` loop is the cleanest way to ensure the cursor-advance runs exactly once per successful dispatch. Alternative: hoist `let dispatched` outside the loop and move the advance after the loop. Either is acceptable; the diff above keeps per-entry locality. + +**Caveat:** "dispatched" here means `await dispatchMessage` returned without throwing. Per the at-least-once invariant work in PR #465, `dispatchMessage` already awaits all registered handlers. If a handler throws inside its own async body, `Promise.allSettled` (used by the adapter's dispatch chain) means the dispatch itself does NOT throw — so `dispatched = true` would still fire. That's the right semantics: we have done our best to deliver, and replaying the same event on next boot is wasteful (Mux dedup short-circuits). The real escape hatch is the next change. + +### Change 2: widen the chat-filter look-back buffer + +**File:** `transport/MultiAddressTransportMux.ts` +**Line:** 990 + +**Current code:** + +```typescript +chatFilter.since = Math.max(0, globalDmSince - NIP17_TIMESTAMP_RANDOMIZATION); +``` + +**Proposed diff:** + +```diff +@@ transport/MultiAddressTransportMux.ts:983-990 @@ + const chatFilter = new Filter(); + chatFilter.kinds = [EventKinds.GIFT_WRAP]; + chatFilter['#p'] = allPubkeys; + // NIP-17 gift wraps have created_at randomized ±2 days for privacy. + // Without this offset, ~50% of messages are silently dropped by the relay + // because their randomized timestamp lands before the `since` filter. + // Math.max(0, ...) prevents negative timestamps when globalDmSince is small. +- chatFilter.since = Math.max(0, globalDmSince - NIP17_TIMESTAMP_RANDOMIZATION); ++ // Issue #473 — DOUBLE the buffer (4 days instead of 2). Two-day buffer ++ // exactly cancels the worst-case NIP-17 randomization (created_at can be ++ // T_pub - 172800), leaving zero net margin when `lastDmTs` was advanced ++ // by a sibling DM that landed AFTER alice's publish but BEFORE alice's ++ // event was observed. Doubling the buffer gives the receiver up to 2 d ++ // of slack between the wall-clock cursor and the worst-case publish ++ // time — enough to cover (a) `lastDmTs` advancing past `T_pub` due to ++ // unrelated DMs, (b) CLI handler-exit-before-completion (the half-bug ++ // Change 1 closes belt-and-brace style), and (c) brief relay indexing ++ // lag (M5). Bounded re-delivery is absorbed by `processedEventIds` ++ // dedup (issue #275 — persistent dedup across process restarts). ++ chatFilter.since = Math.max(0, globalDmSince - 2 * NIP17_TIMESTAMP_RANDOMIZATION); +``` + +### Change 2b (mirror): apply the same widening to NostrTransportProvider + +**File:** `transport/NostrTransportProvider.ts` +**Line:** 3148 + +The original (non-mux) provider has the same buffer pattern in `subscribeToEvents`. To keep the fix internally consistent (tests cover this path), mirror Change 2: + +```diff +@@ transport/NostrTransportProvider.ts:3141-3148 @@ + const chatFilter = new Filter(); + chatFilter.kinds = [EventKinds.GIFT_WRAP]; + chatFilter['#p'] = [nostrPubkey]; + // NIP-17 gift wraps have created_at randomized ±2 days for privacy. +- // Without this offset, ~50% of messages are silently dropped by the relay +- // because their randomized timestamp lands before the `since` filter. +- // Math.max(0, ...) prevents negative timestamps when dmSince is small. +- chatFilter.since = Math.max(0, dmSince - TIMESTAMP_RANDOMIZATION); ++ // Issue #473 — see MultiAddressTransportMux:990 for full rationale. ++ chatFilter.since = Math.max(0, dmSince - 2 * TIMESTAMP_RANDOMIZATION); +``` + +### New constant? Not necessary. + +Both `NIP17_TIMESTAMP_RANDOMIZATION` (line 79 in the Mux) and `TIMESTAMP_RANDOMIZATION` (line 110 in NostrTransportProvider) are already named constants. Doubling them inline with a comment is more readable than introducing `CHAT_SINCE_BUFFER_MS`. The 2× factor is justified by the worst-case math in M1 (see "Why this matches" below). + +If a follow-up wants a tunable knob, the cleanest addition is: + +```typescript +// transport/MultiAddressTransportMux.ts (near line 79) +/** Multiplier applied to NIP-17 randomization when computing chat-filter + * `since`. Larger than 1 covers the worst-case window where the persisted + * cursor was advanced past the event's randomized `created_at` (issue #473). + * 1.0 → no extra margin (pre-#473 behavior). + * 2.0 → 2-day safety margin (current). + * Each step doubles the relay backlog returned on subscription open; dedup + * short-circuits the cost downstream. */ +const CHAT_SINCE_BUFFER_MULTIPLIER = 2; +``` + +Then `chatFilter.since = Math.max(0, globalDmSince - CHAT_SINCE_BUFFER_MULTIPLIER * NIP17_TIMESTAMP_RANDOMIZATION)`. Optional. + +### Tests to add + +#### Test 1 — `routeGiftWrap` does not advance the cursor when dispatch throws + +**File:** `tests/unit/transport/MultiAddressTransportMux.dispatch-await.test.ts` (extend existing) **or** new file `MultiAddressTransportMux.cursor-defer-473.test.ts`. + +```typescript +it('[#473] should NOT advance lastDmEventTs when the dispatch handler throws', async () => { + // Setup: mux with one address, a registered handler that rejects. + const persistedTimestamps: number[] = []; + const mockStorage = { + get: vi.fn().mockResolvedValue(null), + set: vi.fn().mockImplementation((key: string, value: string) => { + if (key.startsWith(STORAGE_KEYS_GLOBAL.LAST_DM_EVENT_TS)) { + persistedTimestamps.push(parseInt(value, 10)); + } + return Promise.resolve(); + }), + }; + const mux = createMuxWithStorage(mockStorage); + await mux.addAddress(0, identity, null); + await mux.armSubscriptions(); + + const adapter = mux.getAdapter(0); + adapter.onMessage(() => { throw new Error('handler failed'); }); + + // Simulate a gift wrap arriving via the live subscription's onEvent callback. + await mux.__test_dispatchGiftWrap(craftGiftWrap({ recipient: identity.transportPubkey })); + + // Cursor should NOT be persisted: handler threw before dispatch completed. + expect(persistedTimestamps).toEqual([]); +}); +``` + +#### Test 2 — `routeGiftWrap` advances the cursor after successful dispatch + +```typescript +it('[#473] should advance lastDmEventTs AFTER dispatchMessage resolves', async () => { + const handlerCompleted = new Deferred(); + const mockStorage = makeMockStorageWithPersistTracking(); + const mux = createMuxWithStorage(mockStorage); + await mux.addAddress(0, identity, null); + await mux.armSubscriptions(); + + const adapter = mux.getAdapter(0); + adapter.onMessage(async () => { + // Simulate a slow handler (e.g., SwapModule's resolve calls). + await new Promise((r) => setTimeout(r, 100)); + handlerCompleted.resolve(); + }); + + const beforeDispatch = Math.floor(Date.now() / 1000); + await mux.__test_dispatchGiftWrap(craftGiftWrap(...)); + await handlerCompleted.promise; + + expect(mockStorage.lastDmTimestamps).toHaveLength(1); + expect(mockStorage.lastDmTimestamps[0]).toBeGreaterThanOrEqual(beforeDispatch); +}); +``` + +#### Test 3 — chat-filter `since` uses 2 × randomization + +**File:** `tests/unit/transport/NostrTransportProvider.test.ts` (extend the existing test at line 724-749). + +```typescript +it('[#473] should apply 2× NIP-17 randomization buffer to chat since filter', async () => { + const mockStorage = { + get: vi.fn().mockImplementation((key: string) => { + if (key.startsWith('last_wallet_event_ts_')) return Promise.resolve('1700000000'); + if (key.startsWith('last_dm_event_ts_')) return Promise.resolve('1699999000'); + return Promise.resolve(null); + }), + set: vi.fn().mockResolvedValue(undefined), + }; + const provider = createProviderWithStorage(mockStorage); + setIdentity(provider); + await provider.connect(); + await provider.armSubscriptions(); + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(mockSubscribe).toHaveBeenCalledTimes(2); + const [chatFilterArg] = mockSubscribe.mock.calls[1]; + const chatFilter = chatFilterArg.toJSON(); + expect(chatFilter.kinds).toContain(1059); + + const TWO_DAYS = 2 * 24 * 60 * 60; + // Pre-#473: chatFilter.since === 1699999000 - TWO_DAYS. + // Post-#473: chatFilter.since === 1699999000 - 2 * TWO_DAYS. + expect(chatFilter.since).toBe(Math.max(0, 1699999000 - 2 * TWO_DAYS)); +}); +``` + +#### Test 4 — end-to-end cross-process simulation (regression for the soak) + +This is the trickiest test because it must simulate two CLI processes. Easiest path: use the existing `tests/integration/` shape and orchestrate two `Sphere` instances in the same Node process, but separate them temporally by tearing down the receiver between alice's publish and bob's check. Sketch: + +```typescript +it('[#473] receiver booted after sender exits sees the swap proposal on first poll', async () => { + // 1. Alice creates a Sphere, sends a swap proposal DM. + const alice = await Sphere.init({ ... }); + const proposal = await alice.swap.proposeSwap(deal, ...); + await alice.destroy(); + + // 2. Force alice's gift-wrap created_at to the worst-case low to maximize + // the chance of catching M1. (Requires test hook on NIP17.createGiftWrap.) + + // 3. Boot bob, expect to find the proposal on FIRST swap list call (no retry). + const bob = await Sphere.init({ ... }); + await ensureSyncForTest(bob, 'nostr'); + const swaps = bob.swap.getSwaps({ role: 'acceptor' }); + expect(swaps.map((s) => s.swapId)).toContain(proposal.swapId); + await bob.destroy(); +}); +``` + +#### Test 5 — pre-existing tests need a 2× update + +The existing test at line 748 hardcodes `chatFilter.since === Math.max(0, 1699999000 - TWO_DAYS)`. After the fix, this becomes `Math.max(0, 1699999000 - 2 * TWO_DAYS)`. Either update the existing assertion or add a separate "buffer doubled per #473" test (Test 3 above) and delete the obsolete assertion. + +### Regression risk + +- Change 1 (defer cursor advance) makes the chat-side cursor follow the same at-least-once invariant the TOKEN_TRANSFER path already follows (see `NostrTransportProvider.ts` lines 1782-1841): cursor advances only after the handler reports it has the event. The persistent dedup at line 1092 (`if (this.processedEventIds.has(event.id)) return`) short-circuits duplicates, so the worst case of re-replay on next boot is a no-op skip. No previously-working flow regresses, because previously the cursor was advanced eagerly; now it advances late, which is strictly safer. +- Change 2 (2× buffer) increases the backlog returned on subscription open by at most a 2 d wider window. Dedup absorbs the cost. The widened window includes more events but does NOT change which events get processed — it only changes which events get *filtered server-side*. No semantic change. + +--- + +## Operator workaround (until the fix lands) + +For #474 trader-roundtrip and any cross-process flow that depends on a DM the sender published in a prior process: + +1. **Always run `sphere payments sync` before any DM-dependent query.** `sync` calls `ensureSync(sphere, 'nostr')` which calls `sphere.fetchPendingEvents()` — at minimum this re-fetches the relay backlog through the OG `NostrTransportProvider.fetchPendingEvents` path (line 2754: `walletFilter.since = now - 86400 - 172800` — uses the wider 3-day window). If the live Mux subscription happens to miss the event due to M1, the one-shot fetch may catch it via its wider window. (But note: this one-shot dispatches handlers on the OG provider, NOT on the Mux adapter — see "What this does NOT address" below.) + +2. **Replace the soak's `swap list` with `payments sync` + `swap list` + retry-with-longer-gap.** Today the soak does `sphere payments sync` → `sphere swap list --role acceptor` → sleep 3 s. Change the retry budget to **at least 30 s gap with up to 5 retries** rather than 3 s with 30 retries. This lets the live subscription's worst-case 5 s EOSE timeout plus handler completion (up to 7 s on a degraded relay per `NostrTransportProvider.ts:442`) settle before the next poll. Per-iteration budget should be `~15 s`, not 3 s. + +3. **Add an explicit `sphere swap wait ` step on bob's side** if the CLI exposes it for the `proposed` state. (The current CLI uses `sphere swap wait --state completed`; if a `--state proposed` mode exists or can be added, it's the right primitive here.) + +4. **Disambiguate dead iterations from successful-but-incomplete iterations.** Today the soak treats `swap list` returning empty as "no proposal" — but the proposal may be IN the Mux's `handleEvent` chain and not yet persisted. Add a `sleep 2` after `payments sync` (instead of 0) before reading the swap list, to give the handler chain time to land. + +5. **Pre-warm bob's wallet** in the soak setup phase by sending bob a benign DM from a third party (e.g., the orchestrator). This advances bob's `lastDmTs` only to the orchestrator-DM's processing time, and the chat filter's existing 2-day buffer is enough as long as that prewarming happens BEFORE alice publishes. (This works around M1's worst-case window: as long as no DM advances `lastDmTs` between alice's publish and the soak's first bob-iteration, the existing buffer suffices.) + +6. **Avoid relay congestion windows.** The bug is intermittent because alice's NIP-17 randomization is uniform — most of the time the randomized `created_at` lands within a 1-day window of `T_pub`, in which case the existing 2-day buffer is enough. The failure rate is roughly the probability that the randomization lands in the **last 172800 s of its range** AND bob's cursor advanced past `T_pub`. Empirically rare but non-zero. Increasing the soak iteration count to absorb the flake is not a real fix. + +--- + +## Why this matches the #473 symptom + +The proposed Change 1 (defer cursor advance) directly closes M1 by enforcing the at-least-once invariant for chat events: the cursor only advances after the handler has been given a fair chance to persist the swap. When bob's CLI exits between unwrap and handler-completion, `lastDmTs` remains at its prior value — so the next boot's `since` filter still includes alice's event, and bob's NEXT iteration catches it. + +Change 2 (2× buffer) is the belt-and-brace defense. Even if some future code path advances `lastDmTs` early (e.g., a self-wrap replay path that we want to count for cursor purposes), the doubled buffer covers the entire worst-case randomization range. Specifically: + +- Without buffer-doubling, an event is visible iff `event.created_at >= lastDmTs - 172800`, i.e., `T_pub - 172800 >= lastDmTs - 172800` (worst-case randomization), i.e., `T_pub >= lastDmTs`. **Zero margin** when `lastDmTs` was set by ANY event processed after `T_pub`. +- With 2× buffer, an event is visible iff `event.created_at >= lastDmTs - 345600`, i.e., `T_pub - 172800 >= lastDmTs - 345600`, i.e., `T_pub + 172800 >= lastDmTs`. **2-day margin** for `lastDmTs` to advance past `T_pub` before alice's event is filtered out. + +In the soak scenario, the gap between alice's publish and bob's processing of any unrelated DM is on the order of seconds to minutes — well inside the 2-day margin. So Change 2 alone would close the symptom for typical traffic, and Change 1 alone would close it for the specific "handler interrupted" case. Both together leave no residual gap from M1 and M5. + +--- + +## What this does NOT address + +- **The Sphere.fetchPendingEvents → OG transport architectural mismatch.** When Sphere is in multi-address mode (always, per `Sphere.ts:7277`), modules register their handlers on the **Mux adapter** but `sphere.fetchPendingEvents()` (`Sphere.ts:2783`) calls **OG `NostrTransportProvider.fetchPendingEvents()`** which dispatches to OG's handlers, not the Mux adapter's. The OG transport buffers messages it can't deliver (`pendingMessages`, line 2287), so for backward-compat the events aren't permanently lost — but they don't reach SwapModule via this path. SwapModule depends entirely on the LIVE Mux subscription. This is a separate fix worth investigating in its own right (PR for #473 should NOT touch it; it's out of scope and would broaden the diff considerably). Track separately as a follow-up. + +- **The 500 ms grace in `ensureSync` (CLI line 897) is still too short.** Even after the SDK-side fix, the CLI can call `getSwaps` before the handler chain has persisted. The fix above ensures the event is **NOT permanently lost** — but bob may still need a retry. The soak's 3 s loop is sufficient post-fix, but a 1 s grace (instead of 500 ms) would further reduce the per-iteration miss probability. This is a sphere-cli change, not a sphere-sdk change. + +- **M5 (relay-level state divergence).** If the relay accepts an event but fails to serve it on subsequent reads, no SDK-side fix helps. Quorum-of-N relay reads would address it but that's a much larger architectural change. Tracked as residual. + +- **In-process delivery to non-handler-registered modules.** PR #465 (sphere-sdk#464) already fixed the `dispatchMessage` → `Promise.allSettled` await chain. This investigation does NOT propose changes there; PR #465's fix remains correct and complementary. + +- **NostrTransportProvider's own `handleGiftWrap` path at line 2148-2192.** The OG provider has the same "advance cursor before dispatch" pattern as the Mux (line 2162: `this.updateLastDmEventTimestamp(Math.floor(Date.now() / 1000))` runs BEFORE `messageHandlers` are called). For full safety, the same Change-1-style fix should be applied to NostrTransportProvider.handleGiftWrap too — but per the architectural mismatch above, the OG path mostly isn't reached for swap DMs in production today. Apply if the same incident surfaces from a non-mux consumer; otherwise defer. + +--- + +## Test plan + +Before claiming closure on #473, the author of the fix should run: + +- [ ] `npm run typecheck` clean. +- [ ] `npm run lint` clean. +- [ ] `npx vitest run tests/unit/transport/MultiAddressTransportMux.dispatch-await.test.ts` (existing) passes unchanged. +- [ ] `npx vitest run tests/unit/transport/NostrTransportProvider.test.ts` passes after updating the `chatFilter.since` assertion at line 748 to `2 * TWO_DAYS`. +- [ ] New tests (1, 2, 3, and ideally 4 above) pass. +- [ ] `npm run test:run` passes (full suite — 2539 tests at last count). +- [ ] Soak: run `manual-test-swap-roundtrip.sh` 20 times back-to-back against `dev` network. Expected: 0 `proposal-ingest-timeout` failures. (Pre-fix baseline: 1-3 timeouts per 20 runs.) +- [ ] Soak: run `manual-test-swap-roundtrip.sh` 20 times back-to-back against `testnet` network. Same expectation. +- [ ] Trader-roundtrip #474 G2 soak: run end-to-end at least 10 times against `testnet`. Expected: 0 hop failures attributable to controller-CLI ↔ tenant DM gap. + +Note: the soak script reports `bob-swap-list-A.log` per iteration. Pre-fix, when failure happens, the log shows "No swaps found." for ALL 30 iterations. Post-fix, even in the worst case, bob should see the proposal within 2-3 iterations (the iteration grace + the relay's subscription replay latency). + +--- + +## Cross-references + +- **Issue #473** — Cross-process Nostr DM flakiness (the bug under investigation). +- **PR #465** (sphere-sdk#464) — `MuxAdapter dispatch await async handler completion`. In-process handler-completion fix; complementary to #473's cross-process fix, NOT a substitute. +- **PR #461** (sphere-sdk#447) — `include terminal swaps in resolveSwapId/getSwaps`. Unrelated to this investigation; mentioned only as recent-history context. +- **PR #459** (sphere-sdk#457) — `fail fast when counterparty transport pubkey missing`. SwapModule sender-side fix; unrelated to receiver-side cursor. +- **PR #423** — Handler-readiness gate (NostrTransportProvider `armSubscriptions`). Already in place; this investigation does NOT change the arm semantics. +- **PR #442** — Subscription gate (Mux `suppressSubscriptions` / `armSubscriptions`). Already in place; this investigation does NOT change the arm semantics. +- **Issue #275** — Persistent dedup via `processedEventIds`. Already in place; the proposed fix relies on this to absorb harmless re-delivery from the widened buffer. +- **Issue #166 / #97** — OUTBOX/SENT crash-safety follow-ups, including the at-least-once invariant for TOKEN_TRANSFER. This investigation extends the same invariant to the chat path. +- **Issue #474** — Trader-roundtrip G2 soak. Hard dependency on #473 closure. +- **Memory entry `project_cross_process_nostr_gap.md`** (2026-05-22): "CLI sender→exit→receiver flow loses Nostr-delivered tokens despite event being on relay; e2e tests use same-process so don't catch it." Same shape as #473 but for TOKEN_TRANSFER events on the wallet filter. Worth checking whether the wallet-filter side has a sibling of M1 (in-process processing-time-vs-publish-time gap). If TOKEN_TRANSFER also uses NIP-17 wrap on some path, the buffer fix here should be ported. +- **`manual-test-swap-roundtrip.sh`** lines 280-298 — the soak loop that surfaces #473. +- **`CLAUDE.md`** — see "Event Timestamp Persistence" and "Transport vs Chain Pubkeys" sections for background. diff --git a/docs/uxf/PROTOCOL-SPEC-DRIFT-474.md b/docs/uxf/PROTOCOL-SPEC-DRIFT-474.md new file mode 100644 index 00000000..c2c5f3aa --- /dev/null +++ b/docs/uxf/PROTOCOL-SPEC-DRIFT-474.md @@ -0,0 +1,287 @@ +# Trader Protocol Spec Drift Audit (issue #474 G4) + +**Spec audited:** `/home/vrogojin/trader-service/docs/protocol-spec.md` v0.1 (Draft, dated 2026-04-03), with §2 marked v0.2 internally (Appendix D notes a 2026-04-03 revision replacing NIP-29 with MarketModule). +**Implementation audited:** `trader-service` at HEAD on `main`, files `src/trader/*.ts` (`acp-types.ts`, `negotiation-handler.ts`, `intent-engine.ts`, `swap-executor.ts`, `trader-command-handler.ts`, `utils.ts`, `types.ts`, `main.ts`), and the controller surface in `sphere-cli-work/sphere-cli/src/trader/trader-commands.ts`. +**Audit date:** 2026-06-10 +**Auditor's verdict:** **DRIFT BLOCKS #474 G2 SOAK** — the rate/volume type drift (D1) is load-bearing for the soak script's unit choice, the human-friendly-float design intent (D-NEW) is also unaddressed, and the CLI surface uses parameter names that the trader doesn't recognize (D2c). Several further drifts (envelope-signature input, deal_id field set, FAILED reason codes, error-code names) are tractable but should be fixed in the spec before this becomes a public reference. + +## Summary + +The implementation is largely faithful to the spec at the **state-machine** level (intent and deal lifecycles match the §6.1/§6.2 tables modulo a couple of pragmatic widenings), the **8-criterion matching rules** (§5.1) are all implemented in `intent-engine.ts`, and the §5.7 lower-pubkey-proposer election is enforced with a 45 s yield-timeout fallback. The privacy/security envelope (anti-replay window, clock-skew, dangerous-key rejection, dedup window of 600 s / 10 000 entries, max active intents, max-concurrent-swaps) all match the documented numbers. + +Where the spec and implementation **disagree** is on the **wire-encoding of monetary values** and on a few small structural details. The most consequential disagreement is the rate/volume type drift (§2.4 says `number`; the implementation has always been `bigint` strings end-to-end, and that's the only encoding the deployed v0.1 image accepts). Other notable drifts are: the NP envelope signature input formula in §3.4 is wrong (spec says `sha256hex(deal_id+":"+msg_id+":"+type)`; implementation hashes canonical JSON of the whole envelope-minus-signature); the deal_id derivation in §3.5 omits four fields the implementation actually hashes (proposer_address, acceptor_address, deposit_timeout_sec, proposer_direction); the spec's error-code names (`INTENT_NOT_FOUND`, `MAX_INTENTS_REACHED`, `INVALID_ADDRESS`, `TRANSFER_FAILED`, `WITHDRAWAL_LOCKED`) don't match the names the handler returns (`NOT_FOUND`, `LIMIT_EXCEEDED`, `INVALID_PARAM`, `WITHDRAW_FAILED`). + +The third-party concern surfaced by the coordinator update — that the **CLI surface should accept human-friendly floats** (`--rate-min 0.08`) and **convert to bigint smallest-units** via a token-registry decimals lookup before sending — is **neither in the spec nor in the implementation today** and surfaces as a three-way drift between design intent, the spec, and the code. The CLI today requires the operator to type fully-scaled bigint strings (`--rate-min 80000000000000000` for 0.08 UCT at 18 decimals), which is a soak-UX hazard. + +## Findings + +### FINDING D1 — Rate/volume types: float-in-spec vs bigint-in-code (BLOCKS SOAK UNIT CHOICE) + +- **Spec:** `protocol-spec.md:139-142` and `:191-195` declares `rate_min/rate_max/volume_min/volume_max` as `number` (JS float). `protocol-spec.md:399-402` repeats this in the canonical TypeScript interface. §7.3 line `1341` writes `assert(rate_min > 0)` style assertions against `number` values. +- **Code (ACP wire shape):** `src/trader/acp-types.ts:20-23` declares `rate_min: string`, `rate_max: string`, `volume_min: string`, `volume_max: string` on `CreateIntentParams`. `acp-types.ts:36-39` does the same for `CreateIntentResult`. `acp-types.ts:74-78` and `:105-106` do the same for `IntentSummary` and `DealSummary`. +- **Code (handler parse):** `src/trader/trader-command-handler.ts:370-378` parses incoming `rate_min/rate_max/volume_min/volume_max` via `safeParseBigint`. `safeParseBigint` at `:120-131` rejects anything that isn't `/^-?\d+$/`, so a float literal like `"0.5"` is rejected as `INVALID_PARAM`. +- **Code (canonical domain shape):** `src/trader/types.ts:72-75` declares the canonical `TradingIntent.rate_min/rate_max/volume_min/volume_max` as `bigint`. `DealTerms.rate` (`types.ts:108-109`) is `bigint`. `intent-engine.ts:832-835` parses params via `BigInt(params.rate_min)`, and `utils.ts:182-186` does the same in `validateIntentParams`. +- **Code (CLI surface):** `sphere-cli-work/sphere-cli/src/trader/trader-commands.ts:386-389` declares `--rate-min `, `--rate-max `, `--volume-min `, `--volume-max ` and forwards the literal strings without conversion (`trader-commands.ts:224-231`). +- **Impact:** Soak operators don't know whether to encode rates as decimal numbers or smallest-unit bigint strings. The actual deployed v0.1 image accepts ONLY bigint strings, so operators MUST use that — but the spec docs read like floats are the wire format. `utils.ts:182-189` catches the `BigInt()` throw and returns the generic `"rate and volume parameters must be valid integer strings"` — operators reading the spec will burn time trying decimals before discovering the actual contract. +- **Recommendation:** patch the spec §2.4 to declare these as bigint strings (canonical JSON tolerates `"42000000000000000000"` strings). Specifically: + - Change the TypeScript interface block at `protocol-spec.md:130-148` to use `string` (with a doc comment `// stringified bigint, smallest units`). + - Change the constraint table at `:191-195` from "Positive finite number" to "Positive bigint (string-encoded, smallest units)". + - Update §5.2's `floor((overlap_min + overlap_max) / 2)` to clarify it's bigint integer division (the implementation in `intent-engine.ts:896` does `Number((rateMin + rateMax) / 2n)` for the MarketModule midpoint, but the on-the-wire midpoint stays bigint). + - Update §7.3 assertions to use bigint comparison operators (`> 0n`). + +### FINDING D-NEW — CLI should accept human-friendly floats; spec/code only see bigints (HIGH; design intent unspoken) + +- **Design intent (from owner):** At the CLI surface, operators work with **human-friendly float numbers** (`--rate-min 0.08 --rate-max 0.12`, `--volume-min 50 --volume-max 50`). The CLI is responsible for converting these to bigint smallest-units internally via a token-registry decimals lookup. The ACP wire format SHOULD remain bigint-string so canonical JSON doesn't lose precision. +- **Spec today:** `protocol-spec.md:139-142, :694-705` describes `number` end-to-end — float at the wire too. No mention of CLI-side conversion. No mention of asset-decimals. +- **Code today (CLI):** `sphere-cli/src/trader/trader-commands.ts:386-389` declares `--rate-min ` and forwards the literal string. `:224-231` builds the ACP payload with `rate_min: opts.rateMin` directly — no conversion, no decimals lookup, no validation that the input is bigint-shaped. +- **Code today (trader):** `trader-command-handler.ts:370-378` enforces bigint-string at the ACP boundary; the trader has no float path. +- **Impact:** Three-way drift between (a) design intent (float at CLI, bigint at wire), (b) spec (float end-to-end), (c) implementation (bigint end-to-end including CLI). An operator who types `sphere trader create-intent --rate-min 0.08` today gets a CLI-side parser error or a downstream `INVALID_PARAM` from the trader. The soak script must currently spell out `80000000000000000` and rely on the operator/script-author to know UCT is 18-decimal. +- **Recommendation (multi-step, do NOT apply now):** + 1. **CLI:** accept floats; add an optional `--rate-min-bigint` fallback for power users. Convert via a token-registry decimals lookup (e.g. a new `MarketModule.decimalsFor(asset)` or extending `TokenRegistry`). For each pair, the conversion is `BigInt(Math.round(floatValue * 10 ** decimals))` with explicit overflow guard. + 2. **ACP wire:** stays bigint-string (matches D1's recommendation). + 3. **Spec §2.4 / §4.2:** patch to declare wire as bigint-string. Add a §2.4-bis subsection: "Recommended CLI UX — accept floats with decimal-registry-based conversion". + 4. **CLI validation:** also catch silly values pre-flight — non-finite floats, negatives, more decimal digits than the asset supports. + +### FINDING D2 — NP envelope signature input formula is wrong in spec + +- **Spec:** `protocol-spec.md:469` declares `signature: string; // ECDSA over sha256hex(deal_id + ":" + msg_id + ":" + type)`. +- **Code:** `negotiation-handler.ts:561-563` and `:815-823` compute the signature input as `sha256hex(canonicalJson(envelope-minus-signature))`. That is, the signature covers EVERY field of the envelope (np_version, msg_id, deal_id, sender_pubkey, type, ts_ms, payload), not just three of them. +- **Impact:** A spec-conformant implementation built off `protocol-spec.md` would sign only three fields and the deployed v0.1 trader would reject every message it sent (signature verification at `:815-823` recomputes the canonical-JSON hash and would fail). The implementation's choice is also more secure — the formula in the spec lets a MITM tamper with `payload` (e.g. `proposer_swap_address`) while keeping the original signature valid, which the implementation's docstring at `:553-560` explicitly flags as a known-bad pattern. +- **Recommendation:** patch §3.4 line 469 to `// ECDSA over sha256hex(canonicalJson(envelope-minus-signature))`. Add a "Rationale: binds every field, prevents payload-substitution" sentence so a future implementor knows why the wider commitment is required. + +### FINDING D3 — Deal ID derivation omits four fields the implementation hashes + +- **Spec:** `protocol-spec.md:487-500` declares deal_id derivation includes the field set `{proposer_pubkey, acceptor_pubkey, proposer_intent_id, acceptor_intent_id, base_asset, quote_asset, rate, volume, escrow_address, created_ms}`. +- **Code:** `negotiation-handler.ts:531-551` (`computeDealId`) hashes the field set `{acceptor_intent_id, acceptor_pubkey, base_asset, created_ms, deposit_timeout_sec, escrow_address, proposer_address, acceptor_address, proposer_direction, proposer_intent_id, proposer_pubkey, quote_asset, rate, volume}` — i.e. the spec's 10 fields PLUS `proposer_address`, `acceptor_address`, `deposit_timeout_sec`, and `proposer_direction`. +- **Impact:** A spec-conformant agent computes a DIFFERENT deal_id than the deployed trader for the same negotiated terms. Cross-implementation interoperability is impossible until they agree. The extra fields in the implementation are good additions — `deposit_timeout_sec` is a money-relevant negotiated value, and `proposer_direction` flips who-deposits-what — but the spec needs to record them. Without `proposer_direction` in the hash, an attacker could swap who-sells-what and the deal_id would be unchanged. +- **Recommendation:** patch §3.5 to add the four missing fields to the canonical JSON input. Document the rationale next to each: addresses bind the on-chain destinations; deposit_timeout_sec binds the funds-at-risk window; proposer_direction prevents who-sells-what flip attacks. + +### FINDING D4 — DealTerms interface omits four fields the implementation carries + +Related to D3 but distinct (since the spec also publishes a `DealTerms` TypeScript interface that doesn't include the extra fields). + +- **Spec:** `protocol-spec.md:505-518, 626-638` declares `DealTerms` with 11 fields including no addresses, no proposer_direction, no `deal_id` at all. +- **Code:** `types.ts:98-114` declares `DealTerms` with 16 fields: spec's 11 PLUS `deal_id`, `proposer_address`, `acceptor_address`, `proposer_direction`. +- **Recommendation:** patch §3.6 to add `deal_id` (the content-addressed ID of the deal, lowercase 64-hex), `proposer_address`, `acceptor_address` (Nostr DM destination addresses, max 256 chars), and `proposer_direction` (`'buy' | 'sell'`). + +### FINDING D5 — ACP command error codes don't match spec names + +The spec publishes a closed set of error codes in §4 and Appendix B.1. The implementation returns DIFFERENT names. + +- **CANCEL_INTENT (§4.3 spec line 754-757):** spec promises `INTENT_NOT_FOUND` / `INTENT_NOT_ACTIVE` / `DEAL_IN_PROGRESS`. Code at `trader-command-handler.ts:479` returns `NOT_FOUND` (not `INTENT_NOT_FOUND`). `INTENT_NOT_ACTIVE` is never returned. `DEAL_IN_PROGRESS` at `:490` matches. +- **CREATE_INTENT (§4.2 spec line 724-728):** spec promises `INVALID_PARAM` / `ASSET_UNKNOWN` / `INSUFFICIENT_BALANCE` / `MAX_INTENTS_REACHED`. Code at `:434` returns `LIMIT_EXCEEDED` instead of `MAX_INTENTS_REACHED`. `ASSET_UNKNOWN` and `INSUFFICIENT_BALANCE` are not returned by `handleCreateIntent` (they're only reachable in WITHDRAW_TOKEN). +- **WITHDRAW_TOKEN (§4.9 spec line 958-962):** spec promises `INSUFFICIENT_BALANCE` / `INVALID_ADDRESS` / `WITHDRAWAL_BLOCKED` / `TRANSFER_FAILED`. Code at `:716` returns `INVALID_PARAM` (NOT `INVALID_ADDRESS`); at `:728` returns `INSUFFICIENT_BALANCE` (matches); at `:761` returns `WITHDRAW_FAILED` (NOT `TRANSFER_FAILED`); `WITHDRAWAL_BLOCKED` / `WITHDRAWAL_LOCKED` (Appendix B.1 line 1604) is never returned. +- **Generic INTERNAL_ERROR:** spec never lists `INTERNAL_ERROR` but the handler returns it at `:462, 511, 540, 569, 624, 686, 868`. (This is fine — it's the canonical fallback — but Appendix B.1 should list it.) +- **Recommendation:** decide per-code whether to rename the spec or rename the code. Recommend renaming the **spec** to match implementation (the codes are already in the deployed trader and are observable behavior). Specifically: + - §4.3: `INTENT_NOT_FOUND` → `NOT_FOUND`. + - §4.2: `MAX_INTENTS_REACHED` → `LIMIT_EXCEEDED`. + - §4.9: `INVALID_ADDRESS` → `INVALID_PARAM` (`to_address must be ...`), `TRANSFER_FAILED` → `WITHDRAW_FAILED`. + - Appendix B.1: add `INTERNAL_ERROR`, remove `WITHDRAWAL_LOCKED` / `INVALID_TXF` / `PROOF_INVALID` / `ASSET_MISMATCH` / `TRANSFER_NOT_TO_AGENT` (none of these are returned by the trader). + +### FINDING D6 — `INTENT_NOT_ACTIVE` is undocumented behavior + +- **Spec:** `protocol-spec.md:756` declares `INTENT_NOT_ACTIVE` is returned when an intent is already filled/cancelled/expired. +- **Code:** `trader-command-handler.ts:468-513` has NO `INTENT_NOT_ACTIVE` branch. `intentEngine.cancelIntent` at `intent-engine.ts:944-978` throws "Cannot cancel intent in terminal state" when called on a terminal intent; the command handler at `:511` wraps this in `INTERNAL_ERROR`. +- **Impact:** an operator who tries to cancel an already-filled intent gets a generic `INTERNAL_ERROR` instead of the documented `INTENT_NOT_ACTIVE`. Not load-bearing for the soak but corrosive to debugging. +- **Recommendation:** code fix (in trader-service, NOT here) — `cancelIntent` should pre-check terminal state and return `INTENT_NOT_ACTIVE`; OR drop `INTENT_NOT_ACTIVE` from §4.3 / Appendix B.1 and rely on `INTERNAL_ERROR`. Recommend the former. + +### FINDING D7 — `SetStrategyResult.strategy` echoes the partial input, not the merged full strategy + +- **Spec:** `protocol-spec.md:857-859` says `strategy: SetStrategyParams; // echoes back the full merged strategy`. +- **Code:** `trader-command-handler.ts:615-617` sets `result.strategy = strategyParams` — the partial input the operator supplied, NOT the full merged result. The merge result is computed at `:597-607` (the `merged` variable) but never returned. +- **Impact:** operator can't verify the merged state without a follow-up call. Not security-sensitive but breaks the implicit "set returns the new full state" contract. +- **Recommendation:** code fix — set `result.strategy = merged`. Cheap and contained. + +### FINDING D8 — CLI SET_STRATEGY param names don't match trader expectations + +- **Spec / trader:** `protocol-spec.md:841-850` and `trader-command-handler.ts:579-589` expect param names `auto_match`, `auto_negotiate`, `max_concurrent_swaps`, `max_active_intents`, `min_search_score`, `scan_interval_ms`, `market_api_url`, `trusted_escrows`, `blocked_counterparties`. +- **Code (CLI):** `sphere-cli-work/sphere-cli/src/trader/trader-commands.ts:340-352` sends `rate_strategy` (not in spec or trader), `max_concurrent_negotiations` (trader expects `max_concurrent_swaps`), and `trusted_escrows` (matches). +- **Impact:** `sphere trader set-strategy --max-concurrent N` SILENTLY DOES NOTHING — the trader receives an unknown param and the existing strategy stays unchanged. `--rate-strategy aggressive` similarly disappears. Only `--trusted-escrows` actually takes effect. +- **Recommendation:** code fix in sphere-cli (NOT here): rename to `max_concurrent_swaps` and drop `rate_strategy` (or add a `rate_strategy` field on the trader-side strategy). This is a CLI bug, not a spec bug. + +### FINDING D9 — LIST_INTENTS / LIST_SWAPS use `filter`/`state` interchangeably + +- **Spec:** `protocol-spec.md:767-771, 803-807` declares the param is `filter` (not `state`). Values: `active`, `filled`, `cancelled`, `expired`, `all` for intents; `active`, `completed`, `failed`, `all` for swaps. +- **Code (trader):** `trader-command-handler.ts:521, 550` reads `params['filter']`. Matches spec. +- **Code (CLI):** `sphere-cli/src/trader/trader-commands.ts:283, 301` sends `state` (NOT `filter`). The trader's `matchesIntentFilter` falls through `filter === undefined` → `return true` (`trader-command-handler.ts:202`), so the CLI's `--state filled` request actually returns ALL intents, not just filled. +- **Impact:** another silent CLI bug — operator-facing filter knob is ignored. Soak operator using `sphere trader list-intents --state filled` sees confusing output. +- **Recommendation:** code fix in sphere-cli: rename `state` → `filter`. Trivially safe. + +### FINDING D10 — TIP_VERSION = "0.2" declared by spec but unused in code + +- **Spec:** `protocol-spec.md:105, 382` declares `TIP_VERSION = "0.2"`. +- **Code:** No reference anywhere in `src/trader/*.ts` to `TIP_VERSION`. There is no `tip_version` field on any envelope (TIP-0 is just a thin wrapper over MarketModule's HTTP API and the spec admits "There are no explicit wire-format message types" at line 121). +- **Impact:** harmless today (MarketModule postIntent doesn't carry the TIP version), but a future TIP-1 would have no rollout vector. The spec's claim that the version is meaningful is misleading. +- **Recommendation:** spec patch — either delete the `TIP_VERSION` constant declaration, or add a §2.x noting that "the TIP version is currently an internal compatibility marker; it is not transmitted over the wire because TIP-0 piggybacks the unversioned MarketModule HTTP API. A future TIP-1 will be signaled via an explicit version field on PostIntentRequest." + +### FINDING D11 — IntentSummary spec includes `volume_min`, the result interface does not + +- **Spec:** `protocol-spec.md:781-794` declares `IntentSummary` with 12 fields including `rate_min`, `rate_max`, `volume_max`, `volume_filled` — but NOT `volume_min`. +- **Code:** `acp-types.ts:69-83` declares `IntentSummary` with `volume_min` AND `volume_max` AND `volume_filled`. `trader-command-handler.ts:151-167` (`toIntentSummary`) emits all three. +- **Impact:** spec under-documents the response shape. Soak script consuming `list-intents` output gets a `volume_min` field that isn't in the spec. +- **Recommendation:** spec patch — add `volume_min: string` to §4.4 line ~787. Trivial. + +### FINDING D12 — DealSummary error_code is undocumented + +- **Spec:** `protocol-spec.md:819-831` declares `DealSummary` with 10 fields. No `error_code`. +- **Code:** `acp-types.ts:100-117` adds optional `error_code?: string` carrying the FAILED-state failure reason. `trader-command-handler.ts:192-193` (`toDealSummary`) emits it when present. +- **Impact:** an operator reading the spec to write a list-deals consumer wouldn't know how to surface failure reasons. The set of values it carries (`EXECUTION_TIMEOUT`, `ESCROW_UNREACHABLE`, `INVALID_ESCROW`, `PAYOUT_UNVERIFIED`, `PROPOSE_SWAP_FAILED: ...`) is documented in `types.ts:120-139` but invisible from the spec. +- **Recommendation:** spec patch — add `error_code?: string` to §4.5 `DealSummary` (line ~830), with a footnote listing the canonical values. Reference Appendix B.3 (which DOES list some of these in the spec). + +### FINDING D13 — Deal failure reason codes in Appendix B.3 are incomplete + +- **Spec:** `protocol-spec.md:1622-1628` lists `DEPOSIT_TIMEOUT`, `ESCROW_REJECTED`, `ESCROW_UNREACHABLE`, `COUNTERPARTY_UNRESPONSIVE`, `NETWORK_ERROR`, `INTERNAL_ERROR`. +- **Code:** the actually-emitted values are `EXECUTION_TIMEOUT` (`swap-executor.ts:402`, `:398` variant), `ESCROW_UNREACHABLE` (`types.ts:129`), `INVALID_ESCROW` (`types.ts:130`), `PAYOUT_UNVERIFIED` (`trader-main.ts:646, 666`), `PROPOSE_SWAP_FAILED: ` (`swap-executor.ts:518`), `EXECUTION_TIMEOUT_REJECT_FAILED: ` (`swap-executor.ts:398`), `MISSING_COUNTERPARTY_PUBKEY` (`main.ts:1281`), `PROTOCOL_VERSION_TOO_OLD` (`main.ts:1260`). +- **Impact:** spec promises codes that never appear; code emits codes the spec doesn't list. Operators triaging failures from logs see codes they can't look up. +- **Recommendation:** spec patch — replace Appendix B.3 with the actual emitted set: `EXECUTION_TIMEOUT`, `ESCROW_UNREACHABLE`, `INVALID_ESCROW`, `PAYOUT_UNVERIFIED`, `PROPOSE_SWAP_FAILED`, `MISSING_COUNTERPARTY_PUBKEY`, `PROTOCOL_VERSION_TOO_OLD`. Note that `PROPOSE_SWAP_FAILED` and `EXECUTION_TIMEOUT_REJECT_FAILED` carry a colon-suffix message tail. + +### FINDING D14 — `np.reject_deal` reason_code set is wider than spec promises + +- **Spec:** `protocol-spec.md:568-577, 614-622` declares 8 reason codes: `RATE_UNACCEPTABLE`, `VOLUME_UNACCEPTABLE`, `ESCROW_UNACCEPTABLE`, `TIMEOUT_UNACCEPTABLE`, `INSUFFICIENT_BALANCE`, `STRATEGY_MISMATCH`, `AGENT_BUSY`, `OTHER`. +- **Code (emitted by trader):** `negotiation-handler.ts:1003` emits `UNKNOWN_INTENT` (NOT in spec); `:1109` emits `AGENT_BUSY` (in spec); `:1152, :1293, :1454` emit the FAILED/CANCELLED/COMPLETED state name as the reason_code (NOT in spec); `:1227` emits `ACCEPT_DM_SEND_FAILED` (NOT in spec). +- **Impact:** counterparty implementations don't know how to handle the extra reason codes. They get logged as `UNKNOWN` (`:1496`) and the negotiation just stops. +- **Recommendation:** spec patch — add `UNKNOWN_INTENT`, `ACCEPT_DM_SEND_FAILED`, and the three terminal-state mirror codes (`CANCELLED`, `COMPLETED`, `FAILED`) to the `DEAL_REJECT_REASONS` set in §3.7.3 and Appendix B.2. Document the semantics: "terminal-state mirror codes signal that the deal_id is already finalized; the receiver should drop their copy without further state change." + +### FINDING D15 — Spec NP message validation requires `ts_ms within 300,000 ms` but doesn't address payload `message` length + +- **Spec:** `protocol-spec.md:1389` says "ts_ms is within 300,000 ms of local time". The spec does not say what to do with the optional `message` payload field beyond the 512-char limit declared at `:531, 547, 562` per message subtype. +- **Code:** `negotiation-handler.ts:1697-1705` enforces a 512-char cap on `payload.message` BEFORE dispatching to handlers. The cap is global to all three message types, even though the spec only declares it per-type. +- **Impact:** minor — consistent with the spec's intent. Document it once in §3.4 instead of per subtype. +- **Recommendation:** spec patch — add to §3.4 envelope validation: "`payload.message`, if present, MUST be a string of at most 512 chars." + +### FINDING D16 — Description format §2.8 omits "Expires" line that the implementation emits + +- **Spec:** `protocol-spec.md:363-371` declares the description format with 4 lines: header (direction+volume+assets), rate, escrow, deposit timeout. Example at `:371`: `"Selling 500-1000 ALPHA for USDC. Rate: 450-500 USDC per ALPHA. Escrow: any. Deposit timeout: 300s."` +- **Code:** `utils.ts:73-82` (`encodeDescription`) emits a FIFTH line: `Expires: ${epoch_ms}.` `:107` regex parses it and `:134-140` extracts the epoch ms. +- **Impact:** spec-conformant parsers don't extract `expiry_ms` and fall back to the MarketModule's `expiresAt` (1-day granularity). The implementation comment at `intent-engine.ts:302-304` warns about this: "Prefer the precise expiry_ms from the description (epoch ms) over the MarketModule's coarse expiresAt (1-day granularity)". Spec-conformant agents lose minute-level expiry precision. +- **Recommendation:** spec patch — add the `Expires: {epoch_ms}.` line to §2.8 with the example updated accordingly. Note that the trailing fields are extension-points. + +### FINDING D17 — Spec deal state machine: ACCEPTED can transition to FAILED/COMPLETED in code; spec only allows EXECUTING/CANCELLED + +- **Spec:** `protocol-spec.md:1263-1273` deal state transition table allows from ACCEPTED only `EXECUTING`, `FAILED` (escrow), `CANCELLED` (timeout or reject). +- **Code:** `types.ts:51` declares `ACCEPTED: ['EXECUTING', 'COMPLETED', 'FAILED', 'CANCELLED']`. The COMPLETED branch is reachable on the acceptor path: `swap-executor.ts:471` registers an acceptor deal in `EXECUTING`, but the proposer-side handoff can short-circuit. +- **Impact:** matches the implementation but spec readers see an under-specified machine and could refuse legitimate transitions. +- **Recommendation:** spec patch — add to §6.2 table: `ACCEPTED → COMPLETED` (rare; SDK swap-completed event fired between np.accept_deal and our EXECUTING transition). + +### FINDING D18 — Spec intent state machine: ACTIVE → PARTIALLY_FILLED / FILLED is implemented (acceptor-direct), not in spec + +- **Spec:** `protocol-spec.md:1199-1216` intent state transition table only allows `ACTIVE → MATCHING` / `CANCELLED` / `EXPIRED`. `PARTIALLY_FILLED` is reachable from `NEGOTIATING` only; `FILLED` from `NEGOTIATING` or `PARTIALLY_FILLED`. +- **Code:** `types.ts:29` declares `ACTIVE: ['MATCHING', 'PARTIALLY_FILLED', 'FILLED', 'CANCELLED', 'EXPIRED']`. The code comment at `types.ts:23-28` explicitly justifies the widening: "ACTIVE → PARTIALLY_FILLED / FILLED is permitted because an acceptor never passes through MATCHING: only the side that proposes runs the match-fan-out path that transitions the intent into MATCHING. The acceptor's intent remains ACTIVE until the swap completes." +- **Impact:** this is a real bug-fix that the spec doesn't capture. Spec-conformant agents on the acceptor side would either reject the transition or silently fail to credit volume. +- **Recommendation:** spec patch — add to §6.1 table: `ACTIVE → PARTIALLY_FILLED` and `ACTIVE → FILLED` with guard "acceptor path; deal completed". Explain that acceptor intents bypass MATCHING because the proposer drives the fan-out. + +### FINDING D19 — Spec §5.7 wait period is 30s; implementation uses 45s + +- **Spec:** `protocol-spec.md:1142-1144` says "wait up to 30 seconds for the proposer's message" before some unspecified fallback. +- **Code:** `intent-engine.ts:175` declares `YIELD_TIMEOUT_MS = 45_000` (45 s) and falls through to PROPOSING ourselves at `:411-451` if the lower-priority candidates haven't proposed. +- **Impact:** modest — soak operator expects the deadlock-recovery to happen at 30s and sees it happen at 45s. Doesn't break correctness; only operator expectation. +- **Recommendation:** spec patch — change 30s to 45s and document the fall-through-to-propose behavior (the spec is silent on what happens AFTER the wait). + +### FINDING D20 — Spec NP-0 `np.accept_deal` validation lists nothing about scheduling the SDK swap; spec §3.7.4 understates handshake + +- **Spec:** `protocol-spec.md:585-598` says "After the deal is accepted, the proposer verifies escrow liveness via `pingEscrow()` and then transitions to `EXECUTING`." The implementation does this differently. +- **Code:** `main.ts:719-746` pings trusted escrows from the SWAP-POLL loop (every 3s), NOT from the post-accept handshake. The actual proposeSwap call happens via `onDealAccepted` callback (which is the np.accept_deal acceptor's handler running in the proposer's swap-executor through `executeDeal`, `swap-executor.ts:439-533`). There is NO explicit `pingEscrow` between ACCEPTED and EXECUTING in the proposer's path — the proposer just calls `proposeSwap` and the SDK handles escrow handshake. +- **Impact:** spec promises a verification step that doesn't happen at the documented point. If the escrow is dead, the deal fails at PROPOSE_SWAP_FAILED, not at "ACCEPTED → FAILED (ESCROW_UNREACHABLE)". +- **Recommendation:** spec patch — rewrite §3.7.4 to reflect: "the proposer calls `SwapModule.proposeSwap(deal)` immediately on np.accept_deal receipt; escrow liveness is pre-warmed by an out-of-band poll loop pinging trusted escrows every 3 seconds". Delete the "pingEscrow() before EXECUTING" promise from §6.2 ACCEPTED row. + +### FINDING D21 — Spec ESCROW_UNREACHABLE failure trigger description is wrong + +Related to D20. + +- **Spec:** `protocol-spec.md:1232, 1268` says `pingEscrow()` failure transitions ACCEPTED → FAILED with reason `ESCROW_UNREACHABLE`. +- **Code:** `ESCROW_UNREACHABLE` is listed in `types.ts:129` as a documented reason code but I cannot find any code path that EMITS it. The grep shows it only in docstrings/comments. The actual failure path is `PROPOSE_SWAP_FAILED: ` via `swap-executor.ts:518`. +- **Impact:** documented behavior diverges from observable behavior. Operators looking for ESCROW_UNREACHABLE in logs never find it; the actual code is PROPOSE_SWAP_FAILED with an SDK error message. +- **Recommendation:** code fix (in trader-service) — wrap `swap.proposeSwap` in a path that detects escrow-unreachable errors specifically (e.g. error.message includes "no response" / "transport") and emits `ESCROW_UNREACHABLE`. OR: drop `ESCROW_UNREACHABLE` from the spec and acknowledge `PROPOSE_SWAP_FAILED` as the escrow-down signal. + +## Non-findings (verified OK) + +The following spec sections match the implementation closely enough that no patch is needed: + +- **§3.3 NP message types** — `negotiation-handler.ts:38` declares `NP_VERSION = '0.1'`, `types.ts:170-175` declares the exact 3-tuple `['np.propose_deal', 'np.accept_deal', 'np.reject_deal']`. No extras, no missing types. Dispatch at `:1708-1720`. +- **§3.4 envelope shape** — all required fields match between `types.ts:177-186` and the spec interface. Sender pubkey shape validated at `:796-798`. UUID v4 regex matches the spec. +- **§3.4 size limit** — `negotiation-handler.ts:1608-1612` enforces 64 KiB via `MAX_MESSAGE_SIZE` from `envelope.ts`. +- **§3.4 dangerous keys** — `negotiation-handler.ts:1626-1629` calls `hasDangerousKeys(parsed)`. +- **§5.1 8 matching criteria** — all 8 implemented in `intent-engine.ts:272-350`: opposite direction (1), asset pair (2), rate overlap (3), volume (4), not-expired (5), not-self (6), not-blocked (7), escrow compatible (8). Note: spec criterion numbering puts not-expired at 6 and escrow at 7; code uses 5 and 8; the underlying logic matches. +- **§5.2 rate formula** — `intent-engine.ts:896` matches `(rateMin + rateMax) / 2n` for the midpoint, although the formula appears only at MarketModule-posting time; the actual NP-0 proposed rate is the agreed counterparty value (no formula). The spec's `floor((overlap_min + overlap_max) / 2)` is reachable but only when computing the proposer's offered rate — implementation defers this to `agreedRate` passed by the caller. Functionally equivalent given bigint division. +- **§5.3 volume formula** — `intent-engine.ts:556-572` implements greedy allocation with per-candidate cap at `min(remaining, candidate.volume_max)`. The greedy strategy is documented to BEAT the spec's simple `min(A.available, B.available)` in the multi-counterparty scenario; aligns with §5.5 priority ordering. +- **§5.4 escrow agreement** — `intent-engine.ts:107-121` implements the same branches. +- **§5.5 priority sort** — `intent-engine.ts:454-482` implements the spec's `[rate, time, volume]` sort, except **time priority is REVERSED** (newest first, `timeB - timeA`) per a deliberate choice documented at `:471`: "prefer newest listings (most likely to be live counterparties)". Spec says "earlier first" (`created_ms ASC`). +- **§5.7 proposer election** — `intent-engine.ts:376-380` implements `myKey < cpKey` correctly. +- **§7.1 intent authentication** — implemented at `intent-engine.ts:861` (sign intent_id) and `utils.ts:34-67` (computeIntentId). Server-side ECDSA signed requests are inherited from MarketModule. +- **§7.4 expiry sweep** — `intent-engine.ts:94, 1099` sweeps every 10 s. +- **§7.6 message authentication** — clock-skew 300_000 ms at `negotiation-handler.ts:53`, dedup window 600_000 ms / 10_000 entries at `:41-42`. Sender-participant check at `:1660-1672`. +- **§7.7 DoS mitigations** — `max_active_intents` at `types.ts:208` (default 20, matches spec); proposal flood is bounded by global cap `MAX_INBOUND_PROPOSALS_PER_MIN = 600` at `:426`. Spec says "Max 3 pending proposals per counterparty per 60s" — `negotiation-handler.ts:44-45` declares `RATE_LIMIT_MAX = 3` / `RATE_LIMIT_WINDOW_MS = 60_000`. Matches exactly. Global cap is an undocumented but harmless addition. +- **§7.8 dangerous keys + nesting** — `hasDangerousKeys` enforced at message ingress. +- **§7.9.5 protocolVersion=2 enforcement** — `main.ts:1244-1268` rejects v1 swaps. Matches spec recommendation exactly. +- **§7.9.4 NP-0 ↔ SwapModule term binding** — `main.ts:~1316-1325` compares received SwapDeal fields against negotiated DealTerms (`partyACurrency`, `partyAAmount`, `partyBCurrency`, `partyBAmount`, escrow address, timeout). Matches spec. +- **CREATE_INTENT params shape** (modulo D1/D-NEW) — all 9 spec fields are accepted at the right names; `deposit_timeout_sec` default of 300 at `intent-engine.ts:93`; `escrow_address` default of `"any"` at `:92`. +- **§4.5 LIST_SWAPS / DealSummary** — modulo D12 (`error_code`), all 10 spec fields are emitted. +- **§4.7 GET_PORTFOLIO** — `trader-command-handler.ts:630-688` returns all 5 documented top-level fields. `AssetBalance` 5 sub-fields all emitted at `:646-655` (`asset`, `available`, `total`, `confirmed`, `unconfirmed`). Amounts as strings — bigint convention. +- **§7.9.6 volume reservation atomicity** — `volume-reservation-ledger.ts` (not directly read but referenced from multiple call sites) is the documented invariant-keeper. + +## Recommended actions + +### Spec patches (apply in this order) + +1. **D1 + D-NEW + D2:** the three load-bearing edits. Patch §2.4 / §4.2 to declare wire as bigint-string. Add §2.4-bis "Recommended CLI UX". Patch §3.4 envelope signature input. +2. **D3 + D4:** patch §3.5 deal_id derivation and §3.6 DealTerms to add the 4 missing fields. +3. **D5 + D13 + D14:** Appendix B.1, B.2, B.3 — align error/reason-code sets with what's actually emitted. +4. **D17 + D18 + D19 + D20 + D21:** state machine and §3.7.4 / §5.7 corrections. +5. **D10 + D11 + D12 + D15 + D16:** minor structural fixes. + +### Implementation fixes (do NOT write the fix; describe and file) + +1. **D6 (INTENT_NOT_ACTIVE):** `cancelIntent` should pre-check terminal state and emit `INTENT_NOT_ACTIVE`. File against trader-service. +2. **D7 (SET_STRATEGY echo):** trivial fix — return `merged` instead of `strategyParams`. File against trader-service. +3. **D8 (CLI SET_STRATEGY params):** rename `max_concurrent_negotiations` → `max_concurrent_swaps`; drop `rate_strategy` until the trader supports it. File against sphere-cli-work/sphere-cli. +4. **D9 (CLI list-intents filter):** rename `state` → `filter`. File against sphere-cli-work/sphere-cli. +5. **D21 (ESCROW_UNREACHABLE):** add an escrow-down detection branch in `main.ts` around `swap.proposeSwap`. File against trader-service. + +### Recommended sub-issues to file under #474 + +Two of the findings rise to "block the soak"; the rest are documentation cleanups. Suggested sub-issues: + +**Sub-issue #474.G4.1 — Rate/volume wire encoding (BLOCKS SOAK):** + +> Trader protocol spec §2.4 declares `rate_min/rate_max/volume_min/volume_max` as `number` (float). The deployed v0.1 trader accepts ONLY bigint strings (`trader-command-handler.ts:370-378`, `acp-types.ts:20-23`). Soak operators reading the spec have no way to learn the correct wire encoding without reading the code. +> +> Required spec patches: +> 1. Change §2.4 interface block to use `string` for all four fields, with `// stringified bigint, smallest units` comment. +> 2. Change §2.4 constraint table from "Positive finite number" to "Positive bigint (string-encoded, smallest units)". +> 3. Change §4.2 `CreateIntentParams` interface accordingly. +> 4. Update §5.2/§7.3 formulas/assertions to bigint-style. +> +> Reference: `docs/uxf/PROTOCOL-SPEC-DRIFT-474.md` finding D1. + +**Sub-issue #474.G4.2 — CLI float UX + decimal-aware conversion (BLOCKS SOAK):** + +> The design intent is that CLI operators type human-friendly floats (`--rate-min 0.08`) and the CLI converts to bigint smallest-units via a token-registry decimals lookup. Today's CLI (`sphere-cli/src/trader/trader-commands.ts:386-389`) requires fully-scaled bigint strings, and the spec doesn't document this convention. +> +> Required work: +> 1. CLI: accept `--rate-min `; add `--rate-min-bigint ` escape hatch. +> 2. CLI: look up decimals via `MarketModule.decimalsFor(asset)` (new SDK helper) or `TokenRegistry`. +> 3. CLI: pre-flight validate non-finite, negative, and over-precise floats. +> 4. Spec §2.4-bis: document the recommended CLI UX. +> +> Reference: `docs/uxf/PROTOCOL-SPEC-DRIFT-474.md` finding D-NEW. + +**Sub-issue #474.G4.3 — Spec hygiene cleanup pass:** + +> Patch protocol-spec.md for the 18 documentation drifts identified in findings D2-D21 of `docs/uxf/PROTOCOL-SPEC-DRIFT-474.md`. None are individually load-bearing; together they make the spec a reliable reference. Estimated patch: ~150 line changes across §3, §4, §5, §6, §7, and Appendix B. + +**Sub-issue #474.G4.4 — Trader code: ESCROW_UNREACHABLE + INTENT_NOT_ACTIVE + SET_STRATEGY echo:** + +> Three small implementation fixes to align observable trader behavior with the spec contract: +> 1. Add an escrow-down detection branch around `swap.proposeSwap` in `main.ts` to emit `ESCROW_UNREACHABLE` instead of `PROPOSE_SWAP_FAILED`. (D21) +> 2. `cancelIntent` pre-checks terminal state and emits `INTENT_NOT_ACTIVE`. (D6) +> 3. `handleSetStrategy` returns the merged strategy, not the partial input. (D7) +> +> Reference: `docs/uxf/PROTOCOL-SPEC-DRIFT-474.md` findings D6/D7/D21. + +**Sub-issue #474.G4.5 — sphere-cli trader command param-name fixes:** + +> Two CLI bugs cause silent param drops: +> 1. `sphere trader set-strategy --max-concurrent N` sends `max_concurrent_negotiations`; trader expects `max_concurrent_swaps`. (D8) +> 2. `sphere trader list-intents --state filled` sends `state`; trader expects `filter`. (D9) +> +> Reference: `docs/uxf/PROTOCOL-SPEC-DRIFT-474.md` findings D8/D9. diff --git a/manual-test-trader-roundtrip.sh b/manual-test-trader-roundtrip.sh new file mode 100644 index 00000000..fb85e947 --- /dev/null +++ b/manual-test-trader-roundtrip.sh @@ -0,0 +1,1215 @@ +#!/usr/bin/env bash +# +# manual-test-trader-roundtrip.sh — trader-agent autonomous round-trip soak +# (sphere-sdk#474, the trader-agent G1 walkthrough). +# +# Scenario (verbatim from #474): +# SETUP alice (controller) faucets 100 UCT to her wallet +# bob (controller) faucets 10 ETH to his wallet +# DEPOSIT alice sends 50 UCT to her trader tenant +# bob sends 4.5 ETH to his trader tenant +# INTENTS alice's controller posts a SELL intent on (UCT/ETH) +# bob's controller posts a BUY intent on (UCT/ETH) +# DAEMONS each tenant scans the market, finds the counter-intent, +# negotiates terms via NP-0 over NIP-17 DMs, and executes +# the matched deal through SwapModule.proposeSwap against +# the escrow ($ESCROW, default @escrow-test-02). +# COMPLETE alice's trader ends up with ~+5 ETH (and -50 UCT) +# bob's trader ends up with ~+50 UCT (and -~5 ETH) +# +# Net (tenant view): +# alice-trader -50 UCT +~5 ETH +# bob-trader +50 UCT -~5 ETH +# Exact split is determined by the agreed rate, which the matching +# code computes as floor((overlap_min + overlap_max) / 2) over the +# intersection of both rate bands — see §5.2 of +# /home/vrogojin/trader-service/docs/protocol-spec.md. +# +# This soak is the TRADER analog of: +# - manual-test-swap-roundtrip.sh (swap roundtrip) +# - manual-test-accounting-roundtrip.sh (invoice roundtrip) +# +# --------------------------------------------------------------------------- +# Run: +# bash manual-test-trader-roundtrip.sh +# KEEP=1 bash manual-test-trader-roundtrip.sh # preserve workspace +# KEEP_TENANTS=1 bash manual-test-trader-roundtrip.sh # leave tenants running +# TRADER_TEST_DIR=/tmp/tr bash manual-test-trader-roundtrip.sh +# +# --------------------------------------------------------------------------- +# UX PRINCIPLE — human-friendly floats at the CLI surface +# +# Per the project owner's guidance, the operator works with human-friendly +# floats everywhere — `--rate-min 0.08`, `--volume-min 50`. The CLI is +# responsible for converting those to smallest-unit bigints internally via +# the token registry decimals lookup (UCT and ETH are both 18-decimal on +# testnet). This soak writes the float form as the canonical UX. +# +# TODO(#474 follow-up): the trader CLI (sphere-cli/src/trader/trader- +# commands.ts) currently accepts `--rate-min ` as a STRING-ENCODED +# BIGINT. This is the wrong UX — it must be fixed to accept floats and do +# the smallest-unit conversion internally. Until that lands, this soak's +# `with_float_or_bigint_shim` helper auto-detects the CLI's accepted form: +# it tries the float form first; on INVALID_PARAM it falls back to an +# inline float→bigint conversion (via `python3 -c "print(int( * 10**18))"`) +# and prints a warning. Set TRADER_CLI_FLOAT_NATIVE=0 to skip the float +# attempt entirely and go straight to the bigint shim. +# +# --------------------------------------------------------------------------- +# Env contract (with defaults): +# +# TRADER_TEST_DIR Workspace root. Default /tmp/trader-roundtrip-$$ +# KEEP=0|1 Preserve $TRADER_TEST_DIR on exit. Default 0. +# KEEP_TENANTS=0|1 Leave spawned tenants AND their local HMs +# running on exit. Default 0 (we +# `sphere trader stop` them, which auto-tears +# down the per-user HM when the last tenant +# stops; setting this to 1 forwards `--keep-hm`). +# SUFFIX Unique suffix shared by the alice/bob/tenant +# nametags. Default = epoch-tail + random. +# ESCROW Escrow @nametag or DIRECT://hex. +# Default @escrow-test-02 (per sphere-sdk#456). +# TRADER_RATE_MIN_ETH_PER_UCT Lower edge of the rate band, as a +# **human-friendly float**. Default 0.08 +# (= 0.08 ETH per 1 UCT). +# TRADER_RATE_MAX_ETH_PER_UCT Upper edge of the rate band. Default 0.12. +# TRADER_VOLUME_UCT Volume to trade in **whole UCT**. Default 50. +# TRADER_CLI_FLOAT_NATIVE 0 = skip the float attempt and go straight +# to the bigint shim. Default 1. +# TRADER_DEAL_DEADLINE_S Wall-clock cap for negotiation + settlement. +# Default 900 (15 min). Bumped from 600 to +# cover per-user local-HM bootstrap (two-shot +# drift-guard restart) on top of the trader +# scan interval. TRADER_SCAN_INTERVAL_MS +# defaults to 30 s in the template, so a first +# match round can take up to a minute. +# TRADER_DEPOSIT_TIMEOUT_S Wall-clock cap for the controller→tenant +# deposit confirmation poll. Default 240. +# TRADER_FAUCET_WAIT_S Wall-clock cap waiting for faucet UTXOs to +# land. Default 120. +# MARKET_API_URL Market feed base URL. Default +# https://market-api.unicity.network. +# SPHERE_ALLOW_MNEMONIC_NON_TTY Always exported as 1 — the soak runs +# non-interactively, so it cannot prompt for +# mnemonic entry. +# +# Prerequisites: +# - sphere-cli with `sphere trader spawn` / `sphere trader stop` +# (unicity-sphere/sphere-cli#49 or later). The wrapper brings up a +# per-user local Host Manager scoped to the current wallet's +# controller pubkey + spawns the trader tenant in one command. The +# public Host Manager is reserved for shared infra (escrow, faucet) +# and is NOT used by this soak. +# - Docker available locally — the wrapper drives docker for the +# per-user HM container. +# +# --------------------------------------------------------------------------- +# KNOWN LIMITATIONS +# +# 1. Cross-process Nostr DM flakiness (sphere-sdk#473). +# The controller `sphere trader ...` commands are CLI-process: they +# boot, send one DM to the tenant, wait for a reply, exit. The tenant +# stays subscribed, but its `since` cursor and the relay's per-pubkey +# retention can drop occasional inbound DMs from a freshly-booted +# controller process. Mitigations baked in here: +# - `with_retry` wraps every `sphere trader ...` controller call +# (3 attempts × 5 s back-off). +# - After spawn we run `sphere trader portfolio` as a warm-up to +# prime the tenant's `since` cursor before doing anything load- +# bearing. +# - The §8 deal-completion poll uses TRADER_DEAL_DEADLINE_S (default +# 15 min, i.e. ~30× TRADER_SCAN_INTERVAL_MS) so a missed DM is +# recovered by the next scan iteration. +# +# 2. CLI float-vs-bigint UX (covered by TODO(#474 follow-up) above). +# The soak writes the float form (post-fix UX), with an inline shim +# that converts to smallest-unit bigints when the CLI rejects floats +# with INVALID_PARAM. Verify the deployed CLI form against +# `sphere trader create-intent --help` before running. The shim +# assumes UCT and ETH have 18 decimals (true on production testnet); +# override TRADER_*_DECIMALS env vars if your registry differs. +# +# 3. Trader image staleness (vrogojin/agentic_hosting#26). +# The trader image tagged `ghcr.io/vrogojin/agentic-hosting/trader:v0.1` +# was built before the SDK-side rotations in: +# - sphere-sdk#456 (DEFAULT_ESCROW_ADDRESS = @escrow-test-02) +# - sphere-sdk#457 (counterparty transport pubkey fail-fast) +# - sphere-sdk#464 (MuxAdapter dispatch await) +# Until the v0.2 image lands (vrogojin/agentic_hosting#26), this soak +# may fail in §8 with one of: +# - tenant times out negotiating because old SwapModule does not +# fail fast on missing transport pubkey; +# - tenant uses a stale default escrow that does not match $ESCROW +# and the swap proposal never gets accepted. +# Remediation: rebuild and republish the trader image upstream. +# The `sphere trader spawn` wrapper accepts `--hm-image` for the host +# manager image but the trader image itself is pinned by the template +# registry (config/templates.json). +# +# 4. Some intent state values are not enumerated in this soak. It ASSUMES +# that --state filters on list-intents/list-deals accept the canonical +# uppercase forms documented in protocol-spec.md §6. If a future +# revision changes the wire shape, the §11 cleanup may need adjusting. +# +# Canonical end-to-end walkthrough: sphere-sdk#474 (G1). + +set -euo pipefail + +# --------------------------------------------------------------------------- +# Workspace +# --------------------------------------------------------------------------- +ROOT="${TRADER_TEST_DIR:-/tmp/trader-roundtrip-$$}" +SNAP="$ROOT/snapshots" +mkdir -p "$SNAP" + +SUFFIX="${SUFFIX:-$(date +%s | tail -c 5)$(printf '%04x' $((RANDOM % 65536)))}" +ALICE_TAG="alice-$SUFFIX" +BOB_TAG="bob-$SUFFIX" +ALICE_TRADER_TAG="alice-trader-$SUFFIX" +BOB_TRADER_TAG="bob-trader-$SUFFIX" +ALICE_TRADER_INSTANCE="alice-trader-$SUFFIX" +BOB_TRADER_INSTANCE="bob-trader-$SUFFIX" + +echo "ALICE_TAG=$ALICE_TAG" +echo "BOB_TAG=$BOB_TAG" +echo "ALICE_TRADER_TAG=$ALICE_TRADER_TAG" +echo "BOB_TRADER_TAG=$BOB_TRADER_TAG" + +PEER_ALICE="$ROOT/alice-peer" +PEER_BOB="$ROOT/bob-peer" +mkdir -p "$PEER_ALICE" "$PEER_BOB" + +ESCROW="${ESCROW:-@escrow-test-02}" + +# Human-friendly floats. The CLI is responsible for converting these +# to smallest-units (post-fix UX). The shim mode below converts inline +# when the CLI still demands bigints. +TRADER_RATE_MIN_ETH_PER_UCT="${TRADER_RATE_MIN_ETH_PER_UCT:-0.08}" +TRADER_RATE_MAX_ETH_PER_UCT="${TRADER_RATE_MAX_ETH_PER_UCT:-0.12}" +TRADER_VOLUME_UCT="${TRADER_VOLUME_UCT:-50}" +TRADER_CLI_FLOAT_NATIVE="${TRADER_CLI_FLOAT_NATIVE:-1}" + +# Decimals — UCT and ETH are both 18-decimal on testnet. +TRADER_UCT_DECIMALS="${TRADER_UCT_DECIMALS:-18}" +TRADER_ETH_DECIMALS="${TRADER_ETH_DECIMALS:-18}" + +# Bumped 600→900 to cover per-user local-HM bootstrap on top of the +# trader scan interval. The wrapper performs a two-shot drift-guard +# restart of the HM before the trader tenant is ready, which the old +# 10-min budget did not account for. +TRADER_DEAL_DEADLINE_S="${TRADER_DEAL_DEADLINE_S:-900}" +TRADER_DEPOSIT_TIMEOUT_S="${TRADER_DEPOSIT_TIMEOUT_S:-240}" +TRADER_FAUCET_WAIT_S="${TRADER_FAUCET_WAIT_S:-120}" + +MARKET_API_URL="${MARKET_API_URL:-https://market-api.unicity.network}" + +# KEEP_TENANTS=1 forwards `--keep-hm` to `sphere trader stop` so the +# per-user local HM stays running for inspection. Note: `sphere trader +# stop` does NOT have a --keep-data flag; the tenant data dir is +# preserved by default. +KEEP_HM_FLAG="" +if [[ "${KEEP_TENANTS:-0}" == "1" ]]; then + KEEP_HM_FLAG="--keep-hm" +fi + +echo "ESCROW=$ESCROW" +echo "TRADER_RATE_MIN_ETH_PER_UCT=$TRADER_RATE_MIN_ETH_PER_UCT (float)" +echo "TRADER_RATE_MAX_ETH_PER_UCT=$TRADER_RATE_MAX_ETH_PER_UCT (float)" +echo "TRADER_VOLUME_UCT=$TRADER_VOLUME_UCT (whole UCT)" +echo "TRADER_CLI_FLOAT_NATIVE=$TRADER_CLI_FLOAT_NATIVE" +echo "TRADER_DEAL_DEADLINE_S=$TRADER_DEAL_DEADLINE_S" +echo "MARKET_API_URL=$MARKET_API_URL" +echo "KEEP_HM_FLAG=${KEEP_HM_FLAG:-}" + +export SPHERE_ALLOW_MNEMONIC_NON_TTY=1 + +# --------------------------------------------------------------------------- +# Derived expected values (smallest-units) for §10 assertions. +# Both rates and volume are floats here; convert to bigint smallest-units +# via python (arbitrary precision). +# --------------------------------------------------------------------------- +EXPECTED_UCT_SMALLEST=$(python3 -c "print(int($TRADER_VOLUME_UCT * 10**$TRADER_UCT_DECIMALS))") +# ETH band: low/high bounds on the expected ETH delta for the agreed volume. +ETH_LOW_SMALLEST=$(python3 -c " +print(int($TRADER_RATE_MIN_ETH_PER_UCT * $TRADER_VOLUME_UCT * 10**$TRADER_ETH_DECIMALS)) +") +ETH_HIGH_SMALLEST=$(python3 -c " +print(int($TRADER_RATE_MAX_ETH_PER_UCT * $TRADER_VOLUME_UCT * 10**$TRADER_ETH_DECIMALS)) +") +ETH_MID_SMALLEST=$(python3 -c " +rmid = ($TRADER_RATE_MIN_ETH_PER_UCT + $TRADER_RATE_MAX_ETH_PER_UCT) / 2 +print(int(rmid * $TRADER_VOLUME_UCT * 10**$TRADER_ETH_DECIMALS)) +") + +echo "EXPECTED_UCT_SMALLEST=$EXPECTED_UCT_SMALLEST (alice trader sells, bob trader receives)" +echo "ETH_LOW_SMALLEST =$ETH_LOW_SMALLEST (alice trader minimum receive)" +echo "ETH_HIGH_SMALLEST =$ETH_HIGH_SMALLEST (alice trader maximum receive)" +echo "ETH_MID_SMALLEST =$ETH_MID_SMALLEST (midpoint expectation)" + +cleanup() { + local rc=$? + # Stop spawned tenants on the way out via `sphere trader stop`. The + # wrapper auto-tears down each peer's per-user local Host Manager when + # the last tenant attached to it stops; pass --keep-hm (via + # KEEP_HM_FLAG, set from KEEP_TENANTS) to leave the HM containers + # running for inspection. + # + # Even with KEEP_TENANTS=1 we still issue `sphere trader stop` (with + # --keep-hm) so the tenant process exits cleanly — this differs from + # the previous behavior, which skipped cleanup entirely. We do this + # because the wrapper's bookkeeping (tenant registry, HM ref count) + # is the source of truth; leaving the tenant alive but unregistered + # would orphan it. If you genuinely want a tenant left running for + # ACP probing, comment out the `sphere trader stop` lines. + if [[ -n "${PEER_ALICE:-}" && -d "$PEER_ALICE" ]]; then + ( + cd "$PEER_ALICE" 2>/dev/null && \ + sphere wallet use alice 2>/dev/null && \ + sphere trader stop --name "$ALICE_TRADER_INSTANCE" $KEEP_HM_FLAG \ + 2>&1 | tee -a "$SNAP/alice-trader-stop.log" || true + ) || true + fi + if [[ -n "${PEER_BOB:-}" && -d "$PEER_BOB" ]]; then + ( + cd "$PEER_BOB" 2>/dev/null && \ + sphere wallet use bob 2>/dev/null && \ + sphere trader stop --name "$BOB_TRADER_INSTANCE" $KEEP_HM_FLAG \ + 2>&1 | tee -a "$SNAP/bob-trader-stop.log" || true + ) || true + fi + if [[ "${KEEP_TENANTS:-0}" == "1" ]]; then + echo "=== KEEP_TENANTS=1: per-user HMs left running (--keep-hm); tenant processes stopped ===" + fi + if [[ "${KEEP:-0}" != "1" ]]; then + rm -rf "$ROOT" 2>/dev/null || true + else + echo "=== KEEP=1: workspace preserved at $ROOT ===" + fi + return "$rc" +} +trap cleanup EXIT INT TERM + +banner() { + echo + echo "================================================================" + echo "$@" + echo "================================================================" +} + +# --------------------------------------------------------------------------- +# Integer-only confirmed balance extractor for `sphere balance` output. +# Same convention as manual-test-{swap,accounting}-roundtrip.sh: both UCT +# and ETH are 18-decimal coins on the production testnet registry, so we +# pad fractional parts to 18 chars to get a smallest-unit integer. +# --------------------------------------------------------------------------- +extract_confirmed_smallest_units() { + local symbol="$1" + local line decimal int_part frac_part + line=$(grep -E "^${symbol}:" || true) + if [[ -z "$line" ]]; then + echo "0" + return + fi + decimal=$(echo "$line" | sed -E -e "s/^${symbol}:[[:space:]]+//" -e 's/[[:space:]]+\(.+$//') + if [[ "$decimal" == *.* ]]; then + int_part="${decimal%.*}" + frac_part="${decimal#*.}" + else + int_part="$decimal" + frac_part="" + fi + while (( ${#frac_part} < 18 )); do frac_part="${frac_part}0"; done + if (( ${#frac_part} > 18 )); then + echo "ERROR: ${symbol} fractional part >18 digits ($decimal)" >&2 + return 1 + fi + local combined="${int_part}${frac_part}" + combined=$(echo "$combined" | sed -E 's/^0+//') + [[ -z "$combined" ]] && combined="0" + echo "$combined" +} + +# Helper for grep-based assertions that keep the ASSERT lines uniform. +assert_grep() { + local label="$1" pattern="$2" file="$3" + if grep -qE "$pattern" "$file"; then + echo "ASSERT OK ($label): pattern matched in $file" + return 0 + fi + echo "ASSERT FAIL ($label): pattern '$pattern' NOT found in $file" >&2 + echo "--- $(basename "$file") tail ---" >&2 + tail -20 "$file" >&2 || true + return 1 +} + +# --------------------------------------------------------------------------- +# Retry helper for controller → tenant DM calls (KNOWN LIMITATION #1). +# +# Usage: with_retry