Page MenuHomePhabricator

[avalanche] add a fetchOrCreatePeer with ProofValidationStatus
AbandonedPublic

Authored by PiRK on Apr 15 2021, 11:32.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

PeerManager::fetchOrCreatePeer is useful to add a new peer from a
proof and check the proof at the same time. The current implementation
does not provide access to the validation state, so we just know if the
proof is good or bad. It creates a ProofValidationStatus that is not
used.

This revision add a version of fetchOrCreatePeer that takes a
validation state as an output argument, so that we can inspect the
reason of failure more in detail. This will be useful when we need to
decide whether or not to keep the proof in an orphan pool.

This is a small piece of D9370.

Test Plan

ninja all check-all

I did not add a unit test for this because it would require to make the method public, which seems to be an unecessary drawback for such a trivial change. When fetchOrCreatePeer is used in a public method (e.g. addProof), it will be indirectly tested.

Event Timeline

PiRK requested review of this revision.Apr 15 2021, 11:32

Tail of the build log:

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

remove the dummy state that is thrown away so that the output argument is actually used

PiRK abandoned this revision.EditedApr 15 2021, 12:12

I think I just had a better idea: do the proof verification inside fetchCreatePeer, so that we don't need another public method adding a proof (getProofId already does that).