p2p: mildly discourage peers that violate feefilter #20895
Conversation
This is much cleaner and we can have ATMP return a const. Nobody else should be editing validation results.
|
In general: I think the concept of Misbehaving scores should go away. Either something is acceptable behavior, and we should permit it, or it's not and we should disconnect (or disconnect+discourage if it's actually a DoS risk). Concept NACK on this. There is no violation of a specification here, so assigning a misbevariour score is dangerous: it could result in honest nodes banning each other, leaving the network more vulnerable to partition attacks. There is no DoS risk here either. BIP133 is a way to save bandwidth for our peers, by helpfully telling them that we're going to ignore transactions with a too low fee anyway, so they don't need to bother sending it. But clearly if they had transactions that were higher fee, we'd be prepared to process them. The question you have to ask yourself is if this is an effective way for an attacker to abuse our resources, and the answer is: if they wanted to attack they could construct (invalid) high-fee transactions just as well, and we'd be processing those anyway. Longer term, I think the correct way to deal with resource usage attacks is keeping track of what resources we spend on behalf of every peer, and if we run, deprioritize processing messages from them. |
|
Ahh thank you @sipa very instructive, this makes sense. I misunderstood what feefilter was supposed to do, sorry about that! 🤗 |
I found it surprising that we don't care if a peer sends us a transaction with fee lower than the fee filter we send them. This is in the spec for BIP 133 so it's not a bug. But it seems like we send fee filters to limit resource usage and if we don't care when peers violate it, it's kind of ineffective. I can see why we wouldn't want to severely punish - particularly because it's possible that they're responding to a
GETDATAwe sent before theFEEFILTER.This PR adds
Misbehaving(1)when peers send us transactions that get rejected for fee-related mempool policy and the fee is lower than the feefilter we sent them. This means they get disconnected after 100 of these transactions. Hopefully this seems reasonable.(Note that the first commit is from a refactor from #20833... I needed it to check the fee after calling ATMP).