fix: address review feedback from @UdjinM6 on #7303

src/net_processing.cpp

@@ -5469,11 +5469,12 @@ void PeerManagerImpl::ProcessMessage(
         uint256 hash = spork.GetHash();
         CInv spork_inv{MSG_SPORK, hash};
         WITH_LOCK(::cs_main, EraseObjectRequest(pfrom.GetId(), spork_inv));
-        if (!m_sporkman.IsSporkValid(spork)) {
+        auto opt_signer = m_sporkman.GetValidSporkSigner(spork);
+        if (!opt_signer) {
             Misbehaving(pfrom.GetId(), 100, strprintf("invalid spork received. peer=%d", pfrom.GetId()));
             return;
         }
-        if (m_sporkman.ProcessSpork(spork)) {
+        if (m_sporkman.ProcessSpork(spork, *opt_signer, strprintf(" peer=%d", pfrom.GetId()))) {
             RelayInv(spork_inv);
         }
         return;

src/spork.cpp

@@ -123,31 +123,27 @@ void CSporkManager::CheckAndRemove()
     }
 }
 
-bool CSporkManager::IsSporkValid(const CSporkMessage& spork) const
+std::optional<CKeyID> CSporkManager::GetValidSporkSigner(const CSporkMessage& spork) const
 {
     if (spork.nTimeSigned > GetAdjustedTime() + 2 * 60 * 60) {
         LogPrint(BCLog::SPORK, "CSporkManager::%s -- ERROR: too far into the future\n", __func__);
-        return false;
+        return std::nullopt;
     }
 
     auto opt_keyIDSigner = spork.GetSignerKeyID();
 
     if (opt_keyIDSigner == std::nullopt || WITH_LOCK(cs, return !setSporkPubKeyIDs.count(*opt_keyIDSigner))) {
         LogPrint(BCLog::SPORK, "CSporkManager::%s -- ERROR: invalid signature\n", __func__);
-        return false;
+        return std::nullopt;
     }
-    return true;
+    return opt_keyIDSigner;
 }
 
-bool CSporkManager::ProcessSpork(const CSporkMessage& spork)
+bool CSporkManager::ProcessSpork(const CSporkMessage& spork, const CKeyID& keyIDSigner, std::string_view peer_log_suffix)
 {
     uint256 hash = spork.GetHash();
-    std::string strLogMsg{strprintf("SPORK -- hash: %s id: %d value: %10d", hash.ToString(), spork.nSporkID,
-                                    spork.nValue)};
-
-    if (!spork.GetSignerKeyID().has_value()) return false;
-
-    auto keyIDSigner = spork.GetSignerKeyID().value();
+    std::string strLogMsg{strprintf("SPORK -- hash: %s id: %d value: %10d%s", hash.ToString(), spork.nSporkID,
+                                    spork.nValue, peer_log_suffix)};
 
     {
         LOCK(cs); // make sure to not lock this together with cs_main

src/spork.h

@@ -244,22 +244,25 @@ class CSporkManager : public SporkStore
     void CheckAndRemove() EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     /**
-     * IsSporkValid validate signed time and pubkey
-     * If these values mismatch function returns false [spork is invalid]
-     * If spork validation failed, peer should be punished
+     * GetValidSporkSigner validates signed time and recovers the signer pubkey.
+     * Returns the signer's CKeyID on success, or std::nullopt if the spork is invalid
+     * (peer should be punished in that case).
      */
-    [[nodiscard]] bool IsSporkValid(const CSporkMessage& spork) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
+    [[nodiscard]] std::optional<CKeyID> GetValidSporkSigner(const CSporkMessage& spork) const
+        EXCLUSIVE_LOCKS_REQUIRED(!cs);
     /**
-     * ProcessSpork is used to handle the 'spork' p2p message.
-     *
-     * For 'spork', it validates the spork and adds it to the internal spork storage and
-     * performs any necessary processing.
+     * ProcessSpork adds the spork to local state. Returns true if the spork was new or
+     * updated and should be relayed. `keyIDSigner` must be the signer key previously
+     * recovered via GetValidSporkSigner. `peer_log_suffix` is appended to log lines for
+     * cross-referencing with the source peer (e.g. " peer=42").
      */
-    [[nodiscard]] bool ProcessSpork(const CSporkMessage& spork)
+    [[nodiscard]] bool ProcessSpork(const CSporkMessage& spork, const CKeyID& keyIDSigner,
+                                    std::string_view peer_log_suffix = {})
         EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_cache);
 
     /**
-     * ActiveSporks is used to handle the 'getsporks' p2p message.
+     * ActiveSporks returns a snapshot of currently active sporks indexed by SporkId then
+     * signer CKeyID. Used by net_processing to answer the 'getsporks' p2p message.
      */
     std::unordered_map<SporkId, std::map<CKeyID, CSporkMessage>> ActiveSporks() const EXCLUSIVE_LOCKS_REQUIRED(!cs);