Page MenuHomePhabricator

[avalanche] implement getavalanchepeerinfo
ClosedPublic

Authored by PiRK on Mar 2 2021, 14:27.

Details

Reviewers
deadalnix
majcosta
Group Reviewers
Restricted Project
Commits
rABCe213a4b92326: [avalanche] implement getavalanchepeerinfo
Summary

This adds a RPC command similar to existing command getpeerinfo, but for avalanche peers. It is useful for debugging.

Test Plan

Manually add a few avalanche peers, run the RPC and verify the displayed information.

src/bitcoin-cli getavalanchepeerinfo

Event Timeline

PiRK requested review of this revision.Mar 2 2021, 14:27
PiRK planned changes to this revision.

Tail of the build log:

-- Installing: /results/artifacts/include/secp256k1_recovery.h
-- Installing: /results/artifacts/include/secp256k1_schnorr.h
[327/503] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/uint256.cpp.o
[328/503] Building CXX object src/CMakeFiles/util.dir/util/message.cpp.o
[329/503] Building CXX object src/CMakeFiles/util.dir/util/settings.cpp.o
[330/503] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[331/503] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/util/strencodings.cpp.o
[332/503] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/pubkey.cpp.o
[333/503] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/primitives/transaction.cpp.o
[334/503] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/consensus/tx_check.cpp.o
[335/503] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[336/503] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[337/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[338/503] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[339/503] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[340/503] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[341/503] Building CXX object src/CMakeFiles/util.dir/util/time.cpp.o
[342/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[343/503] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[344/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[345/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[346/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[347/503] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[348/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[349/503] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqabstractnotifier.cpp.o
[350/503] Building CXX object src/CMakeFiles/util.dir/util/system.cpp.o
[351/503] Linking CXX static library src/libutil.a
[352/503] Linking CXX static library src/librpcclient.a
[353/503] Linking CXX static library src/libbitcoinconsensus.a
[354/503] Linking CXX static library src/libscript.a
[355/503] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[356/503] Linking CXX static library src/libcommon.a
[357/503] Linking CXX shared library src/libbitcoinconsensus.so.0.22.15
[358/503] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[359/503] Linking CXX executable src/bitcoin-cli
[360/503] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[361/503] Linking CXX executable src/bitcoin-tx
[362/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[363/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[364/503] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqnotificationinterface.cpp.o
[365/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[366/503] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[367/503] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[368/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[369/503] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqpublishnotifier.cpp.o
[370/503] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqrpc.cpp.o
[371/503] Linking CXX static library src/zmq/libzmq.a
[372/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[373/503] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[374/503] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[375/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[376/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[377/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[378/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[379/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[380/503] Linking CXX static library src/wallet/libwallet.a
[381/503] Linking CXX static library src/wallet/libwallet-tool.a
[382/503] Linking CXX executable src/bitcoin-wallet
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1
PiRK planned changes to this revision.Mar 2 2021, 14:31

Tail of the build log:

[341/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[342/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[343/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[344/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[345/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[346/503] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[347/503] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[348/503] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[349/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[350/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[351/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[352/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[353/503] Linking C static library src/secp256k1/libsecp256k1.a
[354/503] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[355/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[356/503] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[357/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[358/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[359/503] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[360/503] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[361/503] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[362/503] Linking CXX static library src/libbitcoinconsensus.a
[363/503] Linking C executable src/secp256k1/ecmult-bench
[364/503] Linking C executable src/secp256k1/internal-bench
[365/503] Linking C executable src/secp256k1/sign-bench
[366/503] Linking C executable src/secp256k1/recover-bench
[367/503] Linking C executable src/secp256k1/verify-bench
[368/503] 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
[369/503] Linking CXX static library src/libscript.a
[370/503] Linking CXX static library src/libcommon.a
[371/503] Linking CXX shared library src/libbitcoinconsensus.so.0.22.15
[372/503] Linking CXX executable src/bitcoin-cli
[373/503] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[374/503] Linking CXX executable src/bitcoin-tx
[375/503] Linking CXX static library src/wallet/libwallet.a
[376/503] Linking CXX static library src/wallet/libwallet-tool.a
[377/503] Linking CXX executable src/bitcoin-wallet
[378/503] Building CXX object src/CMakeFiles/server.dir/avalanche/processor.cpp.o
FAILED: src/CMakeFiles/server.dir/avalanche/processor.cpp.o 
/usr/bin/ccache /usr/bin/clang++  -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-protector-all -Wstack-protector -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 -Wformat-security -Wredundant-move -Wshadow -Wshadow-field -Wno-unused-parameter -Wno-implicit-fallthrough -pthread -std=gnu++17 -MD -MT src/CMakeFiles/server.dir/avalanche/processor.cpp.o -MF src/CMakeFiles/server.dir/avalanche/processor.cpp.o.d -o src/CMakeFiles/server.dir/avalanche/processor.cpp.o -c ../../src/avalanche/processor.cpp
../../src/avalanche/processor.cpp:591:5: error: reading variable 'peerManager' requires holding mutex 'cs_peerManager' [-Werror,-Wthread-safety-analysis]
    peerManager->getPeers(vpeers);
    ^
../../src/avalanche/processor.cpp:595:5: error: reading variable 'peerManager' requires holding mutex 'cs_peerManager' [-Werror,-Wthread-safety-analysis]
    peerManager->getNodes(vnodes);
    ^
2 errors generated.
[379/503] Building CXX object src/CMakeFiles/server.dir/rpc/avalanche.cpp.o
[380/503] Building CXX object src/CMakeFiles/server.dir/init.cpp.o
[381/503] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[382/503] Building CXX object src/CMakeFiles/server.dir/net_processing.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:

[380/436] bitcoin: testing bip32_tests
[381/436] bitcoin: testing finalization_tests
[382/436] bitcoin: testing sync_tests
[383/436] Running utility command for check-bitcoin-finalization_tests
[384/436] Running utility command for check-bitcoin-bip32_tests
[385/436] Running utility command for check-bitcoin-sync_tests
[386/436] bitcoin: testing torcontrol_tests
[387/436] Running utility command for check-bitcoin-torcontrol_tests
[388/436] bitcoin: testing scriptpubkeyman_tests
[389/436] bitcoin: testing streams_tests
[390/436] bitcoin: testing timedata_tests
[391/436] Running utility command for check-bitcoin-scriptpubkeyman_tests
[392/436] Running utility command for check-bitcoin-streams_tests
[393/436] Running utility command for check-bitcoin-timedata_tests
[394/436] bitcoin: testing undo_tests
[395/436] bitcoin: testing uint256_tests
[396/436] Running utility command for check-bitcoin-undo_tests
[397/436] Running utility command for check-bitcoin-uint256_tests
[398/436] bitcoin: testing script_standard_tests
[399/436] Running utility command for check-bitcoin-script_standard_tests
[400/436] bitcoin: testing versionbits_tests
[401/436] bitcoin: testing schnorr_tests
[402/436] bitcoin: testing radix_tests
[403/436] Running utility command for check-bitcoin-versionbits_tests
[404/436] Running utility command for check-bitcoin-radix_tests
[405/436] Running utility command for check-bitcoin-schnorr_tests
[406/436] bitcoin: testing blockstatus_tests
[407/436] Running utility command for check-bitcoin-blockstatus_tests
[408/436] bitcoin: testing wallet_tests
[409/436] bitcoin: testing cuckoocache_tests
[410/436] Running utility command for check-bitcoin-wallet_tests
[411/436] bitcoin: testing cashaddr_tests
[412/436] bitcoin: testing validation_block_tests
[413/436] Running utility command for check-bitcoin-cuckoocache_tests
[414/436] bitcoin: testing getarg_tests
[415/436] Running utility command for check-bitcoin-cashaddr_tests
[416/436] Running utility command for check-bitcoin-validation_block_tests
[417/436] Running utility command for check-bitcoin-getarg_tests
[418/436] bitcoin: testing script_tests
[419/436] bitcoin: testing crypto_tests
[420/436] bitcoin: testing validation_tests
[421/436] Running utility command for check-bitcoin-script_tests
[422/436] Running utility command for check-bitcoin-crypto_tests
[423/436] Running utility command for check-bitcoin-validation_tests
[424/436] bitcoin: testing blockcheck_tests
[425/436] Running utility command for check-bitcoin-blockcheck_tests
[426/436] bitcoin: testing skiplist_tests
[427/436] Running utility command for check-bitcoin-skiplist_tests
[428/436] bitcoin: testing coinselector_tests
FAILED: src/test/CMakeFiles/check-bitcoin-coinselector_tests 
cd /work/abc-ci-builds/build-clang-tidy/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-clang-tidy/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-clang-tidy/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-clang-tidy/test/log/bitcoin-coinselector_tests.log /work/abc-ci-builds/build-clang-tidy/src/test/test_bitcoin --run_test=coinselector_tests --logger=HRF,message:JUNIT,message,bitcoin-coinselector_tests.xml --catch_system_errors=no
Segmentation fault (core dumped)
[429/436] bitcoin: testing op_reversebytes_tests
[430/436] Running utility command for check-bitcoin-op_reversebytes_tests
[431/436] bitcoin: testing transaction_tests
[432/436] Running utility command for check-bitcoin-transaction_tests
[433/436] bitcoin: testing coins_tests
[434/436] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

hold the mutex before using peerManager

PiRK planned changes to this revision.Mar 2 2021, 14:40

fix RPCExamples and remove duplicated RPCHelpMan

PiRK planned changes to this revision.Mar 2 2021, 14:54

fix pushKV --> push_back for array of node ids

PiRK planned changes to this revision.Mar 2 2021, 15:18
PiRK retitled this revision from implement getavalanchepeerinfo to [avalanche] implement getavalanchepeerinfo.Mar 2 2021, 17:08
PiRK edited the summary of this revision. (Show Details)
PiRK added a reviewer: deadalnix.
majcosta requested changes to this revision.Mar 3 2021, 11:11
majcosta added a subscriber: majcosta.
majcosta added inline comments.
src/rpc/avalanche.cpp
211 ↗(On Diff #27859)

avalanche*

This revision now requires changes to proceed.Mar 3 2021, 11:11

address review: fix typo in RPC command name in help message

try not to loop over all nodes for each peer just to find its node.

Add an index to NodeSet to be able to find nodes by peerId.

apparently the additional NodeSet index is not necessary. You can do a partial search on only the first key in a composite_key.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_p2p_avalanche.py ======

------- Stdout: -------
2021-03-04T08:26:30.540000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210304_082549/abc_p2p_avalanche_137
2021-03-04T08:26:32.616000Z TestFramework (INFO): Poll for the chain tip...
2021-03-04T08:26:32.670000Z TestFramework (INFO): Poll for a selection of blocks...
2021-03-04T08:26:32.726000Z TestFramework (INFO): Poll for a selection of blocks, but some are now invalid...
2021-03-04T08:26:32.799000Z TestFramework (INFO): Poll for unknown blocks...
2021-03-04T08:26:32.867000Z TestFramework (INFO): Trigger polling from the node...
2021-03-04T08:26:32.909000Z TestFramework (INFO): Testing getavalanchepeerinfo...
2021-03-04T08:26:32.910000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 128, in main
    self.run_test()
  File "/work/test/functional/abc_p2p_avalanche.py", line 269, in run_test
    assert_equal(avapeerinfo[0]["nodes"], list(range(1, 17)))
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not([4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1, 2, 3] == [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
2021-03-04T08:26:32.962000Z TestFramework (INFO): Stopping nodes
2021-03-04T08:26:33.269000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210304_082549/abc_p2p_avalanche_137
2021-03-04T08:26:33.269000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210304_082549/abc_p2p_avalanche_137/test_framework.log
2021-03-04T08:26:33.270000Z TestFramework (ERROR): 
2021-03-04T08:26:33.270000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210304_082549/abc_p2p_avalanche_137' to consolidate all logs
2021-03-04T08:26:33.270000Z TestFramework (ERROR): 
2021-03-04T08:26:33.270000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2021-03-04T08:26:33.271000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2021-03-04T08:26:33.271000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_p2p_avalanche.py

Failed tests logs:

====== Bitcoin ABC functional tests: abc_p2p_avalanche.py ======

------- Stdout: -------
2021-03-04T08:27:47.202000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210304_082521/abc_p2p_avalanche_199
2021-03-04T08:27:50.694000Z TestFramework (INFO): Poll for the chain tip...
2021-03-04T08:27:50.748000Z TestFramework (INFO): Poll for a selection of blocks...
2021-03-04T08:27:50.806000Z TestFramework (INFO): Poll for a selection of blocks, but some are now invalid...
2021-03-04T08:27:50.939000Z TestFramework (INFO): Poll for unknown blocks...
2021-03-04T08:27:50.994000Z TestFramework (INFO): Trigger polling from the node...
2021-03-04T08:27:51.009000Z TestFramework (INFO): Testing getavalanchepeerinfo...
2021-03-04T08:27:51.010000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 128, in main
    self.run_test()
  File "/work/test/functional/abc_p2p_avalanche.py", line 269, in run_test
    assert_equal(avapeerinfo[0]["nodes"], list(range(1, 17)))
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not([2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1] == [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
2021-03-04T08:27:51.061000Z TestFramework (INFO): Stopping nodes
2021-03-04T08:27:51.413000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210304_082521/abc_p2p_avalanche_199
2021-03-04T08:27:51.413000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210304_082521/abc_p2p_avalanche_199/test_framework.log
2021-03-04T08:27:51.414000Z TestFramework (ERROR): 
2021-03-04T08:27:51.414000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210304_082521/abc_p2p_avalanche_199' to consolidate all logs
2021-03-04T08:27:51.414000Z TestFramework (ERROR): 
2021-03-04T08:27:51.414000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2021-03-04T08:27:51.414000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2021-03-04T08:27:51.414000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_p2p_avalanche.py

Failed tests logs:

====== Bitcoin ABC functional tests: abc_p2p_avalanche.py ======

------- Stdout: -------
2021-03-04T08:28:26.223000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210304_082632/abc_p2p_avalanche_313
2021-03-04T08:28:28.290000Z TestFramework (INFO): Poll for the chain tip...
2021-03-04T08:28:28.343000Z TestFramework (INFO): Poll for a selection of blocks...
2021-03-04T08:28:28.399000Z TestFramework (INFO): Poll for a selection of blocks, but some are now invalid...
2021-03-04T08:28:28.472000Z TestFramework (INFO): Poll for unknown blocks...
2021-03-04T08:28:28.526000Z TestFramework (INFO): Trigger polling from the node...
2021-03-04T08:28:28.536000Z TestFramework (INFO): Testing getavalanchepeerinfo...
2021-03-04T08:28:28.537000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 128, in main
    self.run_test()
  File "/work/test/functional/abc_p2p_avalanche.py", line 269, in run_test
    assert_equal(avapeerinfo[0]["nodes"], list(range(1, 17)))
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not([2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1] == [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
2021-03-04T08:28:28.588000Z TestFramework (INFO): Stopping nodes
2021-03-04T08:28:28.839000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210304_082632/abc_p2p_avalanche_313
2021-03-04T08:28:28.839000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210304_082632/abc_p2p_avalanche_313/test_framework.log
2021-03-04T08:28:28.839000Z TestFramework (ERROR): 
2021-03-04T08:28:28.839000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210304_082632/abc_p2p_avalanche_313' to consolidate all logs
2021-03-04T08:28:28.840000Z TestFramework (ERROR): 
2021-03-04T08:28:28.840000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2021-03-04T08:28:28.840000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2021-03-04T08:28:28.840000Z TestFramework (ERROR):
====== Bitcoin ABC functional tests with the next upgrade activated: abc_p2p_avalanche.py ======

------- Stdout: -------
2021-03-04T08:31:42.192000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210304_082945/abc_p2p_avalanche_81
2021-03-04T08:31:44.633000Z TestFramework (INFO): Poll for the chain tip...
2021-03-04T08:31:44.700000Z TestFramework (INFO): Poll for a selection of blocks...
2021-03-04T08:31:44.780000Z TestFramework (INFO): Poll for a selection of blocks, but some are now invalid...
2021-03-04T08:31:44.904000Z TestFramework (INFO): Poll for unknown blocks...
2021-03-04T08:31:44.972000Z TestFramework (INFO): Trigger polling from the node...
2021-03-04T08:31:45.002000Z TestFramework (INFO): Testing getavalanchepeerinfo...
2021-03-04T08:31:45.003000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 128, in main
    self.run_test()
  File "/work/test/functional/abc_p2p_avalanche.py", line 269, in run_test
    assert_equal(avapeerinfo[0]["nodes"], list(range(1, 17)))
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not([3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1, 2] == [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
2021-03-04T08:31:45.054000Z TestFramework (INFO): Stopping nodes
2021-03-04T08:31:45.310000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210304_082945/abc_p2p_avalanche_81
2021-03-04T08:31:45.310000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210304_082945/abc_p2p_avalanche_81/test_framework.log
2021-03-04T08:31:45.310000Z TestFramework (ERROR): 
2021-03-04T08:31:45.311000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210304_082945/abc_p2p_avalanche_81' to consolidate all logs
2021-03-04T08:31:45.311000Z TestFramework (ERROR): 
2021-03-04T08:31:45.311000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2021-03-04T08:31:45.311000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2021-03-04T08:31:45.312000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_p2p_avalanche.py
Bitcoin ABC functional tests with the next upgrade activated: abc_p2p_avalanche.py

fix functional test by sorting the list of node ids before comparing with range(1, 17)

Tested with

for i in `seq 1 10`; do test/functional/test_runner.py abc_p2p_ava*; done
majcosta requested changes to this revision.Mar 9 2021, 22:48
majcosta added inline comments.
src/avalanche/peermanager.cpp
441–454 ↗(On Diff #27886)

why not return std::vector<NodeId> and std::vector<Peer> instead of using an out parameter? same for the Processor methods

test/functional/abc_p2p_avalanche.py
269–270 ↗(On Diff #27886)

now that 16 is proliferating, might be a good idea to make it QUORUM_NODE_COUNT or something

This revision now requires changes to proceed.Mar 9 2021, 22:48

address comments: add a QUORUM_NODE_COUNT=16 to test, return veector of peers instead of passing a reference

lgtm, just a few suggestions for conciseness

src/avalanche/processor.cpp
590–598 ↗(On Diff #27910)

since it returns by value I don't see the need for this to be const, but I suppose it doesn't hurt

src/rpc/avalanche.cpp
272–276 ↗(On Diff #27910)

now that your functions have a simpler API, reap the benefits :)

301–304 ↗(On Diff #27910)

ditto

This revision is now accepted and ready to land.Mar 10 2021, 14:43

reap the benefits of the simpler API, and remove uneccessary const

This revision was automatically updated to reflect the committed changes.