Page MenuHomePhabricator

[backport#19988 4/9] Change transaction request logic to use txrequest
AbandonedPublic

Authored by majcosta on May 17 2021, 17:57.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

This removes most transaction request logic from net_processing, and
replaces it with calls to a global TxRequestTracker object.

The major changes are:

  • Announcements from outbound (and whitelisted) peers are now always preferred over those from inbound peers. This used to be the case for the first request (by delaying the first request from inbound peers), and a bias afters. The 2s delay for requests from inbound peers still exists, but after that, if viable outbound peers remain for any given transaction, they will always be tried first.
  • No more hard cap of 100 in flight transactions per peer, as there is less need for it (memory usage is linear in the number of announcements, but independent from the number in flight, and CPU usage isn't affected by it). Furthermore, if only one peer announces a transaction, and it has over 100 in flight and requestable already, we still want to request it from them. The cap is replaced with an additional 2s delay (possibly combined with the existing 2s delays for inbound connections, and for txid peers when wtxid peers are available).

Includes functional tests written by Marco Falke and Antoine Riard.


Partial backport of core#19988
https://github.com/bitcoin/bitcoin/pull/19988/commits/242d16477df1a024c7126bad23dde39cad217eca

Depends on D9538

Test Plan
ninja all check check-functional

Event Timeline

Tail of the build log:

[334/505] Linking C executable src/secp256k1/verify-bench
[335/505] Linking C executable src/secp256k1/recover-bench
[336/505] Linking CXX static library src/libbitcoinconsensus.a
[337/505] Installing component secp256k1
-- Install configuration: "Debug"
-- Install component: "secp256k1"
-- Installing: /results/artifacts/lib/libsecp256k1.a
-- Installing: /results/artifacts/include/secp256k1.h
-- Installing: /results/artifacts/include/secp256k1_preallocated.h
-- Installing: /results/artifacts/include/secp256k1_recovery.h
-- Installing: /results/artifacts/include/secp256k1_schnorr.h
[338/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[339/505] Building CXX object src/CMakeFiles/script.dir/script/standard.cpp.o
[340/505] Building CXX object src/CMakeFiles/script.dir/script/sign.cpp.o
[341/505] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[342/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[343/505] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[344/505] Linking C executable src/secp256k1/ecmult-bench
[345/505] Building CXX object src/CMakeFiles/script.dir/script/signingprovider.cpp.o
[346/505] Building CXX object src/CMakeFiles/script.dir/script/descriptor.cpp.o
[347/505] Building CXX object src/CMakeFiles/bitcoin-wallet.dir/bitcoin-wallet.cpp.o
[348/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[349/505] Linking CXX static library src/libscript.a
[350/505] Linking CXX static library src/libcommon.a
[351/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[352/505] Linking CXX shared library src/libbitcoinconsensus.so.0.23.4
[353/505] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[354/505] Linking CXX executable src/bitcoin-cli
[355/505] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[356/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[357/505] Linking CXX executable src/bitcoin-tx
[358/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[359/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[360/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[361/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[362/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[363/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[364/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[365/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[366/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[367/505] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[368/505] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[369/505] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[370/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[371/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[372/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[373/505] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[374/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[375/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[376/505] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[377/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[378/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[379/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[380/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[381/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[382/505] Linking CXX static library src/wallet/libwallet.a
[383/505] Linking CXX static library src/wallet/libwallet-tool.a
[384/505] Linking CXX executable src/bitcoin-wallet
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1

Tail of the build log:

[340/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[341/505] Linking CXX static library src/libscript.a
[342/505] Installing component secp256k1
-- Install configuration: "RelWithDebInfo"
-- Install component: "secp256k1"
-- Installing: /results/artifacts/lib/libsecp256k1.a
-- Installing: /results/artifacts/include/secp256k1.h
-- Installing: /results/artifacts/include/secp256k1_preallocated.h
-- Installing: /results/artifacts/include/secp256k1_recovery.h
-- Installing: /results/artifacts/include/secp256k1_schnorr.h
[343/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[344/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[345/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[346/505] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[347/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[348/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[349/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[350/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[351/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[352/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[353/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[354/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[355/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[356/505] Linking CXX static library src/libcommon.a
[357/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[358/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[359/505] Linking CXX shared library src/libbitcoinconsensus.so.0.23.4
[360/505] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[361/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[362/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[363/505] Linking CXX executable src/bitcoin-cli
[364/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[365/505] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[366/505] Linking CXX executable src/bitcoin-tx
[367/505] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[368/505] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[369/505] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[370/505] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[371/505] Linking CXX static library src/wallet/libwallet.a
[372/505] Linking CXX static library src/wallet/libwallet-tool.a
[373/505] Building CXX object src/CMakeFiles/server.dir/banman.cpp.o
[374/505] Linking CXX executable src/bitcoin-wallet
[375/505] Building CXX object src/CMakeFiles/server.dir/node/context.cpp.o
[376/505] Building CXX object src/CMakeFiles/server.dir/node/transaction.cpp.o
[377/505] Building CXX object src/CMakeFiles/server.dir/interfaces/chain.cpp.o
[378/505] Building CXX object src/CMakeFiles/server.dir/txrequest.cpp.o
FAILED: src/CMakeFiles/server.dir/txrequest.cpp.o 
/usr/bin/ccache /usr/bin/c++  -DBOOST_AC_USE_STD_ATOMIC -DBOOST_SP_USE_STD_ATOMIC -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/leveldb/helpers/memenv -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -I../../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wredundant-decls -Wsign-compare -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -std=gnu++17 -MD -MT src/CMakeFiles/server.dir/txrequest.cpp.o -MF src/CMakeFiles/server.dir/txrequest.cpp.o.d -o src/CMakeFiles/server.dir/txrequest.cpp.o -c ../../src/txrequest.cpp
../../src/txrequest.cpp:79:21: error: ‘{anonymous}::Announcement::m_state’ is too small to hold all values of ‘enum class {anonymous}::State’ [-Werror]
     State m_state : 3;
                     ^
cc1plus: all warnings being treated as errors
[379/505] Building CXX object src/CMakeFiles/server.dir/interfaces/node.cpp.o
[380/505] Building CXX object src/CMakeFiles/server.dir/avalanche/processor.cpp.o
[381/505] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[382/505] Building CXX object src/CMakeFiles/server.dir/rpc/net.cpp.o
[383/505] Building CXX object src/CMakeFiles/server.dir/net_processing.cpp.o
[384/505] Building CXX object src/CMakeFiles/server.dir/init.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1

Tail of the build log:

[311/452] Building CXX object src/CMakeFiles/server.dir/rpc/server.cpp.o
[312/452] Building CXX object src/CMakeFiles/server.dir/script/scriptcache.cpp.o
[313/452] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[314/452] Building CXX object src/CMakeFiles/server.dir/script/sigcache.cpp.o
[315/452] Building CXX object src/CMakeFiles/server.dir/timedata.cpp.o
[316/452] Building CXX object src/CMakeFiles/server.dir/txdb.cpp.o
[317/452] Building CXX object src/CMakeFiles/server.dir/torcontrol.cpp.o
[318/452] Building CXX object src/CMakeFiles/server.dir/validationinterface.cpp.o
[319/452] Building CXX object src/CMakeFiles/server.dir/txmempool.cpp.o
[320/452] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[321/452] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[322/452] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[323/452] Building CXX object src/CMakeFiles/server.dir/versionbits.cpp.o
[324/452] Building C object src/secp256k1/CMakeFiles/sign-bench.dir/src/bench_sign.c.o
[325/452] Building C object src/secp256k1/CMakeFiles/verify-bench.dir/src/bench_verify.c.o
[326/452] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[327/452] Building CXX object src/CMakeFiles/server.dir/dummywallet.cpp.o
[328/452] Building C object src/secp256k1/CMakeFiles/recover-bench.dir/src/bench_recover.c.o
[329/452] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[330/452] Linking C static library src/secp256k1/libsecp256k1.a
[331/452] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[332/452] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[333/452] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[334/452] Linking C executable src/secp256k1/ecmult-bench
[335/452] Linking C executable src/secp256k1/internal-bench
[336/452] Linking C executable src/secp256k1/sign-bench
[337/452] Linking C executable src/secp256k1/verify-bench
[338/452] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[339/452] Linking C executable src/secp256k1/recover-bench
[340/452] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[341/452] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[342/452] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[343/452] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[344/452] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[345/452] Linking CXX static library src/libcommon.a
[346/452] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[347/452] Linking CXX static library src/libscript.a
[348/452] Linking CXX static library src/libbitcoinconsensus.a
[349/452] Linking CXX shared library src/libbitcoinconsensus.so.0.23.4
[350/452] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[351/452] Linking CXX executable src/bitcoin-cli
[352/452] Linking CXX executable src/bitcoin-tx
[353/452] Building CXX object src/CMakeFiles/server.dir/node/context.cpp.o
[354/452] Building CXX object src/CMakeFiles/server.dir/node/transaction.cpp.o
[355/452] Building CXX object src/CMakeFiles/server.dir/interfaces/chain.cpp.o
[356/452] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[357/452] Building CXX object src/CMakeFiles/server.dir/interfaces/node.cpp.o
[358/452] Building CXX object src/CMakeFiles/server.dir/txrequest.cpp.o
FAILED: src/CMakeFiles/server.dir/txrequest.cpp.o 
/usr/bin/ccache /usr/bin/c++  -DBOOST_AC_USE_STD_ATOMIC -DBOOST_SP_USE_STD_ATOMIC -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/leveldb/helpers/memenv -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -I../../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -Werror -g -O2 -fPIC -fvisibility=hidden   -fstack-reuse=none -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wredundant-decls -Wsign-compare -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -std=gnu++17 -MD -MT src/CMakeFiles/server.dir/txrequest.cpp.o -MF src/CMakeFiles/server.dir/txrequest.cpp.o.d -o src/CMakeFiles/server.dir/txrequest.cpp.o -c ../../src/txrequest.cpp
../../src/txrequest.cpp:79:21: error: ‘{anonymous}::Announcement::m_state’ is too small to hold all values of ‘enum class {anonymous}::State’ [-Werror]
     State m_state : 3;
                     ^
cc1plus: all warnings being treated as errors
[359/452] Building CXX object src/CMakeFiles/server.dir/avalanche/processor.cpp.o
[360/452] Building CXX object src/CMakeFiles/server.dir/rpc/net.cpp.o
[361/452] Building CXX object src/CMakeFiles/server.dir/net_processing.cpp.o
[362/452] Building CXX object src/CMakeFiles/server.dir/init.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1