Page MenuHomePhabricator

[avalanche] Don't rely on the service bit for sending the avalanche message
ClosedPublic

Authored by Fabien on Jul 4 2022, 14:32.

Details

Summary

This leverage the avahello message to determine if a peer has avalanche enabled rather than relying on the service bit or the ability to ignore the unsupported messages. This will let poll only nodes (that don't set the service bit) participate in the address and compact proof messaging.

Sending the getavaaddr and getavaproofs are now sent upon reception of the avahello message instead of the version message.

The tests are updated in a way that makes them closer to the actual real use case: the nodes are non longer connected via RPC but use the regular peer discovery mechanism instead.

Depends on D11723.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_use_sendava_replace_service_bit
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 19542
Build 38803: Build Difflint-circular-dependencies · build-diff · build-clang · build-debug · build-without-wallet · build-clang-tidy
Build 38802: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Jul 4 2022, 14:32
deadalnix requested changes to this revision.Jul 5 2022, 12:58
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/net_processing.cpp
4911–4941

The fact the message name made no sense was already a hint, but the fact all of this needs to be moved here is another. When you need to do a handshake before you do the handshake, you know you have one too many handshake.

This revision now requires changes to proceed.Jul 5 2022, 12:58

Use the avahello message instead of another handshake

sdulfari requested changes to this revision.Jul 12 2022, 16:31
sdulfari added a subscriber: sdulfari.
sdulfari added inline comments.
test/functional/abc_p2p_getavaaddr.py
313 ↗(On Diff #34296)

Is this necessary?

test/functional/test_framework/avatools.py
287 ↗(On Diff #34296)

It seems like this should be the default interface to use in tests, so naming it AvaP2PInterface is more appropriate. There are not very many direct usages of the base interface, so maybe name the base something like NoHandshakeAvaP2PInterface to make it clear.

This revision now requires changes to proceed.Jul 12 2022, 16:31
test/functional/abc_p2p_getavaaddr.py
313 ↗(On Diff #34296)

Yes, because you're now connecting more peers (all the below avapeers)

Rebase, make the handshake interface the default

This revision is now accepted and ready to land.Jul 14 2022, 22:57