Page MenuHomePhabricator

Use the DifferentialIndexedItemFormatter for the compact blocks
ClosedPublic

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

Details

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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

Tail of the build log:

wallet_keypool.py                                      | ○ Skipped | 0 s
wallet_keypool_topup.py                                | ○ Skipped | 0 s
wallet_keypool_topup.py --descriptors                  | ○ Skipped | 0 s
wallet_labels.py                                       | ○ Skipped | 0 s
wallet_labels.py --descriptors                         | ○ Skipped | 0 s
wallet_listreceivedby.py                               | ○ Skipped | 0 s
wallet_listsinceblock.py                               | ○ Skipped | 0 s
wallet_listsinceblock.py --descriptors                 | ○ Skipped | 0 s
wallet_listtransactions.py                             | ○ Skipped | 0 s
wallet_listtransactions.py --descriptors               | ○ Skipped | 0 s
wallet_multiwallet.py                                  | ○ Skipped | 0 s
wallet_multiwallet.py --usecli                         | ○ Skipped | 0 s
wallet_reorgsrestore.py                                | ○ Skipped | 0 s
wallet_resendwallettransactions.py                     | ○ Skipped | 0 s
wallet_send.py                                         | ○ Skipped | 0 s
wallet_startup.py                                      | ○ Skipped | 0 s
wallet_txn_clone.py                                    | ○ Skipped | 0 s
wallet_txn_clone.py --mineblock                        | ○ Skipped | 0 s
wallet_txn_doublespend.py                              | ○ Skipped | 0 s
wallet_txn_doublespend.py --mineblock                  | ○ Skipped | 0 s
wallet_watchonly.py                                    | ○ Skipped | 0 s
wallet_watchonly.py --usecli                           | ○ Skipped | 0 s

ALL                                                    | ✓ Passed  | 630 s (accumulated) 
Runtime: 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

This revision is now accepted and ready to land.Jun 7 2022, 08:21
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 ↗(On Diff #33850)

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 ↗(On Diff #33850)

Needs to test the other side of the boundary (MAX_SIZE) to make sure it's valid.

515 ↗(On Diff #33850)

Same here.

531 ↗(On Diff #33850)

Same here.

This revision now requires changes to proceed.Jun 7 2022, 15:57
This revision is now accepted and ready to land.Jun 7 2022, 20:46