doc: Add documentation for the new whitelist permissions #16629

NicolasDorier · 2019-08-16T04:43:24Z

Documenting the new feature #16248 . Ping Sjors .

DrahtBot · 2019-08-16T05:32:54Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

Sjors · 2019-08-16T08:07:50Z

@NicolasDorier can you include my "inbound" clarification from #16555 so I can close that?

src/init.cpp

NicolasDorier · 2019-08-16T08:19:02Z

@Sjors let's merge yours first.

NicolasDorier · 2019-08-20T04:07:41Z


  -whitebind=<[(permissions,)*@]addr>
       Bind to given address and whitelist peers connecting to it. Use
       [host]:port notation for IPv6. If the permission marker '@' is
       specified, you can add a comma separated list of the following
       available permissions 'bloomfilter, noban, forcerelay, relay,
       mempool'. If the permission marker '@' is not specified, it will
       default to 'noban,mempool' with `whitelistrelay` and
       `whitelistforcerelay` optionally adding 'relay' and 'forcerelay'
       respectively. Can be specified multiple times.

  -whitelist=<[(permissions,)*@]IP address or network>
       Whitelist peers connecting from the given IP address (e.g. 1.2.3.4) or
       CIDR notated network (e.g. 1.2.3.0/24). If the permission marker
       '@' is specified, you can add a comma separated list of the
       following available permissions 'bloomfilter, noban, forcerelay,
       relay, mempool'. If the permission marker '@' is not specified,
       it will default to 'noban,mempool' with `whitelistrelay` and
       `whitelistforcerelay` optionally adding 'relay' and 'forcerelay'
       respectively. Can be specified multiple times.

  -whitelistforcerelay
       Force relay of transactions from whitelisted inbound peers even if the
       transactions were already in the mempool or violate local relay
       policy, this parameter is ignored if specific permissions are
       applied via whitebind/whitelist (default: 0)

  -whitelistrelay
       Accept relayed transactions received from whitelisted inbound peers even
       when not relaying transactions, this parameter is ignored if
       specific permissions are applied via whitebind/whitelist
       (default: 1)

NicolasDorier · 2019-08-20T04:09:58Z

@MarcoFalke instead of using constant, I built in during runtime, it makes sure that the documentation is updated if the list of permission is updated.

src/init.cpp

harding · 2019-08-21T00:55:31Z

@NicolasDorier I think this could be a bit more concise. If possible, it'd also be good if it could briefly describe what the various options meant (even I had to look at the code to figure out what mempool provide, although BIP35 support was one of my guesses). Here's a suggestion for white(bind|list), the other two looked ok to me:

  -whitebind=<[(permissions,)*@]addr>
       Bind to given address and whitelist peers connecting to it. Use
       [host]:port notation for IPv6.  Allowed permissions are
       bloomfilter (allow requesting BIP37 filtered blocks and
       transactions), noban (do not ban for misbehavior), forcerelay
       (relay even non-standard transactions), relay (relay even in
       -blocksonly mode), and mempool (allow requesting BIP35 mempool
       contents).  Specify multiple permissions separated by commas
       (default: noban,mempool).  Can be specified multiple times

  -whitelist=<[(permissions,)*@]IP address or network>
       Whitelist peers connecting from the given IP address (e.g. 1.2.3.4) or
       CIDR notated network (e.g. 1.2.3.0/24).  Uses same permissions as
       -whitebind.  Can be specified multiple times

faebf62 rpc: Use Join helper in rpc/util (MarcoFalke) fa8cd6f util: Add Join helper to join a list of strings (MarcoFalke) Pull request description: We have a lot of enumerations in the code and sometimes those enumerations need to be mentioned in the RPC or command line documentation. Previously, each caller would have a couple of lines inline to join the strings or the joined string is hardcoded in the documentation. A helper to join strings would make code such as #16629 (comment) less verbose and easier to read. Also, warnings commonly accumulate in complex RPCs, since a warning doesn't lead to an early return. A helper to join those warnings would make code such as https://github.com/bitcoin/bitcoin/pull/16394/files#r309324997 less verbose and easier to read. ACKs for top commit: practicalswift: ACK faebf62 Tree-SHA512: 80f2db86a05c63b686f510585c1c631250271a8958fd71fafaac91559ffd2ec25d609bf7d53412ba27f87eff5893ac9dd9c2f296fc0c73581556e1d6a734a36f

NicolasDorier · 2019-08-22T16:09:09Z

@harding I like it, @MarcoFalke it means that we won't need your new string join util. :(
Will rebase tomorrow.

maflcko · 2019-08-22T16:28:12Z

@MarcoFalke it means that we won't need your new string join util. :(

Good to hear. I like the new text as well. Will take a closer look in a couple of days.

NicolasDorier · 2019-08-23T04:34:09Z

I just updated to use @harding suggestion, added documentation for bloomfilter permission as well.
I needed to change the doc for -whitelistforcerelay and -whitelistrelay as well.

NicolasDorier · 2019-08-23T04:39:31Z

the linter says whitebind is undocumented?

harding · 2019-08-23T05:01:10Z

@NicolasDorier bloomfilter was in my description so now you have it twice. :-)

faebf62 rpc: Use Join helper in rpc/util (MarcoFalke) fa8cd6f util: Add Join helper to join a list of strings (MarcoFalke) Pull request description: We have a lot of enumerations in the code and sometimes those enumerations need to be mentioned in the RPC or command line documentation. Previously, each caller would have a couple of lines inline to join the strings or the joined string is hardcoded in the documentation. A helper to join strings would make code such as bitcoin#16629 (comment) less verbose and easier to read. Also, warnings commonly accumulate in complex RPCs, since a warning doesn't lead to an early return. A helper to join those warnings would make code such as https://github.com/bitcoin/bitcoin/pull/16394/files#r309324997 less verbose and easier to read. ACKs for top commit: practicalswift: ACK faebf62 Tree-SHA512: 80f2db86a05c63b686f510585c1c631250271a8958fd71fafaac91559ffd2ec25d609bf7d53412ba27f87eff5893ac9dd9c2f296fc0c73581556e1d6a734a36f

NicolasDorier · 2019-08-23T09:38:45Z

oops, fixed it.

NicolasDorier · 2019-08-23T09:51:36Z

Ok somehow this break the tests oO checking what happen.

Sjors · 2019-08-23T10:45:33Z

ACK d536a10

Nit: remove spaces in examples default: noban,mempool,relay and 1.2.3.0/24.

74e3876 Release notes: add previously undocumented changes (David A. Harding) 7e1634a Release notes: edit previously-detached notes (David A. Harding) e7415a5 Doc: move detached release notes into release-notes.md (David A. Harding) Pull request description: Merges in detached release notes, edits each change down to a single paragraph bullet point (or, in a couple cases, two individual bullet points in separate sections each with a single paragraph). Adds notes for some undocumented changes I found reviewing `git log --merges`. Also tries something new: adds the PR number(s) after each entry to make it easier for both reviewers and end-user readers to look up the details behind each change. (If the PR numbers are unwanted, they're easy to remove either in this PR or later in the release process.) I also checked the 0.18 branch but I didn't find anything in the current release notes that had been backported. A particular focus in my editing was trying to keep things concise, particularly by pointing to RPC documentation when available (or upcoming, as in #16629). I do suspect that one downside of detached notes is that people write longer summaries than they would if they knew there were already 300 other lines of release notes. :-) The first commit only moves notes, puts them in bullet form, adjusts indentation appropriately, and drops unneeded headers. It can be reviewed with `git diff --color-moved=dimmed-zebra` for a little bit of a speedup, but unfortunately I wasn't smart enough to split my copy/pasting and line wrapping into separate commits, so it's not a transparently move-only change. ACKs for top commit: fanquake: ACK 74e3876 - Thanks for doing this. It's nice to have a gauge of what's going to be in the `0.19.0` release. meshcollider: ACK 74e3876 Tree-SHA512: 676668765849d5a67520dd8ac49de85ac1bfb5ba2dc09504e75db77d79c7e2c58b5cee16c58591ec575cb3682e630231baba7fd07565d19f8d02243e06fcb9ab

NicolasDorier · 2019-08-24T04:01:56Z

Indeed, removed the spaces. Should be ready now.

NicolasDorier · 2019-08-24T14:17:39Z

I broke the tests again. This is the hardest PR doc I did in my life.

NicolasDorier · 2019-08-24T14:45:58Z

I rebased, now the error rpc_invalidateblock does not seem related to this PR.

maflcko · 2019-08-24T14:53:23Z

That is a known bug. See #16444. You don't need a passing travis on a doc-commit, it can be merged in any case after review.

74e3876 Release notes: add previously undocumented changes (David A. Harding) 7e1634a Release notes: edit previously-detached notes (David A. Harding) e7415a5 Doc: move detached release notes into release-notes.md (David A. Harding) Pull request description: Merges in detached release notes, edits each change down to a single paragraph bullet point (or, in a couple cases, two individual bullet points in separate sections each with a single paragraph). Adds notes for some undocumented changes I found reviewing `git log --merges`. Also tries something new: adds the PR number(s) after each entry to make it easier for both reviewers and end-user readers to look up the details behind each change. (If the PR numbers are unwanted, they're easy to remove either in this PR or later in the release process.) I also checked the 0.18 branch but I didn't find anything in the current release notes that had been backported. A particular focus in my editing was trying to keep things concise, particularly by pointing to RPC documentation when available (or upcoming, as in bitcoin#16629). I do suspect that one downside of detached notes is that people write longer summaries than they would if they knew there were already 300 other lines of release notes. :-) The first commit only moves notes, puts them in bullet form, adjusts indentation appropriately, and drops unneeded headers. It can be reviewed with `git diff --color-moved=dimmed-zebra` for a little bit of a speedup, but unfortunately I wasn't smart enough to split my copy/pasting and line wrapping into separate commits, so it's not a transparently move-only change. ACKs for top commit: fanquake: ACK 74e3876 - Thanks for doing this. It's nice to have a gauge of what's going to be in the `0.19.0` release. meshcollider: ACK 74e3876 Tree-SHA512: 676668765849d5a67520dd8ac49de85ac1bfb5ba2dc09504e75db77d79c7e2c58b5cee16c58591ec575cb3682e630231baba7fd07565d19f8d02243e06fcb9ab

NicolasDorier · 2019-08-26T07:26:36Z

I think this one is ready to be merged @MarcoFalke

Sjors · 2019-08-26T10:23:53Z

Indeed, re-ACK 66ad754

…missions 66ad754 [Doc] Add documentation for the new whitelist permissions (nicolas.dorier) Pull request description: Documenting the new feature bitcoin#16248 . Ping Sjors . ACKs for top commit: Sjors: Indeed, re-ACK 66ad754 Tree-SHA512: e6860bb6fae921287da7920a8db534e6a1a23871dd78dd6da030f00adf23e204cd23b194d67361bf34d4ef5a7815fc3fd7c81a3f2f35e4cfbe6ee2f2e6daec25

7e8bca8 rpc: Use Join helper in rpc/util (MarcoFalke) 96e4c78 util: Add Join helper to join a list of strings (MarcoFalke) Pull request description: We have a lot of enumerations in the code and sometimes those enumerations need to be mentioned in the RPC or command line documentation. Previously, each caller would have a couple of lines inline to join the strings or the joined string is hardcoded in the documentation. A helper to join strings would make code such as bitcoin/bitcoin#16629 (comment) less verbose and easier to read. Also, warnings commonly accumulate in complex RPCs, since a warning doesn't lead to an early return. A helper to join those warnings would make code such as https://github.com/bitcoin/bitcoin/pull/16394/files#r309324997 less verbose and easier to read. ACKs for top commit: practicalswift: ACK 7e8bca8 Tree-SHA512: 80f2db86a05c63b686f510585c1c631250271a8958fd71fafaac91559ffd2ec25d609bf7d53412ba27f87eff5893ac9dd9c2f296fc0c73581556e1d6a734a36f

Summary: Backport of Core [[bitcoin/bitcoin#16629 | PR16629]] Depends on D7976 Test Plan: `ninja && src/bitcoind -help` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D7977

…missions 66ad754 [Doc] Add documentation for the new whitelist permissions (nicolas.dorier) Pull request description: Documenting the new feature bitcoin#16248 . Ping Sjors . ACKs for top commit: Sjors: Indeed, re-ACK 66ad754 Tree-SHA512: e6860bb6fae921287da7920a8db534e6a1a23871dd78dd6da030f00adf23e204cd23b194d67361bf34d4ef5a7815fc3fd7c81a3f2f35e4cfbe6ee2f2e6daec25

fanquake added the Docs label Aug 16, 2019

fanquake changed the title ~~[Doc] Add documentation for the new whitelist permissions~~ doc: Add documentation for the new whitelist permissions Aug 16, 2019

fanquake added this to the 0.19.0 milestone Aug 16, 2019

Sjors reviewed Aug 16, 2019

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

DrahtBot added the Needs rebase label Aug 19, 2019

NicolasDorier force-pushed the doc/permissions branch from 42bc1eb to 38aeb4d Compare August 20, 2019 04:08

DrahtBot removed the Needs rebase label Aug 20, 2019

maflcko reviewed Aug 20, 2019

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

maflcko mentioned this pull request Aug 20, 2019

util: Add Join helper to join a list of strings #16670

Merged

harding mentioned this pull request Aug 23, 2019

Prepare release notes for 0.19 #16690

Merged

NicolasDorier force-pushed the doc/permissions branch from 38aeb4d to 9f02395 Compare August 23, 2019 04:32

NicolasDorier force-pushed the doc/permissions branch from 9f02395 to b408508 Compare August 23, 2019 04:40

NicolasDorier force-pushed the doc/permissions branch from b408508 to 13d8cc3 Compare August 23, 2019 09:12

NicolasDorier force-pushed the doc/permissions branch from 13d8cc3 to d536a10 Compare August 23, 2019 09:57

NicolasDorier force-pushed the doc/permissions branch 2 times, most recently from af8a634 to cdb5ce0 Compare August 24, 2019 04:01

[Doc] Add documentation for the new whitelist permissions

66ad754

NicolasDorier force-pushed the doc/permissions branch from cdb5ce0 to 66ad754 Compare August 24, 2019 14:23

maflcko merged commit 66ad754 into bitcoin:master Aug 26, 2019

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

doc: Add documentation for the new whitelist permissions #16629

doc: Add documentation for the new whitelist permissions #16629

Conversation

NicolasDorier commented Aug 16, 2019 • edited by fanquake Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Aug 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Sjors commented Aug 16, 2019

Uh oh!

Uh oh!

NicolasDorier commented Aug 16, 2019

Uh oh!

NicolasDorier commented Aug 20, 2019

Uh oh!

NicolasDorier commented Aug 20, 2019

Uh oh!

Uh oh!

harding commented Aug 21, 2019

Uh oh!

NicolasDorier commented Aug 22, 2019

Uh oh!

maflcko commented Aug 22, 2019

Uh oh!

NicolasDorier commented Aug 23, 2019

Uh oh!

NicolasDorier commented Aug 23, 2019

Uh oh!

harding commented Aug 23, 2019

Uh oh!

NicolasDorier commented Aug 23, 2019

Uh oh!

NicolasDorier commented Aug 23, 2019

Uh oh!

Sjors commented Aug 23, 2019

Uh oh!

NicolasDorier commented Aug 24, 2019

Uh oh!

NicolasDorier commented Aug 24, 2019

Uh oh!

NicolasDorier commented Aug 24, 2019

Uh oh!

maflcko commented Aug 24, 2019

Uh oh!

NicolasDorier commented Aug 26, 2019

Uh oh!

Sjors commented Aug 26, 2019

Uh oh!

Uh oh!

NicolasDorier commented Aug 16, 2019 •

edited by fanquake

Loading

DrahtBot commented Aug 16, 2019 •

edited

Loading