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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dagurval created this revision.Feb 10 2020, 20:30
Owners added a reviewer: Restricted Owners Package.Feb 10 2020, 20:30
Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 10 2020, 20:30
dagurval edited the summary of this revision. (Show Details)Feb 10 2020, 20:40
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

jasonbcox added inline comments.Feb 10 2020, 22:34
src/policy/mempool.cpp
6 ↗(On Diff #16230)

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

dagurval updated this revision to Diff 16239.Feb 10 2020, 23:24
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 planned changes to this revision.Feb 11 2020, 09:44
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.

dagurval updated this revision to Diff 16261.Feb 11 2020, 10:29

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 accepted this revision.Feb 11 2020, 11:14
Fabien added inline comments.
src/test/mempool_policy_tests.cpp
1 ↗(On Diff #16262)

2019 ?

dagurval added inline comments.Feb 11 2020, 11:39
src/test/mempool_policy_tests.cpp
1 ↗(On Diff #16262)

Yes, this is code derived from the activation tests

jasonbcox accepted this revision.Feb 11 2020, 22:09
deadalnix accepted this revision.Feb 13 2020, 01:36
This revision is now accepted and ready to land.Feb 13 2020, 01:36
This revision was automatically updated to reflect the committed changes.