Add a block policy to reject turbo blocks
DraftPublic
Actions

Authored by Fabien on Mon, Jun 17, 14:50.

Details

Reviewers

None

Group Reviewers

Restricted Project

Summary

This is a draft of the heartbeat feature. When -enablertt=1, the node will reject blocks that are received too soon after the previous one.
Based on the research from https://ledger.pitt.edu/ojs/ledger/article/download/195/187/1008.

This needs to be split into pieces, and ideally the double computation should be removed for int256 computation. I also left some debug prints under a new experimental category that should eventually get removed.

Test Plan

ninja all check check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Branch

block_policy_blocktime

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 29351
Build 58238: Build Diff	lint-circular-dependencies · build-diff · build-without-wallet · build-debug · build-clang-tidy · build-clang
Build 58237: arc lint + arc unit

Event Timeline

Fabien created this revision.Mon, Jun 17, 14:50

Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Jun 17, 14:50

Harbormaster completed remote builds in B29319: Diff 48266.Mon, Jun 17, 15:05

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

Tail of the build log:

[585/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/chacha_poly_aead.cpp.o
[586/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/chacha20.cpp.o
[587/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoin-qt-base_autogen/mocs_compilation.cpp.o
[588/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/intro.cpp.o
[589/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/checkqueue.cpp.o
[590/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsdialog.cpp.o
[591/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoingui.cpp.o
[592/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/crypto_hash.cpp.o
[593/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/data.cpp.o
[594/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/examples.cpp.o
[595/638] Building CXX object src/CMakeFiles/bitcoinkernel.dir/validation.cpp.o
[596/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/gcs_filter.cpp.o
[597/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o
[598/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/hashpadding.cpp.o
[599/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/lockedpool.cpp.o
[600/638] Linking CXX static library src/libbitcoinkernel.a
[601/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/guiutil.cpp.o
[602/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoin.cpp.o
[603/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/merkle_root.cpp.o
[604/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/nanobench.cpp.o
[605/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/pool.cpp.o
[606/638] Linking CXX executable src/bitcoin-chainstate
FAILED: src/bitcoin-chainstate 
: && /usr/bin/c++ -Werror -g -O2 -fuse-ld=gold -Wl,-z,noexecstack -Wl,-z,relro -Wl,-z,now -lstdc++fs -fPIE -pie src/CMakeFiles/bitcoin-chainstate.dir/bitcoin-chainstate.cpp.o -o src/bitcoin-chainstate  /usr/lib/x86_64-linux-gnu/libjemalloc_pic.a  -lstdc++fs  src/libbitcoinkernel.a  src/crypto/libcrypto.a  src/crypto/libcrypto_sse4.1.a  src/crypto/libcrypto_avx2.a  src/crypto/libcrypto_shani.a  src/univalue/libunivalue.a  src/secp256k1/libsecp256k1.a  src/leveldb/libleveldb.a  src/leveldb/libleveldb-sse4.2.a  src/leveldb/libmemenv.a  /usr/lib/x86_64-linux-gnu/libjemalloc_pic.a  -lm  -pthread  -ldl  -lstdc++fs && :
../../src/./policy/block/rtt.h:30: error: undefined reference to 'vtable for RTTPolicy'
/usr/bin/ld.gold: the vtable symbol may be undefined because the class is missing its key function
collect2: error: ld returned 1 exit status
[607/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/poly1305.cpp.o
[608/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/prevector.cpp.o
[609/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rollingbloom.cpp.o
[610/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/peertablemodel.cpp.o
[611/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/block_assemble.cpp.o
[612/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/streams_findbyte.cpp.o
[613/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/util_time.cpp.o
[614/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/verify_script.cpp.o
[615/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/crypto_aes.cpp.o
[616/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/utilitydialog.cpp.o
[617/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/checkblock.cpp.o
[618/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrc_bitcoin_locale.cpp.o
[619/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrc_bitcoin.cpp.o
[620/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/splashscreen.cpp.o
[621/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/load_external.cpp.o
[622/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/duplicate_inputs.cpp.o
[623/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/chained_tx.cpp.o
[624/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/mempool_eviction.cpp.o
[625/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o
[626/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/peer_eviction.cpp.o
[627/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/bench.cpp.o
[628/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/mempool_stress.cpp.o
[629/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_mempool.cpp.o
[630/638] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_blockchain.cpp.o
[631/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/trafficgraphwidget.cpp.o
[632/638] Linking CXX static library src/qt/libbitcoin-qt-base.a
[633/638] Automatic MOC for target bitcoin-qt
[634/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt.dir/bitcoin-qt_autogen/mocs_compilation.cpp.o
[635/638] Linking CXX executable src/bench/bitcoin-bench
[636/638] Building CXX object src/qt/CMakeFiles/bitcoin-qt.dir/main.cpp.o
[637/638] Linking CXX executable src/qt/bitcoin-qt
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1

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

Tail of the build log:

[0m[1;30mchronik_script_confirmed_txs.py            | ○ Skipped | 0 s
[0m[1;30mchronik_script_history.py                  | ○ Skipped | 0 s
[0m[1;30mchronik_script_unconfirmed_txs.py          | ○ Skipped | 0 s
[0m[1;30mchronik_script_utxos.py                    | ○ Skipped | 0 s
[0m[1;30mchronik_serve.py                           | ○ Skipped | 0 s
[0m[1;30mchronik_spent_by.py                        | ○ Skipped | 0 s
[0m[1;30mchronik_token_alp.py                       | ○ Skipped | 0 s
[0m[1;30mchronik_token_broadcast_txs.py             | ○ Skipped | 0 s
[0m[1;30mchronik_token_burn.py                      | ○ Skipped | 0 s
[0m[1;30mchronik_token_id_group.py                  | ○ Skipped | 0 s
[0m[1;30mchronik_token_parse_failure.py             | ○ Skipped | 0 s
[0m[1;30mchronik_token_script_group.py              | ○ Skipped | 0 s
[0m[1;30mchronik_token_slp_fungible.py              | ○ Skipped | 0 s
[0m[1;30mchronik_token_slp_mint_vault.py            | ○ Skipped | 0 s
[0m[1;30mchronik_token_slp_nft1.py                  | ○ Skipped | 0 s
[0m[1;30mchronik_tx.py                              | ○ Skipped | 0 s
[0m[1;30mchronik_tx_removal_order.py                | ○ Skipped | 0 s
[0m[1;30mchronik_ws.py                              | ○ Skipped | 0 s
[0m[1;30mchronik_ws_ordering.py                     | ○ Skipped | 0 s
[0m[1;30mchronik_ws_ping.py                         | ○ Skipped | 0 s
[0m[1;30mchronik_ws_script.py                       | ○ Skipped | 0 s
[0m[1;30mfeature_bind_port_discover.py              | ○ Skipped | 0 s
[0m[1;30mfeature_bind_port_externalip.py            | ○ Skipped | 0 s
[0m[1;30minterface_usdt_net.py                      | ○ Skipped | 0 s
[0m[1;30minterface_usdt_utxocache.py                | ○ Skipped | 0 s
[0m[1;30minterface_usdt_validation.py               | ○ Skipped | 0 s
[0m[1m
ALL                                        | ✓ Passed  | 3104 s (accumulated) 
[0mRuntime: 623 s

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

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

OK
[237/509] bitcoin: testing rtt_tests
FAILED: src/test/CMakeFiles/check-bitcoin-rtt_tests 
cd /work/abc-ci-builds/build-debug/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-debug/test/log/bitcoin-rtt_tests.log /work/abc-ci-builds/build-debug/src/test/test_bitcoin --run_test=rtt_tests --logger=HRF,message:JUNIT,message,bitcoin-rtt_tests.xml --catch_system_errors=no
../../src/policy/block/rtt.cpp:83 GetNextRTTWorkRequired: Assertion `pprev' failed.
Aborted (core dumped)
[280/509] Running secp256k1 test suite
PASSED: secp256k1 test suite
[468/509] Running pow test suite
PASSED: pow test suite
[482/509] Running seeder test suite
PASSED: seeder test suite
[489/509] Running avalanche test suite
PASSED: avalanche test suite
[494/509] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[506/509] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1

Remove redundant Assume, unbreak bitcoin-chainstate build

bytesofman added a subscriber: bytesofman.Mon, Jun 17, 16:08

bytesofman added inline comments.

src/node/miner.cpp
49 ↗	(On Diff #48266)	what is `getAdjustedTime` and how does it differ from what is currently being used?
src/policy/block/rtt.cpp
93 ↗	(On Diff #48266)	could be read as "min" === "minute"
src/test/arith_uint256_tests.cpp
670 ↗	(On Diff #48266)	is this testing cpp...? seems like this test is asserting a definition but I suppose it's safer to include than to not.
src/test/rtt_tests.cpp
73 ↗	(On Diff #48266)	prob me just not understanding BOOST_CHECK or cpp syntax: `prevRttWork` is defined before this `for` loop, and `nextWork` is calculated for each new value of `t` in this loop is this confirming that `nextWork` is always the same as the previously calculated `prevRttWork`, i.e. it is not changing with `t`?

Harbormaster completed remote builds in B29321: Diff 48273.Mon, Jun 17, 16:35

Fabien marked an inline comment as done.Mon, Jun 17, 18:42

Fabien added inline comments.

src/node/miner.cpp
49 ↗	(On Diff #48266)	The adjusted time is the node local time plus/minus an offset which is (iirc) the median of the time difference against the other peers, if you have enough peers. So if your node local time is off it could be adjusted by looking at the time error wrt connected peers. I'm not sure whether this is relevant here. Also note that the adjusted time is actually bugged (see timedata.cpp).
src/test/arith_uint256_tests.cpp
670 ↗	(On Diff #48266)	It's testing the ability of the class arith_uint256 to represent a double, see the roundtrip conversion to/from double. This only works for some values though since a double cannot represent any integer value without losing precision, which is why there is this comment: it's about the limit of the loop.
src/test/rtt_tests.cpp
73 ↗	(On Diff #48266)	Each loop iteration compute the next difficulty target, check it is greater than the previous one, and copies the new value as the previous in preparation for the next iteration. The `BOOST_CHECK(nBitsLE(prevRttWork, *nextWork));` asserts that the previous real time target is <= than the new one (if the target increases the difficulty decreases: it becomes easier to find a hash that is below this target). The overall loop checks the target increases (so difficulty decreases) over time t.