Page MenuHomePhabricator

[Chronik] Bridge txs in `bridge_block`
ClosedPublic

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

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Add through tests, make more robust

Remove unnecessary blocks vector

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
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
tobias_ruck edited the summary of this revision. (Show Details)
tobias_ruck edited the test plan for this revision. (Show Details)
tobias_ruck edited the summary of this revision. (Show Details)

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

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

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 marked an inline comment as done.

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

make commentary even better

remove #include <test/lcg.h>

This revision is now accepted and ready to land.Mar 25 2023, 12:47
This revision was automatically updated to reflect the committed changes.