-
Notifications
You must be signed in to change notification settings - Fork 912
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
Ignore fees per channel #6398
Ignore fees per channel #6398
Conversation
cf4774a
to
ebee017
Compare
ebee017
to
5462a4b
Compare
If you have time to rebase it, I will give a shot for the review today :) |
5462a4b
to
0859e1c
Compare
Done, thanks! |
https://fedorapeople.org/~wtogami/a/2023/0001-DO-NOT-COMMIT-Temporary-workaround-for-LND-update_fe.patch Your Would it be possible to add a print to let you know every time an ignore fee event happened? What I like about my ugly hack is the print tells you exactly what is happening. It is diagnostically useful to know because it should happen JUST ONCE per channel to recover from LND's bug after their fee estimator was broken in the past. If it happens more than once the remote peer's fee estimator is currently broken. You want to know that so there is a chance to talk to the sysadmin to be able to fix it. It appears this PR currently would be silent when it ignores which is less informative? |
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.
The critical logic LGMT but I had some comments from the API point of view.
I was wondering if the following CI failure is related?
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
ERROR tests/test_opening.py::test_anchor_min_emergency - ValueError:
Node errors:
- lightningd-1: had bad gossip messages
Global errors:
======= 59 passed, 3 skipped, 4 warnings, 1 error in 2441.54s (0:40:41) ========
For now, it's set from the global config. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…llers. Since it's going to be per-channel, this makes more sense. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `listpeerchannels` has a new field `ignore_fee_limits` Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's always true, now deprecated APIs removed. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Added: JSON-RPC: `setchannel` adds a new `ignorefeelimits` parameter to allow peer to set arbitrary commitment transaction fees on a per-channel basis.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
0859e1c
to
7ec9faa
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.
ACK 7ec9faa
This may be possible, but the current PR works by telling channeld an infinite allowable range. I think we can do logging in lightningd however, if the actual feerate used is weird. I will do this as a separate PR. |
We always had a global
ignore-fee-limits
: this adds an equivalent on a per-channel basis. This is a workaround, of course: letting peers set arbitrary fees is fraught, but there are cases where this can help. For example, there seems to be a problem where LND nodes insist on retransmitting the same fee, even if it knows it's now wrong (e.g. neutrino, or bitcoind restart). We will keep closing the connection on them.With this, you could temporarily
setchannel id=<channel> ignorefeelimits=true
and it will hopefully make progress. You can also use it to "trust" peers, and not get upset if there are fee disagreements.