Skip to content
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

feeadjuster: various updates/fixes #525

Merged
merged 10 commits into from
May 25, 2024

Conversation

daywalker90
Copy link
Contributor

@daywalker90 daywalker90 commented May 8, 2024

Ok so i was bored and went over the feeadjuster plugin because @grubles said there were problems with it.

I found a compatibility issue in #457 with listconfigs so i made a wrapper function to support both the old and new structure of listconfigs.
I went over all the feeadjuster issues that were reporting bugs and most of them are probably users who updated their node but not their pyln-client, since we can't raise the version in requirements.txt ourselves because that would also break compatibility (maybe fixed by this ?).

But i also found an actual issue in #199 which i fixed by always getting the channel balance from a rpc call to listpeers/listpeerchannels instead of doing math with the data from the forward_event. We need to do this because between forwards there can be invoices getting paid, payments sent out or channel splices. Listening for these events is rather pointless since they don't contain data about what channel balances changed, so we would have to do rpc calls anyways. Unfortunately listpeers/listpeerchannels does not update quite as fast as the forward_event so we have to wait a bit.

Then @grubles pointed me to the PR #467 from @m-schmoock and i incorporated it together with my fixes.

Then i tested feeadjuster against CLN 23.02.2, 23.05.2, 23.08.1, 23.11.2 and 24.02.2

I had to adjust the tests a bit to accomodate for the bad gossip retransmission in older CLN versions (it's still slightly flaky but who cares)

just rewrite it in rust lul

@chrisguida
Copy link
Collaborator

Excellent work!

@chrisguida
Copy link
Collaborator

chrisguida commented May 24, 2024

@m-schmoock @telelvis care to approve this?

@telelvis
Copy link
Contributor

fantastic work by @daywalker90 , imo.
I am all for better feeadjuster!

@chrisguida chrisguida merged commit 5ea35a4 into lightningd:master May 25, 2024
6 checks passed
@chrisguida
Copy link
Collaborator

Closes #457 and #199

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