Small tweaks post #633/#684 #690
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #690 +/- ##
=======================================
Coverage 91.96% 91.96%
=======================================
Files 35 36 +1
Lines 20154 20161 +7
=======================================
+ Hits 18534 18541 +7
Misses 1620 1620
Continue to review full report at Codecov.
|
ariard
reviewed
Sep 15, 2020
lightning/src/ln/chan_utils.rs
Outdated
| /// a broadcaster's commitment transactions. This key is static across every commitment | ||
| /// transaction. | ||
| /// The public key on which the non-broadcaster (ie the countersignatory) receives an immediately | ||
| /// spendable primary channel balance. This key is static across every commitment transaction. |
There was a problem hiding this comment.
Is broadcaster's commitment transaction clearer ?
There was a problem hiding this comment.
Its a little wordy now, but at least no one is going to confuse it...better?
TheBlueMatt
force-pushed
the
2020-09-633-bindings
branch
3 times, most recently
from
2c2724a to
8407ae6
Compare
|
Further updated bindings to include #683. |
They all have a specific structure, so having them in the mess that is functional_tests isn't really conducive to readability. More importantly, functional_tests is so big it slows down compilation, so even dropping a few hundred lines is a win.
* Channel::get_counterparty_htlc_minimum_msat() returned holder_htlc_minimum_msat, which was obviously incorrect. * ChannelManager::get_channel_update set htlc_minimum_msat to Channel::get_holder_htlc_minimum_msat(), but the spec explicitly states we "MUST set htlc_minimum_msat to the minimum HTLC value (in millisatoshi) that the channel peer will accept." This makes sense because the reason we're rejecting the HTLC is because our counterparty's HTLC minimum value is too small for us to send to them, our own HTLC minimum value plays no role. Further, our router already expects this - looking at the same directional channel info as it does fees. Finally, we add a test in the existing onion router test cases which fails if either of the above is incorrect (the second issue discovered in the process of writing the test).
TheBlueMatt
force-pushed
the
2020-09-633-bindings
branch
from
8407ae6 to
0f6b000
Compare
|
Chatted a bit with @ariard about this one on IRC. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes one bug that was noticed in review #633 (see #680 (comment) / #633 (comment)), updates the bindings for both 633 and 684, and tweaks one comment from 633.