This makes sure that mempool reverts to pre-phonon policies in case of a chain rollback.
Depends on D5244
Details
Details
- Reviewers
deadalnix markblundeberg jasonbcox - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Project
Test added.
./test_runner.py && ./test_runner.py --with-phonon
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- trim-mempool-03
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 9399 Build 16728: Default Diff Build & Tests Build 16727: arc lint + arc unit
Event Timeline
Comment Actions
Build Bitcoin-ABC / Diffs / Diff Testing failed.
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)| 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. |
Comment Actions
| 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. |
| 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 # |
Comment Actions
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 ↗ | (On Diff #16377) | This function looks fragile. Check how it's done in abc-op-reversebytes-activation.py:203 |
| 58 ↗ | (On Diff #16377) | 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 ↗ | (On Diff #16377) | This is good, but a similar test needs to done to make sure that long-chains get trimmed even if they were mined. |