Union Protocol: Base Tests + Documentation#228
Open
ValentinVaninetti wants to merge 5 commits intomainfrom
Open
Union Protocol: Base Tests + Documentation#228ValentinVaninetti wants to merge 5 commits intomainfrom
ValentinVaninetti wants to merge 5 commits intomainfrom
Conversation
…s for how to run tests and what does this tests do
- Refactor TEST_GUIDE.md explaining test design principles and what each test validates - Document real-world scenarios and security considerations (Unicode attacks, overflow) - Explain helper architecture (generators, validators, oracles) - Include rationale for testing patterns and edge cases - Add future improvements section (proptest, fuzzing)
Removed references to ARCHITECTURE.md and FINAL_SUMMARY.md from README.
- Add #[cfg(test)] guards + security warnings to test key generators - Fix 6 weak assertions using proper output type helpers - Remove 2 redundant weight calculation tests - Add expected panic messages for overflow tests - Document test oracle pattern and BIP-141 references Tests: 168 passing (was 170, removed 2 redundant)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds comprehensive test coverage for Union protocol operations with supporting infrastructure and documentation.
Tests: 168 passing (<30ms total)
Coverage: Fee estimation, output construction, protocol IDs, transaction naming
What's Included
New Test Modules
Fee Estimation (25 tests)
Transaction weight calculation (BIP-141), monotonicity validation, overflow protection, boundary cases
Output Builders (18 tests)
P2WPKH operator outputs and Taproot deposit construction with dust threshold validation
Expanded Coverage
Common Utilities (50 tests)
Protocol ID generation with determinism and collision prevention across committee operations
Indexed Names (73 tests)
Transaction naming format validation with security checks (Unicode edge cases, leading zeros, malformed inputs)
Helper Infrastructure (tests/helpers/)
test_helpers.rs - Deterministic fixtures (keys, committees, Bitcoin constants) with #[cfg(test)] guards
test_generators.rs - Algorithmic test case generation (boundaries, powers of 2/10, fee combinations)
test_validators.rs - Independent validation oracle (Bitcoin Core methodology)
test_assertions.rs - Domain-specific assertions with clear error messages
Scripts
run_union_tests.sh - Filtered test execution with summary output
ci_check.sh - Pre-push validation (format, compile, test)
Key Design Decisions
Test Oracle Pattern:
test_validators.rs uses independent validation logic separate from production code. This follows Bitcoin Core's methodology - if production validation is refactored incorrectly, the independent oracle catches regressions.
Edge Case Coverage:
Bitcoin transaction handling requires testing boundary values (0, 1, u64::MAX), overflow scenarios (attack vectors in fee estimation), and relay policy compliance (546 sat dust threshold).
Safety:
Test-only crypto helpers guarded with #[cfg(test)] and explicit security warnings
Protocol invariant tests reference BIPs (BIP-141 for SegWit, BIP-340 for Taproot)
Removed 2 redundant tests after coverage analysis
**Review Focus:
Do tests validate actual Bitcoin behavior per BIPs?
Are critical invariants (fee estimation, output construction) properly tested?
Is test-only crypto material properly isolated?**