Merge bitcoin/bitcoin#27774: refactor: Add [[nodiscard]] where ignoring a Result return type is an error

fa5680b fix includes for touched header files (iwyu) (MarcoFalke)
dddde27 Add [[nodiscard]] where ignoring a Result return type is an error (MarcoFalke)

Pull request description:

  Only add it for those where it is an error to ignore. Also, fix the gcc compile warning bitcoin/bitcoin#25977 (comment). Also, fix iwyu for touched header files.

ACKs for top commit:
  TheCharlatan:
    ACK fa5680b
  stickies-v:
    ACK fa5680b

Tree-SHA512: c3509103bfeae246e2cf565bc561fcd68d8118515bac5431ba5304c3a63c8254b9c4f40e268b6f6d6b79405675c5a960db9b4eb3bdd14aedca333dc1c9e76415

Author: fanquake

src/addrdb.cpp

@@ -210,7 +210,8 @@ util::Result<std::unique_ptr<AddrMan>> LoadAddrman(const NetGroupManager& netgro
         return util::Error{strprintf(_("Invalid or corrupt peers.dat (%s). If you believe this is a bug, please report it to %s. As a workaround, you can move the file (%s) out of the way (rename, move, or delete) to have a new one created on the next start."),
                                      e.what(), PACKAGE_BUGREPORT, fs::quoted(fs::PathToString(path_addr)))};
     }
-    return std::move(addrman); // std::move should be unneccessary but is temporarily needed to work around clang bug (https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1561270092)
+    return {std::move(addrman)}; // std::move should be unneccessary but is temporarily needed to work around clang bug
+                                 // (https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1561270092)
 }
 
 void DumpAnchors(const fs::path& anchors_db_path, const std::vector<CAddress>& anchors)

src/addrdb.h

@@ -6,20 +6,18 @@
 #ifndef BITCOIN_ADDRDB_H
 #define BITCOIN_ADDRDB_H
 
-#include <net_types.h> // For banmap_t
-#include <univalue.h>
+#include <net_types.h>
 #include <util/fs.h>
 #include <util/result.h>
 
-#include <optional>
+#include <memory>
 #include <vector>
 
 class ArgsManager;
 class AddrMan;
 class CAddress;
 class CDataStream;
 class NetGroupManager;
-struct bilingual_str;
 
 bool DumpPeerAddresses(const ArgsManager& args, const AddrMan& addr);
 /** Only used by tests. */

src/kernel/checks.h

@@ -7,17 +7,14 @@
 
 #include <util/result.h>
 
-struct bilingual_str;
-
 namespace kernel {
 
 struct Context;
 
 /**
  *  Ensure a usable environment with all necessary library support.
  */
-util::Result<void> SanityChecks(const Context&);
-
-}
+[[nodiscard]] util::Result<void> SanityChecks(const Context&);
+} // namespace kernel
 
 #endif // BITCOIN_KERNEL_CHECKS_H

src/node/blockmanager_args.h

@@ -12,7 +12,7 @@
 class ArgsManager;
 
 namespace node {
-util::Result<void> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Options& opts);
+[[nodiscard]] util::Result<void> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Options& opts);
 } // namespace node
 
 #endif // BITCOIN_NODE_BLOCKMANAGER_ARGS_H

src/node/chainstatemanager_args.h

@@ -11,7 +11,7 @@
 class ArgsManager;
 
 namespace node {
-util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts);
+[[nodiscard]] util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts);
 } // namespace node
 
 #endif // BITCOIN_NODE_CHAINSTATEMANAGER_ARGS_H

src/txdb.h

@@ -13,6 +13,9 @@
 #include <util/fs.h>
 #include <util/result.h>
 
+#include <cstddef>
+#include <cstdint>
+#include <functional>
 #include <memory>
 #include <optional>
 #include <string>
@@ -21,11 +24,11 @@
 
 class CBlockFileInfo;
 class CBlockIndex;
+class COutPoint;
 class uint256;
 namespace Consensus {
 struct Params;
 };
-struct bilingual_str;
 
 //! -dbcache default (MiB)
 static const int64_t nDefaultDbCache = 450;
@@ -99,6 +102,6 @@ class CBlockTreeDB : public CDBWrapper
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 };
 
-util::Result<void> CheckLegacyTxindex(CBlockTreeDB& block_tree_db);
+[[nodiscard]] util::Result<void> CheckLegacyTxindex(CBlockTreeDB& block_tree_db);
 
 #endif // BITCOIN_TXDB_H

src/wallet/wallet.h

@@ -1074,7 +1074,7 @@ struct MigrationResult {
 };
 
 //! Do all steps to migrate a legacy wallet to a descriptor wallet
-util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context);
+[[nodiscard]] util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context);
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_WALLET_H