Use the DifferentialIndexedItemFormatter for the compact blocks
ClosedPublic
Actions

Authored by Fabien on Jun 3 2022, 19:52.

Details

Reviewers

PiRK
sdulfari

Group Reviewers

Restricted Project

Commits

rABC5fbd31594a7a: Use the DifferentialIndexedItemFormatter for the compact blocks

Summary

This makes the code more consistent between compact proofs and compact blocks, which will make it easier to factorize the code.

Note that this introduce a change in behavior: previously a malformed compact block with overflowing transaction indexes could have its header attached if valid. Now a malformed compact block due to overflowing indexes is detected invalid at deserialization time and the header is not processed. This is not a concern has a malformed compact block is either the result of a malfunctioning node or a malicious one.

A unit test is added to check the index overflow, similar to what is done for the compact proofs. This code is not factorized because it requires to manually build the stream, which makes it difficult to reuse the code.

Depends on D11568.

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Branch

compactblock_difference_formatter

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 19280
Build 38307: Build Diff	lint-circular-dependencies · build-debug · build-diff · build-without-wallet · build-clang · build-clang-tidy
Build 38306: arc lint + arc unit

Event Timeline

Fabien created this revision.Jun 3 2022, 19:52

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 3 2022, 19:52

Fabien requested review of this revision.Jun 3 2022, 19:52

Harbormaster completed remote builds in B19255: Diff 33815.Jun 3 2022, 20:00

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Tail of the build log:

[0m[1;30mwallet_keypool.py                                      | ○ Skipped | 0 s
[0m[1;30mwallet_keypool_topup.py                                | ○ Skipped | 0 s
[0m[1;30mwallet_keypool_topup.py --descriptors                  | ○ Skipped | 0 s
[0m[1;30mwallet_labels.py                                       | ○ Skipped | 0 s
[0m[1;30mwallet_labels.py --descriptors                         | ○ Skipped | 0 s
[0m[1;30mwallet_listreceivedby.py                               | ○ Skipped | 0 s
[0m[1;30mwallet_listsinceblock.py                               | ○ Skipped | 0 s
[0m[1;30mwallet_listsinceblock.py --descriptors                 | ○ Skipped | 0 s
[0m[1;30mwallet_listtransactions.py                             | ○ Skipped | 0 s
[0m[1;30mwallet_listtransactions.py --descriptors               | ○ Skipped | 0 s
[0m[1;30mwallet_multiwallet.py                                  | ○ Skipped | 0 s
[0m[1;30mwallet_multiwallet.py --usecli                         | ○ Skipped | 0 s
[0m[1;30mwallet_reorgsrestore.py                                | ○ Skipped | 0 s
[0m[1;30mwallet_resendwallettransactions.py                     | ○ Skipped | 0 s
[0m[1;30mwallet_send.py                                         | ○ Skipped | 0 s
[0m[1;30mwallet_startup.py                                      | ○ Skipped | 0 s
[0m[1;30mwallet_txn_clone.py                                    | ○ Skipped | 0 s
[0m[1;30mwallet_txn_clone.py --mineblock                        | ○ Skipped | 0 s
[0m[1;30mwallet_txn_doublespend.py                              | ○ Skipped | 0 s
[0m[1;30mwallet_txn_doublespend.py --mineblock                  | ○ Skipped | 0 s
[0m[1;30mwallet_watchonly.py                                    | ○ Skipped | 0 s
[0m[1;30mwallet_watchonly.py --usecli                           | ○ Skipped | 0 s
[0m[1m
ALL                                                    | ✓ Passed  | 630 s (accumulated) 
[0mRuntime: 126 s

----------------------------------------------------------------------
Ran 10 tests in 0.162s

OK

[168/437] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.005s

OK
[169/437] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[192/437] Running seeder test suite
PASSED: seeder test suite
[198/437] Running pow test suite
PASSED: pow test suite
[204/437] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[205/437] 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
 BOOST_AUTO_TEST_CASE(script_build) {
                      ^~~~~~~~~~~~
[434/437] Running bitcoin test suite
PASSED: bitcoin test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1

Unrelated failure, I restarted the builds

Fabien added a child revision: D11571: Extract the compact block processing into a generic helper class.Jun 4 2022, 01:12

Rebase

PiRK accepted this revision.Jun 7 2022, 08:21

This revision is now accepted and ready to land.Jun 7 2022, 08:21

Harbormaster completed remote builds in B19280: Diff 33850.Jun 7 2022, 08:31

sdulfari requested changes to this revision.Jun 7 2022, 15:57

sdulfari added a subscriber: sdulfari.

sdulfari added inline comments.

src/test/blockencodings_tests.cpp
494	These cases are practically duplicates of the above. Just loop through a vector of test cases that all fail in the same way. `{0, 1, std::numeric_limits<uint32_t>::max()}`
501	Needs to test the other side of the boundary (MAX_SIZE) to make sure it's valid.
515	Same here.
531	Same here.