Skip to content

Small API cleanups pre-0.0.119 #2798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 15, 2023
Merged

Small API cleanups pre-0.0.119 #2798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
90cc993
Un-export the `PrivateRoute` inner field as there are invariants
TheBlueMatt Dec 14, 2023
83e76d7
Drop explicit `bitcoin_hashes` dependency in `lightning-invoice`
TheBlueMatt Dec 15, 2023
bb0ddad
Make `FinalOnionHopData` public
TheBlueMatt Dec 15, 2023
642240e
Use correct default value when comparing to `htlc_maximum_msat`
TheBlueMatt Dec 15, 2023
2aecfa4
Fix dev MSRV build of lightning-transaction-sync
TheBlueMatt Dec 15, 2023
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
2 changes: 2 additions & 0 deletions ci/ci-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ if [[ "$HOST_PLATFORM" != *windows* ]]; then
[ "$RUSTC_MINOR_VERSION" -lt 65 ] && cargo update -p reqwest --precise "0.11.20" --verbose
# Starting with version 1.10.0, the `regex` crate has an MSRV of rustc 1.65.0.
[ "$RUSTC_MINOR_VERSION" -lt 65 ] && cargo update -p regex --precise "1.9.6" --verbose
# Starting with version 0.5.9 (there is no .6-.8), the `home` crate has an MSRV of rustc 1.70.0.
[ "$RUSTC_MINOR_VERSION" -lt 70 ] && cargo update -p home --precise "0.5.5" --verbose

DOWNLOAD_ELECTRS_AND_BITCOIND

Expand Down
3 changes: 1 addition & 2 deletions lightning-invoice/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ rustdoc-args = ["--cfg", "docsrs"]
[features]
default = ["std"]
no-std = ["hashbrown", "lightning/no-std"]
std = ["bitcoin_hashes/std", "num-traits/std", "lightning/std", "bech32/std"]
std = ["bitcoin/std", "num-traits/std", "lightning/std", "bech32/std"]

[dependencies]
bech32 = { version = "0.9.0", default-features = false }
lightning = { version = "0.0.118", path = "../lightning", default-features = false }
secp256k1 = { version = "0.27.0", default-features = false, features = ["recovery", "alloc"] }
num-traits = { version = "0.2.8", default-features = false }
bitcoin_hashes = { version = "0.12.0", default-features = false }
hashbrown = { version = "0.8", optional = true }
serde = { version = "1.0.118", optional = true }
bitcoin = { version = "0.30.2", default-features = false }
Expand Down
12 changes: 6 additions & 6 deletions lightning-invoice/src/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use bech32::{u5, FromBase32};

use bitcoin::{PubkeyHash, ScriptHash};
use bitcoin::address::WitnessVersion;
use bitcoin_hashes::Hash;
use bitcoin_hashes::sha256;
use bitcoin::hashes::Hash;
use bitcoin::hashes::sha256;
use crate::prelude::*;
use lightning::ln::PaymentSecret;
use lightning::routing::gossip::RoutingFees;
Expand Down Expand Up @@ -564,14 +564,14 @@ impl FromBase32 for Fallback {
17 => {
let pkh = match PubkeyHash::from_slice(&bytes) {
Ok(pkh) => pkh,
Err(bitcoin_hashes::Error::InvalidLength(_, _)) => return Err(Bolt11ParseError::InvalidPubKeyHashLength),
Err(bitcoin::hashes::Error::InvalidLength(_, _)) => return Err(Bolt11ParseError::InvalidPubKeyHashLength),
};
Ok(Fallback::PubKeyHash(pkh))
}
18 => {
let sh = match ScriptHash::from_slice(&bytes) {
Ok(sh) => sh,
Err(bitcoin_hashes::Error::InvalidLength(_, _)) => return Err(Bolt11ParseError::InvalidScriptHashLength),
Err(bitcoin::hashes::Error::InvalidLength(_, _)) => return Err(Bolt11ParseError::InvalidScriptHashLength),
};
Ok(Fallback::ScriptHash(sh))
}
Expand Down Expand Up @@ -726,7 +726,7 @@ mod test {
use crate::de::Bolt11ParseError;
use secp256k1::PublicKey;
use bech32::u5;
use bitcoin_hashes::sha256;
use bitcoin::hashes::sha256;
use std::str::FromStr;

const CHARSET_REV: [i8; 128] = [
Expand Down Expand Up @@ -856,7 +856,7 @@ mod test {
use bech32::FromBase32;
use bitcoin::{PubkeyHash, ScriptHash};
use bitcoin::address::WitnessVersion;
use bitcoin_hashes::Hash;
use bitcoin::hashes::Hash;

let cases = vec![
(
Expand Down
13 changes: 6 additions & 7 deletions lightning-invoice/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ pub mod payment;
pub mod utils;

extern crate bech32;
extern crate bitcoin_hashes;
#[macro_use] extern crate lightning;
extern crate num_traits;
extern crate secp256k1;
Expand All @@ -46,7 +45,7 @@ use std::time::SystemTime;
use bech32::u5;
use bitcoin::{Address, Network, PubkeyHash, ScriptHash};
use bitcoin::address::{Payload, WitnessProgram, WitnessVersion};
use bitcoin_hashes::{Hash, sha256};
use bitcoin::hashes::{Hash, sha256};
use lightning::ln::features::Bolt11InvoiceFeatures;
use lightning::util::invoice::construct_invoice_preimage;

Expand Down Expand Up @@ -166,10 +165,10 @@ pub const DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA: u64 = 18;
/// extern crate secp256k1;
/// extern crate lightning;
/// extern crate lightning_invoice;
/// extern crate bitcoin_hashes;
/// extern crate bitcoin;
///
/// use bitcoin_hashes::Hash;
/// use bitcoin_hashes::sha256;
/// use bitcoin::hashes::Hash;
/// use bitcoin::hashes::sha256;
///
/// use secp256k1::Secp256k1;
/// use secp256k1::SecretKey;
Expand Down Expand Up @@ -527,7 +526,7 @@ impl Ord for Bolt11InvoiceSignature {
/// The encoded route has to be <1024 5bit characters long (<=639 bytes or <=12 hops)
///
#[derive(Clone, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
pub struct PrivateRoute(pub RouteHint);
pub struct PrivateRoute(RouteHint);

/// Tag constants as specified in BOLT11
#[allow(missing_docs)]
Expand Down Expand Up @@ -1756,7 +1755,7 @@ impl<'de> Deserialize<'de> for Bolt11Invoice {
#[cfg(test)]
mod test {
use bitcoin::ScriptBuf;
use bitcoin_hashes::sha256;
use bitcoin::hashes::sha256;
use std::str::FromStr;

#[test]
Expand Down
4 changes: 2 additions & 2 deletions lightning-invoice/src/payment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
//! Convenient utilities for paying Lightning invoices.

use crate::Bolt11Invoice;
use crate::bitcoin_hashes::Hash;
use bitcoin::hashes::Hash;

use lightning::ln::PaymentHash;
use lightning::ln::channelmanager::RecipientOnionFields;
Expand Down Expand Up @@ -84,7 +84,7 @@ fn params_from_invoice(invoice: &Bolt11Invoice, amount_msat: u64)
mod tests {
use super::*;
use crate::{InvoiceBuilder, Currency};
use bitcoin_hashes::sha256::Hash as Sha256;
use bitcoin::hashes::sha256::Hash as Sha256;
use lightning::events::Event;
use lightning::ln::channelmanager::{Retry, PaymentId};
use lightning::ln::msgs::ChannelMessageHandler;
Expand Down
6 changes: 3 additions & 3 deletions lightning-invoice/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{Bolt11Invoice, CreationError, Currency, InvoiceBuilder, SignOrCreati

use crate::{prelude::*, Description, Bolt11InvoiceDescription, Sha256};
use bech32::ToBase32;
use bitcoin_hashes::Hash;
use bitcoin::hashes::Hash;
use lightning::chain;
use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
use lightning::sign::{Recipient, NodeSigner, SignerProvider, EntropySource};
Expand Down Expand Up @@ -819,8 +819,8 @@ mod test {
use core::cell::RefCell;
use core::time::Duration;
use crate::{Currency, Description, Bolt11InvoiceDescription, SignOrCreationError, CreationError};
use bitcoin_hashes::{Hash, sha256};
use bitcoin_hashes::sha256::Hash as Sha256;
use bitcoin::hashes::{Hash, sha256};
use bitcoin::hashes::sha256::Hash as Sha256;
use lightning::sign::PhantomKeysManager;
use lightning::events::{MessageSendEvent, MessageSendEventsProvider, Event, EventsProvider};
use lightning::ln::{PaymentPreimage, PaymentHash};
Expand Down
3 changes: 1 addition & 2 deletions lightning-invoice/tests/ser_de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
extern crate bech32;
extern crate bitcoin_hashes;
extern crate lightning;
extern crate lightning_invoice;
extern crate secp256k1;
Expand All @@ -8,7 +7,7 @@ extern crate hex;
use bitcoin::address::WitnessVersion;
use bitcoin::{PubkeyHash, ScriptHash};
use bitcoin::hashes::hex::FromHex;
use bitcoin_hashes::{sha256, Hash};
use bitcoin::hashes::{sha256, Hash};
use lightning::ln::PaymentSecret;
use lightning::routing::gossip::RoutingFees;
use lightning::routing::router::{RouteHint, RouteHintHop};
Expand Down
24 changes: 16 additions & 8 deletions lightning/src/ln/msgs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1668,23 +1668,31 @@ pub trait OnionMessageHandler: EventsProvider {
fn provided_init_features(&self, their_node_id: &PublicKey) -> InitFeatures;
}

#[derive(Clone)]
#[cfg_attr(test, derive(Debug, PartialEq))]
/// Information communicated in the onion to the recipient for multi-part tracking and proof that
/// the payment is associated with an invoice.
pub struct FinalOnionHopData {
/// When sending a multi-part payment, this secret is used to identify a payment across HTLCs.
/// Because it is generated by the recipient and included in the invoice, it also provides
/// proof to the recipient that the payment was sent by someone with the generated invoice.
pub payment_secret: PaymentSecret,
/// The intended total amount that this payment is for.
///
/// Message serialization may panic if this value is more than 21 million Bitcoin.
pub total_msat: u64,
}

mod fuzzy_internal_msgs {
use bitcoin::secp256k1::PublicKey;
use crate::blinded_path::payment::{PaymentConstraints, PaymentRelay};
use crate::prelude::*;
use crate::ln::{PaymentPreimage, PaymentSecret};
use crate::ln::features::BlindedHopFeatures;
use super::FinalOnionHopData;

// These types aren't intended to be pub, but are exposed for direct fuzzing (as we deserialize
// them from untrusted input):
#[derive(Clone)]
#[cfg_attr(test, derive(Debug, PartialEq))]
pub struct FinalOnionHopData {
pub payment_secret: PaymentSecret,
/// The total value, in msat, of the payment as received by the ultimate recipient.
/// Message serialization may panic if this value is more than 21 million Bitcoin.
pub total_msat: u64,
}

pub enum InboundOnionPayload {
Forward {
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/routing/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl<G: Deref<Target = NetworkGraph<L>> + Clone, L: Deref, S: Deref, SP: Sized,
.filter(|details| details.counterparty.features.supports_route_blinding())
.filter(|details| amount_msats <= details.inbound_capacity_msat)
.filter(|details| amount_msats >= details.inbound_htlc_minimum_msat.unwrap_or(0))
.filter(|details| amount_msats <= details.inbound_htlc_maximum_msat.unwrap_or(0))
.filter(|details| amount_msats <= details.inbound_htlc_maximum_msat.unwrap_or(u64::MAX))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I think my reasoning here was that inbound_htlc_maximum_msat should be set for usable channels. So if it is not set, using zero would cause us to not consider this channel.

It will be Some if ChannelContext::counterparty_selected_channel_reserve_satoshis is Some, which seems to be when accept_channel is received:

rust-lightning/lightning/src/ln/channel.rs

Line 6324 in b9797eb

counterparty_selected_channel_reserve_satoshis: None, // Filled in in accept_channel

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yea, I mean it won't be set except if the channel isn't usable, but I'd kinda rather lean on get_inbound_payment_scid() to detect "is channel available for inbound usage" rather than htlc_max, but it doesn't really matter that much.

.filter(|details| network_graph
.node(&NodeId::from_pubkey(&details.counterparty.node_id))
.map(|node_info| node_info.channels.len() >= MIN_PEER_CHANNELS)
Expand Down Expand Up @@ -139,7 +139,7 @@ impl<G: Deref<Target = NetworkGraph<L>> + Clone, L: Deref, S: Deref, SP: Sized,
features: BlindedHopFeatures::empty(),
},
node_id: details.counterparty.node_id,
htlc_maximum_msat: details.inbound_htlc_maximum_msat.unwrap_or(0),
htlc_maximum_msat: details.inbound_htlc_maximum_msat.unwrap_or(u64::MAX),
})
})
.map(|forward_node| {
Expand Down