Page MenuHomePhabricator

[qa] Test that mempool reverts to pre-phonon policies
AbandonedPublic

Authored by dagurval on Mon, Feb 10, 21:00.

Details

Reviewers
deadalnix
markblundeberg
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

This makes sure that mempool reverts to pre-phonon policies in case of a chain rollback.

Depends on D5244

Test Plan

Test added.
./test_runner.py && ./test_runner.py --with-phonon

Diff Detail

Repository
rABC Bitcoin ABC
Branch
trim-mempool-03
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 9495
Build 16903: Bitcoin ABC Buildbot
Build 16902: arc lint + arc unit

Event Timeline

dagurval created this revision.Mon, Feb 10, 21:00
Owners added a reviewer: Restricted Owners Package.Mon, Feb 10, 21:00
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Feb 10, 21:00

Snippet of first build failure:

[21:01:40] :	 [Step 1/1] [287/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[21:01:40] :	 [Step 1/1] [288/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[21:01:40] :	 [Step 1/1] [289/404] Building CXX object src/CMakeFiles/server.dir/rpc/mining.cpp.o
[21:01:41] :	 [Step 1/1] [290/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[21:01:41] :	 [Step 1/1] [291/404] Building Java objects for secp256k1-jni-test-jar.jar
[21:01:41] :	 [Step 1/1] [292/404] Generating CMakeFiles/secp256k1-jni-test-jar.dir/java_class_filelist
[21:01:42] :	 [Step 1/1] [293/404] Creating Java archive secp256k1-jni-test.jar
[21:01:42] :	 [Step 1/1] [294/404] Building CXX object src/CMakeFiles/script.dir/script/interpreter.cpp.o
[21:01:42] :	 [Step 1/1] [295/404] Linking CXX static library src/libscript.a
[21:01:42] :	 [Step 1/1] [296/404] Linking CXX static library src/libbitcoinconsensus.a
[21:01:43] :	 [Step 1/1] [297/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/finaltx.cpp.o
[21:01:43] :	 [Step 1/1] [298/404] Linking CXX shared library src/libbitcoinconsensus.so.0.0.0
[21:01:43] :	 [Step 1/1] [299/404] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[21:01:45] :	 [Step 1/1] [300/404] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[21:01:46] :	 [Step 1/1] [301/404] Linking CXX executable src/bitcoin-tx
[21:01:47] :	 [Step 1/1] [302/404] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[21:01:47] :	 [Step 1/1] [303/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[21:01:47] :	 [Step 1/1] [304/404] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[21:01:48] :	 [Step 1/1] [305/404] Building CXX object src/seeder/CMakeFiles/seeder-netprocessing.dir/bitcoin.cpp.o
[21:01:48] :	 [Step 1/1] [306/404] Building CXX object src/seeder/CMakeFiles/seeder-netprocessing.dir/db.cpp.o
[21:01:48] :	 [Step 1/1] [307/404] Building CXX object src/seeder/CMakeFiles/seeder-netprocessing.dir/dns.cpp.o
[21:01:48] :	 [Step 1/1] [308/404] Linking CXX static library src/seeder/libseeder-netprocessing.a
[21:01:48] :	 [Step 1/1] [309/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[21:01:48] :	 [Step 1/1] [310/404] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[21:01:48] :	 [Step 1/1] [311/404] Building CXX object src/CMakeFiles/server.dir/txmempool.cpp.o
[21:01:48] :	 [Step 1/1] [312/404] Linking CXX executable src/seeder/bitcoin-seeder
[21:01:49] :	 [Step 1/1] [313/404] Building CXX object src/CMakeFiles/server.dir/init.cpp.o
[21:01:51] :	 [Step 1/1] [314/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/init.cpp.o
[21:01:52] :	 [Step 1/1] [315/404] Building CXX object src/CMakeFiles/server.dir/net_processing.cpp.o
[21:01:52] :	 [Step 1/1] [316/404] Building CXX object src/CMakeFiles/server.dir/validationinterface.cpp.o
[21:01:54] :	 [Step 1/1] [317/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[21:01:56] :	 [Step 1/1] [318/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[21:01:56] :	 [Step 1/1] [319/404] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[21:01:57] :	 [Step 1/1] [320/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[21:02:02] :	 [Step 1/1] [321/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[21:02:04] :	 [Step 1/1] [322/404] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[21:02:04] :	 [Step 1/1] [323/404] Linking CXX static library src/wallet/libwallet.a
[21:02:05] :	 [Step 1/1] [324/404] Linking CXX static library src/libserver.a
[21:02:07] :	 [Step 1/1] [325/404] Linking CXX executable src/bitcoind
[21:02:07] :	 [Step 1/1] FAILED: src/bitcoind 
[21:02:07] :	 [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 && :
[21:02:07] :	 [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]':
[21:02:07] :	 [Step 1/1] /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/../src/validation.cpp:747: undefined reference to `GetDefaultAncestorLimit(Consensus::Params const&, CBlockIndex const*)'
[21:02:07] :	 [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*)'
[21:02:07] :	 [Step 1/1] /usr/bin/ld: src/libserver.a(validation.cpp.o): in function `CChainState::DisconnectTip(Config const&, CValidationState&, DisconnectedBlockTransactions*)':
[21:02:07] :	 [Step 1/1] /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/../src/validation.cpp:2288: undefined reference to `GetDefaultAncestorLimit(Consensus::Params const&, CBlockIndex const*)'
[21:02:07] :	 [Step 1/1] /usr/bin/ld: /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/../src/validation.cpp:2289: undefined reference to `GetDefaultAncestorLimit(Consensus::Params const&, CBlockIndex const*)'
[21:02:07] :	 [Step 1/1] /usr/bin/ld: /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/../src/validation.cpp:2292: undefined reference to `GetDefaultDecendantLimit(Consensus::Params const&, CBlockIndex const*)'
[21:02:07] :	 [Step 1/1] /usr/bin/ld: /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/../src/validation.cpp:2293: undefined reference to `GetDefaultDecendantLimit(Consensus::Params const&, CBlockIndex const*)'
[21:02:07] :	 [Step 1/1] collect2: error: ld returned 1 exit status
[21:02:07] :	 [Step 1/1] [326/404] Automatic MOC for target bitcoin-qt-base
[21:02:07] :	 [Step 1/1] ninja: build stopped: subcommand failed.
[21:02:07] :	 [Step 1/1] *** Output of /tmp/sanitizer_logs/*.log.* ***
[21:02:07]W:	 [Step 1/1] ++ print_sanitizers_log
[21:02:07]W:	 [Step 1/1] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[21:02:07]W:	 [Step 1/1] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[21:02:07]W:	 [Step 1/1] ++ cat '/tmp/sanitizer_logs/*.log.*'
[21:02:07]W:	 [Step 1/1] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[21:02:07]W:	 [Step 1/1] Process exited with code 1
[21:02:07]E:	 [Step 1/1] Process exited with code 1 (Step: Command Line)
dagurval planned changes to this revision.Mon, Feb 10, 22:33

Marking as planned changes due to issues with parent diff

Nit: Filename "abc-phonos-disconnectpool.py" should change phonos -> phonon

dagurval updated this revision to Diff 16277.Tue, Feb 11, 13:31

Rebase, rename test, some test cleanup

deadalnix requested changes to this revision.Thu, Feb 13, 01:38
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/validation.cpp
2293 ↗(On Diff #16277)

I don't think this is necessary because the flags change anyways when phonon is enabled. I expect that the test will pass anyways even if you remove this.

This revision now requires changes to proceed.Thu, Feb 13, 01:38
dagurval planned changes to this revision.Thu, Feb 13, 09:35
dagurval updated this revision to Diff 16353.Thu, Feb 13, 14:14

Remove redundant changes in validation.cpp

dagurval retitled this revision from Remove longer ancestor/descendants chain in case of reorg to [qa] Test that mempool reverts to pre-phonon policies.Thu, Feb 13, 14:15
dagurval edited the summary of this revision. (Show Details)
markblundeberg added a subscriber: markblundeberg.

Yes this works because the mempool is being reprocessed now due to another upgrade, as the reversebytes thing D5130 (==D5283) is landed already. 👍
(that's not bad btw, I rely on the same in my now-landed D5224 tests)

test/functional/abc-phonon-mempoolpolicy.py
22 ↗(On Diff #16353)

Phonon, also just use # for comments and reserve """ for docstrings. As far as I'm aware python doesn't have per-variable docstrings.

dagurval planned changes to this revision.Fri, Feb 14, 07:12
dagurval added inline comments.
test/functional/abc-phonon-mempoolpolicy.py
22 ↗(On Diff #16353)

I thought it was standard because sphinx picks up """ docstring for variables when they're added below them. I think you're right though, changing to #

dagurval updated this revision to Diff 16377.Fri, Feb 14, 07:14

typo, docstring fix

jasonbcox requested changes to this revision.Fri, Feb 14, 18:11
jasonbcox added a subscriber: jasonbcox.

Just pointing out that not including this as part of D5244 makes getting this feature in more difficult, since both patches will need to be reviewed green by potentially different sets of people (and preferably landed very soon one after the other).

test/functional/abc-phonon-mempoolpolicy.py
41

This function looks fragile. Check how it's done in abc-op-reversebytes-activation.py:203

58

Make 0.0001 a string, otherwise it's possible to run into this problem: https://stackoverflow.com/questions/32053647/comparing-python-decimals-created-from-float-and-string

95

This is good, but a similar test needs to done to make sure that long-chains get trimmed even if they were mined.

This revision now requires changes to proceed.Fri, Feb 14, 18:11
dagurval abandoned this revision.Fri, Feb 14, 19:54

I'll merge the test into D5244 as advised by @jasonbcox