Page MenuHomePhabricator
Differential D5244

Enable new ancestor/descendants chains limit at fork
ClosedPublic
Actions

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

Details

Reviewers
jasonbcox
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rSTAGING3a535f346e0b: Enable new ancestor/descendants chains limit at fork
rABC3a535f346e0b: Enable new ancestor/descendants chains limit at fork
Summary

This enables longer ancestor/descendants chain at phonon fork time.

Depends on D5243

Test Plan

Update mempool_packages test to test both pre- and postfork limits.
./test_runner && ./test_runner --with-phonon && make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
enable-limit-02
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9497
Build 16907: Default Diff Build & Tests
Build 16906: arc lint + arc unit

Event Timeline

dagurval created this revision.Feb 10 2020, 20:48
Owners added a reviewer: Restricted Owners Package.Feb 10 2020, 20:48
Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 10 2020, 20:48
teamcity added a comment.Feb 10 2020, 20:49

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Feb 10 2020, 20:50
Build Bitcoin-ABC / Diffs / Diff Testing failed.

Snippet of first build failure:

[20:49:58] :	 [Step 1/1] [282/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[20:49:58] :	 [Step 1/1] [283/404] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[20:49:58] :	 [Step 1/1] [284/404] Linking C static library src/secp256k1/libsecp256k1.a
[20:49:59] :	 [Step 1/1] [285/404] Linking CXX static library src/libcommon.a
[20:49:59] :	 [Step 1/1] [286/404] Linking CXX executable src/bitcoin-cli
[20:49:59] :	 [Step 1/1] [287/404] Linking C shared library src/secp256k1/libsecp256k1_jni.so.0.0.0
[20:49:59] :	 [Step 1/1] [288/404] Creating library symlink src/secp256k1/libsecp256k1_jni.so.0 src/secp256k1/libsecp256k1_jni.so
[20:50:01] :	 [Step 1/1] [289/404] Building Java objects for secp256k1-jni-test-jar.jar
[20:50:01] :	 [Step 1/1] [290/404] Generating CMakeFiles/secp256k1-jni-test-jar.dir/java_class_filelist
[20:50:01] :	 [Step 1/1] [291/404] Creating Java archive secp256k1-jni-test.jar
[20:50:01] :	 [Step 1/1] [292/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[20:50:02] :	 [Step 1/1] [293/404] Building CXX object src/CMakeFiles/script.dir/script/interpreter.cpp.o
[20:50:02] :	 [Step 1/1] [294/404] Linking CXX static library src/libscript.a
[20:50:02] :	 [Step 1/1] [295/404] Linking CXX static library src/libbitcoinconsensus.a
[20:50:02] :	 [Step 1/1] [296/404] Linking CXX shared library src/libbitcoinconsensus.so.0.0.0
[20:50:02] :	 [Step 1/1] [297/404] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[20:50:03] :	 [Step 1/1] [298/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/finaltx.cpp.o
[20:50:03] :	 [Step 1/1] [299/404] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[20:50:04] :	 [Step 1/1] [300/404] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[20:50:05] :	 [Step 1/1] [301/404] Building CXX object src/CMakeFiles/server.dir/init.cpp.o
[20:50:05] :	 [Step 1/1] [302/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[20:50:05] :	 [Step 1/1] [303/404] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[20:50:06] :	 [Step 1/1] [304/404] Linking CXX executable src/bitcoin-tx
[20:50:06] :	 [Step 1/1] [305/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[20:50:06] :	 [Step 1/1] [306/404] Building CXX object src/CMakeFiles/server.dir/txmempool.cpp.o
[20:50:06] :	 [Step 1/1] [307/404] Building CXX object src/seeder/CMakeFiles/seeder-netprocessing.dir/dns.cpp.o
[20:50:06] :	 [Step 1/1] [308/404] Building CXX object src/seeder/CMakeFiles/seeder-netprocessing.dir/bitcoin.cpp.o
[20:50:06] :	 [Step 1/1] [309/404] Building CXX object src/seeder/CMakeFiles/seeder-netprocessing.dir/db.cpp.o
[20:50:06] :	 [Step 1/1] [310/404] Linking CXX static library src/seeder/libseeder-netprocessing.a
[20:50:07] :	 [Step 1/1] [311/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[20:50:09] :	 [Step 1/1] [312/404] Building CXX object src/CMakeFiles/server.dir/net_processing.cpp.o
[20:50:11] :	 [Step 1/1] [313/404] Building CXX object src/CMakeFiles/server.dir/validationinterface.cpp.o
[20:50:11] :	 [Step 1/1] [314/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/init.cpp.o
[20:50:11] :	 [Step 1/1] [315/404] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[20:50:11] :	 [Step 1/1] [316/404] Linking CXX executable src/seeder/bitcoin-seeder
[20:50:12] :	 [Step 1/1] [317/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[20:50:14] :	 [Step 1/1] [318/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[20:50:16] :	 [Step 1/1] [319/404] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[20:50:16] :	 [Step 1/1] [320/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[20:50:23] :	 [Step 1/1] [321/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[20:50:24] :	 [Step 1/1] [322/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[20:50:24] :	 [Step 1/1] [323/404] Linking CXX static library src/wallet/libwallet.a
[20:50:25] :	 [Step 1/1] [324/404] Linking CXX static library src/libserver.a
[20:50:27] :	 [Step 1/1] [325/404] Linking CXX executable src/bitcoind
[20:50:27] :	 [Step 1/1] FAILED: src/bitcoind 
[20:50:27] :	 [Step 1/1] : && /usr/bin/c++  -g -O2  -Wl,-z,relro -Wl,-z,now -pie src/CMakeFiles/bitcoind.dir/bitcoind.cpp.o  -o src/bitcoind  src/libserver.a src/wallet/libwallet.a src/libserver.a src/wallet/libwallet.a src/libbitcoinconsensus.a src/leveldb/libleveldb.a src/leveldb/libleveldb-sse4.2.a src/leveldb/libmemenv.a /usr/lib/x86_64-linux-gnu/libminiupnpc.so src/zmq/libzmq.a /usr/lib/x86_64-linux-gnu/libzmq.so /usr/lib/x86_64-linux-gnu/libevent.so /usr/lib/x86_64-linux-gnu/libevent_pthreads.so src/libscript.a src/libcommon.a src/libutil.a src/crypto/libcrypto.a /usr/lib/x86_64-linux-gnu/libcrypto.so src/crypto/libcrypto_sse4.1.a src/crypto/libcrypto_avx2.a src/crypto/libcrypto_shani.a /usr/lib/x86_64-linux-gnu/libboost_filesystem.so /usr/lib/x86_64-linux-gnu/libboost_thread.so /usr/lib/x86_64-linux-gnu/libboost_chrono.so /usr/lib/x86_64-linux-gnu/libboost_system.so /usr/lib/x86_64-linux-gnu/libboost_date_time.so /usr/lib/x86_64-linux-gnu/libboost_atomic.so -pthread src/secp256k1/libsecp256k1.a src/univalue/libunivalue.a /usr/lib/x86_64-linux-gnu/libdb_cxx.so && :
[20:50:27] :	 [Step 1/1] /usr/bin/ld: src/libserver.a(validation.cpp.o): in function `AcceptToMemoryPoolWorker(Config const&, CTxMemPool&, CValidationState&, std::shared_ptr<CTransaction const> const&, bool*, long, bool, Amount, std::vector<COutPoint, std::allocator<COutPoint> >&, bool) [clone .constprop.1798]':
[20:50:27] :	 [Step 1/1] /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/../src/validation.cpp:747: undefined reference to `GetDefaultAncestorLimit(Consensus::Params const&, CBlockIndex const*)'
[20:50:27] :	 [Step 1/1] /usr/bin/ld: /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/../src/validation.cpp:753: undefined reference to `GetDefaultDecendantLimit(Consensus::Params const&, CBlockIndex const*)'
[20:50:27] :	 [Step 1/1] collect2: error: ld returned 1 exit status
[20:50:27] :	 [Step 1/1] [326/404] Automatic MOC for target bitcoin-qt-base
[20:50:27] :	 [Step 1/1] ninja: build stopped: subcommand failed.
[20:50:27] :	 [Step 1/1] *** Output of /tmp/sanitizer_logs/*.log.* ***
[20:50:27]W:	 [Step 1/1] ++ print_sanitizers_log
[20:50:27]W:	 [Step 1/1] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[20:50:27]W:	 [Step 1/1] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[20:50:27]W:	 [Step 1/1] ++ cat '/tmp/sanitizer_logs/*.log.*'
[20:50:27]W:	 [Step 1/1] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[20:50:27]W:	 [Step 1/1] Process exited with code 1
[20:50:27]E:	 [Step 1/1] Process exited with code 1 (Step: Command Line)
Harbormaster failed remote builds in B9398: Diff 16233!Feb 10 2020, 20:50
dagurval added a child revision: D5245: [qa] Test that mempool reverts to pre-phonon policies.Feb 10 2020, 21:01
dagurval updated this revision to Diff 16235.Feb 10 2020, 21:14

Added mempool.cpp to CMakeList.txt

teamcity added a comment.Feb 10 2020, 21:15

Build Bitcoin-ABC / Diffs / Diff Testing started.

dagurval updated this revision to Diff 16236.Feb 10 2020, 21:17

Replaced whitespace with tab in CMakeList.txt (sorry for the noise)

teamcity added a comment.Feb 10 2020, 21:17

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Feb 10 2020, 21:25
Build Bitcoin-ABC / Diffs / Diff Testing failed.

Snippet of first build failure:

[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_estimatefee.py                      | ✓ Passed  | 0 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_fundrawtransaction.py               | ✓ Passed  | 34 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_getblockstats.py                    | ✓ Passed  | 0 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_getchaintips.py                     | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_help.py                             | ✓ Passed  | 0 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_invalidateblock.py                  | ✓ Passed  | 5 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_named_arguments.py                  | ✓ Passed  | 0 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_net.py                              | ✓ Passed  | 1 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_preciousblock.py                    | ✓ Passed  | 1 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_psbt.py                             | ✓ Passed  | 8 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_rawtransaction.py                   | ✓ Passed  | 21 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_scantxoutset.py                     | ✓ Passed  | 3 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_signmessage.py                      | ✓ Passed  | 0 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_signrawtransaction.py               | ✓ Passed  | 0 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_txoutproof.py                       | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_uptime.py                           | ✓ Passed  | 0 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_users.py                            | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mrpc_zmq.py                              | ✓ Passed  | 1 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_abandonconflict.py               | ✓ Passed  | 4 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_basic.py                         | ✓ Passed  | 25 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_createwallet.py                  | ✓ Passed  | 1 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_createwallet.py --usecli         | ✓ Passed  | 1 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_disable.py                       | ✓ Passed  | 0 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_dump.py                          | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_encryption.py                    | ✓ Passed  | 4 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_groups.py                        | ✓ Passed  | 10 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_hd.py                            | ✓ Passed  | 3 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_import_rescan.py                 | ✓ Passed  | 3 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_importmulti.py                   | ✓ Passed  | 1 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_importprunedfunds.py             | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_keypool.py                       | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_keypool_topup.py                 | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_labels.py                        | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_listreceivedby.py                | ✓ Passed  | 12 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_listsinceblock.py                | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_listtransactions.py              | ✓ Passed  | 11 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_multiwallet.py                   | ✓ Passed  | 8 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_multiwallet.py --usecli          | ✓ Passed  | 10 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_resendwallettransactions.py      | ✓ Passed  | 1 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_txn_clone.py                     | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_txn_clone.py --mineblock         | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_txn_doublespend.py               | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_txn_doublespend.py --mineblock   | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;34mwallet_zapwallettxes.py                 | ✓ Passed  | 2 s
[21:24:52] :	 [Step 1/1]  [0m [0;31mmempool_packages.py                     | ✖ Failed  | 19 s
[21:24:52] :	 [Step 1/1]  [0m [0;31m [1m
[21:24:52] :	 [Step 1/1] ALL                                     | ✖ Failed  | 468 s (accumulated) 
[21:24:52] :	 [Step 1/1]  [0m [0mRuntime: 108 s
[21:24:52] :	 [Step 1/1] 
[21:24:52] :	 [Step 1/1] FAILED: test/CMakeFiles/check-functional-upgrade-activated 
[21:24:52] :	 [Step 1/1] cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/test && /usr/bin/python3 ./functional/test_runner.py --with-phononactivation -n "Bitcoin ABC functional tests with the next upgrade activated"
[21:24:52] :	 [Step 1/1] ninja: build stopped: subcommand failed.
[21:24:52] :	 [Step 1/1] *** Output of /tmp/sanitizer_logs/*.log.* ***
[21:24:52]W:	 [Step 1/1] ++ print_sanitizers_log
[21:24:52]W:	 [Step 1/1] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[21:24:52]W:	 [Step 1/1] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[21:24:52]W:	 [Step 1/1] ++ cat '/tmp/sanitizer_logs/*.log.*'
[21:24:52]W:	 [Step 1/1] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[21:24:52]W:	 [Step 1/1] Process exited with code 1
[21:24:52]E:	 [Step 1/1] Process exited with code 1 (Step: Command Line)

Each failure log is accessible here:
Bitcoin ABC functional tests with the next upgrade activated: mempool_packages.py

Harbormaster failed remote builds in B9400: Diff 16235!Feb 10 2020, 21:25
teamcity added a comment.Feb 10 2020, 21:27
Build Bitcoin-ABC / Diffs / Diff Testing failed.

Snippet of first build failure:

[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_estimatefee.py                      | ✓ Passed  | 0 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_fundrawtransaction.py               | ✓ Passed  | 25 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_getblockstats.py                    | ✓ Passed  | 0 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_getchaintips.py                     | ✓ Passed  | 1 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_help.py                             | ✓ Passed  | 0 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_invalidateblock.py                  | ✓ Passed  | 5 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_named_arguments.py                  | ✓ Passed  | 0 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_net.py                              | ✓ Passed  | 1 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_preciousblock.py                    | ✓ Passed  | 1 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_psbt.py                             | ✓ Passed  | 9 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_rawtransaction.py                   | ✓ Passed  | 21 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_scantxoutset.py                     | ✓ Passed  | 3 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_signmessage.py                      | ✓ Passed  | 0 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_signrawtransaction.py               | ✓ Passed  | 0 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_txoutproof.py                       | ✓ Passed  | 2 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_uptime.py                           | ✓ Passed  | 0 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_users.py                            | ✓ Passed  | 2 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mrpc_zmq.py                              | ✓ Passed  | 1 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_abandonconflict.py               | ✓ Passed  | 7 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_basic.py                         | ✓ Passed  | 21 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_createwallet.py                  | ✓ Passed  | 1 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_createwallet.py --usecli         | ✓ Passed  | 1 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_disable.py                       | ✓ Passed  | 0 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_dump.py                          | ✓ Passed  | 2 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_encryption.py                    | ✓ Passed  | 4 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_groups.py                        | ✓ Passed  | 12 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_hd.py                            | ✓ Passed  | 3 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_import_rescan.py                 | ✓ Passed  | 3 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_importmulti.py                   | ✓ Passed  | 1 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_importprunedfunds.py             | ✓ Passed  | 0 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_keypool.py                       | ✓ Passed  | 3 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_keypool_topup.py                 | ✓ Passed  | 3 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_labels.py                        | ✓ Passed  | 3 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_listreceivedby.py                | ✓ Passed  | 11 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_listsinceblock.py                | ✓ Passed  | 3 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_listtransactions.py              | ✓ Passed  | 4 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_multiwallet.py                   | ✓ Passed  | 8 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_multiwallet.py --usecli          | ✓ Passed  | 8 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_resendwallettransactions.py      | ✓ Passed  | 1 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_txn_clone.py                     | ✓ Passed  | 2 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_txn_clone.py --mineblock         | ✓ Passed  | 2 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_txn_doublespend.py               | ✓ Passed  | 2 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_txn_doublespend.py --mineblock   | ✓ Passed  | 2 s
[21:27:12] :	 [Step 1/1]  [0m [0;34mwallet_zapwallettxes.py                 | ✓ Passed  | 3 s
[21:27:12] :	 [Step 1/1]  [0m [0;31mmempool_packages.py                     | ✖ Failed  | 9 s
[21:27:12] :	 [Step 1/1]  [0m [0;31m [1m
[21:27:12] :	 [Step 1/1] ALL                                     | ✖ Failed  | 454 s (accumulated) 
[21:27:12] :	 [Step 1/1]  [0m [0mRuntime: 105 s
[21:27:12] :	 [Step 1/1] 
[21:27:12] :	 [Step 1/1] FAILED: test/CMakeFiles/check-functional-upgrade-activated 
[21:27:12] :	 [Step 1/1] cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/test && /usr/bin/python3 ./functional/test_runner.py --with-phononactivation -n "Bitcoin ABC functional tests with the next upgrade activated"
[21:27:12] :	 [Step 1/1] ninja: build stopped: subcommand failed.
[21:27:12]W:	 [Step 1/1] ++ print_sanitizers_log
[21:27:12]W:	 [Step 1/1] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[21:27:12]W:	 [Step 1/1] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[21:27:12]W:	 [Step 1/1] ++ cat '/tmp/sanitizer_logs/*.log.*'
[21:27:12] :	 [Step 1/1] *** Output of /tmp/sanitizer_logs/*.log.* ***
[21:27:12]W:	 [Step 1/1] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[21:27:12]W:	 [Step 1/1] Process exited with code 1
[21:27:12]E:	 [Step 1/1] Process exited with code 1 (Step: Command Line)

Each failure log is accessible here:
Bitcoin ABC functional tests with the next upgrade activated: mempool_packages.py

Harbormaster failed remote builds in B9401: Diff 16236!Feb 10 2020, 21:27
dagurval planned changes to this revision.Feb 10 2020, 21:37
dagurval added inline comments.
test/functional/mempool_packages.py
43 ↗(On Diff #16236)

Looks like this was a symptom of some underlying problem, and adding this didn't fix whobbling test.

In ~ 1 out of 10 test runs, signature will be missing.

size 191 tx CTransaction(nVersion=2 vin=[CTxIn(prevout=COutPoint(hash=ad5f21686090292701676d95bf667ff28866abf31a359e0c3e9151a4a0194fe3 n=0) scriptSig=4730440220301964ff537de0a8df576dd779cbba9fc5d5384597ad42310572d83f69ec9c0e02202341572c11e67ce73aefae23986e82062ed4b9a3085e9d726be99e66bb5182a94121033d3773ba865be13c3eb129887de71fbac6c594cc282861151fa022a968fab19b nSequence=4294967295)] vout=[CTxOut(nValue=0.24934450 scriptPubKey=76a914062241bc798a17a44bf0e65769c43ef726a954a588ac)] nLockTime=0)
size 85 tx CTransaction(nVersion=2 vin=[CTxIn(prevout=COutPoint(hash=00aa1f9a495e506ebdbd6ce7332b101ddf9c64300d03de2ba8ce83734b454f7a n=0) scriptSig= nSequence=4294967295)] vout=[CTxOut(nValue=0.49988900 scriptPubKey=76a914f37b2817a05feef754236dc65bbcc5bc2b4d64f788ac)] nLockTime=0)

I will remove this padding and investigate the root of the issue.

markblundeberg added a subscriber: markblundeberg.Feb 11 2020, 00:14
markblundeberg added inline comments.
test/functional/mempool_packages.py
43 ↗(On Diff #16236)

At very least you shouldn't pad a tx after signing it, as that means you break the signature validity.

dagurval added inline comments.Feb 11 2020, 09:46
test/functional/mempool_packages.py
43 ↗(On Diff #16236)

haha true, now I just feel dumb

dagurval updated this revision to Diff 16268.Feb 11 2020, 11:37

Ensure test selects inputs with enough coins during the second run

teamcity added a comment.Feb 11 2020, 11:37

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Feb 11 2020, 11:49

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9427: Diff 16268.Feb 11 2020, 11:49
jasonbcox added a subscriber: jasonbcox.Feb 11 2020, 22:25
jasonbcox added inline comments.
src/init.cpp
747 ↗(On Diff #16268)

s/fork/phonon-upgrade

761 ↗(On Diff #16268)

s/fork/phonon-upgrade

test/functional/mempool_packages.py
36 ↗(On Diff #16268)

maxorphantx looks like it belongs in common_params

73 ↗(On Diff #16268)

Why is minimumAmount necessary?

jasonbcox requested changes to this revision.Feb 11 2020, 22:30

Needs an update in doc/release-notes.md

This revision now requires changes to proceed.Feb 11 2020, 22:30
dagurval planned changes to this revision.Feb 12 2020, 07:13
dagurval added inline comments.
test/functional/mempool_packages.py
73 ↗(On Diff #16268)

This ensures that inputs with enough coins are selected.

Without this, the second time the tests runs (post-activation), the test randomly fails, because it would randomly select coins from the first run.

When it selected inputs with too few coins, the test would create transaction with negative values in outputs.

dagurval marked 4 inline comments as done.Feb 12 2020, 07:37
dagurval updated this revision to Diff 16325.Feb 12 2020, 07:48
  • Replace fork with ponon-upgrade in parameter description.
  • Move maxorphantx to common_params
  • Add a comment on why minimumAmount was added
  • Add release notes
teamcity added a comment.Feb 12 2020, 07:48

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Feb 12 2020, 07:59

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9461: Diff 16325.Feb 12 2020, 07:59
Fabien added a subscriber: Fabien.Feb 12 2020, 20:53

Is there any reason for not updating the wallet limit at the same time ? Not doing so would prevent creating a >25 tx chain after the upgrade if -walletrejectlongchains is set.

deadalnix requested changes to this revision.Feb 13 2020, 01:42
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
test/functional/mempool_packages.py
63 ↗(On Diff #16325)

There is already a facility to run test pre and post upgrade. Please use this unless you are specifically testing for the activation.

This revision now requires changes to proceed.Feb 13 2020, 01:42
dagurval added a comment.Feb 13 2020, 08:38
In D5244#127423, @Fabien wrote:

Is there any reason for not updating the wallet limit at the same time ? Not doing so would prevent creating a >25 tx chain after the upgrade if -walletrejectlongchains is set.

I reached out to Fabien in private to discuss this, to check if my reasoning was solid. The reasoing is as follows:

  1. The wallet/protocol is two separate pieces, and the policy change is backward compatible with the wallet as is.
  2. This would belong in its own Diff request.
  3. I did take a stab at changing the limit in the wallet, and no tests failed. Which implies that a test needs to be written for the wallet. This is a challenge, as SelectCoins is an interesting piece of engineering.
dagurval requested review of this revision.Feb 13 2020, 08:49

There is already a facility to run test pre and post upgrade. Please use this unless you are specifically testing for the activation.

I reached out to you, Jason and Fabien to figure out what you meant here. Fabien was online and by facility we think you mean the --with-phononactivation parameter.

So the basic idea would be that:

  • No --with-phononactivation flag: check 25 limit.
  • With --with-phononactivation flag: check 50 limit.

This could be done as either parsing the flag in the test itself, or by accepting limits as arguments to the test and have test_runner.py pass the limits based on if the flag is activated or not.

The problem with that is after 15th of May, the "no-flag" state would start to fail, as the policy would be active on default.

I could do if flag or time.time() > PHONONACTIVATIONTIME, but experience tells me that we don't want phonon activation time coded into the test set.

Another ideas was to add an RPC call to return the limits, but that idea kind of sucks and would not be using an existing pre and post upgrade facility.

So given that, I think the current approach is the best one, unless I'm missing something. With that, I'm pushing this back into review queue without changes.

dagurval updated this revision to Diff 16349.Feb 13 2020, 09:21

Rebase to fix merge conflict with release notes.

teamcity added a comment.Feb 13 2020, 09:21

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Feb 13 2020, 09:31

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9477: Diff 16349.Feb 13 2020, 09:31
markblundeberg added a comment.EditedFeb 14 2020, 01:41

@dagurval this is the only way I know how to do these kinds of tests. We have never written tests that depend explicitly on --with-phononactivation and we avoid writing tests that directly invoke time(), so that would be pretty weird if you did that. In other words we write tests that are either testing activation-independent behaviour, or if they are sensitive then they must handle their own activation parameters and override any effect that --with-phononactivation might have, like you've done here.

The alternative way is to copy-paste the whole test and have one run with ACTIVATION_TIME = 2000000000 and one run with ACTIVATION_TIME = 1000000000. That is nicer in one sense it's slightly easier cleanup: one test copy can be totally wiped after the real activation has passed. But I don't see any strong reason to switch to that. This is easy to clean up as is.

Now of course for the most strenuous activation tests (involving consensus features) it's important to actually step through the activation block by block, see e.g. D5179 and D5130. That lets you check very carefully how certain transactions in mempool before the upgrade are handled during the upgrade, and vice versa -- if the upgrade is rewinded, the mempool needs to be sane still. But as this is nonconsensus it's not really important.

deadalnix requested changes to this revision.Feb 14 2020, 02:46

What @markblundeberg said.

This revision now requires changes to proceed.Feb 14 2020, 02:46
dagurval requested review of this revision.Feb 14 2020, 06:49
In D5244#127826, @deadalnix wrote:

What @markblundeberg said.

Mark is saying that this is good as is. I reached out to Mark, and he does not know why you requested a change either. What change do you want?

deadalnix requested changes to this revision.Feb 14 2020, 15:38

Now of course for the most strenuous activation tests (involving consensus features) it's important to actually step through the activation block by block, see e.g. D5179 and D5130. That lets you check very carefully how certain transactions in mempool before the upgrade are handled during the upgrade, and vice versa -- if the upgrade is rewinded, the mempool needs to be sane still. But as this is nonconsensus it's not really important.

This revision now requires changes to proceed.Feb 14 2020, 15:38
dagurval updated this revision to Diff 16379.Feb 14 2020, 16:23

Remove pre-phonon testing of mempool packages. Instead the D5245 test will be improved to test activation.

teamcity added a comment.Feb 14 2020, 16:23

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Feb 14 2020, 16:33

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9497: Diff 16379.Feb 14 2020, 16:33
jasonbcox mentioned this in D5245: [qa] Test that mempool reverts to pre-phonon policies.Feb 14 2020, 18:11
dagurval updated this revision to Diff 16389.Feb 14 2020, 20:51

Added full activation test

teamcity added a comment.Feb 14 2020, 20:51

Build Bitcoin-ABC / Diffs / Diff Testing started.

deadalnix accepted this revision.Feb 14 2020, 21:00
deadalnix added inline comments.
test/functional/abc-phonon-mempoolpolicy.py
55 ↗(On Diff #16389)

That shoudn't be necessary.

71 ↗(On Diff #16389)

You could have sent the txns the good old way as there is never really a ban situation here.

teamcity added a comment.Feb 14 2020, 21:08

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9503: Diff 16389.Feb 14 2020, 21:08
jasonbcox accepted this revision.Feb 14 2020, 21:15
This revision is now accepted and ready to land.Feb 14 2020, 21:15
Closed by commit rABC3a535f346e0b: Enable new ancestor/descendants chains limit at fork (authored by dagurval). · Explain WhyFeb 14 2020, 21:16
This revision was automatically updated to reflect the committed changes.
dagurval added a commit: rABC3a535f346e0b: Enable new ancestor/descendants chains limit at fork.
dagurval added a commit: rSTAGING3a535f346e0b: Enable new ancestor/descendants chains limit at fork.Apr 7 2020, 23:01

Revision Contents
Changeset List

PathSize
doc/
9 lines
src/
11 lines
10 lines
test/
functional/
24 lines

Diff 16379

View Options

doc/release-notes.md

Loading...
View Options

src/init.cpp

Loading...
View Options

src/validation.cpp

Loading...
View Options

test/functional/mempool_packages.py

Loading...