Expose impl_writeable_msg
#1976
Conversation
tnull
force-pushed
the
2023-01-expose-impl-writeable-msg
branch
4 times, most recently
from
6a338ac
to
8e41db0
Compare
tnull
force-pushed
the
2023-01-expose-impl-writeable-msg
branch
from
8e41db0
to
99fe52e
Compare
Codecov ReportBase: 90.71% // Head: 90.71% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1976 +/- ##
=======================================
Coverage 90.71% 90.71%
=======================================
Files 97 97
Lines 50677 50676 -1
Branches 50677 50676 -1
=======================================
+ Hits 45971 45972 +1
+ Misses 4706 4704 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
It seems like it would be an improvement to support serializing TLV fields as other than just |
| /// [`Readable`]: crate::util::ser::Readable | ||
| /// [`Writeable`]: crate::util::ser::Writeable | ||
| /// [`CustomMessageReader`]: crate::ln::wire::CustomMessageReader | ||
| #[macro_export] | ||
| macro_rules! impl_writeable_msg { | ||
| ($st:ident, {$($field:ident),* $(,)*}, {$(($type: expr, $tlvfield: ident, $fieldty: tt)),* $(,)*}) => { | ||
| impl $crate::util::ser::Writeable for $st { | ||
| fn write<W: $crate::util::ser::Writer>(&self, w: &mut W) -> Result<(), $crate::io::Error> { | ||
| $( self.$field.write(w)?; )* |
There was a problem hiding this comment.
This requires a use $crate::util::ser::Writeable, no?
There was a problem hiding this comment.
Hm, doesn't seem so for the doctest, and also seems to work in a trivial freestanding example?
There was a problem hiding this comment.
Right, in your example you have an explicit use lightning::util::ser::Writeable;, my point was that if there isn't one in the file in question it needs to be in the macro.
There was a problem hiding this comment.
Not sure. a) the example builds fine without the import, it is only needed for the my_custom_msg.write call. Initial version with just the impl works just fine b) If I add the use statement to the macro it's immediately marked as unused. 🤷♂️
There was a problem hiding this comment.
Huh, that's...very surprising to me. Whatever, then.
|
I agree we should make it support non- |
By user request, we expose the
impl_writeable_msgmacro which can be useful for implementing custom message types.