Skip to content

Conversation

NicolasDorier
Copy link
Contributor

As mentioned here, I believe that there should be no reason to not support bloom filter for whitelisted peers.

There is no protocol more efficient than this, working as well in pruned mode to sync a light client from your own full node without the need of some middleware like Electrum.

Even BIP157 comes short for this usecase as it needlessly waste bandwidth. (and I think BIP157, as implemented now does not work in pruned mode -might be wrong here-)

@fanquake fanquake added the P2P label Jun 9, 2019
@jnewbery
Copy link
Contributor

jnewbery commented Jun 9, 2019

This was attempted previously in #8587

I think this comment: #8587 (comment) still stands. We should try to reduce the overloading of -whitelist rather than increase it.

@NicolasDorier
Copy link
Contributor Author

So what about --peerbloomfilters=whitelisted-only ?

@laanwj
Copy link
Member

laanwj commented Jun 9, 2019

I think this comment: #8587 (comment) still stands. We should try to reduce the overloading of -whitelist rather than increase it.

I tend to agree.

Do you have a specific use-case for this @NicolasDorier ?
If so, I guess it makes sense to be able to whitelist nodes for this functionality. There has been discussion in the past of specific whitelist flags instead of a blanket 'whitelist' with ever changing meaning.

@NicolasDorier
Copy link
Contributor Author

NicolasDorier commented Jun 9, 2019

@laanwj yes I have. BTCPay Server is exposing a the full node via a whitebound connection behind a Torv3 hidden service, so the whitebind interface can't be discovered.

I want to make a simple light wallet connecting to this hosted full node (or just slightly modifying Breadwallet to support Tor).

BIP157/full blocks are bad because it consumes lot's of bandwidth. However, I don't want to expose NODE_BLOOM services to node which are not whitelisted because of DDoS concerns.

BIP37 is a fine protocol, just terrible privacy wise and as a DDoS vector. But for connecting to your light wallet to your trusted node, this is the best thing in town.

@maflcko
Copy link
Member

maflcko commented Jun 10, 2019

What is wrong with a new option -whitelistbloomfilter=168.0.0.3? Everything else seems to have drawbacks.

@maflcko
Copy link
Member

maflcko commented Jun 10, 2019

See also:

@NicolasDorier
Copy link
Contributor Author

@MarcoFalke I am using whitebind...

@NicolasDorier
Copy link
Contributor Author

We could duplicate both whitebind/whitelist but I think this is not really useful.

@maflcko
Copy link
Member

maflcko commented Jun 10, 2019

I do think there is a usecase to offer a peer bloom filter, but not re-relay their consensus invalid transactions.

@laanwj
Copy link
Member

laanwj commented Jun 10, 2019

We could duplicate both whitebind/whitelist but I think this is not really useful.

I think adding a [flags] per whitelist range (both per bind and per IP range) would be more general, and less cluttering than duplicating the option. It's also much more flexible than changing the meaning of whitelist throughout the entire daemon.

@maflcko
Copy link
Member

maflcko commented Jun 10, 2019

So yeah. Concept ACK. Approach NACK.

@NicolasDorier
Copy link
Contributor Author

NicolasDorier commented Jun 11, 2019

I do think there is a usecase to offer a peer bloom filter, but not re-relay their consensus invalid transactions.

I agree.

So basically you would Concept ACK for a --whitebindbloomfilter, --whitelistbloomfilter?
I think it makes sense to have it, but at the same time if my peer is in --whitebind --whitelist, I would expect it to be able to use BloomFilter, but not to relay invalid transactions.

Said in another way, is there a scenario where you would like to whitelist a peer, but not let him get bloom filters?

@maflcko
Copy link
Member

maflcko commented Jun 11, 2019

So basically you would Concept ACK for a --whitebindbloomfilter, --whitelistbloomfilter?

I think @laanwj suggested to use -white{bind,list}=bloomfilter${SEPARATOR}${ADDR_PORT}, so that the options are not duplicated in the command line help.

@laanwj
Copy link
Member

laanwj commented Jun 11, 2019

I think @laanwj suggested to use -white{bind,list}=bloomfilter${SEPARATOR}${ADDR_PORT}, so that the options are not duplicated in the command line help.

Yes, ideally I'd like a extensible, flexible mechanism for whitelisting (a set of) peers for special things, instead of a single amorphous "whitelist".
Adding, say, "bloomfilter" into the option name causes a blowup in number of options in the help if more things get added. So I prefer a syntax that moves the kind of whitelisting to the value instead, like that.
Currently this is based on IP range (-whitelist) or incoming port (-whitebind). Say, in the future when authentication is added these flags could be set based on authenticated identity (maybe the wallet on your phone etc…). Each of these should be able to have a set of flags attached instead of the default.

@NicolasDorier
Copy link
Contributor Author

NicolasDorier commented Jun 12, 2019

@laanwj I think this is a good idea.

But still, "the amorphous whitelist" is very useful, please do not remove it, its consequence are very clear: Consider what is whitelisted as friendly (ie, does not want to deanonymise us, or DDoS us)

The fine grained options should be reserved for more specific use case.

If we keep amorphous whitelist/whitebind, then it makes sense to allow Bloom filters for them: What good reason there would be to no allow bloom filter to a friend?

For transaction relay, there is clear good reason: This friend likely does not validate himself the transaction, so we need to validate for him. But for bloom filter?

@ajtowns
Copy link
Contributor

ajtowns commented Jun 12, 2019

Concept ACK. Could consider "whitelisting" to take a pair of information: the node(s) to whitelist and what to whitelist them for, with the latter having some default value. Writing -whitebind=bloom,forcerelay@127.0.0.1:9999 or similar to say "whitelist anyone who connects to blah for bloom filters and forced relaying" seems plausible to me.

@NicolasDorier
Copy link
Contributor Author

@ajtowns I like your format whitebind=bloom,forcerelay@127.0.0.1:9999. If people seems OK with it, I will work on that.

Syntax being: white{bind,list}=({bloom,forcerelay},@)*IP(:Port)?. This would have the nice benefit of not breaking existing stuff.

@laanwj
Copy link
Member

laanwj commented Jun 18, 2019

@ajtowns I like your format whitebind=bloom,forcerelay@127.0.0.1:9999. If people seems OK with it, I will work on that.

Sounds good to me! Nice to fit it into something compatible with the current format.

@NicolasDorier
Copy link
Contributor Author

Closing in favor of #16248

@NicolasDorier
Copy link
Contributor Author

The PR there is finally ready to be reviewed for those interested.

laanwj added a commit that referenced this pull request Aug 14, 2019
c5b404e Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa3 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26de Make whitebind/whitelist permissions more flexible (nicolas.dorier)

Pull request description:

  # Motivation

  In 0.19, bloom filter will be disabled by default. I tried to make [a PR](#16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.

  Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.

  It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.

  When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](#16176 (comment)) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.

  Doing so will also make follow up idea very easy to implement in a backward compatible way.

  # Implementation details

  The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.

  The following permissions exists:
  * ForceRelay
  * Relay
  * NoBan
  * BloomFilter
  * Mempool

  Example:
  * `-whitelist=bloomfilter@127.0.0.1/32`.
  * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.

  If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)

  When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist`  and add to it the permissions granted from `whitebind`.

  To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.

  `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.

  # Follow up idea

  Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:

  * Changing `connect` at rpc and config file level to understand the permissions flags.
  * Changing the permissions of a peer at RPC level.

ACKs for top commit:
  laanwj:
    re-ACK c5b404e

Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
ShengguangXiao pushed a commit to DeFiCh/ain that referenced this pull request Aug 28, 2020
14bdcd3 Add functional tests for flexible whitebind/list (nicolas.dorier)
33d7c65 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
c98ddf1 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
1467e02 Make whitebind/whitelist permissions more flexible (nicolas.dorier)

Pull request description:

  # Motivation

  In 0.19, bloom filter will be disabled by default. I tried to make [a PR](bitcoin/bitcoin#16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.

  Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.

  It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.

  When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](bitcoin/bitcoin#16176 (comment)) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.

  Doing so will also make follow up idea very easy to implement in a backward compatible way.

  # Implementation details

  The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.

  The following permissions exists:
  * ForceRelay
  * Relay
  * NoBan
  * BloomFilter
  * Mempool

  Example:
  * `-whitelist=bloomfilter@127.0.0.1/32`.
  * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.

  If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)

  When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist`  and add to it the permissions granted from `whitebind`.

  To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.

  `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.

  # Follow up idea

  Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:

  * Changing `connect` at rpc and config file level to understand the permissions flags.
  * Changing the permissions of a peer at RPC level.

ACKs for top commit:
  laanwj:
    re-ACK 14bdcd3

Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 18, 2021
c5b404e Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa3 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26de Make whitebind/whitelist permissions more flexible (nicolas.dorier)

Pull request description:

  # Motivation

  In 0.19, bloom filter will be disabled by default. I tried to make [a PR](bitcoin#16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.

  Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.

  It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.

  When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](bitcoin#16176 (comment)) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.

  Doing so will also make follow up idea very easy to implement in a backward compatible way.

  # Implementation details

  The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.

  The following permissions exists:
  * ForceRelay
  * Relay
  * NoBan
  * BloomFilter
  * Mempool

  Example:
  * `-whitelist=bloomfilter@127.0.0.1/32`.
  * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.

  If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)

  When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist`  and add to it the permissions granted from `whitebind`.

  To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.

  `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.

  # Follow up idea

  Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:

  * Changing `connect` at rpc and config file level to understand the permissions flags.
  * Changing the permissions of a peer at RPC level.

ACKs for top commit:
  laanwj:
    re-ACK c5b404e

Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 18, 2021
c5b404e Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa3 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26de Make whitebind/whitelist permissions more flexible (nicolas.dorier)

Pull request description:

  # Motivation

  In 0.19, bloom filter will be disabled by default. I tried to make [a PR](bitcoin#16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.

  Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.

  It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.

  When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](bitcoin#16176 (comment)) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.

  Doing so will also make follow up idea very easy to implement in a backward compatible way.

  # Implementation details

  The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.

  The following permissions exists:
  * ForceRelay
  * Relay
  * NoBan
  * BloomFilter
  * Mempool

  Example:
  * `-whitelist=bloomfilter@127.0.0.1/32`.
  * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.

  If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)

  When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist`  and add to it the permissions granted from `whitebind`.

  To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.

  `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.

  # Follow up idea

  Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:

  * Changing `connect` at rpc and config file level to understand the permissions flags.
  * Changing the permissions of a peer at RPC level.

ACKs for top commit:
  laanwj:
    re-ACK c5b404e

Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 19, 2021
c5b404e Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa3 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26de Make whitebind/whitelist permissions more flexible (nicolas.dorier)

Pull request description:

  # Motivation

  In 0.19, bloom filter will be disabled by default. I tried to make [a PR](bitcoin#16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.

  Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.

  It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.

  When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](bitcoin#16176 (comment)) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.

  Doing so will also make follow up idea very easy to implement in a backward compatible way.

  # Implementation details

  The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.

  The following permissions exists:
  * ForceRelay
  * Relay
  * NoBan
  * BloomFilter
  * Mempool

  Example:
  * `-whitelist=bloomfilter@127.0.0.1/32`.
  * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.

  If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)

  When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist`  and add to it the permissions granted from `whitebind`.

  To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.

  `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.

  # Follow up idea

  Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:

  * Changing `connect` at rpc and config file level to understand the permissions flags.
  * Changing the permissions of a peer at RPC level.

ACKs for top commit:
  laanwj:
    re-ACK c5b404e

Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 19, 2021
c5b404e Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa3 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26de Make whitebind/whitelist permissions more flexible (nicolas.dorier)

Pull request description:

  # Motivation

  In 0.19, bloom filter will be disabled by default. I tried to make [a PR](bitcoin#16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.

  Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.

  It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.

  When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](bitcoin#16176 (comment)) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.

  Doing so will also make follow up idea very easy to implement in a backward compatible way.

  # Implementation details

  The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.

  The following permissions exists:
  * ForceRelay
  * Relay
  * NoBan
  * BloomFilter
  * Mempool

  Example:
  * `-whitelist=bloomfilter@127.0.0.1/32`.
  * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.

  If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)

  When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist`  and add to it the permissions granted from `whitebind`.

  To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.

  `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.

  # Follow up idea

  Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:

  * Changing `connect` at rpc and config file level to understand the permissions flags.
  * Changing the permissions of a peer at RPC level.

ACKs for top commit:
  laanwj:
    re-ACK c5b404e

Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 20, 2021
c5b404e Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa3 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26de Make whitebind/whitelist permissions more flexible (nicolas.dorier)

Pull request description:

  # Motivation

  In 0.19, bloom filter will be disabled by default. I tried to make [a PR](bitcoin#16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.

  Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.

  It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.

  When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](bitcoin#16176 (comment)) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.

  Doing so will also make follow up idea very easy to implement in a backward compatible way.

  # Implementation details

  The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.

  The following permissions exists:
  * ForceRelay
  * Relay
  * NoBan
  * BloomFilter
  * Mempool

  Example:
  * `-whitelist=bloomfilter@127.0.0.1/32`.
  * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.

  If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)

  When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist`  and add to it the permissions granted from `whitebind`.

  To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.

  `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.

  # Follow up idea

  Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:

  * Changing `connect` at rpc and config file level to understand the permissions flags.
  * Changing the permissions of a peer at RPC level.

ACKs for top commit:
  laanwj:
    re-ACK c5b404e

Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 20, 2021
c5b404e Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa3 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26de Make whitebind/whitelist permissions more flexible (nicolas.dorier)

Pull request description:

  # Motivation

  In 0.19, bloom filter will be disabled by default. I tried to make [a PR](bitcoin#16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.

  Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.

  It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.

  When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](bitcoin#16176 (comment)) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.

  Doing so will also make follow up idea very easy to implement in a backward compatible way.

  # Implementation details

  The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.

  The following permissions exists:
  * ForceRelay
  * Relay
  * NoBan
  * BloomFilter
  * Mempool

  Example:
  * `-whitelist=bloomfilter@127.0.0.1/32`.
  * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.

  If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)

  When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist`  and add to it the permissions granted from `whitebind`.

  To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.

  `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.

  # Follow up idea

  Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:

  * Changing `connect` at rpc and config file level to understand the permissions flags.
  * Changing the permissions of a peer at RPC level.

ACKs for top commit:
  laanwj:
    re-ACK c5b404e

Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants