Repository navigation
feat: read L1 through Electrum instead of Esplora - #1101
Conversation
ts-sdk#1110 adds the two seams the Electrum switch needs: a `MessageBus`
`onchainProvider` factory for the worker wallet, and `{ provider }` for the
onchain corridor's chain source. Until a release ships it, both packages
are built from b90b356 and vendored.
The swap tarball asks for `@arkade-os/sdk@0.5.0-rc.25`, which exists on npm
without those seams, so the override forces every `@arkade-os/sdk` in the
tree to the vendored build. The lockfile holds a single instance.
`ws-electrumx-client` 1.0.5, the version the SDK already pins, becomes a
direct dependency: the wallet now opens the `ElectrumWS` itself.
The worker wallet (boarding UTXOs, onchain balance, chain tip, exit broadcasts), exit dating, and the swap onchain corridor (its HTLC watch and the trader's claim) now read L1 through `ElectrumOnchainProvider` at the SDK's `ELECTRUM_WS_URL[network]`. `electrum.ts` keeps one provider per network for the life of the page or worker and never closes it: ws-electrumx-client shares one options object across every `ElectrumWS`, so closing one would stop the others reconnecting. The claim fee rate still comes from mempool's REST API at `ESPLORA_URL[network]`. Electrum's `estimatefee` answers -1 on regtest, where the claim would then refuse to reveal the preimage, and it is not the `fastestFee` tier the claim deliberately uses. `getRestApiExplorerURL` and the explorer `api` URLs had no other reader, so they go.
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Configure and wire the Electrum provider src/lib/electrum.ts, src/wallet-service-worker.ts, src/test/lib/electrum.test.ts, package.json, pnpm-workspace.yaml, vendor/README.md |
The wallet creates and caches an Electrum provider per network and passes it to the MessageBus. Dependencies point to local vendor tarballs, and the vendor README documents their source and regeneration steps. Tests check provider caching and network-specific WebSocket URLs. |
Use Electrum for exit-history lookups src/lib/exitHistory.ts, src/lib/explorers.ts, src/lib/onchainPayout.ts, src/test/lib/exitHistory.test.ts |
Exit-history lookups use the Electrum provider when a network has an Electrum endpoint. Explorer configuration keeps web URLs and removes REST API URL lookup. The shared provider type replaces EsploraProvider in exit-history code, and tests cover Electrum lookup outcomes. Comments in onchainPayout.ts distinguish L1 reads from fee estimates. |
Use Electrum for swap chain sources src/lib/swapClient.ts, src/test/lib/swapClientOnchainClaim.test.ts |
On-chain claims and corridor configuration use the Electrum provider instead of EsploraProvider. Claim fee estimation continues to use onchainClaimEndpoint(network). Tests check the provider passed to the corridor and claim flow. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Refactor
Possibly related issues
arkade-os/ts-sdkissue 1105 concerns lockup-fate detection during on-chain swaps. This change switches the provider used for related on-chain reads, but the incorrect refund classification is not shown as fixed.
Suggested reviewers: pietro909, bordalix
Merge Risk: 
8b0b7
-
No actionable merge-blocking issue was identified. Complete the planned Electrum and regtest checks before release.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "feat: read L1 through Electrum instead of Esplora" matches the main change. The raw summary shows the Electrum provider replacing EsploraProvider in exitHistory.ts, swapClient.ts, and the m… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "feat: read L1 through Electrum instead of Esplora" matches the main change. The raw summary shows the Electrum provider replacing EsploraProvider in exitHistory.ts, swapClient.ts, and the message bus. The objectives confirm that L1 reads and broadcasts move to Electrum over WebSocket. The title is concise and single-sentence. It does not cover the vendored SDK tarballs or the removed explorer API helpers, but those details are secondary.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Deploying wallet-mutinynet with
|
| Latest commit: |
33765b4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e80b154b.arkade-wallet.pages.dev |
| Branch Preview URL: | https://feat-electrum-onchain-provid.arkade-wallet.pages.dev |
Deploying wallet-bitcoin with
|
| Latest commit: |
33765b4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c6f4ffa5.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://feat-electrum-onchain-provid.wallet-bitcoin.pages.dev |
|
@coderabbitai review |
|
The worker's `MessageBus` calls the provider factory on every init and never closes what it returns, so a factory that opened a socket per call would leak one per re-init. This pins the cache that prevents it.
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performed
|
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Summary
Clean, focused migration from Esplora REST to Electrum WebSocket for all L1 reads. The SDK-level interface (OnchainProvider) is preserved, the fee-rate path intentionally stays on Esplora REST (with documented rationale), and the singleton-per-network design is sound for the service-worker context. Findings below in priority order.
1. blockTime: 0 edge case in ElectrumOnchainProvider.getTxStatus — low probability, wrong data persisted
src/lib/exitHistory.ts:68 (confirmedAt)
ElectrumOnchainProvider.getTxStatus (SDK chunk-LPEE2H2M.cjs:~20265) uses this flow:
let blockTime = 0
try {
const header = await this.chain.fetchBlockHeader(merkle.blockHeight)
blockTime = parseBlockHeader(header.hex).timestamp
} catch (err) {
if (!isMissingHeightError(err)) throw err // ← height errors are swallowed; blockTime stays 0
}
return { confirmed: true, blockHeight: merkle.blockHeight, blockTime }
If fetchBlockHeader throws an "unknown height" error (a reorg race or a freshly confirmed tx whose block the server hasn't indexed yet), blockTime is 0 and getTxStatus returns { confirmed: true, blockTime: 0 }.
confirmedAt in exitHistory.ts does:
return status?.confirmed ? status.blockTime : undefinedstatus.blockTime is 0 — not undefined — so it passes the undefined-guard in resolveExits and gets persisted via saveTransactionActivityMetadata(txid, { exitedAt: 0 }). The exit then appears dated at Unix epoch (1970-01-01), sorted to the top/bottom depending on direction, permanently — the record won't be revisited once stored.
Esplora's getTxStatus never returned blockTime: 0 for a confirmed tx; it returned a real block time from the REST response. This is a silent behavioral regression.
Fix: treat blockTime: 0 as undatable in confirmedAt:
return status?.confirmed && status.blockTime ? status.blockTime : undefined2. electrumProvider passes ELECTRUM_WS_URL[network] unchecked for callers that skip the guard
src/lib/electrum.ts:10
exitHistory.ts correctly guards with name && ELECTRUM_WS_URL[name] before calling electrumProvider. swapClient.ts and wallet-service-worker.ts call it unconditionally. ELECTRUM_WS_URL is typed Record<NetworkName, string> so every valid NetworkName has an entry — no runtime hazard today. But electrumProvider's own parameter type is NetworkName, so the contract is enforced at compile time rather than at runtime. Worth a defensive check or an assertion, especially since wallet-service-worker.ts passes the factory directly as onchainProvider and the network is resolved later inside MessageBus.
3. ws-electrumx-client "shared options object" comment is imprecise
src/lib/electrum.ts:6-7
Looking at the ElectrumWS constructor: this.options = Object.assign({ reconnect: true }, options). Each instance gets its own object regardless of what options was passed in. The footgun only fires if the same external options object is passed to multiple ElectrumWS constructors. The wallet never does this — it calls new ElectrumWS(ELECTRUM_WS_URL[network]) with no options argument. The comment explains the right constraint (don't call close()) but the stated mechanism is misleading; a reader who checks the library source will doubt the comment. Consider tightening to: "closing any connection sets options.reconnect = false on the underlying options object; if the same options reference were shared across instances that would silence all reconnection, so we never close."
4. Coupling to swap SDK internals in swapClientOnchainClaim.test.ts
src/test/lib/swapClientOnchainClaim.test.ts:93
expect(chain).toEqual({ chain: [electrum, 'regtest'] })This asserts the internal return shape of chainSourceFrom. If @arkade-os/swap/protocol ever changes the wrapper (e.g., wraps in a class instance rather than a plain object), this assertion will either silently pass (wrong) or need manual updating. The important invariant is that the Electrum provider is threaded through — consider asserting chain.chain[0] is the provider directly rather than the full structure.
5. Testnet Electrum endpoint is dead; any code reaching electrumProvider('testnet') queues forever
src/lib/electrum.ts:10
Per PR description: wss://electrum.blockstream.info:60004 timed out. ELECTRUM_WS_URL.testnet points there. The wallet doesn't ship testnet, and the LOOKUP_TIMEOUT_MS race in confirmedAt covers the exit-history path. But if makeSwapClient or the service worker were ever invoked with network 'testnet', requests would queue on a perpetually-reconnecting socket. Low risk now; worth a note in electrum.ts or a comment in onchainPayout.ts.
6. electrum.test.ts does not verify ElectrumOnchainProvider constructor argument order
src/test/lib/electrum.test.ts:12
The test checks that ElectrumWS was called with the right URLs but mocks ws-electrumx-client and never checks what the returned ElectrumOnchainProvider received. If electrum.ts accidentally swapped the ElectrumWS and network arguments to new ElectrumOnchainProvider(...), the test would still pass. The tsc --noEmit step catches this at compile time, but an explicit check that the provider for 'regtest' is constructed with the regtest network would strengthen the unit.
Minor / no action needed
- Removal of
getRestApiExplorerURL: no other consumers found in the repo or sibling SDK clones. Clean. onchainClaimEndpointstill usingESPLORA_URL: intentional and documented.estimatefee(1)returning -1 on regtest is the right reason to keep this on REST.ElectrumWSreconnect loop: requests block (not reject) while disconnected. The 5s timeout inconfirmedAthandles exit-history correctly; service-worker L1 ops stall until reconnect, which matches the pre-existing behaviour of an unresponsivefetch.- Danger CI: passed.
Blocking on finding #1 (blockTime=0 persisted at epoch). Findings #2–#6 are non-blocking.
Summary
Switches the wallet's L1 reads and broadcasts from Esplora REST to Electrum over WebSocket, at the SDK's per-network defaults (
ELECTRUM_WS_URL):MessageBus({ onchainProvider: electrumProvider }).getTxStatus.corridors.onchain.chain: { provider }) and the trader's L1 claim (chainSourceFrom).src/lib/electrum.tskeeps oneElectrumOnchainProviderper network for the life of the page or worker and never closes it.ws-electrumx-clientshares one options object across everyElectrumWS, so closing one would stop the others reconnecting.getRestApiExplorerURLand the explorerapiURLs had no other reader, so they go.Not changed
/v1/fees/recommendedatESPLORA_URL[network]). Electrum'sestimatefee(1)answers -1 on regtest, which would make the claim refuse to reveal the preimage, and it is not thefastestFeetier the claim uses on purpose.Endpoints
Probed on 2026-10-25 with
blockchain.headers.subscribe:wss://electrum.arkade.shwss://electrum.mutinynet.arkade.shwss://electrum.signet.arkade.shws://localhost:50003testnet's default (
wss://electrum.blockstream.info:60004) timed out. The wallet doesn't ship testnet.Test plan
format:check,lint,tsc --noEmitandbuildare clean, as on master.test:unit: master 105 files / 1045 passed, branch 106 / 1046. The exit-history and swap-claim tests now stand in for the Electrum seam instead ofEsploraProvider;electrum.test.tspins one connection per network across worker re-inits.wsimport.7d1d542f): created a wallet and attached CDP to the service worker. Its socket carries Electrum JSON-RPC (scripthash.listunspent,get_history,scripthash.subscribe,headers.subscribeat height 3533793, then 11block.headerreads for MTP), and its HTTP goes only to the operator, indexer and delegator. No Esplora. The page opens its own socket towss://electrum.mutinynet.arkade.sh.Summary by CodeRabbit