From 513917f999014e37e8418804934334dc7e122870 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 9 May 2026 13:16:33 +0300 Subject: [PATCH] refactor: collapse spork signer recovery to one ECDSA op, restore peer info in logs - Replace IsSporkValid() with GetValidSporkSigner() returning std::optional so the recovered signer key flows directly into ProcessSpork. This drops the ECDSA RecoverCompact() count per inbound spork from three to one (was: once in IsSporkValid, twice in ProcessSpork via has_value() + value()). - ProcessSpork now takes the validated CKeyID as a parameter, removing the unreachable defensive check that was added in 6a4ee52 to replace the original assert. - ProcessSpork takes a peer_log_suffix string_view so the "SPORK -- hash:..." seen/updated/new-signer/new log lines include peer info again (was lost when ProcessSpork stopped taking NodeId). - Tighten ActiveSporks() doc comment to describe what it returns rather than what it "handles". Co-Authored-By: Claude Opus 4.7 --- src/net_processing.cpp | 5 +++-- src/spork.cpp | 18 +++++++----------- src/spork.h | 23 +++++++++++++---------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a8fde29cc39e..0179bf53cced 100644 --- a/src/net_processing.cpp +++ b/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; diff --git a/src/spork.cpp b/src/spork.cpp index 7ca815bcf6e4..98d9086a4f81 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -123,31 +123,27 @@ void CSporkManager::CheckAndRemove() } } -bool CSporkManager::IsSporkValid(const CSporkMessage& spork) const +std::optional 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 diff --git a/src/spork.h b/src/spork.h index f4eb1c00d351..bb1c03bef74b 100644 --- a/src/spork.h +++ b/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 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> ActiveSporks() const EXCLUSIVE_LOCKS_REQUIRED(!cs);