Page MenuHomePhabricator

[avalanche] discourage ava messages if avalanche is disabled
ClosedPublic

Authored by PiRK on Mar 7 2021, 10:32.

Details

Summary

If a node does not have avalanche enabled, it should not receive avalanche related net messages.

Nodes decide to send an AVAHELLO messages to a peer if they see that their NODE_AVALANCHE service flag is set. A node sending such a message when this flag is not set is misbehaving.
This should partially mitigate avalanche message spamming.

Note that for the moment the NODE_AVALANCHE is never set, but I expect that it will be set if -enableavalanche is specified.

To make the functional test work, I had to make sure AvalancheDelegation levels are not None in messages.py, else the serialization fails.

Test Plan

ninja all check-all

Event Timeline

PiRK requested review of this revision.Mar 7 2021, 10:33
PiRK edited the summary of this revision. (Show Details)

minor optimization: don't do and additional check for each message

The tradeoff is that now the Misbehaving line is in 3 different places. But the msg_type checking and verification that avalanche is indeed enabled is no longer duplicated.

Tail of the build log:

[380/436] bitcoin: testing wallet_crypto_tests
[381/436] bitcoin: testing finalization_tests
[382/436] bitcoin: testing sync_tests
[383/436] bitcoin: testing torcontrol_tests
[384/436] Running utility command for check-bitcoin-wallet_crypto_tests
[385/436] Running utility command for check-bitcoin-finalization_tests
[386/436] Running utility command for check-bitcoin-sync_tests
[387/436] Running utility command for check-bitcoin-torcontrol_tests
[388/436] bitcoin: testing streams_tests
[389/436] bitcoin: testing scriptpubkeyman_tests
[390/436] Running utility command for check-bitcoin-streams_tests
[391/436] bitcoin: testing timedata_tests
[392/436] Running utility command for check-bitcoin-scriptpubkeyman_tests
[393/436] Running utility command for check-bitcoin-timedata_tests
[394/436] bitcoin: testing undo_tests
[395/436] Running utility command for check-bitcoin-undo_tests
[396/436] bitcoin: testing uint256_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 radix_tests
[401/436] bitcoin: testing schnorr_tests
[402/436] bitcoin: testing blockcheck_tests
[403/436] bitcoin: testing versionbits_tests
[404/436] bitcoin: testing blockstatus_tests
[405/436] Running utility command for check-bitcoin-radix_tests
[406/436] Running utility command for check-bitcoin-schnorr_tests
[407/436] Running utility command for check-bitcoin-blockcheck_tests
[408/436] Running utility command for check-bitcoin-versionbits_tests
[409/436] Running utility command for check-bitcoin-blockstatus_tests
[410/436] bitcoin: testing wallet_tests
[411/436] bitcoin: testing cashaddr_tests
[412/436] Running utility command for check-bitcoin-wallet_tests
[413/436] Running utility command for check-bitcoin-cashaddr_tests
[414/436] bitcoin: testing getarg_tests
[415/436] bitcoin: testing cuckoocache_tests
[416/436] Running utility command for check-bitcoin-getarg_tests
[417/436] bitcoin: testing validation_block_tests
[418/436] Running utility command for check-bitcoin-cuckoocache_tests
[419/436] Running utility command for check-bitcoin-validation_block_tests
[420/436] bitcoin: testing script_tests
[421/436] Running utility command for check-bitcoin-script_tests
[422/436] bitcoin: testing validation_tests
[423/436] Running utility command for check-bitcoin-validation_tests
[424/436] bitcoin: testing crypto_tests
[425/436] Running utility command for check-bitcoin-crypto_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
Fabien added inline comments.
src/net_processing.cpp
3950

Since you're duplicating the code block, why not specialize it by message ? e.g. unsolicited-avahello

specialize Misbehaving messages: unsolicited-avahello, unsolicited-avapoll and unsolicited-avaresponse

This revision is now accepted and ready to land.Mar 11 2021, 09:09