[feature] invalidate winners by ungaro · Pull Request #240 · plumenetwork/contracts

ungaro · 2025-10-22T14:17:38Z

What's new in this PR?

In bullet point format, please describe what's new in this PR.

Why?

What problem does this solve?
Why is this important?
What's the context?

octane-security-app · 2025-10-22T14:17:46Z

Summary by Octane

New Contracts

No new contracts were added in this PR.

Updated Contracts

Raffle.sol: The smart contract adds winner invalidation, valid winner counting, and includes new error checks to enhance prize management and integrity.

🔗 Commit Hash: 0775919

Copilot

Pull Request Overview

This PR adds functionality to invalidate raffle winners, allowing administrators to mark specific winners as invalid and enabling the prize to be redrawn. The implementation includes new error types, events, helper functions for managing winner validity, and a migration utility to backfill validity flags for existing winners.

Key changes:

Added invalidateWinner function for admins to invalidate specific winners
Introduced valid flag tracking in the Winner struct with helper functions
Added migration utility backfillValidFlags to update existing winner records

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 21 comments.

File	Description
plume/src/spin/Raffle.sol	Implements winner invalidation logic, validity tracking, and migration utilities
plume/test/Raffle.t.sol	Updates all test cases to include the new `formId` parameter in function calls

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-22T14:20:50Z

plume/src/spin/Raffle.sol

            revert NotAWinner();
        }

+        if (!individualWin.claimed) {


The condition is inverted - it should check if (!individualWin.valid) instead of if (!individualWin.claimed). Currently, this reverts when a winner hasn't claimed their prize yet, which is the opposite of the intended behavior. The check should verify that the winner is valid before allowing them to claim.

Suggested change

if (!individualWin.claimed) {

if (!individualWin.valid) {

Copilot · 2025-10-22T14:20:50Z

plume/test/Raffle.t.sol

    function testSpendRaffleInsufficient() public {
        vm.prank(ADMIN);
-        raffle.addPrize("A", "A", 0, 1);
+        raffle.addPrize("A", "A", 0, 1,"0");


[nitpick] Missing space after comma before "0". For consistency with other calls in the file (e.g., line 47, 69), add a space between the comma and the string argument.

Suggested change

raffle.addPrize("A", "A", 0, 1,"0");

raffle.addPrize("A", "A", 0, 1, "0");

Copilot · 2025-10-22T14:20:51Z

plume/test/Raffle.t.sol

    function testRequestWinnerEmitsEvents() public {
        vm.prank(ADMIN);
-        raffle.addPrize("A", "A", 0, 1);
+        raffle.addPrize("A", "A", 0, 1,"0");


[nitpick] Missing space after comma before "0". For consistency with other calls in the file (e.g., line 47), add a space between the comma and the string argument.

Copilot · 2025-10-22T14:20:51Z

plume/test/Raffle.t.sol

    function testClaimPrizeSuccess() public {
        vm.prank(ADMIN);
-        raffle.addPrize("A", "A", 0, 1);
+        raffle.addPrize("A", "A", 0, 1,"0");


[nitpick] Missing space after comma before "0". For consistency with other calls in the file (e.g., line 47), add a space between the comma and the string argument.

Suggested change

raffle.addPrize("A", "A", 0, 1,"0");

raffle.addPrize("A", "A", 0, 1, "0");

plume/test/Raffle.t.sol

octane-security-app · 2025-10-22T14:48:27Z

Overview

Vulnerabilities found:	4
Severity breakdown:	1 High, 1 Medium, 2 Low

Warnings found:	5

Detailed findings

`plume/src/spin/Raffle.sol`

Redundant O(n) scans in Raffle winner selection flow cause DoS of drawing winners. See more
Unbounded iteration in Raffle.getValidWinnersCount used in state-changing flows causes per-prize winner selection DoS. See more
Missing entry freeze during VRF pending state in Raffle causes late-entry dilution and compromised fairness. See more
Missing stale request invalidation in Raffle cancelWinnerRequest allows canceled VRF draw to execute. See more

Warnings

`plume/src/spin/Raffle.sol`

O(n) linear removal and unprunable completed prizes in Raffle causes self-DoS of removePrize. See more
Inconsistent validation allowing zero quantity in Raffle.editPrize causes prize-level DoS and stranded entries. See more
Unnecessarily payable receive() with no withdrawal in Raffle implementation causes ETH to be permanently locked. See more
Missing initialization address validation in Raffle.initialize causes denial of raffle functionality. See more
Unprotected initializer in Raffle implementation causes attacker-controlled raffle under admin misconfiguration, enabling unfair deduction of user raffle tickets. See more

🔗 Commit Hash: 0775919
🛡️ Octane Dashboard: All vulnerabilities

Copilot

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

plume/src/spin/Raffle.sol

Copilot

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-24T14:44:25Z

plume/src/spin/Raffle.sol

+        if (winnersDrawn[prizeId] > 0) {
+            winnersDrawn[prizeId] -= 1;
+        }
+        uint256 c = userWinCount[prizeId][w.winnerAddress];
+        if (c > 0) {
+            userWinCount[prizeId][w.winnerAddress] = c - 1;
+        }


[nitpick] The manual synchronization of winnersDrawn and userWinCount creates potential for inconsistency. Since getValidWinnersCount() now computes the actual count by iterating invalidWinners, consider whether winnersDrawn still serves a purpose or if it should be deprecated to reduce redundancy and maintenance burden.

Copilot · 2025-10-24T14:44:25Z

plume/src/spin/Raffle.sol

+        for (uint256 i = 0; i < arr.length; i++) {
+            if (!invalidWinners[prizeId][i]) {
+                count++;
+            }
+        }


getValidWinnersCount() iterates through all winners on every call. This function is called in requestWinner(), handleWinnerSelection(), setPrizeActive(), and view functions, creating O(n) overhead for each operation. For prizes with many winners, this could become expensive. Consider caching the valid count and updating it when winners are invalidated.

Copilot · 2025-10-24T14:44:25Z

plume/src/spin/Raffle.sol

        for (uint256 i = 0; i < wins.length; i++) {
-            if (wins[i].winnerAddress == user && (!onlyUnclaimed || !wins[i].claimed)) {
+            // Also check that winner is not invalidated
+            if (wins[i].winnerAddress == user && !invalidWinners[prizeId][i] && (!onlyUnclaimed || !wins[i].claimed)) {


[nitpick] This complex conditional with multiple && operators is difficult to parse. Consider extracting the invalidation check or other conditions into named boolean variables to improve readability, e.g., bool isValidWinner = !invalidWinners[prizeId][i];

Suggested change

if (wins[i].winnerAddress == user && !invalidWinners[prizeId][i] && (!onlyUnclaimed || !wins[i].claimed)) {

bool isWinner = wins[i].winnerAddress == user;

bool isValidWinner = !invalidWinners[prizeId][i];

bool isUnclaimedOrAll = !onlyUnclaimed || !wins[i].claimed;

if (isWinner && isValidWinner && isUnclaimedOrAll) {

raffle update invalidate winners

cf69066

Copilot AI review requested due to automatic review settings October 22, 2025 14:17

Copilot AI reviewed Oct 22, 2025

View reviewed changes

ungaro added 2 commits October 23, 2025 11:52

update raffle and tests

c507184

formatting

2e400a0

Copilot AI review requested due to automatic review settings October 23, 2025 15:55

Copilot AI reviewed Oct 23, 2025

View reviewed changes

plume/src/spin/Raffle.sol Outdated Show resolved Hide resolved

plume/src/spin/Raffle.sol Show resolved Hide resolved

ungaro added 2 commits October 23, 2025 11:59

update comments

6181b5d

update gap

0775919

Copilot AI review requested due to automatic review settings October 24, 2025 14:43

Copilot AI reviewed Oct 24, 2025

View reviewed changes

ungaro assigned ungaro and yogurtandjam and unassigned ungaro Oct 24, 2025

yogurtandjam approved these changes Oct 29, 2025

View reviewed changes

yogurtandjam merged commit dbe43fe into main Oct 29, 2025
2 checks passed

yogurtandjam deleted the alp/invalidate-winners branch October 29, 2025 18:18

	raffle.addPrize("A", "A", 0, 1,"0");
	raffle.addPrize("A", "A", 0, 1, "0");

-            if (wins[i].winnerAddress == user && !invalidWinners[prizeId][i] && (!onlyUnclaimed || !wins[i].claimed)) {
+            bool isWinner = wins[i].winnerAddress == user;
+            bool isValidWinner = !invalidWinners[prizeId][i];
+            bool isUnclaimedOrAll = !onlyUnclaimed || !wins[i].claimed;
+            if (isWinner && isValidWinner && isUnclaimedOrAll) {

Conversation

ungaro commented Oct 22, 2025

What's new in this PR?

Why?

Uh oh!

octane-security-app bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Octane

New Contracts

Updated Contracts

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

octane-security-app bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Detailed findings

plume/src/spin/Raffle.sol

Warnings

plume/src/spin/Raffle.sol

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

octane-security-app bot commented Oct 22, 2025 •

edited

Loading

octane-security-app bot commented Oct 22, 2025 •

edited

Loading

`plume/src/spin/Raffle.sol`

`plume/src/spin/Raffle.sol`