This can happen, and in fact does below in our test_autoclean_once
test where we update the datastore, and return from the cmd.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is usually fine, but without this, commando (another branch!) has
a race:
1. A command has multiple parts.
2. We start sending them out.
3. We get a response, which completes the cmd.
4. We go to send the next one out, but it's been freed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. If the tool changes, you need to regenerate since the output may
change.
2. This didn't just filter that out, ignored all but the first
dependency, which made bisecting the bookkeeper plugin a nightmare:
it didn't regenerate the .po file, causing random crashes.
If we want this, try $(filter-out tools/fromschema.py) instead. But I
don't think we want that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
we weren't making records for 'missed' accounts that had a zero balance
at snapshot time (if peer opens channel and is unused)
Fixes: #5502
Reported-By: https://github.com/niftynei/cln-logmaid
Track rebalances, and report income events for them.
Previously `listincome` would report:
- invoice event, debit, outgoing channel
- invoice_fee event, debit, outgoing channel
- invoice event, credit, inbound channel
Now reports:
- rebalance_fee, debit, outgoing channel
(same value as invoice_fee above)
Note: only applies on channel events; if a rebalance falls to chain
we'll use the older style of accounting.
Changelog-None
When traversing the list we call `command_finished` which modifies the
list we are traversing. This ensures we don't end up advancing in the
list iteration.
Reported-by: Rusty Russell <@rustyrussell>
Technically this is a use-after-free since `command_finished` frees
the `cmd` which is also the parent of `p`, so reset it early. All
paths lead to `command_finished` so setting it early is ok.
Reported-by: Rusty Russell <@rustyrussell>
Suspended payments now have the same groupid as the actual attempt,
this allows us to identify pay calls that were suspended due to us and
terminate only those.
Changelog-Fixed pay: Fixed a crash when `pay` was called multiple times while an attempt was already in progress.
This should prevent us from accidentally completing a payment twice,
when replaying the result of an actual attempt against pay call that
was suspended due to it still being pending.
We rely on sqlite3 being present to run unit tests for some bookkeeper
tests; here we effectively disable these tests if not available
Fixes report in #4928
Reported-By: @whitslack
The initial snapshots on an already-running lightningd are expected to
be unbalanced, but this shouldn't cause users to long for the green,
green grass of home.
This controls the Art of Noise.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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#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.
Keep the accounts as an 'append only' log, instead we move the marker
for the 'channel_open' forward when a 'channel_open' comes out.
We also neatly hide the 'channel_proposed' events in 'inspect' if
there's a 'channel_open' for that same event.
If you call inspect before the 'channel_open' is confirmed, you'll see
the tag as 'channel_proposed', afterwards it shows up as
'channel_open'. However the event log rolls forward -- listaccountevents
will show the correct history of the proposal then open confirming (plus
any routing that happened before the channel confirmed).
We need a record of the channel account before you start sending
payments through it. Normally we don't start allowing payments to be
sent until after the channel has locked in but zeroconf does away with
this assumption.
Instead we push out a "channel_proposed" event, which should only show
up for zeroconfs.
This was originally done as part of the bookkeeper introduction, but it deserves its
own patch (and that one didn't remove the bitcoin/chainparams.o from the individual
requirements lines).
Suggested-by: @niftynei
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
`lightningd` has an option --wallet that lets you supply a database dsn
string to connect to a sqlite3/postgres database that's hosted/stored
elsewhere.
This adds the `--bookkeeper-db` option which does the same, except for
the bookkeeping data for a node!
Note that the default is to go in the `lightning-dir` in a database
called `accounts.sqlite3`
First off, when we pull data out of JSON, unescape it so we don't end up
with extraneous escapes in our bookkeeping data. I promise, it's worth
it.
Then, when we print descriptions out to the csvs, we gotta wrap
everything in quotes... but also we have to change all the double-quotes
to singles so that adding the quotes doesn't do anything untoward.
We also just pass it thru json_escape to get rid of linebreaks etc.
Note that in the tests we do a byte comparison instead of converting the
CSV dumps to strings because python will escape the strings on
conversion...
So we print out invoice fees on the same line for those CSVs! This means
we have to do a little bit of gymnastics (but not too bad):
- we save the fee amount onto the income event now so we can use
it later
- we ignore every "invoice_fee" event for the koinly/cointracker
Note that since we're not skipping income events in the loops we also
move the newline character to the start of every `_entry` function so
skipped records dont incur empth lines.
Changelog-Added: bkpr: print out invoice fees on the same line for `koinly` and `cointracker` csv types
It's possible we'll get an "external" event for an account/channel and
then try to do close checks on it and it'll bomb out because we didn't
go look up the originating account to make sure that that had open info
Now, we make sure and do that when we hear about an originating account
If two events for the same (unlogged) account come in and get run
through the "lookup peer" code, we should anticipate that.
We do two things here:
- one, if it's a duplicate "event" to create the channel open
we check for that and just exit early
- two, we were using a copy of the account that was
fetched/pulled from before the RPC ran, so instead we
just re-pull the most up to date account info for the
close checks.
This fixes the crash I was getting in re-running these things
If we expect further events for an onchain output (because we can steal
it away from the 'external'/rightful owner), we mark them.
This prevents us from marking a channel as 'onchain-resolved' before
all events that we're interested in have actually hit the chain.
Case that this matters:
Peer publishes a (cheating) unilateral close and a timeout htlc (which
we can steal).
We then steal the timeout htlc.
W/o the stealable flag, we'd have marked the channel as resolved when
the peer published the timeout htlc, which is incorrect as we're still
waiting for the resolution of that timeout htlc (b/c we *can* steal it).
We were failing to mark channels as resolved b/c we weren't using later
events to external (but originated from this account) events as signals
to run the channel resolution check.
This fixes that, and adds a test.
Adds schema definitions and manpages for bkpr- commands; also renames
the commands to all start with 'bkpr-', so they're easier to identify/
make runes about.
it's nice to know what node your channel was opened with. in theory we
could use listpeers to merge the data after the fact, except that
channels disappear after they've been closed for a bit. it's better to
just save the info.
we print it out in `listbalances`, as that's a great place account level
information
Anchor outputs are ignored by the clightning wallet, but we keep track
of them in the bookkeeper. This causes problems when we do the balance
checks on restart w/ the balance_snapshot -- it results in us printing
out a journal_entry to 'get rid of' the anchors that the clightning node
doesnt know about.
Instead, we mark some outputs as 'ignored' and exclude these from our
account balance sums when we're comparing to the clightning snapshot.
Our consolidate fees had a crash bug (and was pretty convoluted). This
makes it less convoluted and resolves the crash.
The only kinda meh thing is that we have to look up the most recent
timestamp data for the onchain fee entry separately, because of the way
SQL sums work.
For income events, break out the amount paid in routing fees vs the
total amount of the *invoice* that is paid.
Also printout these fees, when available, on listaccountevents
Rusty Russell
niftynei
Christian Decker
William Casarin