Skip to content

refactor(asset-rfq): define the asset RFQ lifecycle as a state machine - #357

Open
d4rp4t wants to merge 1 commit into
mainfrom
refactor/asset-rfq-machine
Open

d4rp4t wants to merge 1 commit into
mainfrom
refactor/asset-rfq-machine

Conversation

@d4rp4t

@d4rp4t d4rp4t commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

The asset RFQ lifecycle is now a state machine, machines/assetRfq.ts, built with defineMachine like lnSend, lnReceive and onchainSend. Before this, db/assetRfqSwaps.ts had its own state type, NON_TERMINAL, EXPOSED and LEGAL_EDGES, and SQL that spelled out state lists by hand. No behaviour or schema change.

What changes

  • AssetRfqMachine has the same six states and edges as the old LEGAL_EDGES: filling is the only exposed state, and quoted, funded and filling are live. It adds two flags for the lists the SQL wrote by hand:
    • holdsOfferScript (quoted, funded, filling, filled) is used by the partial unique index on offer_pk_script and by findLiveByOfferScript;
    • outgoing (filling) is used by the in-flight BTC payout sum.
  • The store reads everything from the machine. AssetRfqSwapState, NON_TERMINAL and EXPOSED come from the machine, and legalEdges is AssetRfqMachine.edges.
  • revertUnsubmitted stays off the graph, as before.

Schema text

sqlList writes 'quoted','funded','filling','filled' with no spaces after the commas, where the old literal had them. The index is CREATE ... IF NOT EXISTS by name, so existing databases keep the index they have, and a new database gets an equivalent one.

Tests

  • test/db/assetRfqMachine.test.ts pins the machine to the lists and edges the store enforced before it existed, and checks that the store's exports come from the machine.
  • pnpm test: 6568/6568 pass, typecheck is clean.

#356 (repositories with SQL in *.sql.ts) touches the same file. Whichever lands second gets a small rebase: the same substitutions, in assetRfqSwaps.sql.ts instead.

@coderabbitai

coderabbitai Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 13453bda-1be4-438f-aaa7-cd7889e5d19a
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@arkana-ai-bot arkana-ai-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review — Arkana

This is a clean structural refactor. No protocol changes, no schema changes, no behavioural delta. The state graph, exposed set, and legal edges are identical to what was previously hardcoded. Notes below:


Correctness: state graph preserved ✓

packages/solver-corridors/src/machines/assetRfq.ts declares the same six states and the same six edge lists that LEGAL_EDGES spelled out:

quoted   → funded, refused
funded   → filling, refused
filling  → filled, stuck
filled   → []
refused  → []
stuck    → []

NON_TERMINAL / EXPOSED / legalEdges in the store all read from the machine. test/db/assetRfqMachine.test.ts pins all three groups to their former literal values.


SQL template literals — no injection risk ✓

packages/solver-corridors/src/db/assetRfqSwaps.ts:199 and :394

AssetRfqMachine.sqlList(AssetRfqMachine.where('holdsOfferScript'))
// → 'quoted','funded','filling','filled'

sqlList is list.map(s => \'${s}'`).join(','). The list is derived entirely from the statically-defined machine; no user input can reach it. The template literal in SCHEMA` is evaluated once at module initialisation, not per-query. No injection surface exists.

The absence of spaces after commas versus the old literal ('quoted', 'funded', 'filling', 'filled') is irrelevant to SQLite. The PR body notes this and correctly explains why CREATE … IF NOT EXISTS by name leaves existing databases untouched.


committedSats — IN vs = ✓

packages/solver-corridors/src/db/assetRfqSwaps.ts:418

Old: state = 'filling'. New: state IN ('filling') (single-element IN, because outgoing is only set on filling). These are semantically identical. SQLite's query planner treats a single-element IN as an equality predicate. No behavioural change.


revertUnsubmitted — correctly off-graph ✓

packages/solver-corridors/src/db/assetRfqSwaps.ts:462

The filling → funded back-edge is not in AssetRfqMachine.edges (the comment is updated accordingly). The raw SQL CAS path is the only route, and that is correct.


FlagsOf<T> type safety ✓

where('holdsOfferScript') and where('outgoing') are statically checked via FlagsOf<T>. Passing an undeclared flag is a compile error.


Minor nit — no test for unknown-flag behaviour

where returns [] silently for a flag not set on any state. All callers pass flags that are type-checked, so this cannot happen in practice. Worth noting if defineMachine grows more callers, but not a problem here.


Cross-repo impact

No other repo clone imports from @arkade-os/solver-corridors/db/assetRfqSwaps or machines/assetRfq. The re-export export type { AssetRfqSwapState } from the db module preserves the public surface for any future consumer.


No changes requested. The refactor holds up — behaviour is preserved, the machine is fully pinned by tests, and the SQL construction is safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants