doc: Update init and reduce-traffic docs for -blocksonly #18391

glowang commented at 6:00 AM on March 20, 2020: contributor

When -blocksonly is set to 1, it interacts with the -walletbroadcast parameter and sets it to 0.

This behavior is not captured by the current documentation, which claims that -blocksonly does not impact any wallet transactions at all.

Fixes #17294

fanquake added the label Docs on Mar 20, 2020

MarcoFalke commented at 1:19 PM on March 20, 2020: member

The walletbroadcast is only set "softly" to 0, i.e. when it was not set otherwise (in the config or on the command line)
RPC transactions are relayed as far as I can tell, see also the second section in the test ./test/functional/p2p_blocksonly.py
It would be nice to extend that test for wallet transactions and maybe even whitelisted peers. (Not necessarily in this pull, it can also be done later)

MarcoFalke added this to the milestone 0.20.0 on Mar 20, 2020

andrewtoth commented at 2:29 PM on March 20, 2020: contributor

Lines 6 and 29 of reduce-traffic.md still show the outdated number of 8 outgoing peers, but this has been changed to 10. Since you're modifying that file you could update those numbers as well.

fanquake added the label Waiting for author on Mar 23, 2020

glowang commented at 5:18 AM on March 25, 2020: contributor

The walletbroadcast is only set "softly" to 0, i.e. when it was not set otherwise (in the config or on the command line)

RPC transactions are relayed as far as I can tell, see also the second section in the test ./test/functional/p2p_blocksonly.py

It would be nice to extend that test for wallet transactions and maybe even whitelisted peers. (Not necessarily in this pull, it can also be done later)

Are you referring to “wallet transaction" as a loose term? I thought wallet transaction includes the ones initiated via rpc already? @MarcoFalke

I would love to add more tests as you mentioned in a follow up PR!

glowang force-pushed on Mar 25, 2020

glowang requested review from MarcoFalke on Mar 25, 2020

fanquake removed the label Waiting for author on Mar 25, 2020

in doc/reduce-traffic.md:52 in 9a7d39bd05 outdated

  47 | @@ -48,3 +48,7 @@ Be reminded of the effects of this setting.
  48 |    wallet is loaded or if you use the node to broadcast transactions.
  49 |  - It makes block propagation slower because compact block relay can only be
  50 |    used when transaction relay is enabled.
  51 | +- This setting sets the flag "-walletbroadcast" to be "0", if it is currently
  52 | +  set to "1", which disables the automatic broadcasting of transactions

MarcoFalke commented at 10:56 AM on March 25, 2020:

  unset, which disables the automatic broadcasting of transactions

The argument is set "softly", see also https://doxygen.bitcoincore.org/class_args_manager.html#ae77153300424d36e02e54f2c718a457f and git grep -W 'SoftSetBoolArg("-walletbr'

glowang commented at 3:16 PM on March 26, 2020:

Ah got it. Thanks!

glowang commented at 4:26 PM on March 26, 2020:

I still have a clarifying question about "softset". My understanding is that this param interaction is not guranteed to happen. It would only happen if the setting was not manually set already by the user.

Therefore, If the user sets -walletbroadcast to 1 first, and later sets -blocksonly to 0, this would effectively means that 1) -walletbroadcast will not be set to 0 and 2) all transaction broadcast happens normally?

MarcoFalke commented at 4:29 PM on March 26, 2020:

Therefore, If the user sets -walletbroadcast to 1 first, and later sets -blocksonly to 0, this would effectively means that 1) -walletbroadcast will not be set to 0 and 2) all transaction broadcast happens normally?

Yes.

in doc/reduce-traffic.md:53 in 9a7d39bd05 outdated

  47 | @@ -48,3 +48,7 @@ Be reminded of the effects of this setting.
  48 |    wallet is loaded or if you use the node to broadcast transactions.
  49 |  - It makes block propagation slower because compact block relay can only be
  50 |    used when transaction relay is enabled.
  51 | +- This setting sets the flag "-walletbroadcast" to be "0", if it is currently
  52 | +  set to "1", which disables the automatic broadcasting of transactions
  53 | +  from inbound peers. If a peer is whitelisted, and "-whitelistforcerelay" is

MarcoFalke commented at 10:57 AM on March 25, 2020:

  from the wallet. If a peer is whitelisted, and "-whitelistforcerelay" is

glowang commented at 4:02 PM on March 26, 2020:

sorry if this is a super naive question...my understanding is transactions from inbound peers (txs our peers ask us to relay) and wallet transactions (ie. txs from or to our wallet) are two different concepts. So actually both of them not relayed when -blocksonly mode is turned on?

MarcoFalke commented at 4:09 PM on March 26, 2020:

Yes, but walletbroadcast is a wallet specific setting and does not control incoming traffic, only outgoing (broadcast) I think it could help to split the walletbroadcast and whitelist settings in two bullet points.

in doc/reduce-traffic.md:51 in 9a7d39bd05 outdated

  47 | @@ -48,3 +48,7 @@ Be reminded of the effects of this setting.
  48 |    wallet is loaded or if you use the node to broadcast transactions.
  49 |  - It makes block propagation slower because compact block relay can only be
  50 |    used when transaction relay is enabled.
  51 | +- This setting sets the flag "-walletbroadcast" to be "0", if it is currently

MarcoFalke commented at 10:58 AM on March 25, 2020:

It could probably make sense to move up this bullet point to after the second bullet point, since they are related.

in doc/reduce-traffic.md:31 in 9a7d39bd05 outdated

  25 | @@ -26,7 +26,7 @@ calculating the target.
  26 |  
  27 |  ## 2. Disable "listening" (`-listen=0`)
  28 |  
  29 | -Disabling listening will result in fewer nodes connected (remember the maximum of 8
  30 | +Disabling listening will result in fewer nodes connected (remember the maximum of 10

andrewtoth commented at 12:56 PM on March 25, 2020:

Line 6 also has the outdated number.

laanwj commented at 4:33 PM on March 25, 2020:

Yup, might want to change the number of outgoing peers there (and mention the composition: 8 full outgoing connections and two blocks-only).

glowang commented at 4:34 PM on March 26, 2020:

Yup, might want to change the number of outgoing peers there (and mention the composition: 8 full outgoing connections and two blocks-only).

This would probably mean that the max number connections now is 127, since we have 10 outgoing and 117 incoming? I will update this part in the code base too then.

MarcoFalke commented at 4:36 PM on March 26, 2020:

No, DEFAULT_MAX_PEER_CONNECTIONS is still 125

in src/init.cpp:374 in 9a7d39bd05 outdated

 370 | @@ -371,7 +371,7 @@ void SetupServerArgs()
 371 |      gArgs.AddArg("-blocknotify=<cmd>", "Execute command when the best block changes (%s in cmd is replaced by block hash)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 372 |  #endif
 373 |      gArgs.AddArg("-blockreconstructionextratxn=<n>", strprintf("Extra transactions to keep in memory for compact block reconstructions (default: %u)", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 374 | -    gArgs.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Transactions from the wallet, RPC and relay whitelisted inbound peers are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 375 | +    gArgs.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Automatic broadcast and rebroadcast of any transactions from inbound peers is disabled, unless '-whitelistforcerelay' is '1', in which case whitelisted peers' transactions will be relayed. rpc transactions are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);

laanwj commented at 4:35 PM on March 25, 2020:

please capitalize 'RPC'

laanwj commented at 4:36 PM on March 25, 2020: member

Thanks for improving the documentation! ACK minus nits

glowang force-pushed on Mar 26, 2020

laanwj commented at 1:47 PM on March 27, 2020: member

ACK 869327ca6e3d5feccb477e4a04e9b894bc2cdddd

in doc/reduce-memory.md:27 in 869327ca6e outdated

  23 | @@ -24,8 +24,8 @@ The size of some in-memory caches can be reduced. As caches trade off memory usa
  24 |  
  25 |  ## Number of peers
  26 |  
  27 | -- `-maxconnections=<n>` - the maximum number of connections, this defaults to `125`. Each active connection takes up some memory. Only significant if incoming
  28 | -   connections are enabled, otherwise the number of connections will never be more than `8`.
  29 | +- `-maxconnections=<n>` - the maximum number of connections, this defaults to `127`. Each active connection takes up some memory. Only significant if incoming

MarcoFalke commented at 2:09 PM on March 27, 2020:

$ ./src/qt/bitcoin-qt -?|grep -C 2 'maxconn'
QSocketNotifier: Can only be used with threads started with QThread
       Automatically create Tor hidden service (default: 1)

  -maxconnections=<n>
       Maintain at most <n> connections to peers (default: 125)

glowang force-pushed on Mar 29, 2020

glowang requested review from MarcoFalke on Mar 29, 2020

glowang requested review from laanwj on Mar 29, 2020

in src/net.h:67 in b4a976d9f1 outdated

  63 | @@ -64,7 +64,7 @@ static const unsigned int MAX_SUBVERSION_LENGTH = 256;
  64 |  /** Maximum number of automatic outgoing nodes over which we'll relay everything (blocks, tx, addrs, etc) */
  65 |  static const int MAX_OUTBOUND_FULL_RELAY_CONNECTIONS = 8;
  66 |  /** Maximum number of addnode outgoing nodes */
  67 | -static const int MAX_ADDNODE_CONNECTIONS = 8;
  68 | +static const int MAX_ADDNODE_CONNECTIONS = 10;

MarcoFalke commented at 11:37 AM on March 29, 2020:

Code changes (behaviour changes) should probably be in a separate pull request from documentation changes.

MarcoFalke approved

MarcoFalke commented at 11:37 AM on March 29, 2020: member

ACK after the code change is removed

Update -blocksonly documentation

When -blocksonly is set to 1, it interacts with the -walletbroadcast
parameter and sets it to 0 if it has not been set already.This behavior
is not captured by the current documentation, which claims that -blocksonly
does not impact any wallet transactions.

Update the max number of outgoing peers from 8 to 10, due to the
addition of two -blocksonly peers.

621e86ee8d

glowang force-pushed on Mar 29, 2020

glowang requested review from MarcoFalke on Mar 29, 2020

MarcoFalke commented at 12:15 PM on March 29, 2020: member

ACK 621e86ee8d0102e2bf41f7656a368083b89b2f83

MarcoFalke merged this on Mar 29, 2020

MarcoFalke closed this on Mar 29, 2020

in doc/reduce-memory.md:27 in 621e86ee8d

  23 | @@ -24,8 +24,7 @@ The size of some in-memory caches can be reduced. As caches trade off memory usa
  24 |  
  25 |  ## Number of peers
  26 |  
  27 | -- `-maxconnections=<n>` - the maximum number of connections, this defaults to `125`. Each active connection takes up some memory. Only significant if incoming
  28 | -   connections are enabled, otherwise the number of connections will never be more than `8`.
  29 | +- `-maxconnections=<n>` - the maximum number of connections, this defaults to `125`. Each active connection takes up some memory. Only significant if incoming connections are enabled, otherwise the number of connections will never be more than `10`. Of the 10 outbound peers, there can be 8 full outgoing connections and 2 -blocksonly peers, in which case they are block/addr peers, but not tx peers.

MarcoFalke commented at 12:38 PM on March 29, 2020:

whoopsie, just realized that blocksonly is different from block-relay-only. Fixed in #18464

deadalnix referenced this in commit d5a049238e on Jan 14, 2021

zkbot referenced this in commit ba4eb241e7 on Mar 31, 2021

DrahtBot locked this on Feb 15, 2022