Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,18 @@ default = []
#lightning-macros = { version = "0.2.0" }
#lightning-dns-resolver = { version = "0.3.0" }

lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144", features = ["std"] }
lightning-types = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144" }
lightning-invoice = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144", features = ["std"] }
lightning-net-tokio = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144" }
lightning-persister = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144", features = ["tokio"] }
lightning-background-processor = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144" }
lightning-rapid-gossip-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144" }
lightning-block-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144", features = ["rest-client", "rpc-client", "tokio"] }
lightning-transaction-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144", features = ["esplora-async-https", "time", "electrum-rustls-ring"] }
lightning-liquidity = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144", features = ["std"] }
lightning-macros = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144" }
lightning-dns-resolver = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144" }
lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c", features = ["std"] }
lightning-types = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c" }
lightning-invoice = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c", features = ["std"] }
lightning-net-tokio = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c" }
lightning-persister = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c", features = ["tokio"] }
lightning-background-processor = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c" }
lightning-rapid-gossip-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c" }
lightning-block-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c", features = ["rest-client", "rpc-client", "tokio"] }
lightning-transaction-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c", features = ["esplora-async-https", "time", "electrum-rustls-ring"] }
lightning-liquidity = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c", features = ["std"] }
lightning-macros = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c" }
lightning-dns-resolver = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c" }

bdk_chain = { version = "0.23.0", default-features = false, features = ["std"] }
bdk_esplora = { version = "0.22.0", default-features = false, features = ["async-https-rustls", "tokio"]}
Expand Down Expand Up @@ -81,13 +81,13 @@ async-trait = { version = "0.1", default-features = false }
vss-client = { package = "vss-client-ng", version = "0.5" }
prost = { version = "0.11.6", default-features = false}
#bitcoin-payment-instructions = { version = "0.6" }
bitcoin-payment-instructions = { git = "https://github.com/jkczyz/bitcoin-payment-instructions", rev = "679dac50cc0d81ec4d31da94b93d467e5308f16a" }
bitcoin-payment-instructions = { git = "https://github.com/benthecarman/bitcoin-payment-instructions", rev = "23bb47b2d568571c3191d59881ff048d21537ebd" }

[target.'cfg(windows)'.dependencies]
winapi = { version = "0.3", features = ["winbase"] }

[dev-dependencies]
lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "369a2cf9c8ef810deea0cd2b4cf6ed0691b78144", features = ["std", "_test_utils"] }
lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "2313bd584d2c46a50d67b8266f488c07516e0b3c", features = ["std", "_test_utils"] }
rand = { version = "0.9.2", default-features = false, features = ["std", "thread_rng", "os_rng"] }
proptest = "1.0.0"
regex = "1.5.6"
Expand Down
19 changes: 12 additions & 7 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ use lightning::util::persist::{
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
Expand All @@ -59,8 +58,9 @@ use crate::fee_estimator::OnchainFeeEstimator;
use crate::gossip::GossipSource;
use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{
read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache,
read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, read_scorer,
open_or_migrate_fs_store, read_all_objects, read_event_queue,
read_external_pathfinding_scores_from_cache, read_network_graph, read_node_metrics,
read_output_sweeper, read_peer_info, read_scorer,
};
use crate::io::vss_store::VssStoreBuilder;
use crate::io::{
Expand Down Expand Up @@ -644,18 +644,23 @@ impl NodeBuilder {
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
///
/// If the storage directory contains data from a v1 filesystem store, it will be
/// automatically migrated to the v2 format.
///
/// [`FilesystemStoreV2`]: lightning_persister::fs_store::v2::FilesystemStoreV2
pub fn build_with_fs_store(&self, node_entropy: NodeEntropy) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let mut storage_dir_path: PathBuf = self.config.storage_dir_path.clone().into();
storage_dir_path.push("fs_store");

fs::create_dir_all(storage_dir_path.clone()).map_err(|e| {
log_error!(logger, "Failed to setup Filesystem store: {}", e);
log_error!(logger, "Failed to set up Filesystem store: {e}");
BuildError::StoragePathAccessFailed
})?;
let kv_store = FilesystemStore::new(storage_dir_path);
let kv_store = open_or_migrate_fs_store(storage_dir_path)?;
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

Expand Down Expand Up @@ -1115,7 +1120,7 @@ impl ArcedNodeBuilder {
self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
pub fn build_with_fs_store(
&self, node_entropy: Arc<NodeEntropy>,
Expand Down
54 changes: 46 additions & 8 deletions src/io/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::io::Write;
use std::ops::Deref;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};

use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet;
Expand All @@ -26,14 +26,16 @@ use lightning::routing::scoring::{
ChannelLiquidities, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
};
use lightning::util::persist::{
KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
NETWORK_GRAPH_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE,
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_KEY,
OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE,
SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
migrate_kv_store_data, KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET,
KVSTORE_NAMESPACE_KEY_MAX_LEN, NETWORK_GRAPH_PERSISTENCE_KEY,
NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_KEY, OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error};
use lightning_types::string::PrintableString;

use super::*;
Expand All @@ -47,7 +49,7 @@ use crate::logger::{log_error, LdkLogger, Logger};
use crate::peer_store::PeerStore;
use crate::types::{Broadcaster, DynStore, KeysManager, Sweeper};
use crate::wallet::ser::{ChangeSetDeserWrapper, ChangeSetSerWrapper};
use crate::{Error, EventQueue, NodeMetrics};
use crate::{BuildError, Error, EventQueue, NodeMetrics};

pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache";

Expand Down Expand Up @@ -619,6 +621,42 @@ pub(crate) fn read_bdk_wallet_change_set(
Ok(Some(change_set))
}

/// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary.
///
/// If the directory contains v1 data (files at the top level), the data is migrated to v2 format
/// in a temporary directory, the original is renamed to `fs_store_v1_backup`, and the migrated
/// directory is moved into place.
pub(crate) fn open_or_migrate_fs_store(
storage_dir_path: PathBuf,
) -> Result<FilesystemStoreV2, BuildError> {
match FilesystemStoreV2::new(storage_dir_path.clone()) {
Ok(store) => Ok(store),
Err(FilesystemStoreV2Error::V1DataDetected(_)) => {
// The directory contains v1 data, migrate to v2.
let mut v1_store = FilesystemStore::new(storage_dir_path.clone());

let mut v2_dir = storage_dir_path.clone();
v2_dir.set_file_name("fs_store_v2_migrating");
fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
let mut v2_store = FilesystemStoreV2::new(v2_dir.clone())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

migrate_kv_store_data(&mut v1_store, &mut v2_store)
.map_err(|_| BuildError::KVStoreSetupFailed)?;

// Swap directories: rename v1 out of the way, move v2 into place.
let mut backup_dir = storage_dir_path.clone();
backup_dir.set_file_name("fs_store_v1_backup");
fs::rename(&storage_dir_path, &backup_dir)
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.

Codex:

  • P1 /tmp/ldk-node-pr872/src/io/utils.rs:650, /tmp/ldk-node-pr872/src/builder.rs:659: the v1 to v2 migration is not crash-recoverable. After fs_store is renamed to fs_store_v1_backup, a crash or error before fs_store_v2_migrating is renamed into place leaves no fs_store. On the next startup,
    build_with_fs_store recreates an empty fs_store, FilesystemStoreV2::new succeeds, and the node can start against empty storage while the real data sits in backup/temp dirs. The migration helper should detect and recover incomplete states before creating a fresh store.

Seems we still might want to double-check we survive an ill-timed crash?

.map_err(|_| BuildError::KVStoreSetupFailed)?;
fs::rename(&v2_dir, &storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;

FilesystemStoreV2::new(storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)
},
Err(_) => Err(BuildError::KVStoreSetupFailed),
}
}

#[cfg(test)]
mod tests {
use super::read_or_generate_seed_file;
Expand Down
11 changes: 2 additions & 9 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1699,15 +1699,8 @@ impl Node {
value: Amount::from_sat(splice_amount_sats),
script_pubkey: address.script_pubkey(),
}];
let contribution = self
.runtime
.block_on(funding_template.splice_out(
outputs,
min_feerate,
max_feerate,
Arc::clone(&self.wallet),
))
.map_err(|e| {
let contribution =
funding_template.splice_out(outputs, min_feerate, max_feerate).map_err(|e| {
log_error!(self.logger, "Failed to splice channel: {}", e);
Error::ChannelSplicingFailed
})?;
Expand Down
5 changes: 5 additions & 0 deletions src/payment/unified.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ impl UnifiedPayment {
PaymentMethod::LightningBolt12(_) => 0,
PaymentMethod::LightningBolt11(_) => 1,
PaymentMethod::OnChain(_) => 2,
PaymentMethod::Cashu(_) => 3,
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.

Seems you also bumped bitcoin-payment-instructions base with this? That's great, but let's make it a separate commit please.

});

for method in sorted_payment_methods {
Expand Down Expand Up @@ -331,6 +332,10 @@ impl UnifiedPayment {
let txid = self.onchain_payment.send_to_address(&address, amt_sats, None)?;
return Ok(UnifiedPaymentResult::Onchain { txid });
},
PaymentMethod::Cashu(_) => {
log_error!(self.logger, "Cashu payment methods are not supported. Skipping.");
continue;
},
}
}

Expand Down
62 changes: 57 additions & 5 deletions src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ impl Wallet {
witness_utxo: Some(input.previous_utxo.clone()),
..Default::default()
};
let weight = Weight::from_wu(input.satisfaction_weight);
let weight = ldk_to_bdk_satisfaction_weight(input.satisfaction_weight);
tx_builder.only_witness_utxo().exclude_unconfirmed();
tx_builder.add_foreign_utxo(input.outpoint, psbt_input, weight).map_err(|e| {
log_error!(self.logger, "Failed to add shared input for fee estimation: {e}");
Expand Down Expand Up @@ -916,7 +916,7 @@ impl Wallet {
witness_utxo: Some(input.previous_utxo.clone()),
..Default::default()
};
let weight = Weight::from_wu(input.satisfaction_weight);
let weight = ldk_to_bdk_satisfaction_weight(input.satisfaction_weight);
tx_builder.add_foreign_utxo(input.outpoint, psbt_input, weight).map_err(|_| ())?;
}

Expand Down Expand Up @@ -955,11 +955,41 @@ impl Wallet {
return Err(());
}

let change_output = unsigned_tx
let mut change_output = unsigned_tx
.output
.into_iter()
.filter(|txout| must_pay_to.iter().all(|output| output != txout))
.next();
.find(|txout| must_pay_to.iter().all(|output| output != txout));

// BDK's `create_tx` applies `ceil(fee_rate * weight)` separately for each
// fee component instead of once over the total weight, and LDK's
// `estimate_transaction_fee` does a single ceiling. Because LDK consumes
// our `change_output.value` verbatim and recomputes its own fee to size
// the new funding output, every sat BDK over-reserves gets funneled into
// the channel instead of staying in change. Absorb the worst-case
// over-reservation by bumping the change output up here.
//
// Two sources of rounding contribute:
//
// * Per-input ceilings — `bdk_wallet`'s `select_sorted_utxos` ceilings
// `fee_rate * (TxIn::default().segwit_weight() + satisfaction_weight)`
// once per input. With `N` inputs, the worst-case over-reservation is
// `N - 1` sats. TODO: drop once
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.

Please put this TODO on a dedicated line. (probably at the start or end of the comment).

Also, can we create an issue for this so we don't forget to drop this?

// https://github.com/bitcoindevkit/bdk_wallet/pull/479 lands in a
// release.
//
// * Initial-tx and change ceilings — `create_tx` ceilings
// `fee_rate * tx.weight()` on the recipient-only tx, and `decide_change`
// ceilings `fee_rate * Weight::from_vb(drain_output_len)`. Two more
// independent ceilings, contributing up to 2 sats.
if let Some(ref mut change) = change_output {
let num_inputs = (confirmed_utxos.len() + must_spend.len()) as u64;
let per_input_rounding_surplus_sat = num_inputs.saturating_sub(1);
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.

Codex:

  • P2 /tmp/ldk-node-pr872/src/wallet/mod.rs:986: the splice fee rounding adjustment adds the worst-case surplus to the change output instead of the actual BDK over-reservation. For example, at 1000 sat/kwu the per-component fee math is exact, so the actual rounding surplus is 0, but this
    still adds N + 1 sats to change. Since LDK derives value_added from inputs minus outputs/change/fees, splice_in(..., amount) can silently add less than the requested amount. This should compute the actual fee delta from the selected inputs/outputs, or cap the change bump to that delta.

let initial_and_change_rounding_surplus_sat = 2;
let bdk_rounding_surplus_sat =
per_input_rounding_surplus_sat + initial_and_change_rounding_surplus_sat;
change.value =
Amount::from_sat(change.value.to_sat().saturating_add(bdk_rounding_surplus_sat));
}

if change_output.is_some() {
locked_wallet.persist(&mut locked_persister).map_err(|e| {
Expand Down Expand Up @@ -1717,6 +1747,28 @@ impl ChangeDestinationSource for WalletKeysManager {
}
}

/// Convert LDK's `Input::satisfaction_weight` to the value BDK's
/// [`bdk_wallet::TxBuilder::add_foreign_utxo`] expects.
///
/// LDK and BDK disagree on what `satisfaction_weight` includes for a SegWit input. LDK
/// treats it as the full weight of the spent input's `script_sig` and `witness` *each
/// with their lengths included* — i.e., the empty `script_sig` length byte (4 WU) and
/// the witness-elements-count varint (1 WU) are part of the value. BDK adds
/// `TxIn::default().segwit_weight()` internally, which already accounts for those same
/// 5 WU (an empty TxIn has a 1-byte empty `script_sig` length and a 1-byte empty
/// witness-count varint). Passing LDK's value directly to BDK therefore double-counts
/// 5 WU per foreign input, which inflates BDK's fee estimate and ultimately funnels the
/// surplus into the new funding output during splice negotiation.
fn ldk_to_bdk_satisfaction_weight(ldk_satisfaction_weight: u64) -> Weight {
const EMPTY_SCRIPT_SIG_WEIGHT: u64 =
1 /* empty script_sig length byte */ * WITNESS_SCALE_FACTOR as u64;
const EMPTY_WITNESS_COUNT_WEIGHT: u64 = 1 /* witness elements count varint */;
Weight::from_wu(
ldk_satisfaction_weight
.saturating_sub(EMPTY_SCRIPT_SIG_WEIGHT + EMPTY_WITNESS_COUNT_WEIGHT),
)
}

// FIXME/TODO: This is copied-over from bdk_wallet and only used to generate `WalletEvent`s after
// applying mempool transactions. We should drop this when BDK offers to generate events for
// mempool transactions natively.
Expand Down
4 changes: 4 additions & 0 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ pub(crate) enum TestChainSource<'a> {
pub(crate) enum TestStoreType {
TestSyncStore,
Sqlite,
FilesystemStore,
}

impl Default for TestStoreType {
Expand Down Expand Up @@ -592,6 +593,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) ->
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
},
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
TestStoreType::FilesystemStore => {
builder.build_with_fs_store(config.node_entropy.into()).unwrap()
},
};

if config.recovery_mode {
Expand Down
Loading
Loading