Page MenuHomePhabricator

[avalanche] Move contender cache to peer manager
Changes PlannedPublic

Authored by roqqit on Wed, Mar 12, 19:26.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

addStakeContender() being called from net_processing is not robust because there are various ways a proof may be introduced (via network, RPC, loaded at startup, etc). This results in the cache being unaware of proofs in various scenarios, eventually breaking contender voting. The robust solution is to put peerManager in charge of adding contenders to the cache whenever a proof is successfully registered. This is the first step to achieving that.

This diff only moves the cache with minimal other changes (excepting some locking fixes). The pass-through API of the cache in peermanager is not ideal, however some of these can be refactored away in future diffs to reduce future maintenance overhead.

There is no change in behavior.

Test Plan
ninja check check-functional

Diff Detail

Event Timeline

roqqit requested review of this revision.Wed, Mar 12, 19:26

Tail of the build log:

/work /work/abc-ci-builds/lint-circular-dependencies
A new circular dependency in the form of "avalanche/peermanager -> avalanche/stakecontendercache -> avalanche/peermanager" appears to have been introduced.

/work/abc-ci-builds/lint-circular-dependencies
Build lint-circular-dependencies failed with exit code 1

Tail of the build log:

[403/580] Linking CXX executable src/bitcoin-cli
[404/580] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[405/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[406/580] Linking CXX shared library src/libbitcoinconsensus.so.0.30.13
[407/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[408/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[409/580] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[410/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[411/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[412/580] Linking CXX executable src/bitcoin-tx
[413/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[414/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[415/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/interfaces.cpp.o
[416/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/receive.cpp.o
[417/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[418/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/backup.cpp.o
[419/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[420/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/encrypt.cpp.o
[421/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/util.cpp.o
[422/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/signmessage.cpp.o
[423/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[424/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/spend.cpp.o
[425/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[426/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[427/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/transaction.cpp.o
[428/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[429/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[430/580] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[431/580] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[432/580] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[433/580] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[434/580] Building CXX object src/CMakeFiles/server.dir/avalanche/stakecontendercache.cpp.o
[435/580] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[436/580] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[437/580] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana.cpp.o
[438/580] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[439/580] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana_formatter.cpp.o
[440/580] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana_interpreter.cpp.o
[441/580] Linking CXX executable src/iguana/iguana
[442/580] Linking CXX static library src/wallet/libwallet.a
[443/580] Building CXX object src/CMakeFiles/server.dir/net_processing.cpp.o
FAILED: src/CMakeFiles/server.dir/net_processing.cpp.o 
/usr/bin/ccache /usr/bin/clang++ -DBOOST_ALL_NO_LIB -DBOOST_NO_CXX98_FUNCTION_BASE -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/work/src/leveldb/helpers/memenv -I/work/src/. -I/work/abc-ci-builds/build-clang/src -I/work/src/univalue/include -I/work/abc-ci-builds/build-clang/src/crypto/.. -I/work/src/secp256k1/include -I/work/src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -Werror -g -O2 -fPIC -fvisibility=hidden -fstack-protector-all -Wstack-protector -fcf-protection=full -fstack-clash-protection -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wgnu -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunreachable-code-loop-increment -Wsign-compare -Wconditional-uninitialized -Wdocumentation -Wformat-security -Wredundant-move -Woverloaded-virtual -Wshadow -Wshadow-field -Wno-unused-parameter -Wno-implicit-fallthrough -Wno-psabi -std=gnu++17 -MD -MT src/CMakeFiles/server.dir/net_processing.cpp.o -MF src/CMakeFiles/server.dir/net_processing.cpp.o.d -o src/CMakeFiles/server.dir/net_processing.cpp.o -c /work/src/net_processing.cpp
/work/src/net_processing.cpp:6959:42: error: calling function 'finalizeStakeContender' requires negative capability '!cs_main' [-Werror,-Wthread-safety-analysis]
                            m_avalanche->finalizeStakeContender(contenderId);
                                         ^
/work/src/net_processing.cpp:9219:22: error: calling function 'addStakeContender' requires negative capability '!cs_main' [-Werror,-Wthread-safety-analysis]
        m_avalanche->addStakeContender(proof);
                     ^
2 errors generated.
[444/580] Building CXX object src/CMakeFiles/server.dir/node/miner.cpp.o
[445/580] Building CXX object src/CMakeFiles/server.dir/avalanche/peermanager.cpp.o
[446/580] Building CXX object src/CMakeFiles/server.dir/node/blockstorage.cpp.o
[447/580] Building CXX object src/CMakeFiles/server.dir/avalanche/processor.cpp.o
[448/580] Building CXX object src/CMakeFiles/server.dir/init.cpp.o
[449/580] Building CXX object src/CMakeFiles/server.dir/rpc/mining.cpp.o
[450/580] Building CXX object src/CMakeFiles/server.dir/rpc/avalanche.cpp.o
[451/580] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1

Tail of the build log:

[393/578] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[394/578] Linking C executable src/secp256k1/ecmult-bench
[395/578] Building CXX object src/CMakeFiles/bitcoind.dir/bitcoind.cpp.o
[396/578] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[397/578] Linking CXX executable src/bitcoin-cli
[398/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockindex.cpp.o
[399/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[400/578] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[401/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[402/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/random.cpp.o
[403/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/coins.cpp.o
[404/578] Building CXX object src/CMakeFiles/bitcoin-wallet.dir/bitcoin-wallet.cpp.o
[405/578] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[406/578] Linking CXX executable src/bitcoin-tx
[407/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[408/578] Building CXX object src/CMakeFiles/server.dir/wallet/init.cpp.o
[409/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[410/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[411/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[412/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[413/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[414/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[415/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[416/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/validation.cpp.o
[417/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[418/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[419/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/txmempool.cpp.o
[420/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[421/578] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[422/578] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[423/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[424/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[425/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/transaction.cpp.o
[426/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/receive.cpp.o
[427/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[428/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/signmessage.cpp.o
[429/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/util.cpp.o
[430/578] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[431/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[432/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/encrypt.cpp.o
[433/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/interfaces.cpp.o
[434/578] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[435/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[436/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/spend.cpp.o
[437/578] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[438/578] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana_formatter.cpp.o
[439/578] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana_interpreter.cpp.o
[440/578] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana.cpp.o
[441/578] Linking CXX executable src/iguana/iguana
[442/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[443/578] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[444/578] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[445/578] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[446/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/backup.cpp.o
[447/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[448/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[449/578] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[450/578] Linking CXX static library src/wallet/libwallet.a
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1
roqqit planned changes to this revision.Wed, Mar 12, 19:39