Page MenuHomePhabricator

Don't penalize peers for unsollicited avalanche messages, just ignore them
AbandonedPublic

Authored by PiRK on May 24 2021, 08:05.

Details

Reviewers
Fabien
deadalnix
Group Reviewers
Restricted Project
Summary

An unsollicited avalanche message is not special in any way, compared to any other form of spam or unknown messages. So just ignore them in the same way.

Test Plan

ninja all check-all

Diff Detail

Event Timeline

PiRK requested review of this revision.May 24 2021, 08:05
PiRK edited the summary of this revision. (Show Details)
Fabien requested changes to this revision.May 25 2021, 20:19
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/net_processing.cpp
2627 ↗(On Diff #28578)

If you just ignore undesired message then there is no reason to ban at all ?
Plus since the -enableavalanche is no longer the same as having the service bit there is no way for the peer to know if it is set or not.

This revision now requires changes to proceed.May 25 2021, 20:19
PiRK retitled this revision from Don't discourage unsollicited AVAHELLO message to Don't penalize peers for unsollicited avalanche messages, just ignore them.
PiRK edited the summary of this revision. (Show Details)

just ignore unsollicited avalanche messages the same way we ignore unknown messages

deadalnix requested changes to this revision.May 26 2021, 15:57
deadalnix added a subscriber: deadalnix.

We probably shouldn't ignore unsolicited messages en general.

But in this specific case, no it is not the same. The proof is in the pudding, the message is "Unknown command \"%s\" from peer=%d\n", which is clearly not true for avalanche related messages.

src/net_processing.cpp
2628

All this achieve is skipping the code handling the fact that the message was ignored. This doesn't seem to be very useful.

2631

This code didn't really made a lot of sense to begin with. The ban is applied inconsistently. Is the message suddenly expected if g_avalanche is unset?

In addition, why isn't this grouped with the rest of the avalanche processing in there? Why isn't IsAvalancheMessageType static?

This revision now requires changes to proceed.May 26 2021, 15:57
PiRK planned changes to this revision.May 31 2021, 09:59