Rather than making the callers do this, make the invoice decoder perform
the various sanity checks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I used `amount_msat_eq(x, AMOUNT_MSAT(0))` because I forgot this
function existed. I probably missed it because the name is surprising,
so add "is" in there to make it clear it's a boolean function.
You'll note almost all the places which did use it are Eduardo's and
Lisa's code, so maybe it's just me.
Fix up a few places which I could use it, too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When you have *lots* of events in your bkpr database looking up a
specific event via calling bkpr-listaccountevents and using jq or
grep to filter gets very slow (and wasteful of CPU and disk resources).
This commit adds the paremeter payment_id to the call to filter for a
specific payment id via a where clause in the request to the database of bkpr.
Changelog-Added: Plugins: Add payment_id parameter to bkpr-listaccountevents to filter events.
Did a sweep to find any others, give this from sanitizer:
```
2024-08-09T18:06:45.1729472Z plugins/bkpr/recorder.c:2057:23: runtime error: load of value 190, which is not a valid value for type 'bool'
2024-08-09T18:06:45.1729877Z SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior plugins/bkpr/recorder.c:2057:23 in
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Valgrind found this. I think two PRs were added in parallel, which is why we
only found it after merge:
```
2024-08-09T01:57:23.7375752Z ==34263== Uninitialised byte(s) found during client check request
2024-08-09T01:57:23.7376275Z ==34263== at 0x172405: memcheck_ (mem.h:247)
2024-08-09T01:57:23.7376661Z ==34263== by 0x172585: db_bind_int (bindings.c:49)
2024-08-09T01:57:23.7377086Z ==34263== by 0x126233: log_chain_event (recorder.c:2057)
2024-08-09T01:57:23.7377544Z ==34263== by 0x11BF8B: json_utxo_deposit (bookkeeper.c:1735)
2024-08-09T01:57:23.7378207Z ==34263== by 0x12BED3: ld_command_handle (libplugin.c:1847)
2024-08-09T01:57:23.7378674Z ==34263== by 0x12C649: ld_read_json_one (libplugin.c:1998)
2024-08-09T01:57:23.7379114Z ==34263== by 0x12C780: ld_read_json (libplugin.c:2018)
2024-08-09T01:57:23.7379534Z ==34263== by 0x2990CB: next_plan (io.c:60)
2024-08-09T01:57:23.7379881Z ==34263== by 0x299D21: do_plan (io.c:422)
2024-08-09T01:57:23.7380230Z ==34263== by 0x299D88: io_ready (io.c:439)
2024-08-09T01:57:23.7380585Z ==34263== by 0x29C1BC: io_loop (poll.c:455)
2024-08-09T01:57:23.7380980Z ==34263== by 0x12D439: plugin_main (libplugin.c:2230)
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It might be nice to let the bookkeeper keep track of external accounts
as well as the internal onchain wallet? To this end, we add some new
custom notifications, which the bookkeeper will ingest and add to its
ledger.
Suggested-By: @chrisguida
Changelog-Added: PLUGINS: `bookkeeper` now listens for two custom events: `utxo_deposit` and `utxo_spend`. This allows for 3rd party plugins to send onchain coin events to the `bookkeeper`. See the new plugins/bkpr/README.md for details on how these work!
We do some fancy accounting for channel closures; since we're tagging
splice txs as closes we need to mark them as splices so we can treat them
as any other 'normal' on chain event.
This avoids globals (and means memleak traverses the variables!): we
only change over the test plugin though, to avoid unnecessary churn.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means we can see the values in listconfigs, even if we haven't set
them yet.
In particular, we now see the following:
* autoclean-cycle.value_int=3600
* bitcoin-rpcclienttimeout.value_int=60
* bitcoin-retry-timeout.value_int=60
* funder-max-their-funding.value_str=4294967295sat
* funder-per-channel-min.value_str=10000sat
* funder-reserve-tank.value_str=0sat
* funder-fund-probability.value_int=100
Changelog-Changed: plugins: libplugin now shows plugin option default values (where they're non-trivial)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Not singular (though we used to have a listinvoice, removed in v0.6.1).
Reported-by: "Plant" via email
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This has the benefit of being shorter, as well as more reliable (you
will get a link error if we can't print it, not a runtime one!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We stopped doing empty journal logs, so we no longer need to switch
our log severity based on whether or not an account exists.
Should make bookkeeper less chatty and remove noisy logs
Changelog-None
We were putting out a lot of empty journal entries. Let's
stop doing that.
Now the wallet balance stays uninitialized until/unless you have
funds in it.
Fixes#5672
Also, we don't need to pass the total length to the field parsers,
just the length for this field (confusingly, this was called
"data_length").
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I know this is an unforgivably large diff, but the spec has changed so
much that most of this amounts to a rewrite.
Some points:
* We no longer have "offer_id" fields, we generate that locally, as all
offer fields are mirrored into invoice_request and then invoice.
* Because of that mirroring, field names all have explicit offer/invreq/invoice
prefixes.
* The `refund_for` fields have been removed from spec: will re-add locally later.
* quantity_min was removed, max == 0 now mean "must specify a quantity".
* I have put recurrence fields back in locally.
This brings us to 655df03d8729c0918bdacac99eb13fdb0ee93345 ("BOLT 12:
add explicit invoice_node_id.")
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is what we do in lightningd, which makes memleak much more forgiving:
you can hang temporaries off cmd without getting reports of leaks (also
when send_outreq called).
We remove all the notleak() calls in plugins which worked around this!
And avoid multiple notleak labels, since both send_outreq() and
command_still_pending() can be called multiple times.
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
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.
`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...
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.
Add ability to filter results by timestamp.
Note that "start_time" is everything *after* that timestamp; and
"end_time" is everything *up to and including* that timestamp.
Prints out the `listincome` events as a CSV formatted file
Current csv_format options:
- koinly
- cointracker
- harmony (https://github.com/harmony-csv/harmony)
- quickbooks*
*Quickbooks expects values in 'USD', whereas we print values out
in <currency> (will be noted in the Description field). This won't work
how you'd expect -> you might need to do some conversion etc before
uploading it.
All amounts emitted as 'btc' w/ *11* decimals.
