Page MenuHomePhabricator

Add new post-fork ancestor and descendants limit.
ClosedPublic

Authored by dagurval on Feb 10 2020, 20:30.

Details

Summary

Adds a getter that returns the correct default limit based on phonon activation status.

Depends on D5242

Test Plan

Added unit test.
make check

Diff Detail

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

Event Timeline

Owners added a reviewer: Restricted Owners Package.Feb 10 2020, 20:30
deadalnix requested changes to this revision.Feb 10 2020, 22:29
deadalnix added a subscriber: deadalnix.

You need to update the cmake build.

src/policy/mempool.cpp
6 ↗(On Diff #16230)

Put this guy on his own line.

src/test/mempool_policy_tests.cpp
39 ↗(On Diff #16230)

You should set the MTP explicitly first.

This revision now requires changes to proceed.Feb 10 2020, 22:29
jasonbcox added inline comments.
src/test/mempool_policy_tests.cpp
27 ↗(On Diff #16230)

Nit: mempool_policy_activation_tests or something similar would be more accurate and descriptive

src/policy/mempool.cpp
6 ↗(On Diff #16230)

For more details on this, see doc/developer-notes.md section Header Inclusions

dagurval marked 4 inline comments as done.

Header inclusion order, explicit SetMTP, rename test

Fabien requested changes to this revision.Feb 11 2020, 09:03
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/Makefile.am
285 ↗(On Diff #16239)

Please also add it to the cmake build (src/CMakeLists.txt).

src/Makefile.test.include
112 ↗(On Diff #16239)

Please also add it to the cmake build (src/test/CMakeLists.txt).

src/test/mempool_policy_tests.cpp
44 ↗(On Diff #16239)

GetDefaultAncestorLimit => GetDefaultDescendantLimit

51 ↗(On Diff #16239)

Dito.

This revision now requires changes to proceed.Feb 11 2020, 09:03
dagurval added inline comments.
src/Makefile.test.include
112 ↗(On Diff #16239)

Thanks, I'll run ninja from now to avoid forgetting this

src/policy/mempool.cpp
6 ↗(On Diff #16230)

Thanks

src/test/mempool_policy_tests.cpp
44 ↗(On Diff #16239)

It appears that I'm blind. Thanks.

test braino, add to cmake

Snippet of first build failure:

[10:30:38] :	 [Step 1/1] [39/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/compress_tests.cpp.o
[10:30:38] :	 [Step 1/1] [40/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/config_tests.cpp.o
[10:30:38] :	 [Step 1/1] [41/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/core_io_tests.cpp.o
[10:30:38] :	 [Step 1/1] [42/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/cuckoocache_tests.cpp.o
[10:30:39] :	 [Step 1/1] [43/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/dbwrapper_tests.cpp.o
[10:30:39] :	 [Step 1/1] [44/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/denialofservice_tests.cpp.o
[10:30:39] :	 [Step 1/1] [45/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/descriptor_tests.cpp.o
[10:30:39] :	 [Step 1/1] [46/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/dstencode_tests.cpp.o
[10:30:39] :	 [Step 1/1] [47/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/excessiveblock_tests.cpp.o
[10:30:39] :	 [Step 1/1] [48/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/finalization_tests.cpp.o
[10:30:39] :	 [Step 1/1] [49/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/addrman_tests.cpp.o
[10:30:39] :	 [Step 1/1] [50/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/getarg_tests.cpp.o
[10:30:39] :	 [Step 1/1] [51/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/hash_tests.cpp.o
[10:30:39] :	 [Step 1/1] [52/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/inv_tests.cpp.o
[10:30:39] :	 [Step 1/1] [53/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/key_tests.cpp.o
[10:30:39] :	 [Step 1/1] [54/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/lcg_tests.cpp.o
[10:30:40] :	 [Step 1/1] [55/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/flatfile_tests.cpp.o
[10:30:40] :	 [Step 1/1] [56/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/key_io_tests.cpp.o
[10:30:40] :	 [Step 1/1] [57/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/limitedmap_tests.cpp.o
[10:30:40] :	 [Step 1/1] [58/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/mempool_tests.cpp.o
[10:30:41] :	 [Step 1/1] [59/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/merkle_tests.cpp.o
[10:30:41] :	 [Step 1/1] [60/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/miner_tests.cpp.o
[10:30:41] :	 [Step 1/1] [61/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/monolith_opcodes_tests.cpp.o
[10:30:42] :	 [Step 1/1] [62/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/multisig_tests.cpp.o
[10:30:42] :	 [Step 1/1] [63/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/net_tests.cpp.o
[10:30:42] :	 [Step 1/1] [64/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/netbase_tests.cpp.o
[10:30:42] :	 [Step 1/1] [65/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/op_reversebytes_tests.cpp.o
[10:30:42] :	 [Step 1/1] [66/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/pmt_tests.cpp.o
[10:30:42] :	 [Step 1/1] [67/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/policyestimator_tests.cpp.o
[10:30:42] :	 [Step 1/1] [68/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/pow_tests.cpp.o
[10:30:42] :	 [Step 1/1] [69/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/prevector_tests.cpp.o
[10:30:43] :	 [Step 1/1] [70/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/radix_tests.cpp.o
[10:30:43] :	 [Step 1/1] [71/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/raii_event_tests.cpp.o
[10:30:44] :	 [Step 1/1] [72/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/random_tests.cpp.o
[10:30:44] :	 [Step 1/1] [73/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/rcu_tests.cpp.o
[10:30:44] :	 [Step 1/1] [74/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/reverselock_tests.cpp.o
[10:30:44] :	 [Step 1/1] [75/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/rpc_tests.cpp.o
[10:30:45] :	 [Step 1/1] [76/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/mempool_policy_tests.cpp.o
[10:30:45] :	 [Step 1/1] FAILED: src/test/CMakeFiles/test_bitcoin.dir/mempool_policy_tests.cpp.o 
[10:30:45] :	 [Step 1/1] /usr/bin/ccache /usr/bin/c++  -DBOOST_AC_USE_STD_ATOMIC -DBOOST_SP_USE_STD_ATOMIC -DBOOST_TEST_DYN_LINK -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -I../src/univalue/include -I../src/. -Isrc -Isrc/crypto/.. -I../src/secp256k1/include -I../src/leveldb/include -g -O2 -fPIE -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wformat-security -Wcast-align -Wunused-parameter -Wmissing-braces -Wshadow -Wredundant-decls -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -std=gnu++14 -MD -MT src/test/CMakeFiles/test_bitcoin.dir/mempool_policy_tests.cpp.o -MF src/test/CMakeFiles/test_bitcoin.dir/mempool_policy_tests.cpp.o.d -o src/test/CMakeFiles/test_bitcoin.dir/mempool_policy_tests.cpp.o -c ../src/test/mempool_policy_tests.cpp
[10:30:45] :	 [Step 1/1] In file included from /usr/include/boost/test/test_tools.hpp:45,
[10:30:45] :	 [Step 1/1]                  from /usr/include/boost/test/unit_test.hpp:18,
[10:30:45] :	 [Step 1/1]                  from ../src/test/mempool_policy_tests.cpp:13:
[10:30:45] :	 [Step 1/1] ../src/test/mempool_policy_tests.cpp: In member function ‘void mempool_policy_tests::mempool_policy_activation_tests::test_method()’:
[10:30:45] :	 [Step 1/1] ../src/test/mempool_policy_tests.cpp:44:23: error: ‘GetDefaultDescendantLimit’ was not declared in this scope
[10:30:45] :	 [Step 1/1]                        GetDefaultDescendantLimit(params, &blocks.back()));
[10:30:45] :	 [Step 1/1]                        ^~~~~~~~~~~~~~~~~~~~~~~~~
[10:30:45] :	 [Step 1/1] ../src/test/mempool_policy_tests.cpp:44:23: note: suggested alternative: ‘GetDefaultDecendantLimit’
[10:30:45] :	 [Step 1/1] ../src/test/mempool_policy_tests.cpp:51:23: error: ‘GetDefaultDescendantLimit’ was not declared in this scope
[10:30:45] :	 [Step 1/1]                        GetDefaultDescendantLimit(params, &blocks.back()));
[10:30:45] :	 [Step 1/1]                        ^~~~~~~~~~~~~~~~~~~~~~~~~
[10:30:45] :	 [Step 1/1] ../src/test/mempool_policy_tests.cpp:51:23: note: suggested alternative: ‘GetDefaultDecendantLimit’
[10:30:46] :	 [Step 1/1] [77/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/blockstatus_tests.cpp.o
[10:30:46] :	 [Step 1/1] [78/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/feerate_tests.cpp.o
[10:30:47] :	 [Step 1/1] [79/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/crypto_tests.cpp.o
[10:30:48] :	 [Step 1/1] [80/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/merkleblock_tests.cpp.o
[10:30:50] :	 [Step 1/1] [81/124] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/rpc_server_tests.cpp.o
[10:30:50] :	 [Step 1/1] ninja: build stopped: subcommand failed.
[10:30:50]W:	 [Step 1/1] Process exited with code 1
[10:30:50]E:	 [Step 1/1] Process exited with code 1 (Step: Command Line)
Fabien added inline comments.
src/test/mempool_policy_tests.cpp
1 ↗(On Diff #16262)

2019 ?

src/test/mempool_policy_tests.cpp
1 ↗(On Diff #16262)

Yes, this is code derived from the activation tests

This revision is now accepted and ready to land.Feb 13 2020, 01:36