From feffaf8bbc5b6687463088d8c6091384b88303a5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 26 Aug 2024 19:25:46 +0000 Subject: [PATCH] Return owned `String`s for onion message message types Returning a reference from a trait method is relatively difficult to map in bindings and is currently handled by storing the object in the trait instance, returning a reference to the local field. This is fine when the object we're returning only needs to live as long as the trait, but when it needs to be `'static` (as is the case for onion message `msg_type`s), there's not really a good way to map them at all. Instead, here, condition on `#[cfg(c_bindings)]` we return a fully owned `String`. This is obviously relatively less effecient, but the extra allocation and `memcpy` isn't the end of the world, especially given it should be released relatively quickly. Note that this breaks doctests in with `c_bindings`. --- ci/ci-tests.sh | 8 ++++--- lightning/src/ln/peer_handler.rs | 3 +++ lightning/src/onion_message/async_payments.rs | 13 +++++++++++ .../src/onion_message/functional_tests.rs | 10 ++++++++ lightning/src/onion_message/offers.rs | 23 +++++++++++++------ lightning/src/onion_message/packet.rs | 15 ++++++++++++ 6 files changed, 62 insertions(+), 10 deletions(-) diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh index 22d0783a9..aff1a46c4 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -87,7 +87,9 @@ cargo test -p lightning --verbose --color always --no-default-features --feature cargo test -p lightning --verbose --color always --features no-std echo -e "\n\nTesting c_bindings builds" -RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test --verbose --color always +# Note that because `$RUSTFLAGS` is not passed through to doctest builds we cannot selectively +# disable doctests in `c_bindings` so we skip doctests entirely here. +RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test --verbose --color always --lib --bins --tests for DIR in lightning-invoice lightning-rapid-gossip-sync; do # check if there is a conflict between no-std and the c_bindings cfg @@ -95,9 +97,9 @@ for DIR in lightning-invoice lightning-rapid-gossip-sync; do done # Note that because `$RUSTFLAGS` is not passed through to doctest builds we cannot selectively -# disable tests in `c_bindings` so we skip doctests entirely here. +# disable doctests in `c_bindings` so we skip doctests entirely here. RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test -p lightning-background-processor --verbose --color always --features futures --no-default-features --lib --bins --tests -RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test -p lightning --verbose --color always --no-default-features --features=no-std +RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test -p lightning --verbose --color always --no-default-features --features=no-std --lib --bins --tests echo -e "\n\nTesting other crate-specific builds" # Note that outbound_commitment_test only runs in this mode because of hardcoded signature values diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index a1fedec89..dda1ca8b9 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -170,6 +170,9 @@ impl CustomOnionMessageHandler for IgnoringMessageHandler { impl OnionMessageContents for Infallible { fn tlv_type(&self) -> u64 { unreachable!(); } + #[cfg(c_bindings)] + fn msg_type(&self) -> String { unreachable!(); } + #[cfg(not(c_bindings))] fn msg_type(&self) -> &'static str { unreachable!(); } } diff --git a/lightning/src/onion_message/async_payments.rs b/lightning/src/onion_message/async_payments.rs index 37e4a5341..89756d9f1 100644 --- a/lightning/src/onion_message/async_payments.rs +++ b/lightning/src/onion_message/async_payments.rs @@ -77,6 +77,11 @@ impl OnionMessageContents for ReleaseHeldHtlc { fn tlv_type(&self) -> u64 { RELEASE_HELD_HTLC_TLV_TYPE } + #[cfg(c_bindings)] + fn msg_type(&self) -> String { + "Release Held HTLC".to_string() + } + #[cfg(not(c_bindings))] fn msg_type(&self) -> &'static str { "Release Held HTLC" } @@ -107,6 +112,14 @@ impl OnionMessageContents for AsyncPaymentsMessage { Self::ReleaseHeldHtlc(msg) => msg.tlv_type(), } } + #[cfg(c_bindings)] + fn msg_type(&self) -> String { + match &self { + Self::HeldHtlcAvailable(_) => "Held HTLC Available".to_string(), + Self::ReleaseHeldHtlc(msg) => msg.msg_type(), + } + } + #[cfg(not(c_bindings))] fn msg_type(&self) -> &'static str { match &self { Self::HeldHtlcAvailable(_) => "Held HTLC Available", diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index 700f275e5..56c540057 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/lightning/src/onion_message/functional_tests.rs @@ -108,6 +108,11 @@ impl OnionMessageContents for TestCustomMessage { TestCustomMessage::Pong => CUSTOM_PONG_MESSAGE_TYPE, } } + #[cfg(c_bindings)] + fn msg_type(&self) -> String { + "Custom Message".to_string() + } + #[cfg(not(c_bindings))] fn msg_type(&self) -> &'static str { "Custom Message" } @@ -656,6 +661,11 @@ fn invalid_custom_message_type() { // Onion message contents must have a TLV >= 64. 63 } + #[cfg(c_bindings)] + fn msg_type(&self) -> String { + "Invalid Message".to_string() + } + #[cfg(not(c_bindings))] fn msg_type(&self) -> &'static str { "Invalid Message" } diff --git a/lightning/src/onion_message/offers.rs b/lightning/src/onion_message/offers.rs index 1b3274ae1..f93c0854e 100644 --- a/lightning/src/onion_message/offers.rs +++ b/lightning/src/onion_message/offers.rs @@ -99,6 +99,16 @@ impl OffersMessage { _ => Err(Bolt12ParseError::Decode(DecodeError::InvalidValue)), } } + + fn get_msg_type(&self) -> &'static str { + match &self { + OffersMessage::InvoiceRequest(_) => "Invoice Request", + OffersMessage::Invoice(_) => "Invoice", + #[cfg(async_payments)] + OffersMessage::StaticInvoice(_) => "Static Invoice", + OffersMessage::InvoiceError(_) => "Invoice Error", + } + } } impl fmt::Debug for OffersMessage { @@ -131,14 +141,13 @@ impl OnionMessageContents for OffersMessage { OffersMessage::InvoiceError(_) => INVOICE_ERROR_TLV_TYPE, } } + #[cfg(c_bindings)] + fn msg_type(&self) -> String { + self.get_msg_type().to_string() + } + #[cfg(not(c_bindings))] fn msg_type(&self) -> &'static str { - match &self { - OffersMessage::InvoiceRequest(_) => "Invoice Request", - OffersMessage::Invoice(_) => "Invoice", - #[cfg(async_payments)] - OffersMessage::StaticInvoice(_) => "Static Invoice", - OffersMessage::InvoiceError(_) => "Invoice Error", - } + self.get_msg_type() } } diff --git a/lightning/src/onion_message/packet.rs b/lightning/src/onion_message/packet.rs index 553d70216..f65ed0d53 100644 --- a/lightning/src/onion_message/packet.rs +++ b/lightning/src/onion_message/packet.rs @@ -148,6 +148,16 @@ impl OnionMessageContents for ParsedOnionMessageContent &ParsedOnionMessageContents::Custom(ref msg) => msg.tlv_type(), } } + #[cfg(c_bindings)] + fn msg_type(&self) -> String { + match self { + ParsedOnionMessageContents::Offers(ref msg) => msg.msg_type(), + #[cfg(async_payments)] + ParsedOnionMessageContents::AsyncPayments(ref msg) => msg.msg_type(), + ParsedOnionMessageContents::Custom(ref msg) => msg.msg_type(), + } + } + #[cfg(not(c_bindings))] fn msg_type(&self) -> &'static str { match self { ParsedOnionMessageContents::Offers(ref msg) => msg.msg_type(), @@ -174,6 +184,11 @@ pub trait OnionMessageContents: Writeable + core::fmt::Debug { /// Returns the TLV type identifying the message contents. MUST be >= 64. fn tlv_type(&self) -> u64; + #[cfg(c_bindings)] + /// Returns the message type + fn msg_type(&self) -> String; + + #[cfg(not(c_bindings))] /// Returns the message type fn msg_type(&self) -> &'static str; } -- 2.39.5