Approved workflow

settle() derives entropy only from the secrets that were actually revealed, and forfeited tickets stay in the ticketsSold modulus while the forward walk redirects their probability to the next revealed ticket. A buyer holding k tickets therefore does not face one draw; he faces 2^k draws, because every subset of his own tickets that he chooses to reveal produces a different entropy and a different winner.

He enumerates all 2^k subsets off-chain after the honest reveals are visible in the mempool, then reveals only the subset that names one of his own tickets.

Withholding is free: the stake behind each forfeited ticket is still in the pot he collects, so a successful attack costs nothing beyond gas.

This is an amplification of the commit-reveal weakness the brief mandates and the README already discloses in general terms (README.md:88 'Holding k tickets gives up to 2^k choices'), but neither the README nor the launch.json notes state the magnitude or that the attack is cost-free, and apps/control-plane admission reviews economics on the basis of those statements.

Fixing it requires changing the agreed winner rule (for example drawing only over the revealed tickets, or requiring a forfeitable reveal bond), which is a scope decision for the brief owner, not a change a reviewer should prescribe. The concrete, non-design part that should be corrected now is the disclosure: README.md:88 and the launch.json notes understate this as a last-revealer nuisance rather than a near-deterministic capture by any multi-ticket holder.

claim() sets r.claimed = true and then reads amount = r.pot but never writes r.pot back to zero (src/CommitRevealRaffle.sol:215-219).

There is no double-spend, because claimed guards the second call and settle only reads pot for the round it is settling, so the on-chain accounting stays solvent (verified by a 256-run fuzz over 10 rounds with rollovers and interleaved late claims: token.balanceOf(raffle) equals the sum of unclaimed winning pots plus the current round's pot at every step). The defect is in reported state: every consumer of getRound() sees a paid-out round advertising its full prize.

The website this stage feeds is specified to 'see the round and its winner, claim', so it will read exactly this field. The test suite does not catch it because test_claimPaysPot (test/CommitRevealRaffle.t.sol:476) asserts the token balances and claimed, and never asserts anything about pot after the payout. Zeroing r.pot before the transfer keeps checks-effects-interactions and changes no behaviour any other function depends on.

buyTicket rejects a commitment already used in the round. The commitment is public in the pending transaction, so anyone watching the mempool can submit the identical commitment with a higher gas price; the victim's transaction then reverts with CommitmentAlreadyUsed and the victim must generate a new secret and pay gas again.

The README (line 44 area, 'Commitments must be non-zero and unique within a round, which stops someone copying a commitment and later replaying its revealed secret') states a rationale the check does not actually provide: reveal() is already gated on t.buyer == msg.sender, so a copied commitment cannot be used to reveal someone else's ticket, and the copier who does not know the preimage can only forfeit the ticket he bought.

The check therefore buys no security while creating this griefing vector. The attacker pays TICKET_PRICE (100 RAFL) per grief and the ticket he bought is dead weight he cannot reveal, so the impact is bounded; but the extra unrevealed ticket also enlarges the ticketsSold modulus and, through the forward walk at src/CommitRevealRaffle.sol:198, shifts probability onto whichever ticket follows it.

Removing the uniqueness check, or binding the commitment to the buyer (keccak256(secret, msg.sender)), both remove the vector; either is a behavioural change and should be decided by the brief owner rather than applied by a reviewer.

README.md:80 asserts 'The raffle's RAFL balance always equals the sum of unclaimed winning pots, plus the pot of the current round (which includes any rollover).' That is false for any RAFL sent to the contract by a plain token transfer rather than through buyTicket: those tokens are credited to no round's pot, and because the contract has no owner, no sweep and no withdrawal path other than claim(), they are stuck forever.

This is the correct consequence of the no-owner design and is not itself a defect worth changing the contract for, but the stated invariant is what a monitor, a subgraph or the planned website would use to reconcile the contract's balance against its rounds, and it will disagree with the chain the first time a user mistypes a transfer to the raffle address (an address the website will display).

The invariant should be restated as 'balance >= sum of unclaimed pots plus the current round's pot, with any excess being unrecoverable donations'.

testFuzz_conservation only ever plays round 1 and claims the prize immediately, and test_claimAfterLaterRoundsStillWorks covers exactly two rounds with one deferred claim.

The property that actually matters for a contract that pays every round from one shared token balance - that token.balanceOf(raffle) always equals the sum of unclaimed winning pots plus the current round's pot - is never asserted for the case the contract is built around: several settled-but-unclaimed rounds outstanding at once, mixed with no-reveal rollovers.

I reconstructed that test and ran it, and the invariant holds (256 fuzz runs, 10 rounds each, random ticket counts 1-4, random reveal masks, random interleaved late claims; every leftover winner was still payable at the end). So this is a coverage gap with no latent bug behind it, but it is the one property whose absence would have hidden a real loss of funds, and it is cheap to add.