revert protocol change (feefilter now ignored) #21839

1. 
   rebroad commented at 7:37 am on May 3, 2021: contributor

   #19260 was merged, which now causes older nodes to not receive TXs when they ask for them. the feefilter message was established as a way for nodes to request TXs after IBD. It was not only used for bloom filters.

   There’s no benefit in having it disabled, and it breaks things by disabling it as it risks older nodes not receiving TXs from newer nodes.

   It was claimed in #19260 that feefilters are a better way to block TXs from being sent, but this seem to go on the assumption that all nodes are running the feefilter logic, which old nodes would not be.

   So perhaps this raises the question - should we be forcing people to update their node software, or should we be trying to ensure that older nodes are respected?

2. rebroad added the label Bug on May 3, 2021
3. 
   MarcoFalke commented at 7:43 am on May 3, 2021: member

   This change was intentional. You can read the first comment on the pull request for the rationale.
4. MarcoFalke closed this on May 3, 2021

   ---

5. 
   MarcoFalke commented at 7:46 am on May 3, 2021: member

   should we be trying to ensure that older nodes are respected?

   There is no obligation for any participant in the p2p network to support an optional message type just because it is mentioned in a BIP or because a previous version of the software supported that message type. Even more so when such behaviour isn’t even advertised as described in the BIP.

6. 
   jnewbery commented at 7:51 am on May 3, 2021: member

   Totally agree with what @MarcoFalke is saying here. Overloading the fRelay and filterclear for non-bloom peers was an abuse of BIP37. Being able to limit support for BIP37 to trusted nodes is a big improvement for security and resource usage on Bitcoin Core nodes, and support for BIP37 may be removed entirely at some point in the future. Clients shouldn’t have been designed to rely on undocumented behaviour.
7. 
   rebroad commented at 8:19 am on May 3, 2021: contributor

   @jnewbery why was it “overloading”? fRelay seems pretty descriptive of what it does - if some functionality (e..g bloom) introduces a variable - that doesn’t mean it ends up with exclusive mandate over what that variable is used for forever into the future, does it?
8. 
   rebroad commented at 8:20 am on May 3, 2021: contributor

   should we be trying to ensure that older nodes are respected?

   There is no obligation for any participant in the p2p network to support an optional message type just because it is mentioned in a BIP or because a previous version of the software supported that message type. Even more so when such behaviour isn’t even advertised as described in the BIP.

   I agree, but surely we should be factoring in the protocol version nevertheless?

9. 
   MarcoFalke commented at 8:24 am on May 3, 2021: member

   I agree, but surely we should be factoring in the protocol version nevertheless?

   No. There is still no obligation for any participant in the p2p network to support an optional message type just because they sent you a specific protocol version. If this was documented in some kind of BIP and both peers in the connection supported that BIP, then sure. Though, this case in this thread isn’t documented anywhere and wouldn’t make sense anyway.

10. fanquake removed the label Bug on May 17, 2021
11. DrahtBot locked this on Aug 18, 2022

Contributors
rebroad MarcoFalke jnewbery