Merge bitcoin/bitcoin#34729: Reduce log noise

### doc/developer-notes.md
@@ -803,6 +803,11 @@ logging messages. They should be used as follows:
   severe enough to warrant shutting down the node (e.g., system time
   appears to be wrong, unknown soft fork appears to have activated).
 
+- `LogWarnThenDebug(BCLog::CATEGORY, fmt, params...)` should be used
+  for errors that indicate a systematic problem that should be addressed
+  (and thus warrant a warning) but are likely to occur frequently if they
+  occur at all, so later occurrences are demoted to debug level.
+
 - `LogTrace(BCLog::CATEGORY, fmt, params...)` should be used in place of
   `LogDebug` for log messages that would be unusable on a production
   system, e.g. due to being too noisy in normal use, or too resource

### src/bitcoin-tx.cpp
@@ -660,7 +660,9 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
                 UniValue v = prevOut["redeemScript"];
                 std::vector<unsigned char> rsData(ParseHexUV(v, "redeemScript"));
                 CScript redeemScript(rsData.begin(), rsData.end());
-                tempKeystore.AddCScript(redeemScript);
+                if (!tempKeystore.AddCScript(redeemScript)) {
+                    throw std::runtime_error(strprintf("Error adding redeemscript=%s", HexStr(redeemScript)));
+                }
             }
         }
     }

### src/common/pcp.cpp
@@ -18,7 +18,6 @@
 #include <util/time.h>
 
 #include <algorithm>
-#include <atomic>
 #include <compare>
 #include <cstring>
 #include <functional>
@@ -391,14 +390,9 @@ std::variant<MappingResult, MappingError> NATPMPRequestPortMap(const CNetAddr &g
         uint16_t result_code = ReadBE16(response.data() + NATPMP_RESPONSE_HDR_RESULT_OFS);
         if (result_code != NATPMP_RESULT_SUCCESS) {
             if (result_code == NATPMP_RESULT_NOT_AUTHORIZED) {
-                static std::atomic<bool> warned{false};
-                if (!warned.exchange(true)) {
-                    LogWarning("natpmp: Port mapping failed with result %s\n", NATPMPResultString(result_code));
-                } else {
-                    LogDebug(BCLog::NET, "natpmp: Port mapping failed with result %s\n", NATPMPResultString(result_code));
-                }
+                LogWarnThenDebug(BCLog::NET, "natpmp: Port mapping failed with result %s", NATPMPResultString(result_code));
             } else {
-                LogWarning("natpmp: Port mapping failed with result %s\n", NATPMPResultString(result_code));
+                LogWarning("natpmp: Port mapping failed with result %s", NATPMPResultString(result_code));
             }
             if (result_code == NATPMP_RESULT_NO_RESOURCES) {
                 return MappingError::NO_RESOURCES;
@@ -534,14 +528,9 @@ std::variant<MappingResult, MappingError> PCPRequestPortMap(const PCPMappingNonc
     CNetAddr external_addr{PCPUnwrapAddress(response.subspan(PCP_HDR_SIZE + PCP_MAP_EXTERNAL_IP_OFS, ADDR_IPV6_SIZE))};
     if (result_code != PCP_RESULT_SUCCESS) {
         if (result_code == PCP_RESULT_NOT_AUTHORIZED) {
-            static std::atomic<bool> warned{false};
-            if (!warned.exchange(true)) {
-                LogWarning("pcp: Mapping failed with result %s\n", PCPResultString(result_code));
-            } else {
-                LogDebug(BCLog::NET, "pcp: Mapping failed with result %s\n", PCPResultString(result_code));
-            }
+            LogWarnThenDebug(BCLog::NET, "pcp: Mapping failed with result %s", PCPResultString(result_code));
         } else {
-            LogWarning("pcp: Mapping failed with result %s\n", PCPResultString(result_code));
+            LogWarning("pcp: Mapping failed with result %s", PCPResultString(result_code));
         }
         if (result_code == PCP_RESULT_NO_RESOURCES) {
             return MappingError::NO_RESOURCES;

### src/index/txindex.cpp
@@ -226,7 +226,7 @@ std::optional<TxIndexResult> TxIndex::FindLegacyTx(const Txid& tx_hash) const
 
     AutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, /*fReadOnly=*/true)};
     if (file.IsNull()) {
-        LogError("OpenBlockFile failed");
+        LogWarning("%s: OpenBlockFile failed", GetName());
         return std::nullopt;
     }
     CBlockHeader header;
@@ -236,11 +236,11 @@ std::optional<TxIndexResult> TxIndex::FindLegacyTx(const Txid& tx_hash) const
         file.seek(postx.nTxOffset, SEEK_CUR);
         file >> TX_WITH_WITNESS(tx);
     } catch (const std::exception& e) {
-        LogError("Deserialize or I/O error - %s", e.what());
+        LogWarning("%s: Deserialize or I/O error - %s", GetName(), e.what());
         return std::nullopt;
     }
     if (tx->GetHash() != tx_hash) {
-        LogError("txid mismatch");
+        LogWarning("%s: txid mismatch", GetName());
         return std::nullopt;
     }
     return TxIndexResult{header.GetHash(), std::move(tx)};

### src/kernel/coinstats.cpp
@@ -136,7 +136,7 @@ static std::optional<CCoinsStats> ComputeUTXOStats(T hash_obj, const CCoinsViewD
             outputs[key.n] = std::move(coin);
             stats.coins_count++;
         } else {
-            LogError("%s: unable to read value\n", __func__);
+            LogDebug(BCLog::COINDB, "%s: unable to read value", __func__);
             return std::nullopt;
         }
         pcursor->Next();

### src/netbase.cpp
@@ -395,7 +395,7 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a
         IntrRecvError recvr;
         LogDebug(BCLog::NET, "SOCKS5 connecting %s\n", strDest);
         if (strDest.size() > 255) {
-            LogError("Hostname too long\n");
+            LogDebug(BCLog::PROXY, "Hostname too long for SOCKS5 proxying: %s", strDest);
             return false;
         }
         // Construct the version identifier/method selection message
@@ -412,19 +412,19 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a
         sock.SendComplete(vSocks5Init, g_socks5_recv_timeout, g_socks5_interrupt);
         uint8_t pchRet1[2];
         if (InterruptibleRecv(pchRet1, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) {
-            LogInfo("Socks5() connect to %s:%d failed: InterruptibleRecv() timeout or other failure\n", strDest, port);
+            LogDebug(BCLog::PROXY, "Socks5() connect to %s:%d failed: InterruptibleRecv() timeout or other failure", strDest, port);
             return false;
         }
         if (pchRet1[0] != SOCKSVersion::SOCKS5) {
-            LogError("Proxy failed to initialize\n");
+            LogDebug(BCLog::PROXY, "SOCKS5 Proxy failed to initialize");
             return false;
         }
         if (pchRet1[1] == SOCKS5Method::USER_PASS && auth) {
             // Perform username/password authentication (as described in RFC1929)
             std::vector<uint8_t> vAuth;
             vAuth.push_back(0x01); // Current (and only) version of user/pass subnegotiation
             if (auth->username.size() > 255 || auth->password.size() > 255) {
-                LogError("Proxy username or password too long\n");
+                LogWarnThenDebug(BCLog::PROXY, "SOCKS5 Proxy username or password too long");
                 return false;
             }
             vAuth.push_back(auth->username.size());
@@ -435,17 +435,17 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a
             sock.SendComplete(vAuth, g_socks5_recv_timeout, g_socks5_interrupt);
             uint8_t pchRetA[2];
             if (InterruptibleRecv(pchRetA, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) {
-                LogError("Error reading proxy authentication response\n");
+                LogDebug(BCLog::PROXY, "Error reading SOCKS5 proxy authentication response");
                 return false;
             }
             if (pchRetA[0] != 0x01 || pchRetA[1] != 0x00) {
-                LogError("Proxy authentication unsuccessful\n");
+                LogWarnThenDebug(BCLog::PROXY, "SOCKS5 Proxy authentication unsuccessful");
                 return false;
             }
         } else if (pchRet1[1] == SOCKS5Method::NOAUTH) {
             // Perform no authentication
         } else {
-            LogError("Proxy requested wrong authentication method %02x\n", pchRet1[1]);
+            LogWarnThenDebug(BCLog::PROXY, "SOCKS5 Proxy requested wrong authentication method %02x", pchRet1[1]);
             return false;
         }
         std::vector<uint8_t> vSocks5;
@@ -466,22 +466,22 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a
                  * error message. */
                 return false;
             } else {
-                LogError("Error while reading proxy response\n");
+                LogDebug(BCLog::PROXY, "SOCKS5 Error while reading proxy response");
                 return false;
             }
         }
         if (pchRet2[0] != SOCKSVersion::SOCKS5) {
-            LogError("Proxy failed to accept request\n");
+            LogDebug(BCLog::PROXY, "SOCKS5 Proxy failed to accept request");
             return false;
         }
         if (pchRet2[1] != SOCKS5Reply::SUCCEEDED) {
             // Failures to connect to a peer that are not proxy errors
-            LogDebug(BCLog::NET,
-                          "Socks5() connect to %s:%d failed: %s\n", strDest, port, Socks5ErrorString(pchRet2[1]));
+            LogDebug(BCLog::PROXY,
+                          "Socks5() connect to %s:%d failed: %s", strDest, port, Socks5ErrorString(pchRet2[1]));
             return false;
         }
         if (pchRet2[2] != 0x00) { // Reserved field must be 0
-            LogError("Error: malformed proxy response\n");
+            LogDebug(BCLog::PROXY, "malformed SOCKS5 proxy response");
             return false;
         }
         uint8_t pchRet3[256];
@@ -491,30 +491,30 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a
         case SOCKS5Atyp::DOMAINNAME: {
             recvr = InterruptibleRecv(pchRet3, 1, g_socks5_recv_timeout, sock);
             if (recvr != IntrRecvError::OK) {
-                LogError("Error reading from proxy\n");
+                LogDebug(BCLog::PROXY, "Error reading from SOCKS5 proxy");
                 return false;
             }
             int nRecv = pchRet3[0];
             recvr = InterruptibleRecv(pchRet3, nRecv, g_socks5_recv_timeout, sock);
             break;
         }
         default: {
-            LogError("Error: malformed proxy response\n");
+            LogDebug(BCLog::PROXY, "Malformed SOCKS5 proxy response");
             return false;
         }
         }
         if (recvr != IntrRecvError::OK) {
-            LogError("Error reading from proxy\n");
+            LogDebug(BCLog::PROXY, "Error reading from SOCKS5 proxy");
             return false;
         }
         if (InterruptibleRecv(pchRet3, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) {
-            LogError("Error reading from proxy\n");
+            LogDebug(BCLog::PROXY, "Error reading from SOCKS5 proxy");
             return false;
         }
         LogDebug(BCLog::NET, "SOCKS5 connected %s\n", strDest);
         return true;
     } catch (const std::runtime_error& e) {
-        LogError("Error during SOCKS5 proxy handshake: %s\n", e.what());
+        LogDebug(BCLog::PROXY, "Error during SOCKS5 proxy handshake: %s", e.what());
         return false;
     }
 }
@@ -539,7 +539,7 @@ std::unique_ptr<Sock> CreateSockOS(int domain, int type, int protocol)
     // Ensure that waiting for I/O on this socket won't result in undefined
     // behavior.
     if (!sock->IsSelectable()) {
-        LogInfo("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n");
+        LogWarnThenDebug(BCLog::NET, "Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)");
         return nullptr;
     }
 
@@ -548,14 +548,14 @@ std::unique_ptr<Sock> CreateSockOS(int domain, int type, int protocol)
     // Set the no-sigpipe option on the socket for BSD systems, other UNIXes
     // should use the MSG_NOSIGNAL flag for every send.
     if (sock->SetSockOpt(SOL_SOCKET, SO_NOSIGPIPE, &set, sizeof(int)) == SOCKET_ERROR) {
-        LogInfo("Error setting SO_NOSIGPIPE on socket: %s, continuing anyway\n",
+        LogDebug(BCLog::NET, "Error setting SO_NOSIGPIPE on socket: %s, continuing anyway",
                   NetworkErrorString(WSAGetLastError()));
     }
 #endif
 
     // Set the non-blocking option on the socket.
     if (!sock->SetNonBlocking()) {
-        LogInfo("Error setting socket to non-blocking: %s\n", NetworkErrorString(WSAGetLastError()));
+        LogWarnThenDebug(BCLog::NET, "Error setting socket to non-blocking: %s", NetworkErrorString(WSAGetLastError()));
         return nullptr;
     }
 
@@ -581,9 +581,9 @@ static void LogConnectFailure(bool manual_connection, util::ConstevalFormatStrin
 {
     std::string error_message = tfm::format(fmt, args...);
     if (manual_connection) {
-        LogInfo("%s\n", error_message);
+        LogInfo("%s", error_message);
     } else {
-        LogDebug(BCLog::NET, "%s\n", error_message);
+        LogDebug(BCLog::NET, "%s", error_message);
     }
 }
 
@@ -606,7 +606,7 @@ static bool ConnectToSocket(const Sock& sock,
             const Sock::Event requested = Sock::RecvEvent | Sock::SendEvent;
             Sock::Event occurred;
             if (!sock.Wait(timeout, requested, &occurred)) {
-                LogInfo("wait for connect to %s failed: %s\n",
+                LogDebug(BCLog::NET, "wait for connect to %s failed: %s",
                           dest_str,
                           NetworkErrorString(WSAGetLastError()));
                 return false;
@@ -623,7 +623,7 @@ static bool ConnectToSocket(const Sock& sock,
             socklen_t sockerr_len = sizeof(sockerr);
             if (sock.GetSockOpt(SOL_SOCKET, SO_ERROR, &sockerr, &sockerr_len) ==
                 SOCKET_ERROR) {
-                LogInfo("getsockopt() for %s failed: %s\n", dest_str, NetworkErrorString(WSAGetLastError()));
+                LogWarnThenDebug(BCLog::NET, "getsockopt() for %s failed: %s", dest_str, NetworkErrorString(WSAGetLastError()));
                 return false;
             }
             if (sockerr != 0) {
@@ -658,15 +658,15 @@ std::unique_ptr<Sock> ConnectDirectly(const CService& dest,
 {
     auto sock = CreateSock(dest.GetSAFamily(), SOCK_STREAM, IPPROTO_TCP);
     if (!sock) {
-        LogError("Cannot create a socket for connecting to %s\n", dest.ToStringAddrPort());
+        LogDebug(BCLog::NET, "Cannot create a socket for connecting to %s", dest.ToStringAddrPort());
         return {};
     }
 
     // Create a sockaddr from the specified service.
     struct sockaddr_storage sockaddr;
     socklen_t len = sizeof(sockaddr);
     if (!dest.GetSockAddr((struct sockaddr*)&sockaddr, &len)) {
-        LogInfo("Cannot get sockaddr for %s: unsupported network\n", dest.ToStringAddrPort());
+        LogDebug(BCLog::NET, "Cannot get sockaddr for %s: unsupported network", dest.ToStringAddrPort());
         return {};
     }
 
@@ -686,7 +686,7 @@ std::unique_ptr<Sock> Proxy::Connect() const
 #ifdef HAVE_SOCKADDR_UN
     auto sock = CreateSock(AF_UNIX, SOCK_STREAM, 0);
     if (!sock) {
-        LogError("Cannot create a socket for connecting to %s\n", m_unix_socket_path);
+        LogWarnThenDebug(BCLog::PROXY, "Cannot create a socket for connecting to %s", m_unix_socket_path);
         return {};
     }
 
@@ -978,7 +978,7 @@ CService GetBindAddress(const Sock& sock)
     if (sock.GetSockName(sa, &len) == 0) {
         addr_bind.SetSockAddr(sa, len);
     } else {
-        LogWarning("getsockname failed\n");
+        LogWarning("getsockname failed");
     }
     return addr_bind;
 }

### src/script/signingprovider.cpp
@@ -231,7 +231,7 @@ bool FillableSigningProvider::GetKey(const CKeyID &address, CKey &keyOut) const
 bool FillableSigningProvider::AddCScript(const CScript& redeemScript)
 {
     if (redeemScript.size() > MAX_SCRIPT_ELEMENT_SIZE) {
-        LogError("FillableSigningProvider::AddCScript(): redeemScripts > %i bytes are invalid\n", MAX_SCRIPT_ELEMENT_SIZE);
+        LogInfo("FillableSigningProvider::AddCScript(): redeemScripts > %i bytes are invalid", MAX_SCRIPT_ELEMENT_SIZE); // TODO: remove, callers should take care of checking input / reporting errors
         return false;
     }
 

### src/script/signingprovider.h
@@ -324,7 +324,7 @@ class FillableSigningProvider : public SigningProvider
     virtual bool HaveKey(const CKeyID &address) const override;
     virtual std::set<CKeyID> GetKeys() const;
     virtual bool GetKey(const CKeyID &address, CKey &keyOut) const override;
-    virtual bool AddCScript(const CScript& redeemScript);
+    [[nodiscard]] virtual bool AddCScript(const CScript& redeemScript);
     virtual bool HaveCScript(const CScriptID &hash) const override;
     virtual std::set<CScriptID> GetCScripts() const;
     virtual bool GetCScript(const CScriptID &hash, CScript& redeemScriptOut) const override;

### src/test/logging_tests.cpp
@@ -209,6 +209,29 @@ BOOST_FIXTURE_TEST_CASE(logging_SeverityLevels, LogSetup)
     BOOST_CHECK_EQUAL_COLLECTIONS(log_lines.begin(), log_lines.end(), expected.begin(), expected.end());
 }
 
+namespace {
+void CheckLogWarnThenDebug(std::string_view message)
+{
+    LogWarnThenDebug(BCLog::NET, "warn_then_debug: %s", message);
+}
+} // namespace
+
+BOOST_FIXTURE_TEST_CASE(logging_LogWarnThenDebug, LogSetup)
+{
+    ResetLogger();
+    CheckLogWarnThenDebug("first");
+    CheckLogWarnThenDebug("suppressed");
+    LogInstance().EnableCategory(BCLog::NET);
+    CheckLogWarnThenDebug("debug");
+
+    std::vector log_lines{ReadDebugLogLines()};
+    std::vector<std::string> expected = {
+        "[warning] warn_then_debug: first",
+        "[net] warn_then_debug: debug",
+    };
+    BOOST_CHECK_EQUAL_COLLECTIONS(log_lines.begin(), log_lines.end(), expected.begin(), expected.end());
+}
+
 BOOST_FIXTURE_TEST_CASE(logging_Conf, LogSetup)
 {
     // Set global log level

### src/util/log.h
@@ -13,6 +13,7 @@
 #include <util/threadnames.h>
 #include <util/time.h>
 
+#include <atomic>
 #include <cstdint>
 #include <source_location>
 #include <string>
@@ -143,4 +144,15 @@ using Level = util::log::Level;
 #define LogDebug(category, ...) detail_LogIfCategoryAndLevelEnabled(category, util::log::ShouldDebugLog, util::log::Level::Debug, __VA_ARGS__)
 #define LogTrace(category, ...) detail_LogIfCategoryAndLevelEnabled(category, util::log::ShouldTraceLog, util::log::Level::Trace, __VA_ARGS__)
 
+// Log unconditionally the first time this statement is hit, then conditionally afterwards
+#define LogWarnThenDebug(category, ...) \
+    do { \
+        static std::atomic<bool> _warned{false}; \
+        if (!_warned.exchange(true)) { \
+            LogWarning(__VA_ARGS__); \
+        } else { \
+            LogDebug(category, __VA_ARGS__); \
+        } \
+    } while (0)
+
 #endif // BITCOIN_UTIL_LOG_H

### src/validation.cpp
@@ -199,7 +199,7 @@ std::optional<std::vector<int>> CalculatePrevHeights(
                               ? tip.nHeight + 1 // Assume all mempool transaction confirm in the next block.
                               : coin->nHeight;
         } else {
-            LogInfo("ERROR: %s: Missing input %d in transaction \'%s\'\n", __func__, i, tx.GetHash().GetHex());
+            LogWarning("%s: Missing input %d in transaction \'%s\'", __func__, i, tx.GetHash().GetHex());
             return std::nullopt;
         }
     }
@@ -2617,7 +2617,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
         }
     }
     if (!state.IsValid()) {
-        LogInfo("Block validation error: %s", state.ToString());
+        LogWarning("Block validation error: %s", state.ToString());
         return false;
     }
     const auto time_4{SteadyClock::now()};

### src/wallet/scriptpubkeyman.h
@@ -232,7 +232,7 @@ class LegacyDataSPKM : public ScriptPubKeyMan, public FillableSigningProvider
     //! Adds an encrypted key to the store, without saving it to disk (used by LoadWallet)
     bool LoadCryptedKey(const CPubKey &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret, bool checksum_valid);
     //! Adds a CScript to the store
-    bool LoadCScript(const CScript& redeemScript);
+    [[nodiscard]] bool LoadCScript(const CScript& redeemScript);
     //! Load a HD chain model (used by LoadWallet)
     void LoadHDChain(const CHDChain& chain);
     void AddInactiveHDChain(const CHDChain& chain);

### src/wallet/test/wallet_tests.cpp
@@ -1199,7 +1199,7 @@ static size_t CalculateNestedKeyhashInputSize(bool use_max_sig)
 
     // Add inner-script to key store and key to watchonly
     FillableSigningProvider keystore;
-    keystore.AddCScript(inner_script);
+    BOOST_CHECK(keystore.AddCScript(inner_script));
     keystore.AddKeyPubKey(key, pubkey);
 
     // Fill in dummy signatures for fee calculation.