-
Notifications
You must be signed in to change notification settings - Fork 111
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
Tap channels liquidity fixes #1120
Tap channels liquidity fixes #1120
Conversation
"Parent" PR (itest in litd) for more context & linked issues: |
Pull Request Test Coverage Report for Build 10938152046Details
💛 - Coveralls |
3e4f920
to
2ea5adb
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.
Nice, I think this should work.
@@ -140,6 +140,33 @@ func (s *AuxTrafficShaper) PaymentBandwidth(htlcBlob, | |||
return linkBandwidth, nil | |||
} | |||
|
|||
// Get the minimum HTLC amount, which is just above dust. | |||
minHtlcAmt := lnwire.NewMSatFromSatoshis(DefaultOnChainHtlcAmount) |
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.
Hmm, looking at the previous commit, I wondered if this value needed to be dynamic, based on the channel config's dust limit.
But luckily for lnd
to lnd
this value is always static: https://github.com/lightningnetwork/lnd/blob/b58fcf73fff36a0ac45e89fac2d7aa589fa6c47b/lnwallet/wallet.go#L1419
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.
I see, perhaps worth keeping track of somehow, in case we ever have tap channel between different LN node implementations
2ea5adb
to
67b4d2e
Compare
1ea6175
to
a875044
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.
Only nits, other than that LGTM 🌮
a875044
to
ed13ed7
Compare
ed13ed7
to
a6f5c3e
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.
Great fixes, thank you! LGTM 🎉
This PR adds some small fixes for liquidity related bugs that were reported on tap channels.