Page MenuHomePhabricator

Enable new ancestor/descendants chains limit at fork
ClosedPublic

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

Details

Reviewers
jasonbcox
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
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
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.Mon, Feb 10, 20:48
Owners added a reviewer: Restricted Owners Package.Mon, Feb 10, 20:48
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Feb 10, 20:48

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)
dagurval updated this revision to Diff 16235.Mon, Feb 10, 21:14

Added mempool.cpp to CMakeList.txt

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

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

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

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

dagurval planned changes to this revision.Mon, Feb 10, 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 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.Tue, Feb 11, 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.Tue, Feb 11, 11:37

Ensure test selects inputs with enough coins during the second run

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.Tue, Feb 11, 22:30

Needs an update in doc/release-notes.md

This revision now requires changes to proceed.Tue, Feb 11, 22:30
dagurval planned changes to this revision.Wed, Feb 12, 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.Wed, Feb 12, 07:37
dagurval updated this revision to Diff 16325.Wed, Feb 12, 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
Fabien added a subscriber: Fabien.Wed, Feb 12, 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.Thu, Feb 13, 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.Thu, Feb 13, 01:42

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.Thu, Feb 13, 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.Thu, Feb 13, 09:21

Rebase to fix merge conflict with release notes.

markblundeberg added a comment.EditedFri, Feb 14, 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.Fri, Feb 14, 02:46

What @markblundeberg said.

This revision now requires changes to proceed.Fri, Feb 14, 02:46
dagurval requested review of this revision.Fri, Feb 14, 06:49

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.Fri, Feb 14, 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.Fri, Feb 14, 15:38
dagurval updated this revision to Diff 16379.Fri, Feb 14, 16:23

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

dagurval updated this revision to Diff 16389.Fri, Feb 14, 20:51

Added full activation test

deadalnix accepted this revision.Fri, Feb 14, 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.

jasonbcox accepted this revision.Fri, Feb 14, 21:15
This revision is now accepted and ready to land.Fri, Feb 14, 21:15
This revision was automatically updated to reflect the committed changes.