Return owned Strings for onion message message types

[TheBlueMatt]

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_types), 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.

[codecov]

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.84%. Comparing base (ff0874a) to head (feffaf8).
Report is 13 commits behind head on main.

Files	Patch %	Lines
lightning/src/onion_message/offers.rs	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3273      +/-   ##
==========================================
+ Coverage   89.82%   89.84%   +0.01%     
==========================================
  Files         126      126              
  Lines      102988   103142     +154     
  Branches   102988   103142     +154     
==========================================
+ Hits        92513    92669     +156     
+ Misses       7757     7752       -5     
- Partials     2718     2721       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

Grr, tests with c_bindings failed:

$ git diff-tree -U2 9136597f7 feffaf8bb
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
@@ -88,5 +88,7 @@ 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
@@ -96,7 +98,7 @@ 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"
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
@@ -109,4 +109,9 @@ impl OnionMessageContents for TestCustomMessage {
 		}
 	}
+	#[cfg(c_bindings)]
+	fn msg_type(&self) -> String {
+		"Custom Message".to_string()
+	}
+	#[cfg(not(c_bindings))]
 	fn msg_type(&self) -> &'static str {
 		"Custom Message"
@@ -657,4 +662,9 @@ fn invalid_custom_message_type() {
 			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"
$ 

[vincenzopalazzo]

LGTM

[dunxen]

Does what it says on the tin. LGTM.