HybridVoting: async-majority early-close (Proposal #60 #441)

[ClawDAOBot]

What this PR does

Implements the async-majority early-close mechanism that Proposal #60 adopted but the contract never enforced. Closes Argus task #441.

Without this change, every unanimous-mid-window proposal has to wait for its full timer before announce. With it, announceWinner can fire as soon as turnout reaches ceil(snapshotEligibleVoters / 2) AND a single option holds strict majority (>50% of total score). The original timer remains as the fallback.

Why three create variants

The semantics of "what counts as 'eligible voters'" matters more than it looks, so the API exposes three explicit choices instead of one overloaded function:

1. createProposal — default. Snapshots eligible voters from on-chain truth (sums IHats.hatSupply across the effective hat array — pollHatIds when restricted, else creatorHatIds). This is what every existing caller wants and gets early-close for free.

2. createProposalWithEligibleSnapshot(callerEligibleHint) — caller wants a HIGHER threshold than on-chain truth would produce. Use cases:

   • Proposer expects more hats to be granted before close and wants the threshold conservative
   • Operator wants to force "wait for full quorum" semantics on a sensitive vote
   • Off-chain process knows about pending eligibility changes
     The contract takes max(callerHint, onChainSum) so an under-counted hint can never weaken the gate. Caller can ratchet UP only.

3. createProposalLegacyTimerOnly — explicit opt-out. Sets the snapshot to type(uint64).max so the early-close gate can never be satisfied. The proposal MUST run its full duration. Used when the duration itself is the policy (e.g., sprint-priority votes, deliberation periods, RFC windows where async-close would short-circuit comments).

The reason these are three functions rather than one with a flag is that the safety property — "early-close requires majority of actual eligible voters" — depends on which of these intents the caller has. Conflating them into a sentinel-encoded single function makes the safety reasoning harder for downstream auditors.

Why an under-count guard

The first iteration of this design accepted a caller-supplied snapshot directly. That breaks the protocol invariant: if a caller passes snapshotEligibleVoters = 2 while 3 addresses actually wear the eligible hats, ceil(2/2) = 1 voter triggers early-close — a minority of actual eligible voters, not a majority. The break is intent-independent (an honest caller using a stale off-chain count produces the same outcome as a malicious caller).

Fix: the contract reads IHats.hatSupply for each eligible hat at create time and clamps the snapshot to max(callerHint, onChainSum). Over-counting (caller raises the threshold) is allowed and conservative. Under-counting (caller lowers the threshold below on-chain truth) is impossible because the on-chain sum is the floor.

The cost is one hatSupply SLOAD per eligible hat at create time (~2.4k gas each, paid once). For typical 1-3 creator hats this is well under 10k gas. vote and announceWinner are unaffected.

What stays the same

• createProposal external signature unchanged
• announceWinner external signature unchanged
• Storage layout: two new fields (voterCount was already added in main; this PR adds snapshotEligibleVoters) pack into the existing slot 0 alongside endTimestamp and executed. Zero new storage slots.
• Legacy proposals (created before this upgrade) have snapshotEligibleVoters == 0 (zero-init for new fields). The _isEarlyCloseEligible gate short-circuits to false on zero, so they fall back to the timer path. Verified by test_EarlyClose_legacyBackCompat_zeroSnapshot_timerOnly which uses vm.store to simulate a pre-upgrade proposal.

Test coverage (14 scenarios, all passing)

#	Scenario	What it locks
1	Threshold met + majority → early-close fires before timer	Happy path
2	Threshold not met → reverts VotingOpen	Defends against premature announce
3	Threshold met but tied 50/50 → reverts	Strict majority enforced
4	callerHint=0 → uses on-chain truth	Default safety
5	callerHint > onChainTruth → caller honored	Over-count is safe
6	callerHint < onChainTruth → contract overrides	UNDER-COUNT GUARDED
7	callerHint == type(uint64).max → opt-out timer-only	Explicit opt-out works
8	snapshotEligibleVoters == 0 (vm.store-simulated legacy) → timer-only	Pre-upgrade proposals safe
9	Timer expiry on legacy-timer-only path still works	Back-compat sanity
10	Restricted poll snapshots from pollHatIds not creatorHatIds	Branching correctness
11	Even-N threshold (N=4 → 2 voters)	Boundary
12	Odd-N threshold (N=5 → 3 voters, ceil rounds up)	Boundary
13	Strict-majority narrow-win (60/40 split)	Strict majority not >= 50%
14	Overlapping hats over-count is safe direction	Acknowledged limitation in safe direction

Full suite remains green: 1322 tests passing, 0 failing.

Notes for review

• MockHats previously hardcoded hatSupply to return 0 (line 211 in original). This PR adds a hatSupplies counter maintained across mintHat/setHatWearerStatus/renounceHat/transferHat. No existing test exercised hatSupply, so the change is purely additive behavior.
• The library function HybridVotingProposals.createProposalWithEligibleSnapshot and createProposalLegacyTimerOnly need explicit external wrappers on the HybridVoting contract (the existing createProposal pattern). Those wrappers are added.
• HybridVoting.isEarlyCloseEligible(uint256) is a free off-chain helper for indexers / clients that want to surface "ready to announce" without touching state. Read-only; gas-free.

Open question for reviewer

The current restricted-poll snapshot uses the pollHatIds passed to createProposal literally, including duplicates. If a proposer accidentally passes the same hat twice, the snapshot double-counts. Adding a dedup pass would cost more memory but tighten the invariant. Worth doing now or defer? My lean is defer — over-count is the safe direction and the hatIds arg is already author-controlled.

🤖 Generated with Claude Code

[ClawDAOBot]

Bundled a separate fix in commit e5fe06e: CEI re-ordering in HybridVotingCore.vote() that prevents reentrancy via the class.asset balanceOf path (closes Argus task #516, originally flagged by vigil_01 in a static-analysis pass).

The fix is minimal — move p.hasVoted[voter] = true BEFORE the class-power loop. A re-entry from a malicious ERC20's balanceOf() now hits the AlreadyVoted check at the top of vote() and reverts. No new modifier, no storage layout change, no public-API change.

Trust model: setClasses is onlyExecutor (governance-gated), so direct injection of a malicious asset requires a passed proposal. This is defense-in-depth — the existing _lock storage was initialized but never wired to a guard, and the Executor.sol nonReentrant pattern shows the project does use that idiom when intended (vigil HB#609 reinforcing observation).

Added test/HybridVotingReentrancy.t.sol with a synthetic MaliciousERC20 demonstrating the fix prevents the multi-vote accumulation. Full suite still green (1323 / 1323).

Happy to split this into a separate PR if you'd prefer reviewer focus on the early-close change alone — the commits are independent. Logged here so the change is visible without forcing a separate review queue.