Closes #21070
v21.0 introduced a behaviour changed noted in #21070 where using a config value rpcallowip=::0 no longer also permitted ipv4 ip addresses.
The rpc_bind.py functional test covers this new behaviour already by checking that the list of bind addresses exactly matches what is expected so this commit only updates the documentation.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
<!--174a7506f384e20aa4161008e828411d-->
No conflicts as of last run.
595 | @@ -596,7 +596,7 @@ void SetupServerArgs(ArgsManager& argsman) 596 | argsman.AddArg("-blockversion=<n>", "Override block version to test forking scenarios", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION); 597 | 598 | argsman.AddArg("-rest", strprintf("Accept public REST requests (default: %u)", DEFAULT_REST_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); 599 | - argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); 600 | + argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0) or all ipv6 (::/0). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
nit
argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
Thanks, I did write it like this intentially as I felt that the final 3 are all variants of network/CIDR, but writing them as a sub-list didn't read well to me...
Thinking more about it I think they should be included as a sublist more clearly, so perhaps we go with:
argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24, 0.0.0.0/0 (all ipv4) or ::/0 (all ipv6)). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
What do you think?
Sure ;-)
nit:
I would use "or" after "a network/netmask (e.g. 1.2.3.4/255.255.255.0)" now:
- argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24, 0.0.0.0/0 (all ipv4) or ::/0 (all ipv6)). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
+ argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24, 0.0.0.0/0 (all ipv4) or ::/0 (all ipv6)). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
Took your suggestion here @pinheadmz
utACK
Concept ACK
<!--8ac04cdde196e94527acabf64b896448-->
There hasn't been much activity lately. What is the status here?
Finding reviewers may take time. However, if the patch is no longer relevant, please close this pull request. If the author lost interest or time to work on this, please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.
There hasn't been much activity lately. What is the status here?
Happy to close this, if we also close the corresponding issue #21070. But I think it's preferable to keep this open, and merge it to close the issue (and improve the docs).
609 | @@ -610,7 +610,7 @@ void SetupServerArgs(ArgsManager& argsman) 610 | argsman.AddArg("-blockversion=<n>", "Override block version to test forking scenarios", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION); 611 | 612 | argsman.AddArg("-rest", strprintf("Accept public REST requests (default: %u)", DEFAULT_REST_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); 613 | - argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); 614 | + argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0 ), or all ipv6 (::/0). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
nit, extra space ?
argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
(Concept ACK.) While retouching, mind s/Valid for/Valid values for/ or s/source. Valid for <ip> are/source, which can be
@jonatack took the first. @pinheadmz removed the space. Thanks.
Closes #21070
v21.0 introduced a behaviour changed noted in #21070 where using a config value
`rpcallowip=::0` no longer also permitted ipv4 ip addresses.
The rpc_bind.py functional test covers this new behaviour already by checking
that the list of bind addresses exactly matches what is expected so this
commit only updates the documentation.
ACK c8e066461b54d745b85411035fcc00a1a4044d76
reviewed code changes, built locally. tested the option with various (in)valid parameters to confirm the docs match the behavior
<details><summary>Show Signature</summary>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK c8e066461b54d745b85411035fcc00a1a4044d76
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmTUwzsACgkQ5+KYS2KJ
yTpDbhAAzRSmfx29YKWZDQ4gY4bCIKF4/PDAwX4DTo3hh7mOpKai7eKx3TPl2RAd
oy42t0dkImgez3gpKKbXzHtapsmJk2GcHCOm0ylC20YrZsX3moaFEErkWT7zBya3
r8+l6p566+g23QzO7UtCAZ+NlNqNpYXz8Si6DkfPhyLtvIgt8LOR7Eu+9YETGyou
lhnocFnWMc5Dg69mFpNVVlHHrTZvnTG3wDXHNRjzz6Elx5gx1/aUAGSJGQXgInSK
F3r8B9mP38jEUnh57c3+acpNZNqSOV+Np32o4jDcwGE0Tpf2pHXsCF5UCzGQEJB/
Bz5qZr6i8Hhr2NLsiDMGQ2UDRGDspKWQumuJWAL5TmD2Wz5IMzaWWL8aL9c4DoSM
nzjXdzF+1gTvXucZHXYm6MFELROODqcNKbnVbd+BW7fC3BZsteqy9+EcobMjzVbt
v3RpGF0msIxc1nb4/kjecrNBLug3vyw9NjE9th+0cRc+sWJByMqgwq3Mo5TMoRvL
mYdsEsDxIIQiRQ3OE73LiQpWZviftQriGDjI9ls9csqBEkYSr572TY+h9H0qFv67
zyKxzresThm1uew1jXDIzIdzA3Rg/RpZllu8GzwhY0Wg4VKgpi7qykkrY8HDywxN
DQ2lleXyWXy2FCsSUzjbbZmUYftbnSRx8e6mTKtt5zrEV7dvmmI=
=d+m3
-----END PGP SIGNATURE-----
pinheadmz's public key is on keybase
</details>
609 | @@ -610,7 +610,7 @@ void SetupServerArgs(ArgsManager& argsman) 610 | argsman.AddArg("-blockversion=<n>", "Override block version to test forking scenarios", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION); 611 | 612 | argsman.AddArg("-rest", strprintf("Accept public REST requests (default: %u)", DEFAULT_REST_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); 613 | - argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); 614 | + argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid values for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
only if you retouch
argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid values for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). This option can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
ACK c8e066461b54d745b85411035fcc00a1a4044d76
Non-blockers, consider changing the commit message to something like doc: update -rpcallowip config option help documentation (it's not an RPC help), and (i.e., for the future) using PR 21070 rather than #21070 so GitHub doesn't add mentions in that issue on every push.
ACK c8e066461b54d745b85411035fcc00a1a4044d76