test: make governance vote fixtures wire-valid#7450
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Review complete (commit e058152) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Verified the one-line test-only fixture fix at the exact requested head: the 65-byte placeholder satisfies the compact-signature wire-size check, the unknown-parent path returns before cryptographic signature verification, and the vote hash does not include the signature bytes. The single commit is focused and accurately described; no in-scope issue survived verification.
Source: reviewers gpt-5.6-sol (general, dash-core-commit-history) and claude-sonnet-5 (general, dash-core-commit-history); verifier claude-sonnet-5.
Note: GitHub does not allow me to approve my own PR, so the verified APPROVE verdict is posted as a COMMENT review.
e058152 test: make governance vote fixtures wire-valid (PastaClaw) Pull request description: ## Issue being fixed or feature implemented The governance inventory tests merged in dashpay#7442 construct unsigned synthetic votes and round-trip them through the network parser. Develop now requires governance vote signatures to use a structurally valid 65- or 96-byte encoding, so the fixture is rejected as malformed before the authorization behavior under test is reached. This causes six unit-test failures across CI configurations. ## What was done? Give the synthetic `VOTE_SIGNAL_FUNDING` vote a compact-signature-sized placeholder. The 65-byte encoding matches the real ECDSA voting-key form for funding votes. These tests deliberately use a missing parent object, so vote processing takes the orphan path before cryptographic signature verification; production validation is unchanged. ## How Has This Been Tested? - Built `src/test/test_dash` in an isolated macOS arm64 worktree using the depends toolchain - `./src/test/test_dash --run_test=governance_inv_tests` - `./src/test/test_dash --run_test=governance_vote_wire_tests` - `git diff --check` - Mandatory independent pre-PR review gate: `ship` with no findings ## Breaking Changes None. Test-only change. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone ACKs for top commit: PastaPastaPasta: utACK e058152 Tree-SHA512: 5caca1e34ce563444ded42f669ff599f7658c6d1dbea1d541588211ac542ee45f3dd768e158899cb23150c5d45ec8dbcbf412a1472f73a3cc3e72a5ac37636e0
e058152 test: make governance vote fixtures wire-valid (PastaClaw) Pull request description: ## Issue being fixed or feature implemented The governance inventory tests merged in dashpay#7442 construct unsigned synthetic votes and round-trip them through the network parser. Develop now requires governance vote signatures to use a structurally valid 65- or 96-byte encoding, so the fixture is rejected as malformed before the authorization behavior under test is reached. This causes six unit-test failures across CI configurations. ## What was done? Give the synthetic `VOTE_SIGNAL_FUNDING` vote a compact-signature-sized placeholder. The 65-byte encoding matches the real ECDSA voting-key form for funding votes. These tests deliberately use a missing parent object, so vote processing takes the orphan path before cryptographic signature verification; production validation is unchanged. ## How Has This Been Tested? - Built `src/test/test_dash` in an isolated macOS arm64 worktree using the depends toolchain - `./src/test/test_dash --run_test=governance_inv_tests` - `./src/test/test_dash --run_test=governance_vote_wire_tests` - `git diff --check` - Mandatory independent pre-PR review gate: `ship` with no findings ## Breaking Changes None. Test-only change. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone ACKs for top commit: PastaPastaPasta: utACK e058152 Tree-SHA512: 5caca1e34ce563444ded42f669ff599f7658c6d1dbea1d541588211ac542ee45f3dd768e158899cb23150c5d45ec8dbcbf412a1472f73a3cc3e72a5ac37636e0
Issue being fixed or feature implemented
The governance inventory tests merged in #7442 construct unsigned synthetic votes and round-trip them through the network parser. Develop now requires governance vote signatures to use a structurally valid 65- or 96-byte encoding, so the fixture is rejected as malformed before the authorization behavior under test is reached. This causes six unit-test failures across CI configurations.
What was done?
Give the synthetic
VOTE_SIGNAL_FUNDINGvote a compact-signature-sized placeholder. The 65-byte encoding matches the real ECDSA voting-key form for funding votes. These tests deliberately use a missing parent object, so vote processing takes the orphan path before cryptographic signature verification; production validation is unchanged.How Has This Been Tested?
src/test/test_dashin an isolated macOS arm64 worktree using the depends toolchain./src/test/test_dash --run_test=governance_inv_tests./src/test/test_dash --run_test=governance_vote_wire_testsgit diff --checkshipwith no findingsBreaking Changes
None. Test-only change.
Checklist: