- 
                Notifications
    You must be signed in to change notification settings 
- Fork 421
logging every sent receive onion message #2642
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
Conversation
| Codecov ReportAttention:  
 
 ❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@            Coverage Diff             @@
##             main    #2642      +/-   ##
==========================================
- Coverage   88.64%   88.62%   -0.03%     
==========================================
  Files         115      115              
  Lines       90257    90270      +13     
  Branches    90257    90270      +13     
==========================================
- Hits        80009    79999      -10     
- Misses       7842     7866      +24     
+ Partials     2406     2405       -1     ☔ View full report in Codecov by Sentry. | 
| Looks like this doesn't build currently. | 
| See the "Checks" tab on this PR:  | 
| Specifically,  | 
| Could you update/squash your commit history per https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches? I think this is close | 
| Looks like this is failing (doc)tests. | 
| 
 I am very confused about the error here , error is not specific here. | 
| 
 CI shows: Here line 18 refers to the 18th line within the example. So test is panicking because  | 
| Looks like this needs a rebase when you next push, sorry about that. | 
f1abd13    to
    84067e0      
    Compare
  
    4fa24ec    to
    2ad1969      
    Compare
  
    | LGTM. Please squash your commits down to one. | 
ed6ea03    to
    b7d38fb      
    Compare
  
    Logs every sent + receive for P2P messages solves lightningdevkit#2346
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. Patch could be a bit cleaner in a few places but its not a big deal. @jkczyz'd already acked the same patch which was only rather trivially rebased.


This Pr solves issue #2346