Spamming `Grandpa: Neighbor message` increases reputation

[EclesioMeloJunior]

trafficstars

Is there an existing issue?

• [X] I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

• [X] This is not a support question.

Description of bug

I was testing the interoperability between Gossamer and Substrate nodes when, by mistake, I introduced a code to spamming Neighbor Messages to substrate nodes, and when I looked into the Substrate logs I saw the Gossamer node reputation going up.

I extracted some parts of the log, check below:

TRACE tokio-runtime-worker afg: Peer 12D3KooWBCKnUaEKcKEU8BXGzv7pNMgJuoaqNxJVdcTXeT5LGWqZ updated view. Now at Round(2), SetId(0)
TRACE tokio-runtime-worker peerset: Report 12D3KooWBCKnUaEKcKEU8BXGzv7pNMgJuoaqNxJVdcTXeT5LGWqZ: +100 to 116. Reason: Grandpa: Neighbor message
...
TRACE tokio-runtime-worker afg: Peer 12D3KooWBCKnUaEKcKEU8BXGzv7pNMgJuoaqNxJVdcTXeT5LGWqZ updated view. Now at Round(2), SetId(0)
TRACE tokio-runtime-worker peerset: Report 12D3KooWBCKnUaEKcKEU8BXGzv7pNMgJuoaqNxJVdcTXeT5LGWqZ: +100 to 877. Reason: Grandpa: Neighbor message
...
...
...
TRACE tokio-runtime-worker afg: Peer 12D3KooWBCKnUaEKcKEU8BXGzv7pNMgJuoaqNxJVdcTXeT5LGWqZ updated view. Now at Round(2), SetId(0)
TRACE tokio-runtime-worker peerset: Report 12D3KooWBCKnUaEKcKEU8BXGzv7pNMgJuoaqNxJVdcTXeT5LGWqZ: +100 to 4467. Reason: Grandpa: Neighbor message

I believe the intended behavior is to decrease the reputation of a peer when it spams a consensus message, right?

Steps to reproduce

No response

[dmitry-markin]

@EclesioMeloJunior do I understand correctly that you are not getting Duplicate gossip reports? I.e, TRACE tokio-runtime-worker peerset: Report 12D3...: -4 to ###. Reason: Duplicate gossip. Do you have any idea in what aspect your neighbor packets are different?

[dmitry-markin]

Looking at the log messages produced it looks like the packets can only differ in commit_finalized_height

[dmitry-markin]

It looks like sending NeighborPackets with increasing commit_finalized_heights is perfectly legitimate behavior when a peer processes GRANDPA commit messages in mod.rs:567. Therefore it's not trivial to distinguish spamming neighbor messages from legitimate ones. It should be possible to rate-limit neighbor messages (and other types of messages) in GRANDPA and punish non-respecting peers, but, unfortunately, I don't understand the GRANDPA internals on a level to correctly implement this and not break the protocol operation.

@bkchr what do you think?

[EclesioMeloJunior]

@dmitry-markin

do I understand correctly that you are not getting Duplicate gossip reports?

No, I only see the peer reputation increasing:

TRACE tokio-runtime-worker peerset: Report 12D3KooWF1ceU9imShW4bYK7K7HCmWzsf5ztayvJ1BMFoQ635uh3: +100 to 1756. Reason: Grandpa: Neighbor message  

Looking at the log messages produced it looks like the packets can only differ

I executed another tests and I make sure that the neighbor message keep the same, so all the neighbor messages sent from my node to substrate nodes are 02010100000000000000000000000000000000000000, as you can see in the following log

DEBUG    gossiping from host 12D3KooWF1ceU9imShW4bYK7K7HCmWzsf5ztayvJ1BMFoQ635uh3 message of type 5: ConsensusMessage Data=02010100000000000000000000000000000000000000	service.go:L519	pkg=network
TRACE    broadcasting message from notifications sub-protocol /2d42e1c0b9e689cdf2b51a09621be564171bdde521fdb169fe538f69c0adb97a/grandpa/1	service.go:L531	pkg=network
WARN     sent neighbor message to peer 12D3KooWDFpuutUQ7RrM3gkWM6uteh4bLtf1Tq4H2ejBeXDxSxHn: &{1 0 0}	grandpa.go:L559	pkg=grandpa

I provided a full log file here -> https://gist.github.com/EclesioMeloJunior/12e90b8bfbba4dc0136f933a7d898927, in the gist there are two logs, one from the Substrate node and one from Gossamer node, the Gossamer node peer id is 12D3KooWF1ceU9imShW4bYK7K7HCmWzsf5ztayvJ1BMFoQ635uh3 you can see that the Gossamer node sends the same message every time and get the reputation increasing

[dmitry-markin]

I executed another tests and I make sure that the neighbor message keep the same

Thanks, this makes the matter more interesting. Going to investigate further.

[bkchr]

@EclesioMeloJunior could you may provide a way to reproduce this?

[EclesioMeloJunior]

@bkchr I was able to reproduce this bug because by mistake I introduced a code in the gossamer client to spam the same neighbor message (within an interval of some seconds) to a substrate peer (https://github.com/ChainSafe/substrate-node-template/tree/afbe177f3fd40728cd4a7ea1ab3a5c03873f7cd1), both running locally

[bkchr]

Yeah my question was more on how to run gossamer to recreate this spam ;) Or at least to show the code and then @dmitry-markin could maybe adapt it to rust.

[EclesioMeloJunior]

hey @bkchr and @dmitry-markin sure! I've created a gist with instructions of how to reproduce it -> https://gist.github.com/EclesioMeloJunior/e3e003464ab3fe53bd882f9817b07029, please let me know if you face problems trying to reproduce the steps :D

[dmitry-markin]

Thanks @EclesioMeloJunior for the exhaustive gist with instructions! I've reproduced the error and is seeing the reputation of gossamer node increasing every second. Looking into substrate part.