-
Notifications
You must be signed in to change notification settings - Fork 268
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
Fix override-features implementation #1549
Conversation
We were calling `nodeParams.features` from inside the channel, which is problematic because we may have overriden those features for specific peers. This is now fixed. `IncomingPacket.decrypt` needs features to tell whether we are using variable onion or not. What features should we use? Do we have an unwritten rule that node-level features shouldn't be overriden? This happens in the relayer and in the post-restart-htlc-cleaner.
Note that if we change the |
Codecov Report
@@ Coverage Diff @@
## master #1549 +/- ##
=========================================
Coverage ? 87.21%
=========================================
Files ? 139
Lines ? 10902
Branches ? 428
=========================================
Hits ? 9508
Misses ? 1394
Partials ? 0
|
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.
Do we have an unwritten rule that node-level features shouldn't be overriden?
I think so, yes. It feels to me that turning var_onion_optin
off is asking for trouble...
There are some features that we may want to make mandatory at some point, and disallow users turning them off, don't you think? It will also allow us to simplify the code by removing some unnecessary if
statements. I'm thinking that var_onion_optin
is exactly a good candidate for that, there's too much risk of footgun for users if we let them turn it off (it's been battle-tested with 1.5 years in the wild and 0 drawbacks)
This is an alternative to #1498.
We were calling
nodeParams.features
from inside the channel, which isproblematic because we may have overriden those features for specific
peers. This is now fixed.
IncomingPacket.decrypt
needs features to tell whether we are usingvariable onion or not. What features should we use? Do we have an
unwritten rule that node-level features shouldn't be overriden? This
happens in the relayer and in the post-restart-htlc-cleaner.