Merge #18996: net: Remove un-actionable TODO

fabea6d404 net: Run clang-format on protocol.h (MarcoFalke)
facdeea2b2 net: Remove un-actionable TODO (MarcoFalke)

Pull request description:

  The first commit removes a TODO that is infeasible to solve. Currently, most (de)serializable classes in Bitcoin Core have public members. For example `CMessageHeader`, `FlatFilePos`, `CBlock`, `CTransaction`, `CCoin`, ...

  So either this TODO comment should apply to all classes or to none. Fix that discrepancy by removing it from the source code for now. If deemed important, the TODO can be discussed in a brainstorming issue later.

  Also run clang format on the header file in a new commit. Happy to drop this commit if it is too controversial, but I think it is trivial to review and makes the workflow of developers using clang-format-diff easier.

ACKs for top commit:
  practicalswift:
    ACK fabea6d404
  naumenkogs:
    ACK fabea6d. Not sure why that TODO was there in the first place, but Marco's justification seems correct.
  hebasto:
    ACK fabea6d404, agree with both changes: removing TODO and applying the `clang-format-diff.py`.

Tree-SHA512: b79ae07be27e5a40fc9f411a5e9ae91aecb2fdedbcbf74699614a1004f4ef816bf396903ec6c06eb1395fd83a2047620c7583acbaadfb8c4e613319a63062c3c

src/protocol.h

@@ -74,100 +74,100 @@ namespace NetMsgType {
  * receiving node at the beginning of a connection.
  * @see https://bitcoin.org/en/developer-reference#version
  */
-extern const char *VERSION;
+extern const char* VERSION;
 /**
  * The verack message acknowledges a previously-received version message,
  * informing the connecting node that it can begin to send other messages.
  * @see https://bitcoin.org/en/developer-reference#verack
  */
-extern const char *VERACK;
+extern const char* VERACK;
 /**
  * The addr (IP address) message relays connection information for peers on the
  * network.
  * @see https://bitcoin.org/en/developer-reference#addr
  */
-extern const char *ADDR;
+extern const char* ADDR;
 /**
  * The inv message (inventory message) transmits one or more inventories of
  * objects known to the transmitting peer.
  * @see https://bitcoin.org/en/developer-reference#inv
  */
-extern const char *INV;
+extern const char* INV;
 /**
  * The getdata message requests one or more data objects from another node.
  * @see https://bitcoin.org/en/developer-reference#getdata
  */
-extern const char *GETDATA;
+extern const char* GETDATA;
 /**
  * The merkleblock message is a reply to a getdata message which requested a
  * block using the inventory type MSG_MERKLEBLOCK.
  * @since protocol version 70001 as described by BIP37.
  * @see https://bitcoin.org/en/developer-reference#merkleblock
  */
-extern const char *MERKLEBLOCK;
+extern const char* MERKLEBLOCK;
 /**
  * The getblocks message requests an inv message that provides block header
  * hashes starting from a particular point in the block chain.
  * @see https://bitcoin.org/en/developer-reference#getblocks
  */
-extern const char *GETBLOCKS;
+extern const char* GETBLOCKS;
 /**
  * The getheaders message requests a headers message that provides block
  * headers starting from a particular point in the block chain.
  * @since protocol version 31800.
  * @see https://bitcoin.org/en/developer-reference#getheaders
  */
-extern const char *GETHEADERS;
+extern const char* GETHEADERS;
 /**
  * The tx message transmits a single transaction.
  * @see https://bitcoin.org/en/developer-reference#tx
  */
-extern const char *TX;
+extern const char* TX;
 /**
  * The headers message sends one or more block headers to a node which
  * previously requested certain headers with a getheaders message.
  * @since protocol version 31800.
  * @see https://bitcoin.org/en/developer-reference#headers
  */
-extern const char *HEADERS;
+extern const char* HEADERS;
 /**
  * The block message transmits a single serialized block.
  * @see https://bitcoin.org/en/developer-reference#block
  */
-extern const char *BLOCK;
+extern const char* BLOCK;
 /**
  * The getaddr message requests an addr message from the receiving node,
  * preferably one with lots of IP addresses of other receiving nodes.
  * @see https://bitcoin.org/en/developer-reference#getaddr
  */
-extern const char *GETADDR;
+extern const char* GETADDR;
 /**
  * The mempool message requests the TXIDs of transactions that the receiving
  * node has verified as valid but which have not yet appeared in a block.
  * @since protocol version 60002.
  * @see https://bitcoin.org/en/developer-reference#mempool
  */
-extern const char *MEMPOOL;
+extern const char* MEMPOOL;
 /**
  * The ping message is sent periodically to help confirm that the receiving
  * peer is still connected.
  * @see https://bitcoin.org/en/developer-reference#ping
  */
-extern const char *PING;
+extern const char* PING;
 /**
  * The pong message replies to a ping message, proving to the pinging node that
  * the ponging node is still alive.
  * @since protocol version 60001 as described by BIP31.
  * @see https://bitcoin.org/en/developer-reference#pong
  */
-extern const char *PONG;
+extern const char* PONG;
 /**
  * The notfound message is a reply to a getdata message which requested an
  * object the receiving node does not have available for relay.
  * @since protocol version 70001.
  * @see https://bitcoin.org/en/developer-reference#notfound
  */
-extern const char *NOTFOUND;
+extern const char* NOTFOUND;
 /**
  * The filterload message tells the receiving peer to filter all relayed
  * transactions and requested merkle blocks through the provided filter.
@@ -176,7 +176,7 @@ extern const char *NOTFOUND;
  *   70011 as described by BIP111.
  * @see https://bitcoin.org/en/developer-reference#filterload
  */
-extern const char *FILTERLOAD;
+extern const char* FILTERLOAD;
 /**
  * The filteradd message tells the receiving peer to add a single element to a
  * previously-set bloom filter, such as a new public key.
@@ -185,7 +185,7 @@ extern const char *FILTERLOAD;
  *   70011 as described by BIP111.
  * @see https://bitcoin.org/en/developer-reference#filteradd
  */
-extern const char *FILTERADD;
+extern const char* FILTERADD;
 /**
  * The filterclear message tells the receiving peer to remove a previously-set
  * bloom filter.
@@ -194,20 +194,20 @@ extern const char *FILTERADD;
  *   70011 as described by BIP111.
  * @see https://bitcoin.org/en/developer-reference#filterclear
  */
-extern const char *FILTERCLEAR;
+extern const char* FILTERCLEAR;
 /**
  * Indicates that a node prefers to receive new block announcements via a
  * "headers" message rather than an "inv".
  * @since protocol version 70012 as described by BIP130.
  * @see https://bitcoin.org/en/developer-reference#sendheaders
  */
-extern const char *SENDHEADERS;
+extern const char* SENDHEADERS;
 /**
  * The feefilter message tells the receiving peer not to inv us any txs
  * which do not meet the specified min fee rate.
  * @since protocol version 70013 as described by BIP133
  */
-extern const char *FEEFILTER;
+extern const char* FEEFILTER;
 /**
  * Contains a 1-byte bool and 8-byte LE version number.
  * Indicates that a node is willing to provide blocks via "cmpctblock" messages.
@@ -215,43 +215,43 @@ extern const char *FEEFILTER;
  * "cmpctblock" message rather than an "inv", depending on message contents.
  * @since protocol version 70014 as described by BIP 152
  */
-extern const char *SENDCMPCT;
+extern const char* SENDCMPCT;
 /**
  * Contains a CBlockHeaderAndShortTxIDs object - providing a header and
  * list of "short txids".
  * @since protocol version 70014 as described by BIP 152
  */
-extern const char *CMPCTBLOCK;
+extern const char* CMPCTBLOCK;
 /**
  * Contains a BlockTransactionsRequest
  * Peer should respond with "blocktxn" message.
  * @since protocol version 70014 as described by BIP 152
  */
-extern const char *GETBLOCKTXN;
+extern const char* GETBLOCKTXN;
 /**
  * Contains a BlockTransactions.
  * Sent in response to a "getblocktxn" message.
  * @since protocol version 70014 as described by BIP 152
  */
-extern const char *BLOCKTXN;
+extern const char* BLOCKTXN;
 /**
  * getcfcheckpt requests evenly spaced compact filter headers, enabling
  * parallelized download and validation of the headers between them.
  * Only available with service bit NODE_COMPACT_FILTERS as described by
  * BIP 157 & 158.
  */
-extern const char *GETCFCHECKPT;
+extern const char* GETCFCHECKPT;
 /**
  * cfcheckpt is a response to a getcfcheckpt request containing a vector of
  * evenly spaced filter headers for blocks on the requested chain.
  * Only available with service bit NODE_COMPACT_FILTERS as described by
  * BIP 157 & 158.
  */
-extern const char *CFCHECKPT;
+extern const char* CFCHECKPT;
-};
+}; // namespace NetMsgType
 
 /* Get a vector of all valid message types (see above) */
-const std::vector<std::string> &getAllNetMessageTypes();
+const std::vector<std::string>& getAllNetMessageTypes();
 
 /** nServices flags */
 enum ServiceFlags : uint64_t {
@@ -320,7 +320,8 @@ void SetServiceFlagsIBDCache(bool status);
  * == GetDesirableServiceFlags(services), ie determines whether the given
  * set of service flags are sufficient for a peer to be "relevant".
  */
-static inline bool HasAllDesirableServiceFlags(ServiceFlags services) {
+static inline bool HasAllDesirableServiceFlags(ServiceFlags services)
+{
     return !(GetDesirableServiceFlags(services) & (~services));
 }
 
@@ -328,7 +329,8 @@ static inline bool HasAllDesirableServiceFlags(ServiceFlags services) {
  * Checks if a peer with the given service flags may be capable of having a
  * robust address-storage DB.
  */
-static inline bool MayHaveUsefulAddressDB(ServiceFlags services) {
+static inline bool MayHaveUsefulAddressDB(ServiceFlags services)
+{
     return (services & NODE_NETWORK) || (services & NODE_NETWORK_LIMITED);
 }
 
@@ -360,8 +362,6 @@ public:
         READWRITEAS(CService, *this);
     }
 
-    // TODO: make private (improves encapsulation)
-public:
     ServiceFlags nServices;
 
     // disk and network only
@@ -370,20 +370,19 @@ public:
 
 /** getdata message type flags */
 const uint32_t MSG_WITNESS_FLAG = 1 << 30;
-const uint32_t MSG_TYPE_MASK    = 0xffffffff >> 2;
+const uint32_t MSG_TYPE_MASK = 0xffffffff >> 2;
 
 /** getdata / inv message types.
  * These numbers are defined by the protocol. When adding a new value, be sure
  * to mention it in the respective BIP.
  */
-enum GetDataMsg
+enum GetDataMsg {
-{
     UNDEFINED = 0,
     MSG_TX = 1,
     MSG_BLOCK = 2,
     // The following can only occur in getdata. Invs always use TX or BLOCK.
-    MSG_FILTERED_BLOCK = 3,  //!< Defined in BIP37
+    MSG_FILTERED_BLOCK = 3,                           //!< Defined in BIP37
-    MSG_CMPCT_BLOCK = 4,     //!< Defined in BIP152
+    MSG_CMPCT_BLOCK = 4,                              //!< Defined in BIP152
     MSG_WITNESS_BLOCK = MSG_BLOCK | MSG_WITNESS_FLAG, //!< Defined in BIP144
     MSG_WITNESS_TX = MSG_TX | MSG_WITNESS_FLAG,       //!< Defined in BIP144
     MSG_FILTERED_WITNESS_BLOCK = MSG_FILTERED_BLOCK | MSG_WITNESS_FLAG,
@@ -410,7 +409,6 @@ public:
     std::string GetCommand() const;
     std::string ToString() const;
 
-public:
     int type;
     uint256 hash;
 };