Skip to content

feat: pubky swaps as paykit route - #182

Open
ovitrif wants to merge 48 commits into
masterfrom
feat/pubky-swaps-route
Open

ovitrif wants to merge 48 commits into
masterfrom
feat/pubky-swaps-route

Conversation

@ovitrif

@ovitrif ovitrif commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Builds on #193, merged.
Refs:

Description

  • Adds a pubky_swaps module so that a Paykit payment routes through a Pubky Swaps provider when no balance the wallet holds reaches an endpoint the contact published. The user never picks the swap; the router does.
  • Extends the Paykit router of feat: pay contacts via spark #193 with swap/1 routes, following Paykit Capabilities §9.1. It runs the router's direct and service-leg check first. When no balance reaches the contact, it turns each provider's offer into swap/1 edges, pre-ranks them by indicative fee (base + ppm + onchain, enforced minimum), asks the cheapest three for a firm quote and picks the smallest debit for the exact amount the contact must receive. It returns whichever costs less in fees, the cheapest live service leg or the swap; a swap wins a tie.
  • Adds the first route, a reverse swap: the wallet pays a hold invoice over Lightning, the provider locks bitcoin on chain, and bitkit-core claims it straight to the contact's on-chain address. The lock covers the wallet's own claim fee, so the contact receives exactly the amount asked for.
  • Negotiates over signed paykit-cap.* messages (Capabilities §10) and refuses anything unsigned:
    • Only providers whose offer lists paykit-cap-v1 and direct-rpc-v1 are quoted. The offer is unsigned, so it only says who to ask; the route is priced on the signed quote.
    • swap-common's CapClient checks each reply. The quote's terms_hash binds this wallet, the provider, the request and the quote. The contract's commit_hash binds it to this wallet's commit. The hold invoice arrives sealed under a hash the contract signs.
    • A reply must be signed by the provider's own key, or by the capability signing key its published records delegate to (§5, Sign descriptors and terms with the delegated capability signing key (#127) coreyphillips/pubky-swap#129). The chain is the identity-signed Noise Key Authorization, then the routing key's delegation for the same generation. bitkit-core reads both records with its existing pubky client and checks the same bytes as paykit_identity::resolve_signing_key; the tests hold records upstream's code wrote.
    • Any other signer, and any unsigned reply to a signed request, is refused.
    • The commit carries the signed quote, and a provider that holds the swap without its signed contract gets the commit again. The signed contract is kept in the swap record.
  • Derives each swap's claim key and preimage from the seed and a random index, and sends the index to the provider as the swap's owner_ref (Owners can now list their swaps with SwapListRequest, paged and scoped like status queries, and a swap-client restore helper rebuilds them from the listing for a wallet that has only its seed coreyphillips/pubky-swap#113). A restored wallet or a second device never repeats a published preimage.
  • Adds pubky_swaps_restore for a wallet restored from its seed:
    • It lists the swaps of each provider that advertises swap-list-v1, all at once, derives their keys from owner_ref, and checks the network, the hash and the contract against them.
    • A swap gets a record only if the app's sent_payments holds a Lightning payment for its hash whose lockup is at least min_reverse_lockup_sat of it, and no other record holds that hash. Finished swaps get none.
    • A claim the provider lists counts only once the chain shows it with the preimage in its witness, or once the app's payment for it settled; then the wallet claims again once the lockup has its confirmations. With the payment pending, the listing counts for nothing.
    • Restored swaps claim to the wallet's own payout address, since the listing doesn't carry the contact's. The report names the swaps restored, already known and finished, and the providers whose list was cut short.
  • Keeps watching a broadcast claim on every pubky_swaps_advance until it confirms, sends it again if it drops out of the mempool, and fails a swap whose lockup the provider refunded or never made before the timeout. A spend of the lockup is a claim only when its witness carries the preimage, so a claim from another device counts and a refund never does.
  • Starts a claim only while the lockup's timeout is more than CLAIM_ABORT_MARGIN (6) blocks away, and keeps asking about a commit the provider reports as pending until it accepts or refuses.
  • Checks everything the provider sends before the app pays: the quote against the request, the contract against the quote and the chain tip, the HTLC rebuilt from the wallet's own keys, and the hold invoice's hash, amount and expiry. These are pubky-swap's own swap-common validators.
  • Finds providers in the caller's candidate list, usually the user's contacts: each is asked for its live offer over iroh pubky-swap/direct/1 at once, with 20 s to answer, and those that run no provider are skipped. iroh needs no homeserver on either side.
  • Derives the wallet's provider identity, claim keys and preimages from the seed, under domain strings of their own. Swap records hold the contract and the key index; the saved claim's witness carries the preimage, before the broadcast too.
  • Raises the Android Bindgen job's timeout from 60 to 90 minutes: master already takes 50 to 55 minutes, and iroh pushes the four-ABI build past the limit.
  • New exports: pubky_swaps_route_paykit_payment, pubky_swaps_commit_route, pubky_swaps_advance, pubky_swaps_provider_edges, pubky_swaps_restore.

This head is the prerelease 0.8.0-swaps.9. The app twins are synonymdev/bitkit-android#1732 and synonymdev/bitkit-ios#1170.

Cross-platform impact

  • New dependencies on Android and iOS: swap-common (git, coreyphillips/pubky-swap at f45e204, unaudited, which brings chacha20poly1305), iroh 1.2.0 and electrum-client 0.25. ed25519-dalek 2 becomes a direct dependency; it was already in the tree. The library size change against a release build is not measured yet.
  • The Swift package links Network.framework: iroh reads network interfaces through it, and apps that link BitkitCore without it fail with undefined nw_* symbols.
  • For the apps, beside the new exports:
    • PaykitRouteRequest has service_legs, and PaykitRoute has a Service case (from feat: pay contacts via spark #193).
    • PubkySwapRouteState has ClaimBroadcast, and Claimed means the claim is confirmed.
    • PubkySwapRouteQuote.quote_id is a route id the wallet assigns.
    • pubky_swaps_restore takes the wallet's sent Lightning payments, with whether each settled, and returns a PubkySwapRestoreReport.
  • Swap routes are refused on mainnet until the claim fee can be bumped.
  • No existing export outside this PR and feat: pay contacts via spark #193 changes.

What it needs to ship

  • Providers: mainnet Pubky Swaps providers with Lightning and on-chain liquidity, and a way to choose them. Today the app passes a configured list; Paykit Capabilities discovery (descriptors, follow graph, curated lists) is not built.
  • swap/1 in Paykit:
  • Delegated signer, live: the §5 path is covered by unit tests on upstream's records. The regtest run used a provider without a homeserver, which signs with its own key.
  • Backup of swap state: swap records are not in the wallet backup; pubky_swaps_restore rebuilds them from providers' listings, within their retention window. An app restart between paying and claiming leaves the claim to a later pubky_swaps_advance call; the app does not resume it yet. Funds stay safe through the timeouts: the provider cancels its hold invoice, or refunds its own lock.
  • Claim fee bumping: the claim needs a change output back to the wallet so confirm_or_bump can raise its fee until it confirms (up to timeout - CLAIM_ABORT_MARGIN) while the contact's output stays exact; until then, swap routes are refused on mainnet.
  • Resume: the apps need to resume a swap on start and wake when the lockup reaches depth, instead of their 10-minute regtest deadline. The app killed after commit, after funding and before the claim, and a seed restore mid-swap, are first-class test cases.
  • Restored payments: an unfinished swap rebuilt after a restore claims to the wallet's own address, so the contact is not paid by it. The apps need to show it as returned to Savings, pass the payments their node sent as sent_payments, and restore again once a pending one settles: a swap already on record then picks up a listed claim. Finished swaps get no record.
  • One execution path: move the route onto the persistence, backup, recovery and claim handling of @coreyphillips's feat/pubky-swap-boltz once it builds without a local path.
  • Fees: the quote covers the provider's service fee, its on-chain cost and the wallet's claim fee. The wallet's claim fee rate is fixed at 2 sat/vB in the Android caller and should come from Bitkit's fee estimates. Lightning routing fees to the provider are on top.
  • Out of scope here: submarine routes (savings to a Lightning-only contact), USDT (Pubky Swaps is Bitcoin and Lightning only), auto-pay route selection, and cooperative signing.

Preview

The Android twin's journey: a wallet with only a spending balance pays a contact who accepts only on-chain bitcoin. The router finds a Pubky Swaps provider among the wallet's contacts, the confirm screen shows the route, and the contact receives exactly 20,000 sats on chain (2x speed, regtest). This recording is from the twin's own bitkit-core build, before negotiation was signed; the signed path is proven at the bitkit-core level below.

swaps-route-android2x.mp4

QA Notes

Setup

  • coreyphillips/pubky-swap at f45e204: docker compose -f docker-compose.regtest.yml up -d (bitcoind, electrs, two LND nodes), then ./scripts/setup-regtest-lnd.sh, which opens a balanced channel from lnd-a to lnd-b.
  • The provider, on lnd-a: cargo build --release -p swap-provider --features full, then swap-provider --network regtest --directions submarine,reverse --min-amount 10000 --max-amount 1000000 --base-fee 100 --fee-ppm 2500 --confirmations 1 --wallet lnd --lnd-address https://127.0.0.1:10011 --lnd-cert <lnd-a tls.cert> --lnd-macaroon <lnd-a admin.macaroon> --electrum-url tcp://127.0.0.1:60001 --data-dir <dir> --no-homeserver, with PUBKY_SWAP_RECOVERY_PHRASE__FILE pointing at a throwaway BIP39 phrase. It prints its pubky.
  • lnd-b is the Lightning-only payer.

Automated Checks

  • added cap.rs tests: a quote signed by the provider is taken. These are refused:
    • a quote signed by another key, until that key is delegated;
    • a tampered quote, or one issued to another wallet;
    • a contract over another commit, a tampered contract or invoice blob, and a contract before any signed quote;
    • a status whose snapshot differs from the signed contract.
      A committed swap's status without its contract asks for the commit again.
  • added delegation.rs tests on records written by pubky-swap's paykit_identity at f45e204 (its golden vector included): the chain resolves to the delegated key for generations 1 and 2. These are refused: a delegation for another generation, records for another identity, and a swapped key, routing key, missing signature or unknown field.
  • added a transport.rs test: an unsigned reply to a signed request is refused.
  • extended the client.rs live test (ignored by default) to the full journey. It routes, commits and checks the signed contract, then runs PUBKY_SWAPS_PAY_CMD <invoice> alongside and PUBKY_SWAPS_MINE_CMD between pubky_swaps_advance calls until Claimed, and checks the claim pays the contact exactly the amount asked. Run with PUBKY_SWAPS_PROVIDER=<pubky> PUBKY_SWAPS_ELECTRUM=tcp://127.0.0.1:60001 PUBKY_SWAPS_DESTINATION=<bcrt1 address> PUBKY_SWAPS_PAY_CMD='docker exec lnd-b lncli -n regtest --lnddir=/home/lnd/.lnd payinvoice --force' PUBKY_SWAPS_MINE_CMD='docker exec bitcoin bitcoin-cli -regtest -rpcport=43782 -rpcuser=polaruser -rpcpassword=polarpass -generate 1' cargo test --lib pubky_swaps::client::live_tests -- --ignored --nocapture
  • ran cargo fmt --check, cargo clippy --lib --tests and cargo test --lib modules::pubky_swaps (41 tests): pass
  • ran the live test against the provider above, at d9a0e15 with the live test from ebbafb5: pass in 35 s.
    • The signed quote was for a 20,000 sat delivery: lock 20,278 sat, debit 20,986 sat.
    • lnd-b paid the 20,986 sat hold invoice, which stayed IN_FLIGHT until the claim and then SUCCEEDED.
    • pubky_swaps_advance reported AwaitingLockup, ClaimBroadcast and then Claimed.
    • The claim, 9fb48cd13ef28eec63109beab2fbd0b67b4c4f359aabfdc04248984352e177ed, pays exactly 20,000 sat to the contact's btc-regtest-p2wpkh address.

@ovitrif ovitrif self-assigned this Oct 29, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator

Reviewed from the pubky-swap side at 1abc3ff. The router, edges and swap-common validators line up with swap/1, and the commit path checks what it should before the app pays. Four things need fixing before mainnet, all in the reverse claim path, and then a proposal for converging with an older integration.

  1. Check the claim window before the first claim.

    • advance_blocking builds and broadcasts the claim as soon as the lockup has its confirmations (client.rs). It never checks against timeout_block_height.
    • A resume near the timeout therefore reveals the preimage in a race the wallet can't win: the provider can settle the hold invoice and still refund its lock.
    • pubky-swap's client refuses to start a claim within CLAIM_ABORT_MARGIN (6 blocks) of the timeout (reverse.rs; the reason is in timelock.rs).
    • swap_common::timelock::claim_start_is_safe is already in your dependency tree.
  2. Bump the claim fee until the claim confirms.

    • The fee is fixed at routing time from the caller's rate, which is 2 sat/vB on Android.
    • The record turns Claimed at broadcast, and nothing checks the claim or rebroadcasts it afterwards.
    • If the claim is evicted or stuck, the HTLC sits unclaimed after the provider has already settled. A provider that wanted to could then refund it after the timeout.
    • pubky-swap's client keeps bumping with swap_common::fee_bump::confirm_or_bump until timeout - CLAIM_ABORT_MARGIN (reverse.rs).
    • "The contact receives exactly the amount entered" leaves RBF no room, because the contact's output is the claim's only output. A change output back to the wallet can absorb the bumps and keep the delivered amount exact.
  3. The 10-minute claim deadline in the apps.

    • On mainnet the claim waits for 2 confirmations, which is swap-common's floor, MIN_MAINNET_CONFIRMATIONS.
    • So even when the provider funds within a minute, about 3 in 4 payments would show the payment error while the swap is still fine.
    • With no resume, the user's Lightning payment then stays held until the provider refunds its lock after the 144-block timeout and cancels the invoice. That is roughly a day, and at most 216 blocks.
    • This needs a resume path, and ideally a wake when the lockup reaches depth, before any deadline like this.
  4. Identity and pending.

One more, smaller: the edges use btc-{network}-* (router.rs), which PaymentEndpointIdentifier::new rejects. I've asked on pubky/paykit-rs#229 for swap/1 to list identifiers explicitly.

Converging with feat/pubky-swap-boltz.

  • My unmerged bitkit-core branch, feat/pubky-swap-boltz, runs Pubky swaps through the Boltz module via pubky-swap-boltz.
  • It already has much of what this PR lists under "What it needs to ship":
    • backup and restore of swap records (backup.rs);
    • recovery;
    • a claim window check;
    • swaps that pay an address (send.rs);
    • the user's own Pubky identity, including one imported through Ring.
  • I'd rather Bitkit ships one execution path: this PR's router and Paykit glue, on top of that branch's persistence, backup, recovery and claim handling.
  • The branch isn't a PR yet, and its Cargo.toml still points pubky-swap-boltz at a local path. So for now it's for reading, not building.

@ovitrif

ovitrif commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips thanks for the careful read.

  1. Check the claim window before the first claim.

Fixed in 160d443: advance_blocking now checks claim_start_is_safe(tip, timeout_block_height) before it builds the claim. Inside the margin, it reports Failed and leaves the swap to your refund, so the preimage never goes out in that race.

Separately, commit_route follows up on pending only once.

Also in 160d443: commit_route keeps querying status every 2 s while the provider answers pending, for up to 60 s. Nothing is paid before an acceptance, so a give-up costs nothing.

  1. Bump the claim fee until the claim confirms.

Agreed, and I'll take your change-output design: a change output back to the wallet absorbs the bumps, and the contact's output stays exact. The record then waits for the claim to confirm instead of turning Claimed at broadcast, and confirm_or_bump runs until timeout - CLAIM_ABORT_MARGIN. It's listed under What it needs to ship; this PR stays regtest-only until then.

  1. The 10-minute claim deadline in the apps.

Agreed. The 10 minutes is a regtest demo bound. Mainnet needs a resume on app start plus a background wake when the lockup reaches depth, with the error screen only for a real failure. Listed under What it needs to ship, together with the app-killed and seed-restore cases from pubky/paykit-rs#229.

  1. Identity and pending.

On identity: one seed-derived key keeps the user's Pubky unlinked from every provider. It also misses contact allowlists (coreyphillips/pubky-swap#88) and Encrypted Link recovery, as you say. That trade-off is better settled for swap/1 on pubky/paykit-rs#229 than here, so this PR keeps the separate key until it is.

the edges use btc-{network}-*, which PaymentEndpointIdentifier::new rejects

Right. The wildcard only labels the edge's family; the route itself always targets the contact's concrete identifier, for example btc-regtest-p2wpkh. I'll switch the edges to explicit identifiers once pubky/paykit-rs#229 lists them for swap/1.

I'd rather Bitkit ships one execution path

Agreed on one path: this PR's router and Paykit glue on top of feat/pubky-swap-boltz's persistence, backup, recovery and claim handling. Once your branch builds without the local path, I'll move the route onto it and drop the duplicate claim code here.

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two independent reviews.

needs changing before merge

  • Restored or reinstalled wallet reuses payment hashes whose preimages are public (src/modules/pubky_swaps/store.rs:56-66). After a seed restore, a reinstall, a wipe, or on a second device with the same seed, the next swap reuses the payment hash of the first swap, and that preimage is already public. store::reserve_index (src/modules/pubky_swaps/store.rs:59-61) returns 0 when <base>/pubky_swaps/next_index is missing. keys::swap_keys is a pure function of seed and index (keys.rs:50-66, and its own test asserts the preimage repeats for the same index). The first swap's claim put that preimage in its witness: swap-common build_htlc_spend pushes it at onchain.rs:287 at 8686702. Every Lightning hop that forwarded its hold-invoice payment also saw it at settlement. A provider (any contact that answers an offer) or a hop that knows the preimage can settle the new hold invoice without ever locking funds, and the wallet loses the debit. What I checked: the store default, the derivation, and the claim witness. Two things mitigate it in some cases. pubky-swap's own provider refuses a hash already in its store (ensure_reverse_hash_available, swap-provider/src/lib.rs:1828), so the same honest provider turns this into an invalid_request refusal; a different or dishonest provider has no such check. I did not verify whether the app's LDK node refuses to re-pay a hash it paid before; that depends on its payment store surviving the restore and is outside this repo. A random per-swap nonce mixed into the derivation and kept in the record would remove the reuse and still keep secrets out of the record. So would an index that cannot repeat, such as one seeded from the clock. The index file is also written without fsync, which gives a smaller version of the same reuse after a power loss.
  • Claim is broadcast once at a fixed fee and reported Claimed, with mainnet allowed (src/modules/pubky_swaps/client.rs:525-538). A claim that does not confirm before the timeout loses the whole debit, even to an honest provider, and the exports accept Network::Bitcoin. advance_blocking (client.rs:525-538) signs the claim with record.claim_fee_sat. That fee was fixed at route time from the app's fee_rate_sat_vb, which the description says is 2 sat/vB in the Android caller. It broadcasts once and stores claim_txid, and every later call returns Claimed at line 458 without looking at the chain again. The pinned provider settles the hold invoice as soon as it sees the claim in Electrum history, which includes the mempool (swap-provider/src/reverse.rs:570-600 and find_spend in swap-common chain/electrum.rs at 8686702). If that spend is not buried, it falls back to refunding. The claim signals RBF (ENABLE_RBF_NO_LOCKTIME), so once timeout_block_height passes, the provider's refund replaces a claim still sitting in the mempool. The invoice is then settled and the lock refunded. The description lists claim confirmation under what it needs to ship. Nothing in the code limits the exposure: route_paykit_payment and commit_route accept mainnet. Either land a re-sign/bump path that keeps watching until the claim is buried, or refuse Network::Bitcoin in these exports until it exists.
  • Seed restores reuse previously disclosed payment preimages (src/modules/pubky_swaps/store.rs:59-64). Restoring the seed without local swap state can reuse a preimage that an earlier claim already published. reserve_index restarts at zero, and swap_keys derives the preimage only from the seed and index. I confirmed identical preimages and payment hashes before and after removing next_index in an isolated test. A provider knowing the old preimage can settle the new hold invoice without funding its lockup.
  • Unconfirmed claims become permanently terminal (src/modules/pubky_swaps/client.rs:535-538). A broadcast claim receives no further protection even if it never confirms. Saving claim_txid makes every later advance return before checking the chain. I confirmed this with mocked chain responses: an unconfirmed broadcast remained Claimed when the chain became unavailable. If the claim stalls or is evicted, its public preimage lets the provider settle Lightning and later refund the lockup. The claim must remain monitored and bumpable while preserving the recipient's amount.

worth doing, does not block

  • Commit accepts a Taproot contract that advance can never find (src/modules/pubky_swaps/client.rs:389-396). commit_route asks for SwapScript::P2wsh but never checks that accept.script_type came back as P2wsh. swap-common's verify_contract says the caller must do that check (validate.rs:487-488). A Taproot acceptance with a consistent tree passes verification (client.rs:389-396), and the Taproot address is returned to the app as htlc_address. advance_blocking then always rebuilds a P2WSH script (client.rs:468-474), never finds the lockup, and reports AwaitingLockup until the provider refunds. Funds come back when the hold invoice is cancelled, but the wallet's Lightning payment stays locked until then. Reject any script_type other than P2wsh before validating.
  • advance has no terminal state for a missing or already spent lockup (src/modules/pubky_swaps/client.rs:478-538). If the provider never funds, advance_blocking returns AwaitingLockup { confirmations: 0 } forever, even after timeout_block_height has passed (client.rs:496-502). The app gets no signal to stop polling. If the app dies between the broadcast and store::save (client.rs:535-537), the next call rebuilds the same claim and rebroadcasts it. The broadcast fails with already-known or inputs-spent, so every call returns a Chain error and the record never reaches Claimed. The same happens if the record file is lost after a successful claim. The script history already fetched at line 481 contains any spend of the outpoint. Checking it before broadcasting, and returning Failed once the tip passes the timeout with no lockup, would close both.
  • Offers are not bound to the candidate that answered (src/modules/pubky_swaps/client.rs:222-235). The router keys offers and edges by the provider_pkarr written inside the offer (client.rs:229, router.rs:70), not by the candidate whose iroh connection returned it. quote_edge then connects to that claimed key (client.rs:142). So any contact can send the wallet's quote and commit traffic to an endpoint that was never in the candidate list. It can also name another candidate's key, which duplicates that provider's edge and spends the 3-quote fanout on it twice. The validators still apply, so this is not a theft path, but it undoes the restriction to the providers the caller passed. Drop offers where bare_pubky(&offer.provider_pkarr) != provider.
  • Probing every contact through n0 DNS discloses the contact list (src/modules/pubky_swaps/transport.rs:30). DirectClient::bind uses presets::N0 (transport.rs:30). In iroh 1.2.0 that preset resolves endpoint ids over HTTPS and DNS through n0's servers, and publishes the wallet's own address there (endpoint/presets.rs:116-138). route_paykit_payment probes every candidate at once (client.rs:207-221), and the description says the candidates are usually the user's contacts. Each payment that needs a swap therefore sends n0 a burst of lookups for the user's whole contact list, from one stable swap identity and IP. The fan-out is also unbounded, so a large contact list opens that many concurrent connections on a phone. The PR's claim that iroh needs no homeserver is true, but it does depend on n0's infrastructure. Consider capping the candidate count and documenting or replacing the n0 address lookup.
  • Swap store ignores a second init_db path (src/modules/pubky_swaps/store.rs:36-41). store::init saves the base path in a OnceCell and ignores later calls (store.rs:39). init_db replaces the activity, Blocktank and Boltz connections on every call. After a second init_db with a different base path, swap records and next_index keep going to the first directory while everything else moves.
  • Commit accepts contract types that advance cannot claim (src/modules/pubky_swaps/client.rs:388-396) [one reviewer only]. A provider can return TaprootBoltz despite the request selecting P2wsh, and commit still returns the invoice. I confirmed this with a valid Taproot acceptance and hold invoice in an isolated test. verify_contract validates the returned type, but advance_blocking only watches and spends P2WSH, so it cannot claim that lockup.
  • Expired unfunded swaps remain AwaitingLockup forever (src/modules/pubky_swaps/client.rs:496-508). An unfunded swap never becomes terminal after its timeout. The missing-lockup and insufficient-confirmation returns run before the timeout check. I reproduced an empty history at tip 2,000 with timeout 1,144, which still returned AwaitingLockup. Callers waiting for a terminal status can poll forever.
  • Sync permanently preserves an unknown fee rate (src/modules/activity/implementation.rs:1093). An outgoing activity first stored with an unknown rate (0) cannot acquire its rate through later syncs. I reproduced a preserving upsert of zero followed by a confirmed snapshot at 12 sat/vB: the absolute fee and confirmation updated, but the rate stayed zero. Preserve known positive rates while allowing an unknown rate to be filled.
  • The new dependency exceeds the declared Rust minimum (Cargo.toml:51). The pinned iroh 1.2.0 requires Rust 1.91, while this package still advertises Rust 1.90. Dependency validation rejects builds using the advertised minimum. Raise the package minimum or select a compatible dependency.

nits

  • Quote cache never evicts (src/modules/pubky_swaps/client.rs:279-286). QUOTES (client.rs:43) only drops an entry on a successful commit (line 428). Every routed payment the user abandons, and every expired quote, stays in memory for the life of the process. Pruning expired entries on insert would do.

the reviewers disagree, your call

  • Quote ID collisions change the committed recipient (src/modules/pubky_swaps/client.rs:279-285). Objection: Real, but non-blocking rather than blocking: Real: QUOTES is a process-global map keyed by the provider-chosen quote_id, so a second route with the same ID overwrites the first, and committing the first uses the second's address and debit. Not blocking: it needs two uncommitted routes in one process, the redirected recipient is one the wallet itself routed to, and the provider gains nothing.
  • Offers can redirect payments outside the provider list (src/modules/pubky_swaps/client.rs:227-229). Objection: Real, but non-blocking rather than blocking: Real: the queried provider is dropped and offers/edges are keyed by offer.provider_pkarr, which is never compared with the peer that answered. A listed candidate can therefore steer the wallet to an unlisted provider, or overwrite another provider's offer. Not blocking: the HTLC doesn't depend on trusting the provider, the counterparty's quote and contract are still checked, and the review screen shows its real pubky.

@ovitrif

ovitrif commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips all four blocking points are addressed in 7621dfc, with iOS bindings in 64cdede.

Restored or reinstalled wallet reuses payment hashes whose preimages are public

Each swap's claim key and preimage now derive from the seed and a random 32-byte nonce, under new v2 domains. The nonce lives in the record. next_index is gone, so a restore, a reinstall, a wipe or a second device on the same seed can't repeat a published preimage. Records are written with fsync on the file and its directory. Test: fresh_nonces_never_repeat_a_preimage.

Claim is broadcast once at a fixed fee and reported Claimed, with mainnet allowed

I took the option to refuse mainnet. pubky_swaps_route_paykit_payment answers None with a reason on Network::Bitcoin, and pubky_swaps_commit_route refuses it. Fee bumping with a change output stays under What it needs to ship; mainnet opens with it.

Unconfirmed claims become permanently terminal

advance now decides from the HTLC's script history and the tip on every call, in a pure claim::next_step with unit tests:

  • A broadcast claim reports the new ClaimBroadcast until it confirms. Claimed means confirmed.
  • A saved claim missing from the history is broadcast again, whatever the window, since its preimage is public.
  • The claim is saved before its first broadcast. A failed broadcast is logged rather than returned, so the app keeps polling.
  • A refund spend of the lockup is Failed, even after our claim went out.
  • A claim found on chain that the record missed is recorded.

Commit accepts a Taproot contract that advance can never find

Fixed: commit_route rejects any script_type other than P2WSH before validating.

advance has no terminal state for a missing or already spent lockup

Fixed. No lockup by the timeout is Failed. A spent lockup is read from the history before anything is broadcast. A crash between save and broadcast sends the same saved claim again.

Offers are not bound to the candidate that answered

Fixed: an offer whose provider_pkarr isn't the candidate that answered is dropped.

Probing every contact through n0 DNS discloses the contact list

Candidates are capped at 30, in the caller's order, so configured providers come first. The n0 lookups are documented on the cap. Replacing n0 discovery belongs with moving negotiation onto Encrypted Links (pubky/paykit-rs#229).

Swap store ignores a second init_db path

Fixed: the store's base path moves with every init_db.

Sync permanently preserves an unknown fee rate

Not in this PR's diff; it comes from #166 on master. Out of scope here, tracked in #186.

The new dependency exceeds the declared Rust minimum

Fixed: rust-version is now 1.91.

Quote cache never evicts

Fixed: expired quotes are pruned on every insert.

Quote ID collisions change the committed recipient

Fixed: the cache is keyed by an id the wallet assigns to each route. The provider's quote id stays inside the cached entry.

Offers can redirect payments outside the provider list

Fixed with the offer binding above.

@ovitrif

ovitrif commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed cdfdcae, following @coreyphillips's swap/1 notes on pubky/paykit-rs#229 (pubky/paykit-rs#229 (comment)):

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at 61cc8e4, mostly the restore path, from reading the code. Swaps are refused on mainnet, so nothing here is live, but 1 to 3 need fixing before that changes.

1. A listed claim_observed_txid_hex can get the preimage revealed early.

restored_record sets claim_published from the provider's listing, and next_step then returns Rebroadcast before it checks confirmations or the claim window. With no saved claim, advance builds and sends a new one. So a provider can list any string, leave its lockup unconfirmed, read the preimage from the mempool, settle the hold invoice and replace the lockup. That's the attack validate_quote in swap-common guards against. The same value also passes is_saved, so a refund txid listed there reports Claimed.

Treat a listed txid as published only if it's on chain and its witness carries the preimage (swap_common::onchain::extract_preimage). That keeps the stuck claim I raised working while the claim can be fetched. A claim that has left the mempool can't be proven this way, so that path should at least wait for the lockup's confirmations.

2. The restored amount isn't checked.

verify_contract binds the keys, hash and timeout but not the value, and restore skips validate_accept and validate_hold_invoice. So a provider can list the real hash and claim key with a small onchain_amount_sat, fund that, and settle the full hold invoice once the wallet claims. A second listing under a new swap_id works too, because records are keyed on swap_id alone.

Comparing against accept.invoice won't fix this, because the provider can sign a lower invoice on the same hash. The amount has to come from something the wallet holds, such as what its Lightning node paid for that hash. Please also refuse a second record with the same hash. swap_client::restore has the same gap and needs its own fix.

3. A real claim can be reported as a refund.

With no observed txid in the listing, a claim from the original device matches neither the record's claim_txid nor payout_address, so next_step reports "The provider refunded its lockup". The contact was paid, so the user may pay again. Nothing re-reads the listing, so it stays that way. extract_preimage tells a claim from a refund by its witness, and also closes the refund case in 1.

Also worth fixing

  • Finished swaps restore as live. restored_record checks neither snapshot.state nor snapshot.network, and list_swaps asks with active_only: false. swap_client::restore checks both. A finished payment comes back with the wallet's payout address, and the PR tells apps to show that as returned to Savings even though the contact was paid. What do you intend for finished swaps?
  • Restore can overwrite a record. store::load(...).is_ok() treats any read error as no record. It should match only NotFound.
  • Restored ids can be lost. Only the call that writes a record returns its id, and no export lists records. If the app dies first, it has no id for pubky_swaps_advance. RestoreReport.known in swap_client::restore covers this.
  • One failed page loses a provider's list. The ? drops the pages already fetched, and restore still returns Ok.
  • Restore can be slow. It doesn't check the offer for swap-list-v1, isn't capped at MAX_CANDIDATES, and visits providers one at a time. An older provider never replies, so each one costs the 60-second request timeout while a live claim window may be closing.

Minor

  • ClaimBroadcast is also reported when publish_claim sent nothing. The retry is fine, but the state overclaims.
  • README.md and store.rs say a record holds no secret, but the saved claim carries the preimage before broadcast.
  • Please check Package.swift: it still pins v0.8.0-swaps.2 from 30 Oct, and the bindings have changed since.

A listed claim no longer counts as published, a claim is told from a refund by the preimage in its witness, and a restored swap needs a Lightning payment the wallet sent for its hash that covers its lockup. Restore also skips finished swaps and other networks, refuses a second record for a hash, reads only a missing record as absent, keeps partial lists, asks swap-list-v1 providers at once, and reports known and incomplete ids.
@ovitrif

ovitrif commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips thanks, all fixed in ca83325 and baf7655, with the bindings and the prerelease 0.8.0-swaps.5 in 7e1e52f.

1. A listed claim_observed_txid_hex can get the preimage revealed early.

A listed txid no longer counts as published, and nothing trusts it as a claim. The record keeps only a claim_listed flag. next_step takes a spend of the lockup as our claim only when extract_preimage finds the preimage in its witness, so a claim still in the mempool or on chain is found that way. With the flag set and no such spend in the history, the wallet waits for the lockup's confirmations and only then claims again, past the window too. is_saved is gone, so a refund txid listed as the claim fails as a refund.

2. The restored amount isn't checked.

pubky_swaps_restore now takes sent_payments, the Lightning payments the wallet's node sent, by hash and amount. A swap gets a record only if its hash is there and the payment less onchain_amount_sat is at most max_fee_bps of that lockup. That's the bound validate_quote holds a fresh quote to, the one min_reverse_lockup_sat from coreyphillips/pubky-swap#134 inverts. I'll switch to that helper when the swap-common pin moves. The apps report what their node paid, routing fees excluded. A second listing of a hash already on record, or seen earlier in the same restore, gets no record. I saw coreyphillips/pubky-swap#133 and coreyphillips/pubky-swap#134 close the swap_client::restore side.

3. A real claim can be reported as a refund.

Fixed by the same witness check: a claim from the original device is found by its preimage whatever address it pays, and a spend without it is the refund. destination and delivered_sat no longer decide this.

Finished swaps restore as live. What do you intend for finished swaps?

They get no record. A listed swap in a terminal state goes into the report's finished, and a swap on another network is skipped. The PR body now asks the apps to show only an unfinished restored swap as returned to Savings.

Restore can overwrite a record.

It now matches NotFound only. Any other read error skips that swap with a warning.

Restored ids can be lost.

The restore returns a PubkySwapRestoreReport with restored, known and finished, so calling it again lists the ids written by a run that died.

One failed page loses a provider's list.

A failed page keeps the pages read before it, and the provider goes into the report's incomplete_providers so the app can restore again.

Restore can be slow.

It asks for each provider's offer first, with the 20 s offer timeout, lists only the ones that advertise swap-list-v1, caps them at MAX_CANDIDATES and asks them all at once.

ClaimBroadcast is also reported when publish_claim sent nothing.

It's now reported only once a server took the claim. Before that, advance reports AwaitingLockup with the lockup's confirmations and sends the claim again on the next call.

README.md and store.rs say a record holds no secret

Both now say the saved claim's witness carries the preimage before the broadcast, and so does the PR body.

Please check Package.swift

It pins v0.8.0-swaps.5 now, built from this head.

@ovitrif
ovitrif requested a review from coreyphillips November 9, 2026 08:32
Base automatically changed from feat/paykit-router to master November 9, 2026 10:13
@ovitrif

ovitrif commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator Author

#193 merged as 50065eb. This PR already targets master, so its diff now leaves out #193's changes.

@coreyphillips

Copy link
Copy Markdown
Collaborator

@ovitrif thanks. I checked ca83325 and baf7655 at 7e1e52f: items 1 to 3 and the rest are fixed as you describe. The witness check is sound for this script, because a refund can't carry a 32-byte element that hashes to the payment hash. min_reverse_lockup_sat is on pubky-swap main at coreyphillips/pubky-swap@f45e204 for when the pin moves.

One case is left, which my suggestion on 1 didn't cover.

A listed claim goes out past the window. With claim_listed set and the lockup confirmed, next_step claims whatever the window, past the timeout too. So for a wallet restored after the window closed, a provider can list a claim it never saw, replace the wallet's claim with its refund once the timeout passes, and settle the hold invoice with the preimage that claim revealed. The invoice outlives the timeout by design, so the provider keeps both legs. Without the listing, the same swap keeps the preimage and the payment fails back. swap_client does the same.

Past the window, send a listed claim only when the wallet's node shows the payment settled, since only then is the preimage known to be public. PubkySwapSentPayment would need to say whether it settled. While the payment is pending, treat the swap as unlisted. An honest provider settles as soon as it sees a claim, so a listing with the payment still pending has nothing behind it.

Minor: providers past MAX_CANDIDATES are dropped without appearing in incomplete_providers, and so is a list task that panics.

@ovitrif

ovitrif commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips thanks, fixed in af67f55, with the bindings and the prerelease 0.8.0-swaps.6 in 7aa9937.

A listed claim goes out past the window.

PubkySwapSentPayment now carries settled, and a restored record sets claim_listed only when the provider lists a claim and the node shows that payment settled. With the payment pending, the swap restores as unlisted, so past the window it keeps the preimage and the payment fails back. A settled listed claim still waits for the lockup's confirmations. ignores_a_listed_claim_while_the_payment_is_pending covers it.

One limit: a payment that settles after the restore isn't picked up, because the record keeps what the restore saw. If the claim is still in the mempool or on chain, advance finds it by its preimage either way.

Minor: providers past MAX_CANDIDATES are dropped without appearing in incomplete_providers, and so is a list task that panics.

Both now go into incomplete_providers. Providers past the cap are added up front, and any provider whose task never returns a listing is added at the end.

…pubky-swaps-route

# Conflicts:
#	Cargo.lock
#	Cargo.toml
#	Package.swift
#	bindings/android/gradle.properties
Quotes and contracts now come over paykit-cap on direct/1 and are checked by swap-common's CapClient: terms and commit hashes, signatures, the invoice blob and both parties. A reply must be signed by the provider's key or the capability signing key its published records delegate to (Capabilities §5); unsigned replies and providers without paykit-cap-v1 are refused. swap-common moves to f45e204, and restore uses its min_reverse_lockup_sat.
@ovitrif
ovitrif marked this pull request as ready for review November 9, 2026 17:34
@ovitrif

ovitrif commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips ready for review. Since your last pass:

  • d9a0e15 moves negotiation to signed paykit-cap.* over direct/1, checked by swap-common's CapClient. A reply must be signed by the provider's key or the capability signing key its records delegate to (§5, Sign descriptors and terms with the delegated capability signing key (#127) coreyphillips/pubky-swap#129). Unsigned replies, and providers without paykit-cap-v1, are refused. I wrote the §5 chain on bitkit-core's existing pubky client rather than add paykit-identity, which would bring a second pubky, pkarr and mainline into the mobile build; its tests hold records your code wrote, golden vector included.
  • swap-common is now at f45e204, and restore uses min_reverse_lockup_sat.
  • ebbafb5 runs the whole reverse swap against a current provider on regtest. The QA notes have the run: signed quote and contract, lnd-b paying the hold invoice, and the claim paying the contact exactly 20,000 sat.
  • Prerelease 0.8.0-swaps.7 is at 4c8a792.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-09T17:56:01.014672Z 4c8a792 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at 4c8a792. af67f55 closes the gap: a listing counts only once the payment settled, so a late claim reveals nothing the provider doesn't already hold. The incomplete_providers changes cover both cases.

I checked d9a0e15 against pubky-swap too. The delegation check matches paykit_identity byte for byte, the authorization check matches paykit-rs and is slightly stricter, and a signer whose records don't verify or can't be read is refused.

Two follow-ups, not blocking:

  • Once the payment settled, the listing adds nothing, because the settled payment alone shows the preimage is public. Setting claim_listed from payment.settled alone would also claim past the window when a provider settled but left the claim out of its listing. Like the limit you noted, that only matters when the wallet's own claim never confirmed.
  • No CI job runs modules::pubky_swaps. A job like paykit-route-tests would keep these tests running.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4c8a7929f8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/modules/pubky_swaps/claim.rs Resolved
Comment thread src/modules/pubky_swaps/client.rs Resolved
An unsent saved claim waits for the lockup's confirmations again after a reorganization, and a claim of a lockup the provider replaced is built again for the new outpoint.

@talosmachina talosmachina 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.

1 finding (1 P2). Adds Pubky Swaps as a Paykit route: signed paykit-cap quotes and contracts with delegated signer resolution, seed-derived swap keys with an owner_ref index, a reverse-swap claim driver over Electrum, and restore from provider listings. Reviewed 1fee62c, oversized tier (about 3,700 non-generated lines).

What I checked, and 6 candidates I ruled out

Read in full: pubky_swaps/keys.rs, claim.rs, cap.rs (non-test), delegation.rs, router.rs, store.rs, transport.rs, client.rs L106-1112, the paykit_route and pubky/profile.rs changes, Cargo.toml.
Upstream checked: swap-common at f45e204 (validate_quote, validate_accept, validate_hold_invoice, min_reverse_lockup_sat, claim_start_is_safe, swap-client/src/restore.rs).
Not read: client.rs tests and the live regtest test, lib.rs exports, generated bindings.
Build: not built here. talos-pr-run refused because it could not check the author's permission. This review is static.
CI: SwiftPM, manifests, paykit-route, wake tests green; Android/iOS bindgen and usdt pending at review time. No workflow runs cargo test modules::pubky_swaps.
Cold reader: did not finish within this run, so the finding is my own reading of L865-868, L1088 and claim::next_step.

Ruled out

  • Unsigned or wrongly signed replies: checker always pins the provider root key, transport::cap_reply rejects non-cap replies, and a delegated key is accepted only when delegation::verify_chain checks owner, generation and both signatures.
  • Lockup smaller than paid on restore: bounded by min_reverse_lockup_sat against the node's own payment, and the hash must derive from this seed.
  • Early preimage reveal from a provider's listing: a listed claim needs a settled payment and the lockup's confirmations (claim.rs L125-136).
  • Refund mistaken for our claim: spends are classified by preimage in the witness, not by destination (claim.rs L98).
  • Lost broadcast reply hiding a published preimage: claim_published is saved before the send and cleared only on a protocol refusal of the first attempt.
  • Mainnet exposure with an unbumpable claim: best_swap_route and commit_route both refuse Network::Bitcoin.

Merge confidence: 3/5, oversized tier ceiling; the remainder was not read and the PR was not built here.

Comment thread src/modules/pubky_swaps/client.rs Resolved
@ovitrif

ovitrif commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips thanks. Since your approval at 4c8a792 I pushed fixes for three findings from Codex and Talos, so this PR needs your approval again:

  • 2395691: an unsent saved claim waits for the lockup's confirmations again after a reorganization, and a claim of a lockup the provider replaced is built again for the new outpoint.
  • 8663ae7: a restore marks a swap already on record once the provider lists a claim and the node shows that payment settled, so a restore made while the payment was in flight no longer stays unmarked.
  • 31e6e23 is the prerelease 0.8.0-swaps.9. 1fee62c only changed how the live test logs.

I'll take both of your follow-ups in one PR after this merges: setting claim_listed from a settled payment alone, and a CI job for modules::pubky_swaps.

@ovitrif
ovitrif requested a review from coreyphillips November 9, 2026 18:34
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.

3 participants