Page MenuHomePhabricator
Differential D9306

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

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

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC7006b4646f01: [avalanche] add an upper limit for the number of UTXOs in a proof
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 created this revision.Mar 8 2021, 17:04
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 8 2021, 17:04
PiRK requested review of this revision.Mar 8 2021, 17:04
Harbormaster completed remote builds in B15330: Diff 27900.Mar 8 2021, 17:08
teamcity added a comment.Mar 8 2021, 17:08
Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

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 ↗(On Diff #27900)

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
PiRK added a comment.Mar 11 2021, 15:05

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.

PiRK updated this revision to Diff 27924.Mar 11 2021, 15:09

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)Mar 11 2021, 15:19
Harbormaster completed remote builds in B15345: Diff 27924.Mar 11 2021, 15:39
PiRK updated this revision to Diff 27928.Mar 11 2021, 15:54
PiRK edited the summary of this revision. (Show Details)

rebase after landing D9290

Harbormaster completed remote builds in B15348: Diff 27928.Mar 11 2021, 16:12
Fabien accepted this revision.Mar 15 2021, 08:23
This revision is now accepted and ready to land.Mar 15 2021, 08:23
Closed by commit rABC7006b4646f01: [avalanche] add an upper limit for the number of UTXOs in a proof (authored by PiRK). · Explain WhyMar 18 2021, 08:18
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC7006b4646f01: [avalanche] add an upper limit for the number of UTXOs in a proof.
PiRK mentioned this in D9327: [avalanche] implement an avalanche handshake.Mar 18 2021, 08:33

Revision Contents
Changeset List

PathSize
src/
avalanche/
5 lines
5 lines
rpc/
5 lines
test/
functional/
66 lines

Diff 27928

View Options

src/avalanche/processor.h

Loading...
View Options

src/net_processing.cpp

Loading...
View Options

src/rpc/avalanche.cpp

Loading...
View Options

test/functional/abc_p2p_avalanche.py

Loading...