Repository navigation
Conversation
Bootstraps mint/fund wallets, runs intent-solver on the HTTP RFQ path (not OFFER_MARKETS), and verifies quote → fund → fill with fee checks. Proven end-to-end on regtest with seconds-mode VTXO expiry. Co-authored-by: Marco Argentieri <tiero@users.noreply.github.com>
The existing smoke proves the wire: a hand-built rfq_request posted to the solver's HTTP swap port, funding a covenant the harness derives itself. This adds the product path beside it — one client.exchange() per direction against a solver running relay mode, where port 8787 does not exist, so an HTTP fallback cannot silently carry the test. The client is given no transport: discovery.snapshot injects a card carrying discovery_pubkey and the local relay, and the client opens that rendezvous itself. A second relay subscription counts kind-24859 traffic independently of the transport under test, and market.backend is asserted rfq before anything is disclosed — the feed-priced path would settle the same swaps and send nothing. start-solver.sh gains SOLVER_MODE=relay; lib/card.mjs builds the card the solver's own builder refuses to (a ws:// relay); link-sdk.sh packs the unreleased swap package the flow needs. Co-authored-by: Marco Argentieri <tiero@users.noreply.github.com>
The ceiling was 1% of the give amount, which this market can never clear: the fee is the whole concession against the card's price, and on a BTC input that includes the 330-sat carrier the registry schema has no honest field for — 3.2% on a 10k swap. It now comes from the deployment's published terms, bps plus the flat fee for that direction, so MaxFeeExceeded means the price moved rather than the test being miscalibrated. Balances are asserted exact on both legs instead of a lower bound on the take leg. The carrier is what makes that possible: it rides an asset deposit out and returns inside the fill, so the sats delta is take - carrier one way and carrier - give the other. Both held to the satoshi over two runs. Co-authored-by: Marco Argentieri <tiero@users.noreply.github.com>
The positivity comment sat above the fee-ceiling check while describing the take-leg check above it. Co-authored-by: Marco Argentieri <tiero@users.noreply.github.com>
The smoke still fails if the relay is down, still counts kind 24859, still checks exact balances. The essays around that are gone. Co-authored-by: Marco Argentieri <tiero@users.noreply.github.com>
arkana-ai-bot
left a comment
There was a problem hiding this comment.
Asset RFQ smoke harness — review
This is a new smoke harness, no production protocol code changes. Two genuine bugs and several operational issues worth fixing before the run is canonical.
Bug: HTTP smoke does not assert payout amounts
** — throughout **
The Nostr smoke () has which asserts exact equality of the after-balances against the quoted amounts, accounting for the 330-sat carrier both ways. The HTTP smoke collects / snapshots and writes them to the artifact JSON but never asserts them. A wrong payout silently passes. The fee-arithmetic check on the quote (assertEq(BigInt(quote.to_amount), expected, 'to_amount')) is correct, but that only validates the solver's quote reply — not what actually landed.
Suggested fix: port the carrier-aware equality assertion from waitForPayout into smoke-swaps.mjs after waitSettled returns. The logic already exists; it just needs to be called.
Bug: incomplete terminal-state guard in /
**** and ** waitFilled**
Both polling loops only fast-fail on failed and refused. ts-sdk/packages/swap/src/rfq.ts:162 defines five terminal states:
If the solver returns expired, refunded, or stuck, both loops spin for the full 180 s / 240 s before timing out with an unhelpful "not settled" message. Add the remaining terminal states to the early-exit guard.
engine floor understates the requirement
The pinned SDK (@arkade-os/sdk@0.5.0-rc.8) and swap package both declare >=22.12.0. A developer on Node 20 or 21 gets a runtime crash after npm install succeeds. Raise the floor to >=22.12.0 to match the dependency.
silently swallows settle errors
If settle() throws (the usual failure mode when a batch races a commitment tx), the bootstrap continues and the subsequent waitBalance times out after 120 s with "timed out waiting for issuer asset supply" rather than the actual cause. Log the error or rethrow.
Operational notes (not bugs, but worth fixing)
configure-market.mjs:56 — toleranceBps: 9999
99.99% price-feed tolerance is required here because the regtest pricefeed is a static HTTP server. Add a comment stating this must not appear in non-regtest configs; a production market with this value accepts any price the feed serves.
start-solver.sh — pricefeed startup race
The background node process is started and followed by sleep 1 before the script continues. If port 18088 is already taken by a stale process from a prior run, the new process fails silently and the solver later fails to fetch prices. Consider using the same waitHttpOk-style loop that the JS scripts use.
smoke-nostr.mjs — no fetch timeout in assertRelayIngress
Both fetch(ADMIN_URL/api/backends) calls at startup carry no AbortSignal / timeout. If the admin port is unreachable (e.g., solver crashed on startup), the process hangs indefinitely. A 10 s abort controller matches the pattern used elsewhere in the harness.
.gitignore — node_modules/ duplicated (lines 1 and 9). Minor.
Confirming what is correct
- Fee arithmetic in
expectedPayoutExactInand thebtc->assetflat-fee adjustment are correct: carrier is deducted before the bps cut, matchingsellBaseFeeFlaton the market config. waitForPayoutcarrier accounting in the Nostr smoke is correct:+ASSET_CARRIER_SATSonbtc->asset(fill returns the carrier) and-ASSET_CARRIER_SATSonasset->btc(carrier rode the deposit out).fundQuotedOfferaddress-mismatch guard before funding is correct and important.- Relay-mode structural proof (no port 8787 published) is well-reasoned.
- Independent relay watcher (
lib/relayWatch.mjs) sharing no code with the transport under test is the right way to count kind-24859 events. solverPubkeyFromEnv→discovery_pubkey→ responder-check path in the Nostr smoke is correct.- Card built locally to allow
ws://relay is correctly explained and the absence offee_flaton the card is justified.
The main ask before treating this run as canonical: add the payout assertion to the HTTP smoke and add the missing terminal states to both polling loops.
arkana-ai-bot
left a comment
There was a problem hiding this comment.
Asset RFQ smoke harness — review
This is a new smoke harness, no production protocol code changes. Two genuine bugs and several operational issues worth fixing before the run is canonical.
Bug: HTTP smoke does not assert payout amounts
smoke-swaps.mjs — throughout runSwap
The Nostr smoke has waitForPayout which asserts exact equality of the after-balances against the quoted amounts, accounting for the 330-sat carrier both ways. The HTTP smoke collects before / after snapshots and writes them to the artifact JSON but never asserts them. A wrong payout silently passes the test. The fee-arithmetic check on the quote (assertEq(BigInt(quote.to_amount), expected, 'to_amount')) is correct, but that only validates the solver's quote reply — not what actually landed.
Suggested fix: port the carrier-aware equality assertion from waitForPayout into smoke-swaps.mjs after waitSettled returns. The logic already exists in the Nostr smoke; it just needs to be called here too.
Bug: incomplete terminal-state guard in waitSettled / waitFilled
lib/rfq.mjs:90–100 and smoke-nostr.mjs waitFilled
Both polling loops only fast-fail on failed and refused. ts-sdk/packages/swap/src/rfq.ts:162 declares five terminal states:
RFQ_TERMINAL_STATES = ["settled", "refused", "expired", "refunded", "stuck"]
If the solver returns expired, refunded, or stuck, both loops spin for the full 180 s / 240 s before timing out with an unhelpful "not settled" message instead of failing fast with the actual terminal state. Add the remaining three to the early-exit guard.
package.json:5 engine floor understates the requirement
The outer package.json declares "node": ">=20". The pinned @arkade-os/sdk@0.5.0-rc.8 and @arkade-os/swap@0.1.0-rc.11 both require >=22.12.0. A developer on Node 20 or 21 gets a runtime crash after npm install succeeds without warning. Raise to >=22.12.0.
bootstrap.mjs:66 silently swallows settle errors
await issuer.wallet.settle().catch(() => {});If settle() throws (the usual mode when a batch races a commitment tx), the script continues and the subsequent waitBalance times out after 120 s with "timed out waiting for issuer asset supply" rather than the actual cause. At minimum log the error; ideally rethrow after recording it.
Operational notes (not bugs, but worth addressing)
configure-market.mjs:56 — toleranceBps: 9999
99.99% price-feed tolerance is required here because the regtest pricefeed is static. The value is not currently commented. Add a comment that this must never appear in non-regtest configs; a production market with this value accepts any price the feed serves.
start-solver.sh — pricefeed startup race
The background node pricefeed is started and followed by sleep 1. If port 18088 is already bound from a stale prior process, the new background process fails silently and the solver later fails to price. A short waitHttpOk-style loop (matching what the JS scripts use) is more reliable than a fixed sleep.
smoke-nostr.mjs — no fetch timeout in assertRelayIngress
Both fetch(ADMIN_URL/api/...) calls at startup carry no AbortSignal / timeout. If the admin port is unreachable the process hangs indefinitely. A 10 s abort controller matches the pattern used elsewhere in the harness.
.gitignore — node_modules/ is listed twice (lines 1 and 9). Minor.
What is correct
- Fee arithmetic in
expectedPayoutExactInand thebtc->assetflat-fee adjustment are correct: carrier is deducted before the bps cut, matchingsellBaseFeeFlaton the market config. waitForPayoutcarrier accounting in the Nostr smoke is correct:+ASSET_CARRIER_SATSonbtc->asset(fill returns the carrier) and-ASSET_CARRIER_SATSonasset->btc(carrier rode the deposit out).fundQuotedOfferaddress-mismatch guard before funding is correct and important.- Relay-mode structural proof — no port 8787 published — is well-reasoned and the comment in
start-solver.shexplains it clearly. - Independent relay watcher (
lib/relayWatch.mjs) sharing no code with the transport under test is the right way to count kind-24859 events, and the pre-subscription-before-swap ordering is correct for ephemeral events. solverPubkeyFromEnv→discovery_pubkey→ responder-check path in the Nostr smoke is correct.- Card built locally to allow a
ws://relay is correctly explained, and the absence offee_flaton the card is properly justified.
The two items to fix before treating this run as canonical: add the payout assertion to the HTTP smoke and add the missing terminal states to both polling loops.
Drop session notes and assertion essays; keep the commands a human needs to start the stack and run HTTP vs Nostr. Co-authored-by: Marco Argentieri <tiero@users.noreply.github.com>
arkana-ai-bot
left a comment
There was a problem hiding this comment.
1 commit since last pass (docs-only README rewrite); 0 prior issues fixed; 5 prior findings still open.
Prior finding status
Bug 1 — HTTP smoke does not assert payout amounts (smoke-swaps.mjs)
Still open. runSwap captures before / after balance snapshots and writes them to the artifact JSON, but never asserts after - before === quote.to_amount. A wrong payout silently passes. The new commit did not touch smoke-swaps.mjs. This is the top-priority fix.
Bug 2 — Incomplete terminal-state guard in waitSettled (lib/rfq.mjs)
Still open. lib/rfq.mjs still reads:
if (status?.state === 'failed' || status?.state === 'refused') {expired, refunded, and stuck are still missing. Loops spin the full 180 s before timing out with an unhelpful message. Not touched in the new commit.
Issue 3 — Engine floor understates the requirement (package.json:5)
Still open. "node": ">=20" remains. Dependencies require >=22.12.0. Not touched.
Issue 4 — settle() silently swallowed (bootstrap.mjs:88)
Still open. await issuer.wallet.settle().catch(() => {}) is unchanged. A settle failure produces a 120 s timeout rather than the actual error message.
Operational notes (configure-market.mjs, start-solver.sh, smoke-nostr.mjs, .gitignore)
Still open. None of these files were touched in the new commit. The toleranceBps: 9999 comment, pricefeed startup race, missing AbortSignal on admin fetch, and duplicate .gitignore entry all remain.
New finding (this pass)
smoke/asset-rfq/README.md — assertion behaviour no longer documented
The trimmed README removed the explanation of what the Nostr smoke verifies: carrier accounting, independent kind-24859 relay watcher, solverPubkeyFromEnv → discovery_pubkey → responder check, exact-balance equality, and the relay-failure → hard fail behaviour. The operational commands are intact, but a contributor picking up this harness now has no written record of what constitutes a passing run beyond "it ran." The fee-arithmetic formula (to = from - ceil(from * feeBps / 10000)) and the flat-fee adjustment for btc→asset are also gone.
Suggestion: retain a concise "What passes" section in the README — not the full essays, but the key invariants (exact payout amounts, kind-24859 count ≥ 1 in each direction, relay-failure mode). This is a documentation request, not a blocker, but the prior README's explanations were the only place those invariants were written down outside the code.
Summary
The new commit is a clean, focused docs trim — the reduced README is more useful operationally. No regressions introduced. The two bugs (payout assertion, terminal-state guard) from the previous review are the blocking items before this run can be treated as canonical.
A regtest harness for the quoted asset-RFQ path —
arkade:BTC <-> arkade:<asset>againstintent-solver'sASSET_MARKETS, not the offer-packet pathOFFER_MARKETSserves.Two smokes, at two layers, and both are worth keeping:
npm run smokenpm run smoke:nostrPOST /v1/swapserve(port 8787)relay— no swap port at allrfq_request+createOffercreateSwapClient().exchange()The wire smoke
bootstrappersists keys, mints the asset and funds the solver and traders;configure-marketPUTs the market through the admin API and appendsASSET_MARKETS(omitted on first boot, since the process refuses to start naming an asset the console has not priced yet);smokeruns asset→BTC, BTC→asset and a random back-and-forth, checking each payout against the exact-in arithmetic. Amounts are jittered because identical covenant terms derive one offer address and the solver uniques its fills on that script.The Nostr smoke
SOLVER_MODE=relay ./start-solver.shrunssolverd relayagainstws://strfry:7777and publishes no swap port, with health moved to the admin API on 8788. That is the load-bearing part: port 8787 does not exist, so an HTTP fallback cannot quietly carry the test.The client is handed a card, not a transport.
discovery.snapshotinjects one built bylib/card.mjscarryingdiscovery_pubkeyand the local relay, andtransportForis left unset — so the client resolves the rendezvous off the card and opens its own Nostr transport. The card is built in the harness rather than read fromsolver card/GET /api/cardbecause both refuse a relay that is notwss://, and a local strfry isws://. It also omitsfee_flatfor the BTC input direction, mirroring the refusal the solver's own builder makes: the registry field is denominated in quote units and this deployment charges it on the BTC side, so there is no honest field for it.Beyond settlement it asserts:
market.backend === "rfq"before anything is disclosed. This is the regression that matters — the feed-priced path settles these same swaps and sends not one packet.lib/relayWatch.mjs) that shares no code with the transport under test. The kind is ephemeral, so the watcher has to be subscribed before the swap runs, which is what makes the count evidence rather than a query.rfq_statusanswered over Nostr on a fresh transport key — the only way to read the solver'sfill_txidin relay mode.fundingTxidbefore the payout arrives: the orderingaccept()exists to guarantee, read back rather than trusted.take - carrierone way andcarrier - givethe other.If relay ingress is down the run fails rather than skipping, verified by stopping strfry: the solver reports
relay: unreachableand the harness refuses on it with exit 1.Proven
fillede308c2be…filledb58d26da…Balances landed on the quoted amounts to the satoshi in both directions, across two runs.
Notes
link-sdk.shpacks@arkade-os/sdkand@arkade-os/swapout of a localts-sdkcheckout, because the Nostr flow is ts-sdk#886 + ts-sdk#919 and the published swap package still prices that route from the card's feed. It packs rather than links so what the smoke imports is the tarball a release would ship, andsmoke-nostr.mjsasserts the behaviour rather than a version — a stale install fails loudly instead of quietly proving the wrong thing.Stack prerequisites are in the README: arkd's default block-denominated delays flip balances to
recoverablebefore a multi-step smoke can finish, soenv.smoke-rfqpins seconds mode and the scripts mine explicitly after funding.