fix: gate BLS vote-signature failure log behind gobject category - #7556
Conversation
Gate BLS vote-signature failure logs in CGovernanceVote::CheckSignature and CGovernanceObject::ProcessVote behind the BCLog::GOBJECT log category instead of logging unconditionally via LogPrintf. This prevents peers holding valid masternode outpoints with invalid BLS vote signatures or unsupported/invalid signals from spamming debug.log when -debug=gobject is not enabled.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughGovernance vote validation messages now use category-based Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/governance/object.cpp (1)
411-411: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify regression coverage for the logging-category contract.
If existing governance tests do not cover these paths, add a focused test that verifies the messages are emitted only when
BCLog::GOBJECTis enabled. Also cover the related change insrc/governance/vote.cppline 204.As per coding guidelines, governance changes must add small tests that prove the changed invariant.
Also applies to: 462-462
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/governance/object.cpp` at line 411, Add focused governance tests covering the logging paths in object.cpp and vote.cpp, verifying messages are emitted only when BCLog::GOBJECT is enabled and suppressed when it is disabled. Reuse existing logging-test utilities and cover both referenced object.cpp locations plus the related vote.cpp path.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/governance/object.cpp`:
- Line 411: Add focused governance tests covering the logging paths in
object.cpp and vote.cpp, verifying messages are emitted only when BCLog::GOBJECT
is enabled and suppressed when it is disabled. Reuse existing logging-test
utilities and cover both referenced object.cpp locations plus the related
vote.cpp path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e90c0f8-3dff-4ae6-a1aa-e0aff43e1b06
📒 Files selected for processing (2)
src/governance/object.cppsrc/governance/vote.cpp
|
⛔ Blockers found — Opus deferred (commit afe5fda) |
Issue being fixed or feature implemented
In
CGovernanceVote::CheckSignature(const CBLSPublicKey&)(src/governance/vote.cpp), a signature verification failure was logged unconditionally viaLogPrintfinstead ofLogPrint(BCLog::GOBJECT, ...), unlike key ID signature verification and the rest of vote validation.After PR #7527 (orphan governance votes must pass
GovernanceVote::IsValidbefore caching), a peer holding a valid masternode outpoint (public data) plus an invalid BLS signature can reach this path with a vote naming an arbitrary parent hash, producing adebug.logline per message without-debug=gobjectset.Similarly,
CGovernanceObject::ProcessVote(src/governance/object.cpp) logged unsupported vote signals and invalid votes via unconditionalLogPrintf.What was done?
Updated the unconditional
LogPrintfcalls inCGovernanceVote::CheckSignatureandCGovernanceObject::ProcessVoteto useLogPrint(BCLog::GOBJECT, ...).src/governance/vote.cpp:CGovernanceVote::CheckSignature(const CBLSPublicKey&)src/governance/object.cpp:CGovernanceObject::ProcessVote(unsupported vote signal and invalid vote paths)How Has This Been Tested?
./src/test/test_dash --run_test="*gov*") pass.test/lint/lint-logs.py,test/lint/lint-whitespace.py) pass.Breaking Changes
None.
Checklist:
This pull request was created by Codex.