Repository navigation
Conversation
…e and read state lists from it
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
arkana-ai-bot
left a comment
There was a problem hiding this comment.
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.
The asset RFQ lifecycle is now a state machine,
machines/assetRfq.ts, built withdefineMachinelikelnSend,lnReceiveandonchainSend. Before this,db/assetRfqSwaps.tshad its own state type,NON_TERMINAL,EXPOSEDandLEGAL_EDGES, and SQL that spelled out state lists by hand. No behaviour or schema change.What changes
AssetRfqMachinehas the same six states and edges as the oldLEGAL_EDGES:fillingis the only exposed state, andquoted,fundedandfillingare 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 onoffer_pk_scriptand byfindLiveByOfferScript;outgoing(filling) is used by the in-flight BTC payout sum.AssetRfqSwapState,NON_TERMINALandEXPOSEDcome from the machine, andlegalEdgesisAssetRfqMachine.edges.revertUnsubmittedstays off the graph, as before.Schema text
sqlListwrites'quoted','funded','filling','filled'with no spaces after the commas, where the old literal had them. The index isCREATE ... IF NOT EXISTSby name, so existing databases keep the index they have, and a new database gets an equivalent one.Tests
test/db/assetRfqMachine.test.tspins 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, inassetRfqSwaps.sql.tsinstead.