Introduce Rust features - #1061
Conversation
Allow integration runs to constrain randomized chain sources through the LDK_NODE_TEST_CHAIN_SOURCES environment variable. This makes backend-specific feature builds deterministic while preserving all-backend randomization by default. Co-Authored-By: HAL 9000
Keep filesystem-specific code together so generic storage utilities can compile without that backend. This commit only relocates the existing migration behavior and tests. Co-Authored-By: HAL 9000
Hide the concrete Bitcoin Core gossip verifier behind the LDK UTXO lookup trait. This lets common gossip types compile independently of the Bitcoin Core backend without changing verification behavior. Co-Authored-By: HAL 9000
Keep human-readable-name resolution beside the unified payment code that consumes it. This commit only relocates the existing resolver and updates its internal import paths. Co-Authored-By: HAL 9000
Define the cfg-selected Builder name next to the implementation it represents. Feature-specific binding exports can now live there without changing the native and lock-wrapped builder APIs. Co-Authored-By: HAL 9000
|
👋 Thanks for assigning @benthecarman as a reviewer! |
Keep the native defaults while letting applications select only required chain sources, storage backends, and unified payment support. At least one chain source remains mandatory. PostgreSQL now has a storage-prefixed feature name. Co-Authored-By: HAL 9000
Separate backend-specific binding methods into cfg-gated implementation blocks. Method bodies and availability remain unchanged, making later conditional exports easier to review. Co-Authored-By: HAL 9000
Keep binding signatures and documentation next to their Rust implementations. Leave only the UDL object declaration so backend features can add methods without failure stubs. Co-Authored-By: HAL 9000
Build published bindings with uniffi-default and without native default features. Let local builds add backend features through LDK_NODE_EXTRA_FEATURES. Co-Authored-By: HAL 9000
Build and test UniFFI with its lean preset so excluded backends stay excluded. Check all features and test targets to catch incompatible optional dependencies without running the suite twice. Co-Authored-By: HAL 9000
List each chain, storage, payment, and binding feature and explain the unchanged native defaults. Show both lean and custom binding builds so optional backend dependencies can be selected deliberately. Co-Authored-By: HAL 9000
9b4022f to
7f339e5
Compare
benthecarman
left a comment
There was a problem hiding this comment.
Would be good to have CI cover the missed feature configs that claude caught
| feature = "chain-electrum", | ||
| feature = "chain-bitcoind" | ||
| )))] | ||
| compile_error!("at least one chain source feature must be enabled"); |
There was a problem hiding this comment.
should do same with db
| @@ -47,47 +47,6 @@ interface ProbingConfigBuilder { | |||
| }; | |||
|
|
|||
There was a problem hiding this comment.
bindings/ldk_node.udl:73 — unified_payment() is declared unconditionally in the UDL while the Rust side is gated on unified-payments, so the same combo also fails with E0425. Effectively uniffi requires storage-vss + unified-payments, but nothing enforces or documents that.
| use crate::config::{BackgroundSyncConfig, Config, WALLET_SYNC_INTERVAL_MINIMUM_SECS}; | ||
| use crate::fee_estimator::OnchainFeeEstimator; | ||
| use crate::logger::{log_debug, log_error, log_info, log_trace, LdkLogger, Logger}; | ||
| use crate::runtime::Runtime; |
There was a problem hiding this comment.
the vss_client import isn’t gated on storage-vss, so any uniffi build without it fails (E0433). The README’s own example command cargo build --no-default-features --features uniffi,chain-bitcoind,storage-postgres doesn’t compile.
| @@ -213,6 +213,7 @@ impl Wallet { | |||
| self.inner.lock().expect("lock").tx_graph().full_txs().map(|tx_node| tx_node.tx).collect() | |||
| } | |||
There was a problem hiding this comment.
src/wallet/mod.rs:2813— the in-crate wallet unit tests callChainSource::new_esploraungated, socargo testbreaks on any set withoutchain-esplora.
| @@ -362,25 +362,52 @@ pub(crate) fn setup_bitcoind_and_electrsd() -> (BitcoinD, ElectrsD) { | |||
| pub(crate) fn random_chain_source<'a>( | |||
There was a problem hiding this comment.
tests/common/mod.rs:45 — the integration-test harness imports SqliteStore unconditionally, so postgres-only test runs break.
| fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?; | ||
| recover_incomplete_fs_store_migration(&storage_dir_path)?; | ||
| if !storage_dir_path.exists() { | ||
| fs::create_dir_all(storage_dir_path.clone()) |
| let v1_store = FilesystemStore::new(storage_dir_path.clone()); | ||
|
|
||
| let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating"); | ||
| fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?; |
Closes #900
Previously, we omitted making use of Rust features to dis-/enable specific functionality, with the exception of
uniffiand most recently adding a tentativepostgresfeature.Here we finally change that and allow users to selectively disable and enable specific parts of the code base, hence also reducing their dependency tree.