Bookkeeper migration part 1: the cleanups #8445
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustyrussell
force-pushed
the
bkpr-part1
branch
from
b48f4bc to
5f1bdad
Compare
Merged
niftynei
reviewed
Aug 12, 2025
| assert(mvt_tags_valid(tags)); | ||
|
|
||
| /* We put the *primary* tag first */ | ||
| for (size_t i = 0; i < NUM_MVT_TAGS; i++) { |
|
|
||
| enum mvt_tag *new_tag_arr(const tal_t *ctx, enum mvt_tag tag); | ||
| /* Convenience macro for creating tag bitmaps */ | ||
| #define mk_mvt_tags(...) mk_mvt_tags_(__VA_ARGS__, 999) |
rustyrussell
force-pushed
the
bkpr-part1
branch
from
5f1bdad to
dcdfec6
Compare
rustyrussell
force-pushed
the
bkpr-part1
branch
from
dcdfec6 to
5ee11b8
Compare
We can use these to test migrations. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Print the error! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means it doesn't have to be a tal ptr. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes our final balance not match our wallet: 1. We only spend the anchor when we need to boost the commitment tx, which we don't always do (sometimes the peer does, sometimes it's not worth it). 2. We don't put the UTXO in our wallet, because we don't consider it "ours": anyone can spend it after 16 blocks. We used to use the tag "ignored" for this, but that's overly complex IMHO. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't actually set it any more. The bookkeeper db does a migration for old anchors. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is how we handle amount_msat and amount_sat everywhere these days, and this wasn't updated. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…cations. Rather than converting to a generic coin_mvt struct, use these directly in the notification, which is more explicit. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we only ever use `struct chain_coin_mvt` or `struct channel_coin_mvt`. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The part id is *only* unique within a group. The payment_hash / partid / groupid tuple is unique. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Added: Plugins: `coin_movement` notification with `part_id` field now always has `group_id` field.
More readable for me. Also, change order so we definitely break compilation on all callers (putting enum before amount). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…others. This means we can keep a pointer to the channel directly, *or* a string. This avoids gratuitous formatting (on creation) and lookups (later). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make the common fields the first ones, and make part_and_group and payment_hash const pointers. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Prefix MVT_ to them, for clarity. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This isn't a robust assumption, so sort them before comparison. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Undocumented, but the first tag in the coin_movement notification is considered the primary tag, and the others are optional. The bookkeeper plugin relies on this! Enforce that this is true, and in the process document in the code which is the primary tag. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than open-coding in json_parse. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to store them in the db this way, so I thought I'd see what it looks like if we lift that interface all the way through. We use a struct, so that types are checked strictly. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…d `extra_tags`. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Deprecated: JSON-RPC: `coin_movement` notification `tags` array (use `primary_tag` and `extra_tags`). Changelog-Added: JSON-RPC: `coin_movement` notification `primary_tag` and `extra_tags`.
This is not particularly relevant now (it's always the current time) but will be useful when we implement the list commands. Note that timestamp is set to be "u32" in various schemas. This will only become a problem on Sun 07 Feb 2106 06:28:15 UTC. I apologize to my grandchildren in advance. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's always set, and in fact we assume it is (journal entries are not internal to lightningd, so we won't see them in lightningd/notification.c: that comment is misleading). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…st functions. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Deprecated: JSON-RPC: `coin_movement` notification `utxo_txid`, `vout` and `txid` fields (use `utxo` and `spending_txid`). Changelog-Added: JSON-RPC: `coin_movement` notification `utxo` field. Changelog-Added: JSON-RPC: `coin_movement` notification `spending_txid` field.
…common/coin_mvt.h They're scattered and reproduced in many places: unify them. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Only used in tests. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to get rid of this concept, but the main change is that the account_get_balance API can be drastically simplified: account_get_credit_debit() accesses the raw fields, never fails, but returns the a flag which tells us if the account doesn't actually have any events. The one place we care about the balance, calculate by hand. Then account_get_balance() (and struct account_balance) can simply be moved to th test. Subtly, without the "GROUP BY" clause, you always get one row, even if there are no rows (but the SUM are null). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We still output the fields, they're just always the currency of the node. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Changed: Plugins: `bookkeeper` now explicitly assumes every transaction is in the same currency as the node (true unless you added manually)
rustyrussell
force-pushed
the
bkpr-part1
branch
from
5ee11b8 to
dacc037
Compare
bookkeeper used to generate these as channel events, now lightningd does. We also add a "journal" event, which we will need later too. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
bkpr-part1
branch
from
dacc037 to
3699f66
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does various cleanups in anticipation of moving the audit log of moves into the core.
The next part will do the rest (unfortunately, that's a giant PR because we need to do the bookeeper -> internal migration at the end, and we must do that before anyone records coin movements).