test: add coverage for governance and timelock authorization, initialization, and boundary paths (closes #1421)

[Cyber-Mitch]

closes #1421

Summary

Adds test coverage for the governance and timelock contracts, focused on authorization boundaries, initialization edge cases, boundary/failure paths, and event assertions. Closes #1421.

Changes

• contracts/governance/src/lib.rs (tests module, +29 tests)
• contracts/timelock/src/test.rs (+28 tests, +1 helper)

No contract logic (impl blocks, storage, error enums) was touched — git status --short confirms only test code changed.

Recon Summary

• Test locations confirmed: governance has no separate test file — tests live inline in lib.rs under #[cfg(test)] mod tests, so new tests were appended there. Timelock's tests are split across test.rs (canonical, 3 files total including a mock-auth/mock-target idiom) plus integration_test.rs; security_properties.rs is doc-only invariants, not test code.
• Baseline: cargo fmt --all -- --check, cargo clippy --workspace --all-targets --all-features, cargo test --workspace all passed unmodified (35 test binaries, 0 failures). Per-crate counts matched the issue exactly: governance 12, timelock 12.
• Pre-existing clippy warnings in both crates (e.g. dead-code in governance/lib.rs:120) are baseline and untouched.
• Neither crate had any existing event-assertion tests, so the event-assertion pattern was borrowed from elsewhere in the workspace (env.events().all() + try_into_val).

Governance mechanics (as found in code)

• Privileged (require_admin = require_auth + caller == Admin, else NotAuthorized): set_total_staked, set_voter_stake. Admin read uses .expect("not init") — a bare panic pre-init.
• Open (caller-authed, no admin check): create_proposal (require_auth + stake ≥ MIN_STAKE_TO_PROPOSE = 1), vote (require_auth), cancel_proposal (require_auth + proposer identity check). Plus read-only finalize_proposal, execute_proposal, get_proposal, proposal_count.
• Init: rejects double-init (AlreadyInitialized).
• Voting: weight = voter's stake read at vote time (a live read, not a proposal-creation snapshot); double-vote blocked; voting window enforced via now <= voting_ends_at.
• Quorum/threshold: quorum = total_staked × 1000 / 10000 (10%). Passes iff total_votes >= quorum and votes_for > votes_against (strict — a tie is Rejected).
• Timelock delay: execute_after = voting_ends_at + TIMELOCK_SECS (48h); rejects if now < execute_after (boundary inclusive at exactly execute_after). Executed proposals reject re-execution with ProposalAlreadyExecuted.
• Events: proposal_created, vote_cast (pid, voter, support, weight), proposal_finalized (pid, for, against, quorum_met), proposal_executed (pid, param_key, proposed_value), proposal_cancelled (pid).

Timelock mechanics (as found in code)

• Privileged (admin: require_auth + admin == Admin, else NotAuthorized): cancel, unpause, set_min_delay, expire. Some paths use bare .unwrap() → panic pre-init.
• Multisig-gated: emergency_pause — insufficient signers → InsufficientMultisigApprovals; each signer checked via require_auth + membership, else NotAuthorized.
• Open (no auth — the delay itself is the security control): execute.
• Init: double-init → AlreadyInitialized; min > max or min == 0 → InvalidDelay.
• Queue → execute → cancel/expire: delay bounded [min, max]; execute checks Paused → hash match → now >= eta (boundary inclusive) → now < eta + GRACE_PERIOD (14 days) before invoking. Cancel removes the queued entry (verified via re-execute attempt, not just the cancel call's return). Double-execute → TransactionNotQueued (entry already removed). expire requires the grace period to have elapsed.
• Events: queued, executed, cancelled, emergency_paused, expired (tx_hash, eta).

Coverage matrix (before this PR)

Criterion	Governance	Timelock
Unauthorized-caller rejection	cancel had it; set_total_staked/set_voter_stake MISSING	queue had it; cancel/unpause/set_min_delay/expire/emergency_pause membership MISSING
Double-init	MISSING	MISSING
Pre-init call	MISSING	MISSING
Delay boundary (exact vs. −1)	had −1 only, exact MISSING	had loose >/< eta checks, exact-eta + eta−1 pair MISSING
Cancel-then-execute (storage-verified)	n/a (no queue)	had 1 case; double-cancel re-verify MISSING
Double-execute	had it	MISSING
Double-vote	had it	n/a
Quorum/threshold exact & −1	had "not reached" only	n/a
Zero/overflow arithmetic	MISSING	delay-bounds partial; below-min/above-max/min>max/zero MISSING
Pause blocks execution	n/a	MISSING
Expire grace period	n/a	MISSING
Event assertions	ALL MISSING	ALL MISSING

Test List

Governance (+29 → 41 total):

• set_total_staked_unauthorized_caller_rejected, set_voter_stake_unauthorized_caller_rejected → NotAuthorized
• double_init_rejected → AlreadyInitialized
• set_total_staked_before_init_panics (bare panic), create_proposal_before_init_rejects_insufficient_stake → InsufficientStake (zeroed state), vote_before_init_not_found → ProposalNotFound
• execute_one_second_before_timelock_rejected → TimelockNotElapsed; execute_exactly_at_timelock_boundary_succeeds → Executed (boundary inclusive)
• vote_on_nonexistent_proposal_rejected, vote_after_voting_period_rejected (VotingNotEnded), vote_on_inactive_proposal_rejected (ProposalNotActive)
• finalize_before_voting_ends_rejected (VotingNotEnded), finalize_nonexistent_proposal_rejected, finalize_already_finalized_rejected (ProposalNotActive)
• execute_unfinalized_proposal_rejected, execute_rejected_proposal_rejected → ProposalNotPassed (proves proposals can't execute without passing), execute_nonexistent_proposal_rejected
• cancel_nonexistent_proposal_rejected, cancel_already_finalized_rejected (ProposalNotActive)
• quorum_exactly_at_threshold_passes (100_000 == quorum → Passed, asserts on votes_for), quorum_one_below_threshold_rejected (99_999 → Rejected), tie_vote_is_rejected (equal for/against → Rejected), zero_total_staked_makes_quorum_trivial → Passed (flagged as a FINDING below), vote_counting_overflow_panics (#[should_panic("overflow")], i128::MAX + 1, flagged below)
• create_proposal_emits_event, vote_emits_event, finalize_emits_event, execute_proposal_emits_event, cancel_emits_event — assert exact topics and decode full event-data tuples (ids, addresses, weights, values)

Timelock (+28 → 40 total):

• non_admin_cannot_cancel/unpause/set_min_delay/expire → NotAuthorized; non_member_cannot_emergency_pause; emergency_pause_insufficient_signers_rejected (1 of 2 required signers) → InsufficientMultisigApprovals
• double_init_rejected → AlreadyInitialized; init_min_greater_than_max_rejected → InvalidDelay; execute_before_init_returns_error; cancel_before_init_panics (#[should_panic], bare .unwrap())
• queue_delay_below_min_rejected, queue_delay_above_max_rejected (604801) → InvalidDelay
• execute_one_second_before_eta_rejected (TimelockNotMet), execute_exactly_at_eta_succeeds (real target invoked, confirmed via post-state)
• double_execute_rejected (TransactionNotQueued), execute_while_paused_rejected (ContractPaused, even with valid eta)
• cancel_nonexistent_rejected, cancel_confirms_removal (re-cancel → TransactionNotQueued, proving removal), expire_before_grace_elapsed_rejected (TimestampNotMet), expire_after_grace_removes_operation (then re-execute → TransactionNotQueued), expire_nonexistent_returns_error
• set_min_delay_zero_rejected, set_min_delay_above_max_rejected → InvalidDelay
• queue_emits_event, execute_emits_event, cancel_emits_event, emergency_pause_emits_event, expire_emits_event — assert exact namespace+name topics and decode full event data (hash, target, function, eta)

Ambiguous Behavior — Needs Maintainer Confirmation

No test was written to encode a guess for any of the following; each was left as-is with the currently-observed behavior only.

1. Governance pre-init inconsistency: create_proposal and cancel_proposal are callable before init (they don't read Admin; only set_* functions panic pre-init). create_proposal pre-init returns InsufficientStake rather than an explicit "not initialized" error. Tests pin the currently-observed typed errors — is this the intended contract, or should all entrypoints uniformly reject pre-init?
2. Governance vote-weighting timing: vote reads the voter's current stake at vote time, not a proposal-creation-time snapshot — despite a VoterSnapshot-shaped storage key existing and an existing baseline test (flash_stake_voting_prevented) whose own comments suggest snapshotting was intended, deferred pending a staking-pool cross-call integration. Is current-stake voting the intended production behavior, or a known placeholder? (Related to Finding 1 below.)
3. get_stake_for key reuse: reuses DataKey::Voted(0, voter) as a stake-weight storage slot, per an inline comment suggesting it's a placeholder pending the real staking-pool cross-call. Is this namespace reuse intentional or a temporary shim?

Implementation Findings

No test demonstrates a bypassable timelock delay, a non-cancelling cancel, a double-execute, or an executable-without-passing proposal — all core guards from the issue's stated risk list hold. Two findings surfaced during boundary testing, tied to the issue's stated "governance capture" and arithmetic-safety concerns:

• FINDING (Medium) — governance quorum is defeatable via zero/misconfigured total_staked. With total_staked = 0, the required quorum computes to 0, so a single minimal-stake vote can pass a proposal with effectively no quorum. set_total_staked has no floor/validation, so a wrong or zero value silently removes quorum protection. Demonstrated by zero_total_staked_makes_quorum_trivial.
• FINDING (Low) — unguarded arithmetic in vote tallying. votes_for/votes_against += weight uses plain i128 addition and can overflow-panic given admin-set adversarial stake weights (demonstrated by vote_counting_overflow_panics). Low severity since it requires an admin-controlled input, but it's an unguarded arithmetic path where the rest of the codebase generally uses overflow-checked (rather than wrapping) arithmetic.

Per the issue's instructions, contract logic was not modified and no separate issue was opened — a follow-up issue should be filed covering (i) a quorum/total-staked floor and (ii) confirming intended vote-weighting timing (ambiguity #2 above).

Separately noted, out of scope: running the full workspace test suite regenerated a pre-existing stale snapshot at contracts/contract_access/tests/delegate_permission_grants_access.1.json (a u32: 6 value is now serialized as a map). This is unrelated to governance/timelock, was reverted before committing so it's not in this diff, but indicates contract_access has an out-of-date committed snapshot worth a separate look.

Test Results

$ cargo fmt --all -- --check
PASS — clean, no diff

$ cargo clippy --workspace --all-targets --all-features
exit 0 — governance and timelock at baseline warning counts, zero new warnings introduced

$ cargo test --workspace
exit 0 — 35 test binaries, zero failures
governance: running 41 tests → ok. 41 passed; 0 failed
timelock: running 40 tests → ok. 40 passed; 0 failed

Checklist

Governance (governance/src/lib.rs):

• Every privileged function has an unauthorized-caller test — set_total_staked L764, set_voter_stake L777
• Init edge cases — double-init L792, pre-init panic L808, pre-init typed errors L821/L840
• Failure/boundary paths — timelock boundary L876/L892, quorum L1098/L1120, tie L1137, zero-quorum finding L1162, execute/cancel failure paths L909–L1090
• Events asserted — L307 (all 5 events)
• Meaningful assertions — decoded event-data tuples and specific typed error variants, not just pass/fail

Timelock (timelock/src/test.rs):

• Every privileged function has an unauthorized-caller test — cancel L333, unpause L365, set_min_delay L384, expire/emergency_pause L462 (queue was pre-existing)
• Init edge cases — double-init, pre-init L509/L529
• Failure/boundary paths — eta boundary L615/L640, double-execute L667, paused L699, cancel/expire/delay bounds L554/L583/L918/L936
• Events asserted — L4xx (all 5 events)
• Meaningful assertions — typed Err(Ok(TimelockError::…)) + decoded event data + post-state re-execution checks
• cargo fmt / clippy / test --workspace all pass

[vercel]

@Cyber-Mitch is attempting to deploy a commit to the pope-h's projects Team on Vercel.

A member of the Team first needs to authorize it.

[drips-wave]

@Cyber-Mitch Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits