Make bitcoind optional: add transaction-sync as an alternative chain backend

[bitwalt]

Implements #125.

Reworked per @zoedberg's review: instead of replacing lightning-block-sync with lightning-transaction-sync, this adds transaction-sync as an alternative chain backend and keeps block-sync intact. The sync mode is chosen explicitly at unlock time.

Sync modes

The /unlock payload gains a sync_mode tag:

• block_sync (default): consume full blocks from a trusted/local bitcoind over JSON-RPC (lightning-block-sync). The bitcoind_rpc_* parameters live inside this variant, so they are required exactly when this mode is selected and rejected otherwise — no manual "all or none" validation. This is the more trust-minimized / local-first option.
• transaction_sync: sync through the configured electrum/esplora indexer_url (lightning-transaction-sync). No bitcoind required — lighter to deploy for indexer-based setups.

Cargo features

Each backend is gated behind a feature (block-sync, transaction-sync), both enabled by default, so a user who wants only one sync type can build with only the dependency they need. Both single-feature builds compile cleanly, and requesting a mode that wasn't compiled in returns a clear error.

Implementation notes

• LDK is wired against trait objects for the fee estimator, broadcaster and gossip UTXO lookup, so a single set of type aliases serves both backends.
• Because lightning-block-sync's GossipVerifier requires the P2PGossipSync to be typed with Arc<Self>, a small BlockSyncGossipVerifier provides the block-sync gossip UTXO lookup against the shared trait-object gossip sync (mirroring the transaction-sync IndexerGossipVerifier).

Tests

Adds an integration test for the transaction-sync backend (as suggested): two nodes are unlocked in transaction-sync mode, an RGB asset channel is opened, a node is restarted (re-establishing the channel after syncing via the indexer) and the channel is cooperatively closed. The existing block-sync tests continue to pass unchanged.

[zoedberg]

Please check my comment #125 (comment), I explained why we should not replace the block-sync with the transaction-sync but add it instead as an alternative sync mode. Please update this PR accordingly

[zoedberg]

Hi @bitwalt, here #125 (comment) you said you agreed in updating this PR to add this as an alternative sync mode. Are you still willing to complete this PR?

[bitwalt]

Hi Zoe, sure I'll try to complete this

[bitwalt]

Thanks @zoedberg — I've reworked the PR along the lines you suggested.

Rather than replacing block-sync, it now keeps lightning-block-sync and adds lightning-transaction-sync as an alternative, selectable via an explicit sync_mode on the unlock request:

• block_sync (default) — full blocks from a trusted/local bitcoind. The bitcoind_rpc_* params are embedded in this variant so they're required exactly when this mode is chosen (no manual all-or-none checking).
• transaction_sync — sync through the configured electrum/esplora indexer only, no bitcoind.

Both are behind Cargo features (block-sync, transaction-sync), enabled by default, so a user can compile just the sync dependency they want; both single-feature builds compile cleanly.

I added the integration test you described (unlock 2 nodes, open a channel, restart a node, close the channel) exercising the transaction-sync path, and the existing block-sync tests still pass.

Two notes for review:

1. The /unlock payload shape changes (the bitcoind params now sit under the sync_mode: block_sync variant). You mentioned not worrying about existing setups in this phase, but happy to reshape if you'd prefer a different layout.
2. For the block-sync gossip UTXO lookup I added a small BlockSyncGossipVerifier instead of lightning-block-sync's GossipVerifier, because the latter requires the P2PGossipSync to be typed with Arc<Self>, which is incompatible with the trait-object gossip sync needed to serve both backends from one PeerManager type. Let me know if you'd rather approach that differently.

[zoedberg]

Thanks for updating this PR! Please check the requested changes and sign the commit

[bitwalt]

Thanks for the thorough review @zoedberg — reworked and squashed into a single signed commit. Rundown per thread:

chain_source (was ChainSource) — routes.rs / openapi.yaml

• Renamed to LdkChainSync and switched to #[serde(tag = "mode", content = "config")]. I kept rename_all = "snake_case" so the wire values stay block_sync / transaction_sync — dropping it flips them to BlockSync / TransactionSync, which would diverge from the README/openapi. Happy to drop it if you'd rather match the suggestion literally.
• Removed the doc comments on the (private) enum and its variants.
• Feature-gated both variants and dropped APIError::UnsupportedSyncMode accordingly.
• Removed #[serde(flatten)] — chain_source is now a nested object; openapi.yaml updated to match and all the per-field descriptions I'd added are gone.
• Moved the type to its alphabetical position.
• Added an optional indexer_url to the transaction_sync config, so the RGB wallet and LDK can use different indexer sources.

indexer.rs

• esplora get_best_block is now atomic: the height is derived from the tip hash we just fetched (get_block_status), so there's no hash/height mismatch if a block arrives mid-call.
• The transaction-sync gossip lookup now enforces the same six-confirmation depth as the block-sync path.
• On the "redundant electrum pass" (sync_electrum_registered_txs): I first deleted it and relied on ElectrumSyncClient::sync, but that regresses — the electrum transaction_sync test then deterministically hangs at funding lock-in (the channel never reaches channel_ready), while esplora is fine. So it's compensating for a concrete failure rather than being redundant. I kept it with a comment explaining exactly that, and reworked it so the registered_txs lock is not held across the electrum network calls (your other comment).

bitcoind.rs

• Ported upstream GossipVerifier's block cache into BlockSyncGossipVerifier.

Build / features

• Added the compile_error! guard in main.rs when neither feature is enabled, and a dedicated build-features CI job that builds each single feature.

README.md

• Moved the sync-mode + Cargo-feature explanation into a dedicated Sync modes section, dropped the "(default build)" wording, removed the sync_mode bullets from the network-specific service lists, and fixed the indexer wording.

esplora coverage

• The esplora backend was untested — added a blockstream/esplora service to compose.yaml (+ regtest.sh wiring) and split the test into transaction_sync_electrum and transaction_sync_esplora. The esplora variant also exercises the dedicated-LN-indexer path (LDK on esplora, wallet on electrum).

Verification

• Builds with default / block-sync-only / transaction-sync-only, and correctly fails with neither feature.
• cargo clippy -D warnings and cargo fmt --check clean across those feature sets.
• Both integration tests (transaction_sync_electrum, transaction_sync_esplora) pass against a real regtest bitcoind + electrs + esplora.

[bitwalt]

Thanks again @zoedberg — went through this round and pushed an updated (still signed) commit. Per thread:

ldk_chain_backend module + dedup

• Added src/ldk_chain_backend/ with block_sync.rs (was bitcoind.rs) and transaction_sync.rs (was indexer.rs); mod.rs now holds the chain-backend objects moved out of ldk.rs (ChainBackend, ChainSetup, DynFeeEstimator/DynBroadcaster, sync_chain_data) plus the shared fee logic that was duplicated across the two backends — default_fee_buckets, the FeeEstimator body (fee_from_bucket) and the fee-estimate store loop (store_fee_estimates, which was triplicated).

routes.rs / naming

• Dropped rename_all — the mode values are now BlockSync / TransactionSync, matching the existing style (JsonRpc, Nia).
• Renamed the field (and every helper/param) chain_source → ldk_chain_sync; start_node_with_sync → start_node_with.

indexer.rs (now transaction_sync.rs)

• Dropped the chain_hash check and removed the unused impl UtxoLookup for IndexerClient; the lookup logic now lives directly in IndexerGossipVerifier::get_utxo (and the now-unused network field is gone).

error.rs

• FailedBitcoindConnection and NetworkMismatch are now #[cfg(feature = "block-sync")] instead of allow(dead_code).

openapi.yaml

• ldk_chain_sync now uses oneOf + discriminator/mapping (on mode), like Assignment.

Cargo.toml / CI

• Multi-line [features]; the single-feature build job is in place.

README.md

• Moved the Sync-modes section down between Use and Test, dropped the inline JSON example (openapi covers it), switched to cargo install ... --no-default-features --features <backend>, and added a cross-reference from Install.

Tests

• Gated with transaction-sync (module-level #[cfg]).

• Extended to a 3-node node1 -> node2 -> node3 topology with two announced asset channels; node1 makes a multihop RGB payment to node3, which (no route hint for the public far channel) forces node1 to resolve the route from gossip and verify node2 -> node3's funding output through the indexer. Runs against both electrum and esplora, then restarts all nodes (re-establishing via the indexer) and cooperatively closes a channel. Both transaction_sync_electrum and transaction_sync_esplora pass against a real regtest bitcoind + electrs + esplora.

  One note: I kept a single cooperative close in the test. With two back-to-back coop closes, the second one (peer-initiated, on the node that had just processed the first close) didn't settle its returned on-chain balance within the wait under transaction-sync — the multihop payment itself (what covers the gossip lookup) and a single close both settle fine. Happy to dig into the double-close behaviour separately if you think it's worth a closer look.

Verification: fmt + clippy -D warnings clean on default / block-sync-only / transaction-sync-only; no-feature build fails with the compile_error!; both integration tests green.

[zoedberg]

@bitwalt please double check your changes and my requests, there are some unaddressed things

[zoedberg]

@bitwalt could you please rebase this PR on the updated master tip? Also please check my previous change requests, some were unaddressed. And finally note that we decided to drop the default indexer (see #147), so this PR will need to take that into account.

[codecov]

Codecov Report

❌ Patch coverage is 86.12245% with 102 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.82%. Comparing base (3e1223a) to head (85be6af).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/ldk_chain_backend/transaction_sync.rs	79.78%	75 Missing ⚠️
src/ldk.rs	91.21%	18 Missing ⚠️
src/ldk_chain_backend/block_sync.rs	90.80%	8 Missing ⚠️
src/error.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #126      +/-   ##
==========================================
- Coverage   87.96%   87.82%   -0.15%     
==========================================
  Files          14       16       +2     
  Lines        7829     8394     +565     
==========================================
+ Hits         6887     7372     +485     
- Misses        942     1022      +80     

Flag	Coverage Δ
rust	87.82% <86.12%> (-0.15%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[zoedberg]

Thanks for sticking with this.

I pushed a commit on top of yours instead of sending another round of comments, because most of it needed digging that was quicker to just do.

I found out why ElectrumSyncClient::sync was not confirming the funding transaction. LDK ignores the script_pubkey you pass to Filter::register_tx, and instead looks up the history of the transaction's first output. In RGB transactions that first output is the opret commitment, an OP_RETURN, and electrum servers don't index those, so the history comes back empty and the transaction never confirms. BDK hit exactly the same bug, which I fixed in bitcoindevkit/bdk#2195.

So the fix belongs in rust-lightning, not here. I patched our fork in RGB-Tools/rust-lightning#37 and dropped sync_electrum_registered_txs completely. Your workaround was compensating for a real bug, it just wasn't at the right layer. There's a new test that reproduces it in a few seconds, so a future LDK bump can't bring it back.

Other changes:

• the gossip lookup now checks that the funding output is unspent, like the block-sync path already did. Without it a node accepts replayed announcements of channels that have already been closed
• indexer_url in the TransactionSync config is no longer optional, since master dropped the default indexer
• the single-feature builds moved from build.yaml into lint.yaml, as a matrix over all 9 valid feature combinations
• the test suite no longer needs block-sync to compile
• mine() now waits for esplora too, but only in the test that syncs through it
• a few small cleanups

None of this changes the design you landed on, it all sits on top. @bitwalt please have a look and let me know if it looks good to you.

[bitwalt]

Hi Zoe, thanks for the clarification and for completing this. LGTM