p2p: mildly discourage peers that violate feefilter #20895

1. 
   glozow commented at 11:57 pm on January 9, 2021: member

   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 GETDATA we sent before the FEEFILTER.

   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).

2. [refactor] add MempoolAcceptResult struct

   This is much cleaner and we can have ATMP
   return a const. Nobody else should be editing
   validation results.

   d5581bb809
3. [p2p] mildly discourage peers who violate fee filter 18a0552880
4. [test] fee filter violations d0988dd322
5. 
   sipa commented at 0:09 am on January 10, 2021: member

   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.

6. 
   glozow commented at 0:24 am on January 10, 2021: member

   Ahh thank you @sipa very instructive, this makes sense. I misunderstood what feefilter was supposed to do, sorry about that! 🤗
7. glozow closed this on Jan 10, 2021

   ---

8. glozow deleted the branch on Jan 10, 2021
9. DrahtBot locked this on Aug 18, 2022

Contributors
glozow sipa