Skip to content

One final fix + cut 0.0.103 #1153

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
merged 3 commits into from
Nov 3, 2021

Conversation

TheBlueMatt
Copy link
Collaborator

No description provided.

@TheBlueMatt TheBlueMatt added this to the 0.0.103 milestone Nov 3, 2021
@codecov
Copy link

codecov bot commented Nov 3, 2021

Codecov Report

Merging #1153 (2b837bb) into main (55fc0a1) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1153      +/-   ##
==========================================
- Coverage   90.15%   90.14%   -0.01%     
==========================================
  Files          70       70              
  Lines       36386    36386              
==========================================
- Hits        32804    32801       -3     
- Misses       3582     3585       +3     
Impacted Files Coverage Δ
lightning/src/routing/scorer.rs 34.42% <ø> (ø)
lightning/src/ln/functional_tests.rs 97.29% <0.00%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55fc0a1...2b837bb. Read the comment docs.

Copy link

@ariard ariard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise SGTM. I checked the change logs since 0.0.102 and that all relevant new stuff is mentioned.

@TheBlueMatt TheBlueMatt force-pushed the 2021-11-0.0.103 branch 2 times, most recently from e4511b0 to b43e814 Compare November 3, 2021 02:12
@ariard
Copy link

ariard commented Nov 3, 2021

ACK b43e814

@TheBlueMatt
Copy link
Collaborator Author

Squashed without diff. Will land after CI 🎉

$ git diff-tree -U1 b43e81402 2b837bb27
$

@TheBlueMatt TheBlueMatt merged commit e3bdfa0 into lightningdevkit:main Nov 3, 2021
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.

Post-merge review of the CHANGELOG. All else LGTM! 🎉

## API Updates
* This release is almost entirely focused on a new API in the
`lightning-invoice` crate - the `InvoicePayer`. `InvoicePayer` is a
struct which takes a reference to a `ChannelManager` and a `NetworkGraph`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should replace NetworkGraph with Router. I almost wonder if we should move the Router trait to the lightning crate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, IMO.

Comment on lines +44 to +48
* If a payment is sent, creating an outbound HTLC and sending it to our
counterparty (implying the `ChannelMonitor` was persisted on disk), but the
`ChannelManager` was not persisted prior to shutdown/crash, no
`Event::PaymentPathFailed` event will be generated if the HTLC is eventually
failed on chain (#1104).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be worded in terms of what the bug was whereas the previous bullet is worded in terms of how the bug was fixed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, we've generally worded in terms of the bug, but I wasn't really sure how to word the previous in terms of the bug and still have it be clear. I'll try, though.

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