From ec4395cf6eae14605cd41fc715a08ac0bcd786c3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 20 Oct 2023 17:31:42 +0000 Subject: [PATCH 01/10] Apply a default max fee rather than none when paying for BOLT12 If the user declines to specify a `max_total_routing_fee_msat` in the new BOLT12 payment methods, rather than defaulting to no limit on the fee we pay at all, we should default to our "usual default", ie the one calculated in `RouteParameters::from_payment_params_and_value`. We do this here, as well as documenting the behavior on the payment methods. --- lightning/src/ln/channelmanager.rs | 6 ++++++ lightning/src/ln/outbound_payment.rs | 14 +++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c9eb534db08..4f3caef03e3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7362,6 +7362,9 @@ where /// invoice. If abandoned, or an invoice isn't received before expiration, the payment will fail /// with an [`Event::InvoiceRequestFailed`]. /// + /// If `max_total_routing_fee_msat` is not specified, The default from + /// [`RouteParameters::from_payment_params_and_value`] is applied. + /// /// # Privacy /// /// Uses a one-hop [`BlindedPath`] for the refund with [`ChannelManager::get_our_node_id`] as @@ -7421,6 +7424,9 @@ where /// - `amount_msats` if overpaying what is required for the given `quantity` is desired, and /// - `payer_note` for [`InvoiceRequest::payer_note`]. /// + /// If `max_total_routing_fee_msat` is not specified, The default from + /// [`RouteParameters::from_payment_params_and_value`] is applied. + /// /// # Payment /// /// The provided `payment_id` is used to ensure that only one invoice is paid for the request diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index a0aca510574..0fbb0f5eaf4 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -762,7 +762,6 @@ impl OutboundPayments { } } - #[allow(unused)] pub(super) fn send_payment_for_bolt12_invoice( &self, invoice: &Bolt12Invoice, payment_id: PaymentId, router: &R, first_hops: Vec, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, @@ -779,7 +778,7 @@ impl OutboundPayments { SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { let payment_hash = invoice.payment_hash(); - let mut max_total_routing_fee_msat = None; + let max_total_routing_fee_msat; match self.pending_outbound_payments.lock().unwrap().entry(payment_id) { hash_map::Entry::Occupied(entry) => match entry.get() { PendingOutboundPayment::AwaitingInvoice { retry_strategy, max_total_routing_fee_msat: max_total_fee, .. } => { @@ -795,11 +794,12 @@ impl OutboundPayments { hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice), }; - let route_params = RouteParameters { - payment_params: PaymentParameters::from_bolt12_invoice(&invoice), - final_value_msat: invoice.amount_msats(), - max_total_routing_fee_msat, - }; + let pay_params = PaymentParameters::from_bolt12_invoice(&invoice); + let amount_msat = invoice.amount_msats(); + let mut route_params = RouteParameters::from_payment_params_and_value(pay_params, amount_msat); + if let Some(max_fee_msat) = max_total_routing_fee_msat { + route_params.max_total_routing_fee_msat = Some(max_fee_msat); + } self.find_route_and_send_payment( payment_hash, payment_id, route_params, router, first_hops, &inflight_htlcs, From 50c55dcf32466e3ccf17acea50697fc664950deb Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 20 Oct 2023 17:34:12 +0000 Subject: [PATCH 02/10] Use `Default::default()` for scoring params in tests In 26c1639ab69d6780c97a118f09e42cb42304088a we switched to using `Default::default()` to initialize `()` for scoring parameters in tests. A number of `()`s slipped back in recently, which we replace here. --- lightning/src/ln/payment_tests.rs | 2 +- lightning/src/routing/router.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index e25d8f06e45..b593a73eeb1 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -1871,7 +1871,7 @@ fn do_test_intercepted_payment(test: InterceptTest) { let route_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat); let route = get_route( &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph.read_only(), None, - nodes[0].logger, &scorer, &(), &random_seed_bytes + nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes ).unwrap(); let (payment_hash, payment_secret) = nodes[2].node.create_inbound_payment(Some(amt_msat), 60 * 60, None).unwrap(); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index fe73b37cf8b..8e45a877962 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -5417,7 +5417,7 @@ mod tests { max_total_routing_fee_msat: Some(149_999) }; if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route( &our_id, &route_params, &network_graph.read_only(), None, Arc::clone(&logger), - &scorer, &(), &random_seed_bytes) { + &scorer, &Default::default(), &random_seed_bytes) { assert_eq!(err, "Failed to find a sufficient route to the given destination"); } else { panic!(); } } @@ -7357,7 +7357,7 @@ mod tests { let route_params = RouteParameters::from_payment_params_and_value( payment_params, amt_msat); if let Err(LightningError { err, .. }) = get_route( - &our_id, &route_params, &netgraph, None, Arc::clone(&logger), &scorer, &(), &random_seed_bytes + &our_id, &route_params, &netgraph, None, Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes ) { assert_eq!(err, "Failed to find a path to the given destination"); } else { panic!() } @@ -7414,7 +7414,7 @@ mod tests { let route_params = RouteParameters::from_payment_params_and_value( payment_params, amt_msat); if let Err(LightningError { err, .. }) = get_route( - &our_id, &route_params, &netgraph, None, Arc::clone(&logger), &scorer, &(), &random_seed_bytes + &our_id, &route_params, &netgraph, None, Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes ) { assert_eq!(err, "Failed to find a path to the given destination"); } else { panic!() } @@ -7492,7 +7492,7 @@ mod tests { payment_params, amt_msat); if let Err(LightningError { err, .. }) = get_route( &our_id, &route_params, &netgraph, Some(&first_hops.iter().collect::>()), - Arc::clone(&logger), &scorer, &(), &random_seed_bytes + Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes ) { assert_eq!(err, "Failed to find a path to the given destination"); } else { panic!() } @@ -7578,7 +7578,7 @@ mod tests { let route = get_route( &our_id, &route_params, &netgraph, Some(&first_hops.iter().collect::>()), - Arc::clone(&logger), &scorer, &(), &random_seed_bytes + Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes ).unwrap(); assert_eq!(route.paths.len(), 1); assert_eq!(route.get_total_amount(), amt_msat); From d974a07e96199932e98f6c34b8f3e1f151b53500 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 20 Oct 2023 17:38:19 +0000 Subject: [PATCH 03/10] Avoid a redundant allocation in `InvoiceError` handling in one case ... by passing an owned `String`, rather than taking an `&str` and `to_owned()`ing it. --- lightning/src/ln/channelmanager.rs | 8 ++++---- lightning/src/offers/invoice_error.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 4f3caef03e3..20dba4cf8e6 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8967,10 +8967,10 @@ where match invoice.sign(|invoice| self.node_signer.sign_bolt12_invoice(invoice)) { Ok(invoice) => Ok(OffersMessage::Invoice(invoice)), Err(SignError::Signing(())) => Err(OffersMessage::InvoiceError( - InvoiceError::from_str("Failed signing invoice") + InvoiceError::from_string("Failed signing invoice".to_string()) )), Err(SignError::Verification(_)) => Err(OffersMessage::InvoiceError( - InvoiceError::from_str("Failed invoice signature verification") + InvoiceError::from_string("Failed invoice signature verification".to_string()) )), }); match response { @@ -8986,7 +8986,7 @@ where OffersMessage::Invoice(invoice) => { match invoice.verify(expanded_key, secp_ctx) { Err(()) => { - Some(OffersMessage::InvoiceError(InvoiceError::from_str("Unrecognized invoice"))) + Some(OffersMessage::InvoiceError(InvoiceError::from_string("Unrecognized invoice".to_owned()))) }, Ok(_) if invoice.invoice_features().requires_unknown_bits_from(&self.bolt12_invoice_features()) => { Some(OffersMessage::InvoiceError(Bolt12SemanticError::UnknownRequiredFeatures.into())) @@ -8994,7 +8994,7 @@ where Ok(payment_id) => { if let Err(e) = self.send_payment_for_bolt12_invoice(&invoice, payment_id) { log_trace!(self.logger, "Failed paying invoice: {:?}", e); - Some(OffersMessage::InvoiceError(InvoiceError::from_str(&format!("{:?}", e)))) + Some(OffersMessage::InvoiceError(InvoiceError::from_string(format!("{:?}", e)))) } else { None } diff --git a/lightning/src/offers/invoice_error.rs b/lightning/src/offers/invoice_error.rs index 441dae265cb..5476ad551b7 100644 --- a/lightning/src/offers/invoice_error.rs +++ b/lightning/src/offers/invoice_error.rs @@ -50,10 +50,10 @@ pub struct ErroneousField { impl InvoiceError { /// Creates an [`InvoiceError`] with the given message. - pub fn from_str(s: &str) -> Self { + pub fn from_string(s: String) -> Self { Self { erroneous_field: None, - message: UntrustedString(s.to_string()), + message: UntrustedString(s), } } } From 803fb557e805a27c63db48dd72d311f73e0ad4e7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 20 Oct 2023 18:09:39 +0000 Subject: [PATCH 04/10] Drop unused `use` import. --- lightning/src/chain/package.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 3d29e5a5522..1b938d98137 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -24,7 +24,7 @@ use crate::ln::PaymentPreimage; use crate::ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment}; use crate::ln::chan_utils; use crate::ln::msgs::DecodeError; -use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT, compute_feerate_sat_per_1000_weight, fee_for_weight, FEERATE_FLOOR_SATS_PER_KW}; +use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT, compute_feerate_sat_per_1000_weight, FEERATE_FLOOR_SATS_PER_KW}; use crate::sign::WriteableEcdsaChannelSigner; use crate::chain::onchaintx::{ExternalHTLCClaim, OnchainTxHandler}; use crate::util::logger::Logger; From d7668e1d12249b8e064fbdecabaab1caba6572c8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 20 Oct 2023 18:11:56 +0000 Subject: [PATCH 05/10] Remove a redundant sentence in `ConfirmationTarget` docs ... and correct direction which causes force-closure in another sentence. --- lightning/src/chain/chaininterface.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index dfb90135c88..10676b7190d 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/lightning/src/chain/chaininterface.rs @@ -71,14 +71,12 @@ pub enum ConfirmationTarget { /// [`ChannelConfig::max_dust_htlc_exposure`]: crate::util::config::ChannelConfig::max_dust_htlc_exposure MaxAllowedNonAnchorChannelRemoteFee, /// This is the lowest feerate we will allow our channel counterparty to have in an anchor - /// channel in order to close the channel if a channel party goes away. Because our counterparty - /// must ensure they can always broadcast the latest state, this value being too high will cause - /// immediate force-closures. + /// channel in order to close the channel if a channel party goes away. /// /// This needs to be sufficient to get into the mempool when the channel needs to - /// be force-closed. Setting too low may result in force-closures. Because this is for anchor - /// channels, we can always bump the feerate later, the feerate here only needs to suffice to - /// enter the mempool. + /// be force-closed. Setting too high may result in force-closures if our counterparty attempts + /// to use a lower feerate. Because this is for anchor channels, we can always bump the feerate + /// later; the feerate here only needs to be sufficient to enter the mempool. /// /// A good estimate is the expected mempool minimum at the time of force-closure. Obviously this /// is not an estimate which is very easy to calculate because we do not know the future. Using From d2532dce341411597d098c30c3b5f202de527dba Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 20 Oct 2023 18:19:57 +0000 Subject: [PATCH 06/10] Remove some additional excess words in `ConfirmationTarget` docs --- lightning/src/chain/chaininterface.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index 10676b7190d..73707f05236 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/lightning/src/chain/chaininterface.rs @@ -85,13 +85,10 @@ pub enum ConfirmationTarget { /// (package relay) may obviate the need for this entirely. MinAllowedAnchorChannelRemoteFee, /// The lowest feerate we will allow our channel counterparty to have in a non-anchor channel. - /// This needs to be sufficient to get confirmed when the channel needs to be force-closed. - /// Setting too low may result in force-closures. /// /// This is the feerate on the transaction which we (or our counterparty) will broadcast in - /// order to close the channel if a channel party goes away. Because our counterparty must - /// ensure they can always broadcast the latest state, this value being too high will cause - /// immediate force-closures. + /// order to close the channel if a channel party goes away. Setting this value too high will + /// cause immediate force-closures in order to avoid having an unbroadcastable state. /// /// This feerate represents the fee we pick now, which must be sufficient to enter a block at an /// arbitrary time in the future. Obviously this is not an estimate which is very easy to From bbb8facbe6ded97d7612c2ec6126812e96df1a22 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 21 Oct 2023 01:08:38 +0000 Subject: [PATCH 07/10] Fix (and test) the `c_bindings` build flag Rather than only building with the `c_bindings` flag in certain crates, we go ahead and test all crates with the flag in CI here. --- ci/ci-tests.sh | 12 ++++++----- lightning-background-processor/src/lib.rs | 26 +++++++++++++++-------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh index c24f344f6df..676e94b4478 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -100,14 +100,16 @@ popd echo -e "\n\nTesting no-std flags in various combinations" for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do - pushd $DIR - cargo test --verbose --color always --no-default-features --features no-std + cargo test -p $DIR --verbose --color always --no-default-features --features no-std # check if there is a conflict between no-std and the default std feature - cargo test --verbose --color always --features no-std + cargo test -p $DIR --verbose --color always --features no-std +done +for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do # check if there is a conflict between no-std and the c_bindings cfg - RUSTFLAGS="--cfg=c_bindings" cargo test --verbose --color always --no-default-features --features=no-std - popd + RUSTFLAGS="--cfg=c_bindings" cargo test -p $DIR --verbose --color always --no-default-features --features=no-std done +RUSTFLAGS="--cfg=c_bindings" cargo test --verbose --color always + # Note that outbound_commitment_test only runs in this mode because of hardcoded signature values pushd lightning cargo test --verbose --color always --no-default-features --features=std,_test_vectors diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 2682e3824b6..aa6d0b0615e 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -864,7 +864,7 @@ mod tests { use lightning::ln::peer_handler::{PeerManager, MessageHandler, SocketDescriptor, IgnoringMessageHandler}; use lightning::routing::gossip::{NetworkGraph, NodeId, P2PGossipSync}; use lightning::routing::router::{DefaultRouter, Path, RouteHop}; - use lightning::routing::scoring::{ChannelUsage, ScoreUpdate, ScoreLookUp}; + use lightning::routing::scoring::{ChannelUsage, ScoreUpdate, ScoreLookUp, LockableScore}; use lightning::util::config::UserConfig; use lightning::util::ser::Writeable; use lightning::util::test_utils; @@ -894,6 +894,11 @@ mod tests { fn disconnect_socket(&mut self) {} } + #[cfg(c_bindings)] + type LockingWrapper = lightning::routing::scoring::MultiThreadedLockableScore; + #[cfg(not(c_bindings))] + type LockingWrapper = Mutex; + type ChannelManager = channelmanager::ChannelManager< Arc, @@ -905,7 +910,7 @@ mod tests { Arc>>, Arc, - Arc>, + Arc>, (), TestScorer> >, @@ -927,7 +932,7 @@ mod tests { network_graph: Arc>>, logger: Arc, best_block: BestBlock, - scorer: Arc>, + scorer: Arc>, } impl Node { @@ -1148,6 +1153,9 @@ mod tests { } } + #[cfg(c_bindings)] + impl lightning::routing::scoring::Score for TestScorer {} + impl Drop for TestScorer { fn drop(&mut self) { if std::thread::panicking() { @@ -1179,7 +1187,7 @@ mod tests { let logger = Arc::new(test_utils::TestLogger::with_id(format!("node {}", i))); let genesis_block = genesis_block(network); let network_graph = Arc::new(NetworkGraph::new(network, logger.clone())); - let scorer = Arc::new(Mutex::new(TestScorer::new())); + let scorer = Arc::new(LockingWrapper::new(TestScorer::new())); let seed = [i as u8; 32]; let router = Arc::new(DefaultRouter::new(network_graph.clone(), logger.clone(), seed, scorer.clone(), Default::default())); let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Bitcoin)); @@ -1689,7 +1697,7 @@ mod tests { maybe_announced_channel: true, }], blinded_tail: None }; - $nodes[0].scorer.lock().unwrap().expect(TestResult::PaymentFailure { path: path.clone(), short_channel_id: scored_scid }); + $nodes[0].scorer.write_lock().expect(TestResult::PaymentFailure { path: path.clone(), short_channel_id: scored_scid }); $nodes[0].node.push_pending_event(Event::PaymentPathFailed { payment_id: None, payment_hash: PaymentHash([42; 32]), @@ -1706,7 +1714,7 @@ mod tests { // Ensure we'll score payments that were explicitly failed back by the destination as // ProbeSuccess. - $nodes[0].scorer.lock().unwrap().expect(TestResult::ProbeSuccess { path: path.clone() }); + $nodes[0].scorer.write_lock().expect(TestResult::ProbeSuccess { path: path.clone() }); $nodes[0].node.push_pending_event(Event::PaymentPathFailed { payment_id: None, payment_hash: PaymentHash([42; 32]), @@ -1721,7 +1729,7 @@ mod tests { _ => panic!("Unexpected event"), } - $nodes[0].scorer.lock().unwrap().expect(TestResult::PaymentSuccess { path: path.clone() }); + $nodes[0].scorer.write_lock().expect(TestResult::PaymentSuccess { path: path.clone() }); $nodes[0].node.push_pending_event(Event::PaymentPathSuccessful { payment_id: PaymentId([42; 32]), payment_hash: None, @@ -1733,7 +1741,7 @@ mod tests { _ => panic!("Unexpected event"), } - $nodes[0].scorer.lock().unwrap().expect(TestResult::ProbeSuccess { path: path.clone() }); + $nodes[0].scorer.write_lock().expect(TestResult::ProbeSuccess { path: path.clone() }); $nodes[0].node.push_pending_event(Event::ProbeSuccessful { payment_id: PaymentId([42; 32]), payment_hash: PaymentHash([42; 32]), @@ -1745,7 +1753,7 @@ mod tests { _ => panic!("Unexpected event"), } - $nodes[0].scorer.lock().unwrap().expect(TestResult::ProbeFailure { path: path.clone() }); + $nodes[0].scorer.write_lock().expect(TestResult::ProbeFailure { path: path.clone() }); $nodes[0].node.push_pending_event(Event::ProbeFailed { payment_id: PaymentId([42; 32]), payment_hash: PaymentHash([42; 32]), From 4443db67f9bced58882eaf99d68608ed5be6bba3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 21 Oct 2023 02:42:48 +0000 Subject: [PATCH 08/10] Do not compile the `Simple*` type aliases in `c_bindings` at all Because the bindings changes now require further changes to our type definitions, avoiding building the `Simple*` type aliases entirely makes the patchset there simpler. --- lightning/src/ln/channelmanager.rs | 6 ++++-- lightning/src/ln/peer_handler.rs | 11 ++++++++--- lightning/src/onion_message/messenger.rs | 7 +++++-- lightning/src/onion_message/mod.rs | 4 +++- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 20dba4cf8e6..e75b1a9273e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -827,7 +827,8 @@ struct PendingInboundPayment { /// or, respectively, [`Router`] for its router, but this type alias chooses the concrete types /// of [`KeysManager`] and [`DefaultRouter`]. /// -/// This is not exported to bindings users as Arcs don't make sense in bindings +/// This is not exported to bindings users as type aliases aren't supported in most languages. +#[cfg(not(c_bindings))] pub type SimpleArcChannelManager = ChannelManager< Arc, Arc, @@ -855,7 +856,8 @@ pub type SimpleArcChannelManager = ChannelManager< /// or, respectively, [`Router`] for its router, but this type alias chooses the concrete types /// of [`KeysManager`] and [`DefaultRouter`]. /// -/// This is not exported to bindings users as Arcs don't make sense in bindings +/// This is not exported to bindings users as type aliases aren't supported in most languages. +#[cfg(not(c_bindings))] pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = ChannelManager< &'a M, diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 44d2a87a87c..ba3a733d225 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -24,12 +24,15 @@ use crate::ln::ChannelId; use crate::ln::features::{InitFeatures, NodeFeatures}; use crate::ln::msgs; use crate::ln::msgs::{ChannelMessageHandler, LightningError, SocketAddress, OnionMessageHandler, RoutingMessageHandler}; +#[cfg(not(c_bindings))] use crate::ln::channelmanager::{SimpleArcChannelManager, SimpleRefChannelManager}; use crate::util::ser::{VecWriter, Writeable, Writer}; use crate::ln::peer_channel_encryptor::{PeerChannelEncryptor,NextNoiseStep}; use crate::ln::wire; use crate::ln::wire::{Encode, Type}; -use crate::onion_message::{CustomOnionMessageHandler, OffersMessage, OffersMessageHandler, OnionMessageContents, PendingOnionMessage, SimpleArcOnionMessenger, SimpleRefOnionMessenger}; +#[cfg(not(c_bindings))] +use crate::onion_message::{SimpleArcOnionMessenger, SimpleRefOnionMessenger}; +use crate::onion_message::{CustomOnionMessageHandler, OffersMessage, OffersMessageHandler, OnionMessageContents, PendingOnionMessage}; use crate::routing::gossip::{NetworkGraph, P2PGossipSync, NodeId, NodeAlias}; use crate::util::atomic_counter::AtomicCounter; use crate::util::logger::Logger; @@ -608,7 +611,8 @@ impl Peer { /// SimpleRefPeerManager is the more appropriate type. Defining these type aliases prevents /// issues such as overly long function definitions. /// -/// This is not exported to bindings users as `Arc`s don't make sense in bindings. +/// This is not exported to bindings users as type aliases aren't supported in most languages. +#[cfg(not(c_bindings))] pub type SimpleArcPeerManager = PeerManager< SD, Arc>, @@ -626,7 +630,8 @@ pub type SimpleArcPeerManager = PeerManager< /// But if this is not necessary, using a reference is more efficient. Defining these type aliases /// helps with issues such as long function definitions. /// -/// This is not exported to bindings users as general type aliases don't make sense in bindings. +/// This is not exported to bindings users as type aliases aren't supported in most languages. +#[cfg(not(c_bindings))] pub type SimpleRefPeerManager< 'a, 'b, 'c, 'd, 'e, 'f, 'logger, 'h, 'i, 'j, 'graph, 'k, SD, M, T, F, C, L > = PeerManager< diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 723e105d044..200ee44ee32 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -19,6 +19,7 @@ use crate::blinded_path::BlindedPath; use crate::blinded_path::message::{advance_path_by_one, ForwardTlvs, ReceiveTlvs}; use crate::blinded_path::utils; use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient}; +#[cfg(not(c_bindings))] use crate::ln::channelmanager::{SimpleArcChannelManager, SimpleRefChannelManager}; use crate::ln::features::{InitFeatures, NodeFeatures}; use crate::ln::msgs::{self, OnionMessage, OnionMessageHandler}; @@ -712,10 +713,11 @@ where /// Useful for simplifying the parameters of [`SimpleArcChannelManager`] and /// [`SimpleArcPeerManager`]. See their docs for more details. /// -/// This is not exported to bindings users as `Arc`s don't make sense in bindings. +/// This is not exported to bindings users as type aliases aren't supported in most languages. /// /// [`SimpleArcChannelManager`]: crate::ln::channelmanager::SimpleArcChannelManager /// [`SimpleArcPeerManager`]: crate::ln::peer_handler::SimpleArcPeerManager +#[cfg(not(c_bindings))] pub type SimpleArcOnionMessenger = OnionMessenger< Arc, Arc, @@ -728,10 +730,11 @@ pub type SimpleArcOnionMessenger = OnionMessenger< /// Useful for simplifying the parameters of [`SimpleRefChannelManager`] and /// [`SimpleRefPeerManager`]. See their docs for more details. /// -/// This is not exported to bindings users as general type aliases don't make sense in bindings. +/// This is not exported to bindings users as type aliases aren't supported in most languages. /// /// [`SimpleRefChannelManager`]: crate::ln::channelmanager::SimpleRefChannelManager /// [`SimpleRefPeerManager`]: crate::ln::peer_handler::SimpleRefPeerManager +#[cfg(not(c_bindings))] pub type SimpleRefOnionMessenger< 'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, 'i, 'j, M, T, F, L > = OnionMessenger< diff --git a/lightning/src/onion_message/mod.rs b/lightning/src/onion_message/mod.rs index be800822e4d..07482c3092e 100644 --- a/lightning/src/onion_message/mod.rs +++ b/lightning/src/onion_message/mod.rs @@ -27,7 +27,9 @@ mod packet; mod functional_tests; // Re-export structs so they can be imported with just the `onion_message::` module prefix. -pub use self::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, MessageRouter, OnionMessageContents, OnionMessagePath, OnionMessenger, PeeledOnion, PendingOnionMessage, SendError, SimpleArcOnionMessenger, SimpleRefOnionMessenger}; +pub use self::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, MessageRouter, OnionMessageContents, OnionMessagePath, OnionMessenger, PeeledOnion, PendingOnionMessage, SendError}; +#[cfg(not(c_bindings))] +pub use self::messenger::{SimpleArcOnionMessenger, SimpleRefOnionMessenger}; pub use self::offers::{OffersMessage, OffersMessageHandler}; pub use self::packet::{Packet, ParsedOnionMessageContents}; pub(crate) use self::packet::ControlTlvs; From 4918c415afce25b23a0b10ca8f9dd3aaba708574 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 23 Oct 2023 16:50:05 +0000 Subject: [PATCH 09/10] Drop an unnecessary no-export on ParsedOnionMessageContents --- lightning/src/onion_message/packet.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/lightning/src/onion_message/packet.rs b/lightning/src/onion_message/packet.rs index 19ca6eb963b..ba90c717b34 100644 --- a/lightning/src/onion_message/packet.rs +++ b/lightning/src/onion_message/packet.rs @@ -137,7 +137,6 @@ impl OnionMessageContents for ParsedOnionMessageContent } } -/// This is not exported to bindings users as methods on non-cloneable enums are not currently exportable impl Writeable for ParsedOnionMessageContents { fn write(&self, w: &mut W) -> Result<(), io::Error> { match self { From 32d2d0f1cfa17fc2d530ce315c55a92350fa46de Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 23 Oct 2023 16:49:49 +0000 Subject: [PATCH 10/10] Add relevant no-export tags to functions returning builders Because we can't map move semantics in most languages, we also can't map our current builders. Thus, we have to mark them no-export. --- lightning/src/ln/channelmanager.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e75b1a9273e..2bf60cdc797 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7331,6 +7331,8 @@ where /// Requires a direct connection to the introduction node in the responding [`InvoiceRequest`]'s /// reply path. /// + /// This is not exported to bindings users as builder patterns don't map outside of move semantics. + /// /// [`Offer`]: crate::offers::offer::Offer /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest pub fn create_offer_builder( @@ -7384,6 +7386,8 @@ where /// Errors if a duplicate `payment_id` is provided given the caveats in the aforementioned link /// or if `amount_msats` is invalid. /// + /// This is not exported to bindings users as builder patterns don't map outside of move semantics. + /// /// [`Refund`]: crate::offers::refund::Refund /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice /// [`Bolt12Invoice::payment_paths`]: crate::offers::invoice::Bolt12Invoice::payment_paths