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

547fa52 net processing: clamp -blockreconstructionextratxn to uint32_t bounds (stickies-v)
e451d1e net processing: clamp -maxorphantx to uint32_t bounds (stickies-v)
aa89e04 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 bitcoin/bitcoin#27499 (review).

  Also documents all `PeerManager::Options` members, addressing bitcoin/bitcoin#27499 (comment).

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

Tree-SHA512: 042d47b35bb8a7b29ef3dadd4c0c5d26f13a8f174f33687855d603c19f8de0fcbbda94418453331e149885412d4edd5f402d640d938f6d94b4dcf54e2fdbbcc9

Author: glozow

src/net_processing.h

@@ -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};
     };
 

src/node/peerman_args.cpp

@@ -3,18 +3,21 @@
 #include <common/args.h>
 #include <net_processing.h>
 
+#include <algorithm>
+#include <limits>
+
 namespace node {
 
 void ApplyArgsManOptions(const ArgsManager& argsman, PeerManager::Options& options)
 {
     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;