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

bookkeeper crash fixes #5477

Merged
merged 3 commits into from
Jul 31, 2022
Merged

Conversation

niftynei
Copy link
Collaborator

Fixes two crash reports, and adds back some missing functionality for the snapshots in coin-moves.

@niftynei niftynei added this to the v0.12 milestone Jul 29, 2022
@niftynei niftynei requested a review from cdecker as a code owner July 29, 2022 04:05
tests/test_bookkeeper.py Show resolved Hide resolved
@@ -437,7 +437,7 @@ void json_add_income_event(struct json_stream *out, struct income_event *ev)

const char *csv_filename(const tal_t *ctx, const struct csv_fmt *fmt)
{
return tal_fmt(ctx, "cln_incomestmt_%s_%zu.csv",
return tal_fmt(ctx, "cln_incomestmt_%s_%li.csv",
fmt->fmt_name,
time_now().ts.tv_sec);
Copy link
Contributor

Choose a reason for hiding this comment

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

That will break on some platforms too! Use an "unsigned long" cast...

9cad7d6 changed the behavior of `channel_active`, which slightly
broke how our balance snapshots work (we need info about channels that
aren't on-chain yet).

This patches adds AWAITING_UNILATERAL back in.
First, how we record "our_funds" and then apply pushes vs lease_fees
(for liquidity ad buys/sales) was exactly opposite.

For pushes we were reporting the total funded into the channel, with the
push representing how much we'd later moved to the peer.

For lease_fees we were rerporting the total in the channel, with the
push representing how much was already moved to the peer.

We fix this (from a view perspective) by re-adding lease fees to what's
reported in the channel funding totals. Since this is now new behavior
(for leased channel values), we added new fields so we can take the old
field names thru a deprecation cycle.

We also make it possible to differentiate btw a push and a lease_fee
(before they were all the same), by adding to new fields to `listpeers`:
`fee_paid_msat` and `fee_rcvd_msat`.

This allows us to avoid math in the bookkeeper, instead we just pick
the numbers out directly and record them.

Fixes ElementsProject#5472

Changelog-Added: JSON-RPC: `listpeers` now has a few new fields for `funding` (`remote_funds_msat`, `local_funds_msat`, `fee_paid_msat`, `fee_rcvd_msat`).
Changelog-Deprecated: JSON-RPC: `listpeers`.`funded` fields `local_msat` and `remote_msat` are now deprecated.
@rustyrussell
Copy link
Contributor

Ack 31e5b82

@rustyrussell rustyrussell merged commit 3445882 into ElementsProject:master Jul 31, 2022
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.

2 participants