Allow filterclear messages for enabling TX relay only.

rebroad commented at 2:31 am on September 13, 2016: contributor

Following my comment just now at https://github.com/bitcoin/bitcoin/pull/6579/files#r78485962

An example of where this might be useful is allowing a node to connect blocksonly during IBD but then becoming a full-node once caught up with the latest block. This might also even want to be the default behaviour since during IBD most TXs appear to be orphans, and are routinely dropped (for example when a node disconnects). Therefore, this can waste a lot of bandwidth.

Additionally, another pull could be written to stop relaying of TXs to nodes that are clearly far behind the latest block and are running a node that doesn’t store many orphan TXs, such as recent versions of Bitcoin Core.

An example of where this might be useful is allowing a node to connect blocksonly during IBD but then becoming a full-node once caught up with the latest block. This might also even want to be the default behaviour since during IBD most TXs appear to be orphans, and are routinely dropped (for example when a node disconnects). Therefore, this can waste a lot of bandwidth.

Additionally, another pull could be written to stop relaying of TXs to nodes that are clearly far behind the latest block and are running a node that doesn't store many orphan TXs, such as recent versions of Bitcoin Core.

1f951c67a4

jonasschnelli added the label P2P on Sep 13, 2016

gmaxwell commented at 9:30 am on September 14, 2016: contributor

Concept ACK, though might it be better to just introduce a new, trivial, P2P message for this purpose instead of overloading filterclear?

morcos commented at 11:39 am on September 14, 2016: member

Maybe I’m missing something, but how is that accomplished by just this PR? How are you starting blocksonly? It seems reasonable to me that if you’re not setting NODE_BLOOM that other nodes should respect that and not send you filter messages of any kind.

I think avoiding txs during catch up could be already accomplished using the feefilter message or as @gmaxwell points out by a new mechanism specifically for that purpose if that was an optimization we really needed. I believe it mostly only saves inv traffic though as we don’t request txs if we’re in IBD.

rebroad commented at 7:43 pm on September 14, 2016: contributor

@morcos This is a necessary first step prior to being able to code that - if the code mentioned was to be added to the network now without this pull merged and running on the network then the current behaviour would be to ban the nodes that start blocksonly and later request the TXs. Am I making sense?

Yes, you are correct in that we are talking about savnig the bandwidth from inv messages. Feefilter is a nice idea but it would only be understood by the very latest versions of Bitcoin Core whereas filterclear is understood by most versions in the field - we just need to be careful not to send them to nodes not offering BLOOM whose protocol versions are between 70011 up to the protocol version where filterclear stops being banned - which leads me to think, should protocol version be raised as part of the pull request? @gmaxwell filterclear has always done exactly what is required, i.e. sets RelayTXs to true. I’m not sure what other functionality you are thinking about that would need a new message.

TheBlueMatt commented at 9:07 pm on September 14, 2016: member

Its kind of a shame that we didnt catch this at the time of the BIP, but this isnt really sufficient to use as a feature, now that there are nodes which will disconnect/ban you for sending a filterclear. A new P2P message would certainly require a new BIP and some kind of version bump/signaling capability before we could use this. Alternatively, we could update the BIP and assume no one uses -nodebloom (or its a small enough set that we’d be OK to use filterclear in the future).

rebroad commented at 6:33 am on September 15, 2016: contributor

@TheBlueMatt This pull is safe but the pull that uses filterclear will need to be careful due to the ban-happy nodes you mention. As far as I know only bitcoin core nodes currently do this, and they are easy to spot as they have protocol version 70011 to 70014 and don’t advertise BLOOM services. These nodes are a small minority (since BLOOM is by default on), so it might be so few as to not need to worry about them. The sooner this pull is merged the less the impact will be of the ban-happiness. The only question remaining for me is do I need to modify this pull to increase the protocol version; if it’s not raised, people/nodes won’t be sure whether nodes running protocol 70014 without BLOOM will ban upon receipt of a filterclear message; but as these are such a small minority, I think raising the protocol version isn’t that necessary.

What is involved in updating the BIP?

TheBlueMatt commented at 12:47 pm on September 15, 2016: member

Sure, but ban-happy nodes won’t be identifiable unless we also bump the protocol version with this pull (which i don’t think we should do), so it may be useless without thinking about it further. My preference would be to just do this and update the BIP to note that you should not ban on filterclear, while pointing out that, due to a previous version of the BIP, some nodes may do this. We would then have to analyze the network to find out how many 0.13 nodes set the non-default option to disable NODE_BLOOM before we could seriously consider using filterclear for IBD blocks only mode, though since that is a minor optimization, it could easily wait six months or more.

On September 15, 2016 2:33:08 AM EDT, R E Broadley notifications@github.com wrote:

@TheBlueMatt This pull is safe but the pull that uses filterclear will need to be careful due to the ban-happy nodes you mention. As far as I know only bitcoin core nodes currently do this, and they are easy to spot as they have protocol version 70011 to 70014 and don’t advertise BLOOM services. These nodes are a small minority, so it might be so few as to not need to worry about them.

You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #8709 (comment)

morcos commented at 1:07 pm on September 15, 2016: member

I misunderstood how filterclear and fRelayTxes interact, and now I think this change makes sense.

utACK

pstratem commented at 4:48 am on September 20, 2016: contributor

@rebroad you can achieve the effect you desire by simply reconnecting after IBD

rebroad commented at 7:12 am on September 20, 2016: contributor

@pstratem Sorry, I don’t understand what you mean - what effect are you referring to? Without this patch, my node was banning my SPV wallet. I do agree that to address the overloading of whitelist that I need to change this to require an additional command line option. Perhaps -bloomwhite ?

gmaxwell commented at 6:44 pm on September 22, 2016: contributor

ACK.

rebroad commented at 5:15 am on October 17, 2016: contributor

Is making this optional via a command line option what is needed to get more ACKs?

laanwj merged this on Nov 7, 2016

laanwj closed this on Nov 7, 2016

laanwj referenced this in commit 1e50d22ed2 on Nov 7, 2016

codablock referenced this in commit 9be592806d on Jan 13, 2018

zkbot referenced this in commit f6d09aa822 on Apr 5, 2018

andvgal referenced this in commit a918f2c7d0 on Jan 6, 2019

CryptoCentric referenced this in commit 790752965f on Feb 15, 2019

fanquake referenced this in commit 28ce05d06f on Jun 16, 2020

DrahtBot locked this on Sep 8, 2021

Allow filterclear messages for enabling TX relay only. #8709