Page MenuHomePhabricator

[Chronik] Add `Group` and `GroupHistoryReader/-Writer` to index the tx history of a group
ClosedPublic

Authored by tobias_ruck on Mar 28 2023, 12:49.

Details

Summary

A group can be "address", or "SLP token ID" or other protocols.

This struct allows us to conveniently and generically index the tx history of all such groups. The legacy Chronik indexer only allowed indexing by script/address, however, many people have been asking to add support to index by other predicates.

Since the tx history for some addresses is already +1M, we store the history paginated. This also allows APIs to more easily and efficiently serve paginated tx history.

Txs can be grouped by implementing Group on some struct, which then receives GroupQuerys, and returns Members, which are then grouped and sorted into the DB.

Depends on D13498.

Test Plan

ninja check-crates

Diff Detail

Repository
rABC Bitcoin ABC
Branch
chronik-group-txs
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 22790
Build 45200: Build Diffbuild-chronik
Build 45199: arc lint + arc unit

Event Timeline

Fabien requested changes to this revision.Mar 28 2023, 20:58

More questions that requesting changes, generally LGTM

chronik/chronik-bridge/src/ffi.rs
34

This change seems unrelated

chronik/chronik-db/src/group.rs
16

It would make sense to add time_first_seen to query as a time based group ?

chronik/chronik-db/src/io/group_history.rs
49

Actually sorting in descending order is probably more useful for the common case of displaying txs for an address, as you want to show the most recent txs first

117

Do you know about Duff's device ? No idea if that can be done with rust though (not requesting change here)

152

Can you explain why this is needed ? IIUC you need to check that there is no remaining removes when it completed page_num == 0, but the decrement should be a no-op since the loop will not run again ?

395
418

I don't understand this one, it looks like it's missing make_tx(4, [10], [40, 30, 40]), content

This revision now requires changes to proceed.Mar 28 2023, 20:58
chronik/chronik-bridge/src/ffi.rs
34

I can add it in another diff.

chronik/chronik-db/src/group.rs
16

It's easy enough to add once needed, and having GroupQuery very much simplifies adding new stuff here. Nobody asked for querying by time yet though, so let's add it when needed.

chronik/chronik-db/src/io/group_history.rs
49

Yes. However, storing them in descending order is very tricky, as we have to shift all txs in all pages for every insert. In the ascending case, we just append the new txs in order to the latest page (or start new ones) and we're done. In case of a block delete, we just delete the last few txs of the last page (or delete the latest page and then some txs in the previous page) and we're done.

Plus, we also need them in ascending order when a wallet tries to sync all tx history (e.g. when storing history locally, and only updating the latest txs).

Plus, while the code to transform from ascending db pages to descending history pages is a little hard to wrap one's mind around, it's quite efficient and already written, so if we can make sure that's clean and documented I don't see a reason to store them descendingly.

117
152

page_num -= 1; panics in debug mode if page_num == 0. Welcome to Rust!

Also we'd do one unnecessary DB lookup of page 0xffffffff.

418

No, only txs 2 and 4 have a value=30 in there, and only txs 3 and 4 have a value=40. So this test makes sense.

Simplify the interface by moving the config creation to the Group trait, add page_size and member_num_txs to GroupHistoryWriter.

Extract ValueGroup and other things so we can use them for tests in other places.

Sort mods alphabetically in chronik-db

This revision is now accepted and ready to land.Mar 29 2023, 19:48

Re-use the tx_num param in make_value_tx for the txid.