Skip to content

Fix/combined rebuild quality - #469

Open
Hallab7 wants to merge 6 commits into
TevaLabs:mainfrom
Hallab7:fix/combined-rebuild-quality
Open

Fix/combined rebuild quality#469
Hallab7 wants to merge 6 commits into
TevaLabs:mainfrom
Hallab7:fix/combined-rebuild-quality

Conversation

@Hallab7

@Hallab7 Hallab7 commented Aug 28, 2026

Copy link
Copy Markdown

Combined Quality & Rebuild Fixes

Summary

This PR consolidates four critical quality improvements and feature rebuilds into a single, tested integration:

  1. Access Control Error Variants (Quality: add missing AccessDenied (and related) error variants #384): Added missing AccessDenied and related error codes to ContractError, ensuring proper error handling and binding parity across the contract and TypeScript bindings.

  2. Dispute Window Lifecycle Rebuild (Rebuild: dispute window void_round + finalize_round end-to-end #389): Rebuilt the complete dispute window implementation including void_round (permissionless refunds during window) and finalize_round (settlement after window), with configurable ledger windows, full stake conservation, and comprehensive event coverage.

  3. Security Test Deduplication (Quality: deduplicate quadrupled security.rs test suite #385): Removed ~5,400 lines of quadrupled test code from security.rs, reducing CI time while maintaining full coverage of oracle freshness, nonce replay, domain binding, and network validation.

  4. Precision Commit-Reveal Hardening (Rebuild: Precision commit-reveal with forfeiture and grinding defenses #394): Hardened commit-reveal flow with unrevealed position forfeiture, weak salt protection, and grinding defense mechanisms, including comprehensive E2E tests and event emissions.

Why? These changes address critical gaps in error handling, dispute resolution workflows, test maintainability, and Precision mode security—all essential for production readiness.


Linked issues


Docs reference

  • PROTOCOL_SPEC.md — invariants (I1–I13) updated if affected
    • No protocol invariants affected; changes are implementation-level quality and error handling
  • docs/EVENT_SCHEMA.md — event schema updated if new/changed events
    • Updated: Added dispute window events (dispute_void, dispute_finalized)
    • Updated: Added commit-reveal events (commit_received, reveal_matched, forfeit_unrevealed)
  • MIGRATION.md — breaking changes documented with migration path
    • No breaking changes; new error codes are additive, dispute APIs were incomplete
  • COMPATIBILITY_POLICY.md — MAJOR/MINOR/PATCH classification noted
    • Classification: MINOR (additive error codes, completed dispute APIs, internal test cleanup)
  • SECURITY_REVIEW.md — open findings referenced if affected
    • Addresses: Commit-reveal grinding and forfeiture handling (Precision mode security)
  • contracts/BENCHMARKS.md — benchmark evidence included if hot paths changed
    • No hot path changes; dispute window and commit-reveal are cold paths

Validation

  • cargo test --workspace — All tests pass including new dispute window and commit-reveal E2E tests
  • cargo clippy --workspace --all-targets -- -D warnings — No warnings
  • cargo fmt --all -- --check — Formatting verified
  • cd bindings && npm ci && npm run build — Bindings regenerated with new error codes

Governance checklist

  • I reviewed CONTRIBUTING.md for workflow expectations
  • I reviewed the Contributor Task Matrix for domain-specific requirements
  • I checked CODEOWNERS impact for touched paths
    • Affected: contracts/src/, bindings/, docs/
  • I followed SUPPORT.md disclosure guidance for any security-sensitive change
    • Security-sensitive: Commit-reveal hardening documented in commit messages

Labels

  • protocol — Dispute window settlement logic
  • security — Commit-reveal forfeiture and grinding defenses
  • bug — Missing error variants, incomplete dispute APIs
  • enhancement — Test suite cleanup
  • testing — New E2E coverage for dispute and commit-reveal
  • contract — Core Rust contract changes
  • Rust — Primary language
  • priority: high — Production-blocking quality and security gaps

Smart-Contract Security Checklist

1. Authentication & Access Control

  • Every state-mutating method verifies caller authorization using require_auth() or appropriate admin/oracle checks.
    • void_round is permissionless (by design, during dispute window)
    • finalize_round is permissionless (after window expires)
    • Access control errors now properly mapped with AccessDenied variants
  • Access control policies align with security specifications (e.g., admin-only vs oracle-only vs user-only).
    • Documented in error handling and dispute window logic

2. Safe Arithmetic & Overflow Protection

  • Checked/safe math operations (checked_add, checked_sub, checked_mul, etc.) are used for all state changes.
    • Stake refunds in void_round use safe arithmetic
    • Conservation tests verify no stroop leakage
  • Precision math operations use specialized safe helpers (e.g., payout_add / payout_mul) where applicable.
    • Forfeiture handling uses existing safe payout primitives

3. Lifecycle & State Transitions

  • Mutating actions are correctly gated by runtime mode checks (e.g., disabled during emergency modes, allowed during claims-only).
    • void_round only callable during dispute window (before dispute_ledgers_after_resolution)
    • finalize_round only callable after window expires
    • Commit-reveal enforces reveal deadline and forfeiture rules
  • Invariants (like "exactly one active round") are preserved before/after execution.
    • Dispute void archives round, clearing active state
    • Conservation tests validate stake accounting

4. Event Emission & Observability

  • Canonical events are emitted for all key state transitions (e.g., round created, bet placed, resolution, cancellation).
    • dispute_void event on void_round with refund details
    • dispute_finalized event on finalize_round
    • commit_received, reveal_matched, forfeit_unrevealed events for Precision flow
  • Forensic summary events are generated correctly with compact and stable metadata.
    • Documented in EVENT_SCHEMA.md

5. Tests & Verification

  • Unit tests cover both successful executions and expected failure/rejection paths.
    • Negative tests for AccessDenied errors
    • Void blocked after window; finalize blocked during window
    • Unrevealed positions forfeit, cannot win
    • Weak salt scenarios documented
  • Property/invariant tests or edge cases are added for new protocol changes.
    • Conservation tests for dispute void vs normal resolution
    • E2E commit-reveal grind/grief attempt tests

Critical Path Changes

  • Does this PR modify contract payout, resolution, or claim paths?
    • Yes

Details:

Dispute Window Settlement: Added void_round as an alternative resolution path that refunds all stakes without computing payouts. This path is only active during the dispute window and requires full conservation (every participant receives exactly their stake back). Finalize path remains unchanged but now properly enforces window expiration.

Commit-Reveal Forfeiture: Modified Precision settlement to exclude unrevealed positions from winning eligibility. Unrevealed stakes are redistributed to revealed winners or refunded if no valid reveals exist. This hardens the commit-reveal game against grinding attacks.

  • Are there any new failure modes introduced by these changes?
    • Yes

Details:

New Failure Modes:

  1. Void Race Condition: If void_round and finalize_round are called simultaneously at window boundary, one will fail. Mitigation: Window boundary is strictly enforced via ledger timestamp checks.
  2. Mass Unrevealed Scenario: If all participants commit but none reveal, round enters forfeit-all state. Mitigation: Full refund fallback implemented; documented as expected behavior for failed rounds.
  3. Access Control Errors: New error codes could break client error parsing if not updated. Mitigation: Bindings regenerated with parity tests; breaking change classified as MINOR (additive).

Snapshot policy

  • If snapshot files under contracts/test_snapshots/ changed, I reviewed the diff and confirmed every change is intentional
    • Reviewed: New snapshots for dispute window events and commit-reveal forfeiture flows
    • Reviewed: Security test deduplication removed redundant snapshots (expected)
  • If snapshot drift was reported in CI, I either regenerated snapshots or marked the drift as expected in the PR description
    • Snapshots regenerated for new event schemas and dispute flows

File Change Summary

15 files changed | +749 insertions | -5,579 deletions

Key Changes:

  • Bindings: Updated with new error codes and helpers
  • Contracts:
    • access_control.rs: Added AccessDenied mapping
    • errors.rs: New error variants (AccessDenied, NotOracleRole, etc.)
    • settlement.rs: Complete dispute window implementation
    • betting.rs: Commit-reveal forfeiture logic
  • Tests:
    • dispute_window.rs: New 128-line test suite
    • commit_reveal_e2e.rs: Hardened with grind scenarios
    • security.rs: Reduced by 5,448 lines (deduplicated)
  • Documentation:
    • EVENT_SCHEMA.md: 86 lines updated with new events

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant