Skip to content

Commit 4e6f844

Browse files
UdjinM6claude
authored andcommitted
refactor: collapse spork signer recovery to one ECDSA op, restore peer info in logs
- Replace IsSporkValid() with GetValidSporkSigner() returning std::optional<CKeyID> 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 <noreply@anthropic.com>
1 parent 6a4ee52 commit 4e6f844

3 files changed

Lines changed: 23 additions & 23 deletions

File tree

src/net_processing.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5469,11 +5469,12 @@ void PeerManagerImpl::ProcessMessage(
54695469
uint256 hash = spork.GetHash();
54705470
CInv spork_inv{MSG_SPORK, hash};
54715471
WITH_LOCK(::cs_main, EraseObjectRequest(pfrom.GetId(), spork_inv));
5472-
if (!m_sporkman.IsSporkValid(spork)) {
5472+
auto opt_signer = m_sporkman.GetValidSporkSigner(spork);
5473+
if (!opt_signer) {
54735474
Misbehaving(pfrom.GetId(), 100, strprintf("invalid spork received. peer=%d", pfrom.GetId()));
54745475
return;
54755476
}
5476-
if (m_sporkman.ProcessSpork(spork)) {
5477+
if (m_sporkman.ProcessSpork(spork, *opt_signer, strprintf(" peer=%d", pfrom.GetId()))) {
54775478
RelayInv(spork_inv);
54785479
}
54795480
return;

src/spork.cpp

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -123,31 +123,27 @@ void CSporkManager::CheckAndRemove()
123123
}
124124
}
125125

126-
bool CSporkManager::IsSporkValid(const CSporkMessage& spork) const
126+
std::optional<CKeyID> CSporkManager::GetValidSporkSigner(const CSporkMessage& spork) const
127127
{
128128
if (spork.nTimeSigned > GetAdjustedTime() + 2 * 60 * 60) {
129129
LogPrint(BCLog::SPORK, "CSporkManager::%s -- ERROR: too far into the future\n", __func__);
130-
return false;
130+
return std::nullopt;
131131
}
132132

133133
auto opt_keyIDSigner = spork.GetSignerKeyID();
134134

135135
if (opt_keyIDSigner == std::nullopt || WITH_LOCK(cs, return !setSporkPubKeyIDs.count(*opt_keyIDSigner))) {
136136
LogPrint(BCLog::SPORK, "CSporkManager::%s -- ERROR: invalid signature\n", __func__);
137-
return false;
137+
return std::nullopt;
138138
}
139-
return true;
139+
return opt_keyIDSigner;
140140
}
141141

142-
bool CSporkManager::ProcessSpork(const CSporkMessage& spork)
142+
bool CSporkManager::ProcessSpork(const CSporkMessage& spork, const CKeyID& keyIDSigner, std::string_view peer_log_suffix)
143143
{
144144
uint256 hash = spork.GetHash();
145-
std::string strLogMsg{strprintf("SPORK -- hash: %s id: %d value: %10d", hash.ToString(), spork.nSporkID,
146-
spork.nValue)};
147-
148-
if (!spork.GetSignerKeyID().has_value()) return false;
149-
150-
auto keyIDSigner = spork.GetSignerKeyID().value();
145+
std::string strLogMsg{strprintf("SPORK -- hash: %s id: %d value: %10d%s", hash.ToString(), spork.nSporkID,
146+
spork.nValue, peer_log_suffix)};
151147

152148
{
153149
LOCK(cs); // make sure to not lock this together with cs_main

src/spork.h

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -244,22 +244,25 @@ class CSporkManager : public SporkStore
244244
void CheckAndRemove() EXCLUSIVE_LOCKS_REQUIRED(!cs);
245245

246246
/**
247-
* IsSporkValid validate signed time and pubkey
248-
* If these values mismatch function returns false [spork is invalid]
249-
* If spork validation failed, peer should be punished
247+
* GetValidSporkSigner validates signed time and recovers the signer pubkey.
248+
* Returns the signer's CKeyID on success, or std::nullopt if the spork is invalid
249+
* (peer should be punished in that case).
250250
*/
251-
[[nodiscard]] bool IsSporkValid(const CSporkMessage& spork) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
251+
[[nodiscard]] std::optional<CKeyID> GetValidSporkSigner(const CSporkMessage& spork) const
252+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
252253
/**
253-
* ProcessSpork is used to handle the 'spork' p2p message.
254-
*
255-
* For 'spork', it validates the spork and adds it to the internal spork storage and
256-
* performs any necessary processing.
254+
* ProcessSpork adds the spork to local state. Returns true if the spork was new or
255+
* updated and should be relayed. `keyIDSigner` must be the signer key previously
256+
* recovered via GetValidSporkSigner. `peer_log_suffix` is appended to log lines for
257+
* cross-referencing with the source peer (e.g. " peer=42").
257258
*/
258-
[[nodiscard]] bool ProcessSpork(const CSporkMessage& spork)
259+
[[nodiscard]] bool ProcessSpork(const CSporkMessage& spork, const CKeyID& keyIDSigner,
260+
std::string_view peer_log_suffix = {})
259261
EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_cache);
260262

261263
/**
262-
* ActiveSporks is used to handle the 'getsporks' p2p message.
264+
* ActiveSporks returns a snapshot of currently active sporks indexed by SporkId then
265+
* signer CKeyID. Used by net_processing to answer the 'getsporks' p2p message.
263266
*/
264267
std::unordered_map<SporkId, std::map<CKeyID, CSporkMessage>> ActiveSporks() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
265268

0 commit comments

Comments
 (0)