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

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

Details

Reviewers

deadalnix
sdulfari

Group Reviewers

Restricted Project

Commits

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

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 19622
Build 38963: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 38962: arc lint + arc unit

Event Timeline

Fabien created this revision.Jul 4 2022, 14:32

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 4 2022, 14:32

Fabien requested review of this revision.Jul 4 2022, 14:32

Harbormaster completed remote builds in B19542: Diff 34231.Jul 4 2022, 14:48

Fabien added a child revision: D11707: [avalanche] Request addresses and proofs from our inbounds while the quorum is not established.Jul 4 2022, 21:03

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

Fabien edited the summary of this revision. (Show Details)Jul 7 2022, 13:05

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

Fabien added a parent revision: D11723: [avalanche] Send a avahello message when no proof is supplied.

Fabien removed a parent revision: D11700: [avalanche] Send a sendava message to notify we support avalanche.

Use the avahello message instead of another handshake

Harbormaster completed remote builds in B19591: Diff 34296.Jul 7 2022, 13:42

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.