-
Notifications
You must be signed in to change notification settings - Fork 403
Give us a self when reading a custom onion message #1809
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
Give us a self when reading a custom onion message #1809
Conversation
518dcc0
to
70566de
Compare
af59fc9
to
224a987
Compare
Codecov ReportBase: 90.72% // Head: 90.67% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1809 +/- ##
==========================================
- Coverage 90.72% 90.67% -0.05%
==========================================
Files 87 87
Lines 47364 47910 +546
Branches 47364 47910 +546
==========================================
+ Hits 42970 43442 +472
- Misses 4394 4468 +74
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. |
224a987
to
be5b548
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just a couple of nits.
be5b548
to
9eeee96
Compare
+ remove MaybeReadableArgs trait as it is now unused + remove onion_utils::DecodeInput as it would've now needed to be parameterized by the CustomOnionMessageHandler trait, and we'd like to avoid either implementing DecodeInput in messenger or having onion_utils depend on onion_message::* Co-authored-by: Matt Corallo <[email protected]> Co-authored-by: Valentine Wallace <[email protected]>
9eeee96
to
150c87a
Compare
Needed for the bindings and gives a bit more flexibility for users as well