Merge bitcoin/bitcoin#28149: net processing: clamp PeerManager::Options user input

547fa52443 net processing: clamp -blockreconstructionextratxn to uint32_t bounds (stickies-v)
e451d1e3c6 net processing: clamp -maxorphantx to uint32_t bounds (stickies-v)
aa89e04e07 doc: document PeerManager::Options members (stickies-v)

Pull request description:

  Avoid out-of-bounds user input for `PeerManager::Options` by safely clamping `-maxorphantx` and `-blockreconstructionextratxn`, and avoid platform-specific behaviour by changing `PeerManager::Options::max_extra_txs` from `size_t` to a `uint32_t`. Addresses https://github.com/bitcoin/bitcoin/pull/27499#pullrequestreview-1544114932.

  Also documents all `PeerManager::Options` members, addressing https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1272302469.

ACKs for top commit:
  dergoegge:
    Code review ACK 547fa52443
  glozow:
    reACK 547fa52443

Tree-SHA512: 042d47b35bb8a7b29ef3dadd4c0c5d26f13a8f174f33687855d603c19f8de0fcbbda94418453331e149885412d4edd5f402d640d938f6d94b4dcf54e2fdbbcc9
This commit is contained in:
glozow
2023-08-09 13:39:37 +02:00
2 changed files with 16 additions and 7 deletions

View File

@@ -17,9 +17,10 @@ class ChainstateManager;
/** Whether transaction reconciliation protocol should be enabled by default. */
static constexpr bool DEFAULT_TXRECONCILIATION_ENABLE{false};
/** Default for -maxorphantx, maximum number of orphan transactions kept in memory */
static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100;
/** Default number of orphan+recently-replaced txn to keep around for block reconstruction */
static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100;
static const uint32_t DEFAULT_MAX_ORPHAN_TRANSACTIONS{100};
/** Default number of non-mempool transactions to keep around for block reconstruction. Includes
orphan, replaced, and rejected transactions. */
static const uint32_t DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN{100};
static const bool DEFAULT_PEERBLOOMFILTERS = false;
static const bool DEFAULT_PEERBLOCKFILTERS = false;
/** Threshold for marking a node to be discouraged, e.g. disconnected and added to the discouragement filter. */
@@ -46,11 +47,16 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
{
public:
struct Options {
/** Whether this node is running in -blocksonly mode */
//! Whether this node is running in -blocksonly mode
bool ignore_incoming_txs{DEFAULT_BLOCKSONLY};
//! Whether transaction reconciliation protocol is enabled
bool reconcile_txs{DEFAULT_TXRECONCILIATION_ENABLE};
//! Maximum number of orphan transactions kept in memory
uint32_t max_orphan_txs{DEFAULT_MAX_ORPHAN_TRANSACTIONS};
size_t max_extra_txs{DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN};
//! Number of non-mempool transactions to keep around for block reconstruction. Includes
//! orphan, replaced, and rejected transactions.
uint32_t max_extra_txs{DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN};
//! Whether all P2P messages are captured to disk
bool capture_messages{false};
};

View File

@@ -3,6 +3,9 @@
#include <common/args.h>
#include <net_processing.h>
#include <algorithm>
#include <limits>
namespace node {
void ApplyArgsManOptions(const ArgsManager& argsman, PeerManager::Options& options)
@@ -10,11 +13,11 @@ void ApplyArgsManOptions(const ArgsManager& argsman, PeerManager::Options& optio
if (auto value{argsman.GetBoolArg("-txreconciliation")}) options.reconcile_txs = *value;
if (auto value{argsman.GetIntArg("-maxorphantx")}) {
options.max_orphan_txs = uint32_t(std::max(int64_t{0}, *value));
options.max_orphan_txs = uint32_t((std::clamp<int64_t>(*value, 0, std::numeric_limits<uint32_t>::max())));
}
if (auto value{argsman.GetIntArg("-blockreconstructionextratxn")}) {
options.max_extra_txs = size_t(std::max(int64_t{0}, *value));
options.max_extra_txs = uint32_t((std::clamp<int64_t>(*value, 0, std::numeric_limits<uint32_t>::max())));
}
if (auto value{argsman.GetBoolArg("-capturemessages")}) options.capture_messages = *value;