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

Fix for #5557 #5587

Merged
merged 5 commits into from
Sep 14, 2022
Merged

Fix for #5557 #5587

merged 5 commits into from
Sep 14, 2022

Conversation

niftynei
Copy link
Collaborator

Crash found in bookkeeper around duplicate lease_fees! Here we fix them.

Reproduce crash for ElementsProject#5557!

If we record the channel open because bookkeeper was added after the
channel open request started but the channel confirms later, we end up
with re-recording any associated push or leased fees (paid or rcvd).

In the case where you've paid for these fees, your channel balance goes
negative and the node crashes the next time you call `listbalances`.

Reported-by: @chrisguida
We were double counting channel lease fees because we were double firing
the channel open event sequence (so to speak). If we don't report
balances for unopened channels, we don't have this problem?

Changelog-Changed: Plugins: `balance_snapshot` notification does not send balances for channels that aren't locked-in/opened yet
Clean up for ElementsProject#5557.

If you've got duplicate 'lease_fee' entries, we delete them!
Make sure that we're not issuing duplicate lease_fee events!
test_bookkeeping_closing_trimmed_htlcs fails to find 'all outputs
resolved' occassionally, seems like it's because the
OUR_DELAYED_TO_WALLET doesn't make it into the mempool before we start
mining blocks?

So here make sure there's something in the mempool before before we
start making new blocks.
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack 3e903e6

@rustyrussell rustyrussell merged commit 3ad8347 into ElementsProject:master Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants