Page MenuHomePhabricator

[avalanche] add an upper limit for the number of UTXOs in a proof
ClosedPublic

Authored by PiRK on Mar 8 2021, 17:04.

Details

Summary

We probably need such a limit because the proof will be exchanged over the network when avalanche nodes meet each other.

The test is done when adding an avalanche peer with the addavalanchenode RPC command and when an AVAHELLO message is received. Note that right now the AVAHELLO message does not contain the proof, but this will need to be implemented in a future diff.

One consideration should be that this number of UTXOs allows miner to use coinbases to reach the minimum stake amount. But this minimum amount is currently not decided.
Another consideration is the maximum message size of 2MB that can be transmitted over the network to other nodes. This permits a maximum of 11 000 UTXOs.

Test Plan

ninja all check-all

Diff Detail

Event Timeline

PiRK requested review of this revision.Mar 8 2021, 17:04

Tail of the build log:

[383/436] Running utility command for check-bitcoin-blockcheck_tests
[384/436] Running utility command for check-bitcoin-cuckoocache_tests
[385/436] bitcoin: testing radix_tests
[386/436] bitcoin: testing schnorr_tests
[387/436] Running utility command for check-bitcoin-schnorr_tests
[388/436] Running utility command for check-bitcoin-radix_tests
[389/436] bitcoin: testing uint256_tests
[390/436] bitcoin: testing undo_tests
[391/436] Running utility command for check-bitcoin-uint256_tests
[392/436] Running utility command for check-bitcoin-undo_tests
[393/436] bitcoin: testing wallet_tests
[394/436] bitcoin: testing script_standard_tests
[395/436] Running utility command for check-bitcoin-wallet_tests
[396/436] Running utility command for check-bitcoin-script_standard_tests
[397/436] bitcoin: testing script_tests
[398/436] bitcoin: testing blockstatus_tests
[399/436] bitcoin: testing crypto_tests
[400/436] Running utility command for check-bitcoin-script_tests
[401/436] Running utility command for check-bitcoin-blockstatus_tests
[402/436] Running utility command for check-bitcoin-crypto_tests
[403/436] Running utility command for check-pow-aserti32d_tests
[404/436] Running pow test suite
PASSED: pow test suite
[405/436] bitcoin: testing cashaddr_tests
[406/436] Running utility command for check-seeder-write_name_tests
[407/436] Running utility command for check-bitcoin-cashaddr_tests
[408/436] bitcoin: testing getarg_tests
[409/436] Running utility command for check-seeder-p2p_messaging_tests
[410/436] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_main.cpp.o
[411/436] Running utility command for check-bitcoin-getarg_tests
[412/436] bitcoin: testing versionbits_tests
[413/436] seeder: testing parse_name_tests
[414/436] Running utility command for check-bitcoin-versionbits_tests
[415/436] Running utility command for check-seeder-parse_name_tests
[416/436] Running seeder test suite
PASSED: seeder test suite
[417/436] bitcoin: testing validation_tests
[418/436] bitcoin: testing skiplist_tests
[419/436] Running utility command for check-bitcoin-validation_tests
[420/436] Running utility command for check-bitcoin-skiplist_tests
[421/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)
[422/436] bitcoin: testing validation_block_tests
[423/436] Running utility command for check-bitcoin-validation_block_tests
[424/436] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o
[425/436] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o
[426/436] bitcoin: testing op_reversebytes_tests
[427/436] Running utility command for check-bitcoin-op_reversebytes_tests
[428/436] Linking CXX executable src/qt/test/test_bitcoin-qt
[429/436] bitcoin: testing transaction_tests
[430/436] Running utility command for check-bitcoin-transaction_tests
[431/436] bitcoin-qt: testing test_bitcoin-qt
[432/436] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[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 requested changes to this revision.Mar 11 2021, 09:47
Fabien added a subscriber: Fabien.

There already is a limit for the p2p messages which is 2MB (see MAX_PROTOCOL_MESSAGE_LENGTH). Can't we determine the max number of stakes from this limit ?

test/functional/abc_p2p_avalanche.py
353

You might want to check that AVALANCHE_MAX_PROOF_STAKES works before checking the +1 case fails. By doing that you would notice that there is a discrepancy between the values from C++ and python.

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

My quick calculation gives me 178 bytes per signed stake. We can ignore the rest of the proof, as it is does not change much depending on the number of stakes.
That is a huge number of stakes in 2 MB: 11781.

I tried with 10000, and it made the test time out when generating the blocks.
With 1000 it works without major modification of the test.

fix the discrepency for max number of stakes between the node and the test, increase the max number of stakes to 1000.

PiRK edited the summary of this revision. (Show Details)

rebase after landing D9290

This revision is now accepted and ready to land.Mar 15 2021, 08:23