Skip to content

Small bindings tweaks for 0.0.118 #2679

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.

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 2 commits into from
Oct 23, 2023
Merged
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
6 changes: 3 additions & 3 deletions ci/ci-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,13 @@ popd

echo -e "\n\nTesting no-std flags in various combinations"
for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do
cargo test -p $DIR --verbose --color always --no-default-features --features no-std
[ "$RUSTC_MINOR_VERSION" -gt 50 ] && 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 -p $DIR --verbose --color always --features no-std
[ "$RUSTC_MINOR_VERSION" -gt 50 ] && 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 -p $DIR --verbose --color always --no-default-features --features=no-std
[ "$RUSTC_MINOR_VERSION" -gt 50 ] && 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

Expand Down
42 changes: 21 additions & 21 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ use crate::offers::merkle::SignError;
use crate::offers::offer::{DerivedMetadata, Offer, OfferBuilder};
use crate::offers::parse::Bolt12SemanticError;
use crate::offers::refund::{Refund, RefundBuilder};
use crate::onion_message::{Destination, OffersMessage, OffersMessageHandler, PendingOnionMessage};
use crate::onion_message::{Destination, OffersMessage, OffersMessageHandler, PendingOnionMessage, new_pending_onion_message};
use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, WriteableEcdsaChannelSigner};
use crate::util::config::{UserConfig, ChannelConfig, ChannelConfigUpdate};
use crate::util::wakers::{Future, Notifier};
Expand Down Expand Up @@ -7505,23 +7505,23 @@ where

let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap();
if offer.paths().is_empty() {
let message = PendingOnionMessage {
contents: OffersMessage::InvoiceRequest(invoice_request),
destination: Destination::Node(offer.signing_pubkey()),
reply_path: Some(reply_path),
};
let message = new_pending_onion_message(
OffersMessage::InvoiceRequest(invoice_request),
Destination::Node(offer.signing_pubkey()),
Some(reply_path),
);
pending_offers_messages.push(message);
} else {
// Send as many invoice requests as there are paths in the offer (with an upper bound).
// Using only one path could result in a failure if the path no longer exists. But only
// one invoice for a given payment id will be paid, even if more than one is received.
const REQUEST_LIMIT: usize = 10;
for path in offer.paths().into_iter().take(REQUEST_LIMIT) {
let message = PendingOnionMessage {
contents: OffersMessage::InvoiceRequest(invoice_request.clone()),
destination: Destination::BlindedPath(path.clone()),
reply_path: Some(reply_path.clone()),
};
let message = new_pending_onion_message(
OffersMessage::InvoiceRequest(invoice_request.clone()),
Destination::BlindedPath(path.clone()),
Some(reply_path.clone()),
);
pending_offers_messages.push(message);
}
}
Expand Down Expand Up @@ -7574,19 +7574,19 @@ where

let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap();
if refund.paths().is_empty() {
let message = PendingOnionMessage {
contents: OffersMessage::Invoice(invoice),
destination: Destination::Node(refund.payer_id()),
reply_path: Some(reply_path),
};
let message = new_pending_onion_message(
OffersMessage::Invoice(invoice),
Destination::Node(refund.payer_id()),
Some(reply_path),
);
pending_offers_messages.push(message);
} else {
for path in refund.paths() {
let message = PendingOnionMessage {
contents: OffersMessage::Invoice(invoice.clone()),
destination: Destination::BlindedPath(path.clone()),
reply_path: Some(reply_path.clone()),
};
let message = new_pending_onion_message(
OffersMessage::Invoice(invoice.clone()),
Destination::BlindedPath(path.clone()),
Some(reply_path.clone()),
);
pending_offers_messages.push(message);
}
}
Expand Down
31 changes: 31 additions & 0 deletions lightning/src/onion_message/messenger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ where
///
/// These are obtained when released from [`OnionMessenger`]'s handlers after which they are
/// enqueued for sending.
#[cfg(not(c_bindings))]
pub struct PendingOnionMessage<T: OnionMessageContents> {
/// The message contents to send in an [`OnionMessage`].
pub contents: T,
Expand All @@ -170,6 +171,22 @@ pub struct PendingOnionMessage<T: OnionMessageContents> {
pub reply_path: Option<BlindedPath>,
}

#[cfg(c_bindings)]
/// An [`OnionMessage`] for [`OnionMessenger`] to send.
///
/// These are obtained when released from [`OnionMessenger`]'s handlers after which they are
/// enqueued for sending.
pub type PendingOnionMessage<T: OnionMessageContents> = (T, Destination, Option<BlindedPath>);

pub(crate) fn new_pending_onion_message<T: OnionMessageContents>(
contents: T, destination: Destination, reply_path: Option<BlindedPath>
) -> PendingOnionMessage<T> {
#[cfg(not(c_bindings))]
return PendingOnionMessage { contents, destination, reply_path };
#[cfg(c_bindings)]
return (contents, destination, reply_path);
}

/// A trait defining behavior for routing an [`OnionMessage`].
pub trait MessageRouter {
/// Returns a route for sending an [`OnionMessage`] to the given [`Destination`].
Expand Down Expand Up @@ -286,7 +303,15 @@ pub trait CustomOnionMessageHandler {
///
/// Typically, this is used for messages initiating a message flow rather than in response to
/// another message. The latter should use the return value of [`Self::handle_custom_message`].
#[cfg(not(c_bindings))]
fn release_pending_custom_messages(&self) -> Vec<PendingOnionMessage<Self::CustomMessage>>;

/// Releases any [`Self::CustomMessage`]s that need to be sent.
///
/// Typically, this is used for messages initiating a message flow rather than in response to
/// another message. The latter should use the return value of [`Self::handle_custom_message`].
#[cfg(c_bindings)]
fn release_pending_custom_messages(&self) -> Vec<(Self::CustomMessage, Destination, Option<BlindedPath>)>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to change if PendingOnionMessage is a type alias for a tuple in bindings? If it does, then it doesn't seem like PendingOnionMessage needs to be pub for bindings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, that is the first thing I tried and it didn't really work. Type aliases in bindings are handled a bit specifically for cases we rely on (eg the scorer), so it didn't quite work here. I will drop it to not be pub.

Copy link
Collaborator Author

@TheBlueMatt TheBlueMatt Oct 23, 2023

Choose a reason for hiding this comment

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

Actually, reverted this. onion_messages/mod.rs re-exports it as pub so it doesnt compile as pub-crate, and I'd rather not have any more diff than is required. There's no reason not to export it, the bindings won't include it anyway.

}

/// A processed incoming onion message, containing either a Forward (another onion message)
Expand Down Expand Up @@ -686,15 +711,21 @@ where
fn next_onion_message_for_peer(&self, peer_node_id: PublicKey) -> Option<OnionMessage> {
// Enqueue any initiating `OffersMessage`s to send.
for message in self.offers_handler.release_pending_messages() {
#[cfg(not(c_bindings))]
let PendingOnionMessage { contents, destination, reply_path } = message;
#[cfg(c_bindings)]
let (contents, destination, reply_path) = message;
self.find_path_and_enqueue_onion_message(
contents, destination, reply_path, format_args!("when sending OffersMessage")
);
}

// Enqueue any initiating `CustomMessage`s to send.
for message in self.custom_handler.release_pending_custom_messages() {
#[cfg(not(c_bindings))]
let PendingOnionMessage { contents, destination, reply_path } = message;
#[cfg(c_bindings)]
let (contents, destination, reply_path) = message;
self.find_path_and_enqueue_onion_message(
contents, destination, reply_path, format_args!("when sending CustomMessage")
);
Expand Down
1 change: 1 addition & 0 deletions lightning/src/onion_message/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@ pub use self::messenger::{SimpleArcOnionMessenger, SimpleRefOnionMessenger};
pub use self::offers::{OffersMessage, OffersMessageHandler};
pub use self::packet::{Packet, ParsedOnionMessageContents};
pub(crate) use self::packet::ControlTlvs;
pub(crate) use self::messenger::new_pending_onion_message;
8 changes: 8 additions & 0 deletions lightning/src/onion_message/offers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,15 @@ pub trait OffersMessageHandler {
///
/// Typically, this is used for messages initiating a payment flow rather than in response to
/// another message. The latter should use the return value of [`Self::handle_message`].
#[cfg(not(c_bindings))]
fn release_pending_messages(&self) -> Vec<PendingOnionMessage<OffersMessage>> { vec![] }

/// Releases any [`OffersMessage`]s that need to be sent.
///
/// Typically, this is used for messages initiating a payment flow rather than in response to
/// another message. The latter should use the return value of [`Self::handle_message`].
#[cfg(c_bindings)]
fn release_pending_messages(&self) -> Vec<(OffersMessage, crate::onion_message::Destination, Option<crate::blinded_path::BlindedPath>)> { vec![] }
}

/// Possible BOLT 12 Offers messages sent and received via an [`OnionMessage`].
Expand Down