Page MenuHomePhabricator
Differential D13428

[Chronik] Bridge txs in `bridge_block`
ClosedPublic
Actions

Authored by tobias_ruck on Mar 22 2023, 22:19.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC0b9aebe337a2: [Chronik] Bridge txs in `bridge_block`
Summary

To index txs, we have to bridge them over from the node.

Therefore, we add shared structs for BlockTx, Tx, OutPoint, TxInput, TxOutput, Coin, and bridge them in bridge_block.

We also set the positions for the block and undo data for each individual tx, so we can later read them again. This is a rather dangerous thing and easy to get wrong, which is why we thoroughly test this behavior. We add ReadTxFromDisk and ReadTxUndoFromDisk to blockstorage.h to test this. We'll need this later when querying txs anyway.

Test Plan

ninja test_bitcoin && ./src/test/test_bitcoin -t bridge_tests

Diff Detail

Event Timeline

tobias_ruck created this revision.Mar 22 2023, 22:19
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 22 2023, 22:19
Harbormaster completed remote builds in B22603: Diff 38776.Mar 22 2023, 22:41
tobias_ruck updated this revision to Diff 38832.Mar 24 2023, 10:54

Add through tests, make more robust

tobias_ruck updated this revision to Diff 38833.Mar 24 2023, 11:02

Remove unnecessary blocks vector

Harbormaster completed remote builds in B22650: Diff 38832.Mar 24 2023, 11:11
teamcity added a comment.Mar 24 2023, 11:12
Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.

Tail of the build log:

wallet_disable.py                         | ✓ Passed  | 1 s
wallet_dump.py                            | ✓ Passed  | 4 s
wallet_encryption.py                      | ✓ Passed  | 5 s
wallet_encryption.py --descriptors        | ✓ Passed  | 5 s
wallet_groups.py                          | ✓ Passed  | 13 s
wallet_hd.py                              | ✓ Passed  | 6 s
wallet_hd.py --descriptors                | ✓ Passed  | 5 s
wallet_import_rescan.py                   | ✓ Passed  | 7 s
wallet_import_with_label.py               | ✓ Passed  | 1 s
wallet_importdescriptors.py               | ✓ Passed  | 3 s
wallet_importmulti.py                     | ✓ Passed  | 3 s
wallet_importprunedfunds.py               | ✓ Passed  | 2 s
wallet_importprunedfunds.py --descriptors | ✓ Passed  | 2 s
wallet_keypool.py                         | ✓ Passed  | 3 s
wallet_keypool_topup.py                   | ✓ Passed  | 2 s
wallet_keypool_topup.py --descriptors     | ✓ Passed  | 5 s
wallet_labels.py                          | ✓ Passed  | 2 s
wallet_labels.py --descriptors            | ✓ Passed  | 1 s
wallet_listreceivedby.py                  | ✓ Passed  | 5 s
wallet_listsinceblock.py                  | ✓ Passed  | 5 s
wallet_listsinceblock.py --descriptors    | ✓ Passed  | 5 s
wallet_listtransactions.py                | ✓ Passed  | 4 s
wallet_listtransactions.py --descriptors  | ✓ Passed  | 4 s
wallet_multiwallet.py                     | ✓ Passed  | 10 s
wallet_multiwallet.py --usecli            | ✓ Passed  | 10 s
wallet_reorgsrestore.py                   | ✓ Passed  | 3 s
wallet_resendwallettransactions.py        | ✓ Passed  | 4 s
wallet_send.py                            | ✓ Passed  | 7 s
wallet_startup.py                         | ✓ Passed  | 2 s
wallet_timelock.py                        | ✓ Passed  | 1 s
wallet_txn_clone.py                       | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock           | ✓ Passed  | 3 s
wallet_txn_doublespend.py                 | ✓ Passed  | 1 s
wallet_txn_doublespend.py --mineblock     | ✓ Passed  | 3 s
wallet_watchonly.py                       | ✓ Passed  | 1 s
wallet_watchonly.py --usecli              | ✓ Passed  | 1 s
interface_usdt_net.py                     | ○ Skipped | 0 s
interface_usdt_utxocache.py               | ○ Skipped | 0 s
interface_usdt_validation.py              | ○ Skipped | 0 s

ALL                                       | ✓ Passed  | 1297 s (accumulated) 
Runtime: 260 s

[185/486] Running avalanche test suite
PASSED: avalanche test suite
[205/486] Running pow test suite
PASSED: pow test suite
[218/486] Running seeder test suite
PASSED: seeder test suite
[221/486] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/script_tests.cpp.o
In file included from /usr/include/boost/test/unit_test.hpp:19,
                 from ../../src/test/script_tests.cpp:30:
../../src/test/script_tests.cpp: In member function ‘void script_tests::script_build::test_method()’:
../../src/test/script_tests.cpp:540:22: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  540 | BOOST_AUTO_TEST_CASE(script_build) {
      |                      ^~~~~~~~~~~~
[229/486] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1
tobias_ruck updated this revision to Diff 38834.Mar 24 2023, 11:24

Use Amount unambiguously

Harbormaster completed remote builds in B22651: Diff 38833.Mar 24 2023, 11:25
teamcity added a comment.Mar 24 2023, 11:25
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Harbormaster completed remote builds in B22652: Diff 38834.Mar 24 2023, 11:41
tobias_ruck published this revision for review.Mar 24 2023, 11:47
tobias_ruck edited the summary of this revision. (Show Details)
tobias_ruck edited the test plan for this revision. (Show Details)
tobias_ruck updated this revision to Diff 38836.Mar 24 2023, 11:55
tobias_ruck edited the summary of this revision. (Show Details)

Make bridge_block return Result (e.g. disk failure)

Harbormaster completed remote builds in B22654: Diff 38836.Mar 24 2023, 12:12
tobias_ruck added a comment.Mar 24 2023, 14:49

@bot build-linux64 build-linux-aarch64 build-linux-arm build-osx build-win64

teamcity awarded a token.Mar 24 2023, 14:49
tobias_ruck updated this revision to Diff 38857.Mar 24 2023, 16:45

rebase from master

Harbormaster completed remote builds in B22674: Diff 38857.Mar 24 2023, 17:11
tobias_ruck updated this revision to Diff 38859.Mar 24 2023, 17:25

Factor out CheckTxsEqual

Harbormaster completed remote builds in B22676: Diff 38859.Mar 24 2023, 17:43
Fabien requested changes to this revision.Mar 24 2023, 21:18
Fabien added inline comments.
chronik/chronik-cpp/chronik_bridge.cpp
103 ↗(On Diff #38859)

Maybe add a comment to explain the -1 (the coinbase is not in the undo file)

chronik/test/bridge_tests.cpp
27 ↗(On Diff #38859)

You're missing txid

src/node/blockstorage.cpp
812 ↗(On Diff #38859)

I would love to see some unit tests for these 2 functions so they get coverage outside of chronik, as they are added to the node even if chronik is disabled.

src/node/blockstorage.h
223 ↗(On Diff #38859)
src/test/CMakeLists.txt
108 ↗(On Diff #38859)

The naming is confusing. What's the difference between bridge and chronikbridge (and please don't answer "chronik")

This revision now requires changes to proceed.Mar 24 2023, 21:18
tobias_ruck updated this revision to Diff 38875.Mar 25 2023, 08:48
tobias_ruck marked an inline comment as done.

Add blockstorate_tests.cpp, rename bridge_tests.cpp -> bridgeprimitives_tests.cpp, missing txid test data + check, more commentary

tobias_ruck updated this revision to Diff 38876.Mar 25 2023, 08:51

make commentary even better

tobias_ruck updated this revision to Diff 38877.Mar 25 2023, 08:55

remove #include <test/lcg.h>

Harbormaster completed remote builds in B22690: Diff 38875.Mar 25 2023, 09:03
Harbormaster completed remote builds in B22691: Diff 38876.Mar 25 2023, 09:11
Harbormaster completed remote builds in B22692: Diff 38877.Mar 25 2023, 09:25
tobias_ruck updated this revision to Diff 38880.Mar 25 2023, 10:44

rebase from master

tobias_ruck added a child revision: D13464: [Chronik] Add `load_tx`.Mar 25 2023, 10:45
Harbormaster completed remote builds in B22695: Diff 38880.Mar 25 2023, 11:00
Fabien accepted this revision.Mar 25 2023, 12:47
This revision is now accepted and ready to land.Mar 25 2023, 12:47
Closed by commit rABC0b9aebe337a2: [Chronik] Bridge txs in `bridge_block` (authored by tobias_ruck). · Explain WhyMar 25 2023, 13:07
This revision was automatically updated to reflect the committed changes.
tobias_ruck added a commit: rABC0b9aebe337a2: [Chronik] Bridge txs in `bridge_block`.

Revision Contents
Changeset List

PathSize
chronik/
chronik-bridge/
src/
75 lines
chronik-cpp/
143 lines
util/
22 lines
chronik-indexer/
src/
10 lines
chronik-lib/
src/
7 lines
test/
390 lines
src/
node/
5 lines
38 lines
test/
2 lines
96 lines

Diff 38880

View Options

chronik/chronik-bridge/src/ffi.rs

Loading...
View Options

chronik/chronik-cpp/chronik_bridge.cpp

Loading...
View Options

chronik/chronik-cpp/util/collection.h

Loading...
View Options

chronik/chronik-indexer/src/indexer.rs

Loading...
View Options

chronik/chronik-lib/src/bridge.rs

Loading...
View Options

chronik/test/bridgeprimitives_tests.cpp

Loading...
View Options

src/node/blockstorage.h

Loading...
View Options

src/node/blockstorage.cpp

Loading...
View Options

src/test/CMakeLists.txt

Loading...
View Options

src/test/blockstorage_tests.cpp

Loading...