Skip to content

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

Merged

Conversation

valentinewallace
Copy link
Contributor

Needed for the bindings and gives a bit more flexibility for users as well

@valentinewallace valentinewallace force-pushed the 2022-10-custom-om-self branch 2 times, most recently from 518dcc0 to 70566de Compare October 27, 2022 16:43
@valentinewallace valentinewallace force-pushed the 2022-10-custom-om-self branch 2 times, most recently from af59fc9 to 224a987 Compare October 27, 2022 18:37
@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2022

Codecov Report

Base: 90.72% // Head: 90.67% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (be5b548) compared to base (2e343e7).
Patch coverage: 93.75% of modified lines in pull request are covered.

❗ Current head be5b548 differs from pull request most recent head 150c87a. Consider uploading reports for the commit 150c87a to get more accurate results

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     
Impacted Files Coverage Δ
lightning/src/ln/peer_handler.rs 55.82% <0.00%> (ø)
lightning/src/util/ser.rs 91.66% <ø> (ø)
lightning/src/ln/onion_utils.rs 94.87% <100.00%> (-0.05%) ⬇️
lightning/src/onion_message/functional_tests.rs 95.48% <100.00%> (ø)
lightning/src/onion_message/messenger.rs 89.95% <100.00%> (ø)
lightning/src/onion_message/packet.rs 76.03% <100.00%> (+0.19%) ⬆️
lightning/src/ln/peer_channel_encryptor.rs 93.38% <0.00%> (-0.25%) ⬇️
lightning/src/chain/onchaintx.rs 95.06% <0.00%> (-0.23%) ⬇️
lightning/src/chain/channelmonitor.rs 90.65% <0.00%> (-0.06%) ⬇️
lightning/src/ln/functional_tests.rs 96.96% <0.00%> (-0.02%) ⬇️
... and 3 more

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

@TheBlueMatt
Copy link
Collaborator

Can you drop the second commit? It actually needs cfg(anchors) and @tnull is gonna do it in #1790

Copy link
Contributor

@jkczyz jkczyz left a 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.

+ 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]>
@TheBlueMatt TheBlueMatt merged commit 6957fb6 into lightningdevkit:main Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants