Fix issue #14 by Fiuum1 · Pull Request #67 · BreadchainCoop/safety-net

Fiuum1 · 2026-03-13T16:40:10Z

As requested, this PR implements the solution for Issue #14: the system now uses a 33% contest threshold to trigger a veto.

Here is a technical overview of the modifications I've made, although GitHub already gives us an overview of the modifications:

SafetyNet.sol:

'isContested[]' replaced by 'isVetoed[]'
Added 'contestCount' initialization (=0) in new Request creation in '_withdraw()'
Added 'contestCount' increment in 'contest()'
Removed 'vote()' function
Removed 'requestVotes' parameter (mapping)
Removed "_isVotingOnGoing()' function
Modified 'createRequest()' function, replacing 'yesVotes' and 'noVotes' initialization (=0) with 'contestCount' initialization (=0)
Added in 'contest()' a mechanism to check if contestCount is enough (compared to threshold) to reject withdrawal
Replaced 'consensusTreshold' parameter in 'SafetyNetCreated' event in 'create()' function with 'creationstruct with 'contestTreshold' parameter
Added "emit WithdrawalVetoed()" in contest()
Added 'AlreadyContestedByMember()' error in contest()
Added 'hasContested' parameter (mapping)
Added "if (hasContested[_requestId][msg.sender]) revert AlreadyContestedByMember()" in contest()

ISafetyNet.sol:

'AlreadyContested()' replaced by 'AlreadyVetoed()'
Removed 'WithdrawalContested()' event
Removed 'yesVotes' e 'noVotes', replaced by 'contestCount'
Removed 'vote()' interface
Removed 'Voted()' event
Removed 'AlreadyVoted()' error
Removed 'NotAllVoted()' error
Removed 'votingWindow' parameter from 'SafetyNet' struct
Modified 'RequestEnded()' event
Removed 'VotingWindowsClosed()' error
Replaced 'consensusTreshold' parameter in 'SafetyNet' struct with 'contestTreshold' parameter, representing the percentage of members who contest required to reject a request
Added 'WithdrawalVetoed()' event
Added 'AlreadyContestedByMember()' error

All tests (Unit and Fuzz) have been updated and pass successfully.

Looking forward to your feedback!
Fabio

…eadchainCoop#14)

Fiuum1 · 2026-03-13T16:41:29Z

P.S: Sorry but I forgot to remove modifications.txt from src/contracts/

Copilot

Pull request overview

This PR refactors the SafetyNet withdrawal-request governance flow from a vote-based mechanism to a contest/veto-based mechanism, updating the core contract interface and aligning unit/fuzz tests with the new request model.

Changes:

Replace request voting (yes/no votes + voting window) with contest counting and a veto threshold.
Update ISafetyNet structs/events/errors and the SafetyNet contract storage/execution logic to match the contest/veto flow.
Remove voting-focused fuzz tests and add new fuzz coverage for contesting/veto boundaries and timeout behavior.

Reviewed changes

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

Show a summary per file

File	Description
test/unit/SafetyNetUnit.sol	Updates unit tests to use `contestThreshold`, `contestCount`, and veto semantics.
test/invariants/fuzz/SafetyNetFuzzBase.t.sol	Updates fuzz default config to use contest-based parameters (removes voting window defaults).
test/invariants/fuzz/SafetyNetFuzz_RequestsVoting.t.sol	Removes the voting-oriented fuzz suite.
test/invariants/fuzz/SafetyNetFuzz_RequestsContesting.t.sol	Adds contest/veto-focused fuzz scenarios and threshold-boundary properties.
test/invariants/fuzz/SafetyNetFuzz_DepositWithdraw.t.sol	Updates request struct destructuring for the new `Request` layout.
test/invariants/fuzz/SafetyNetFuzz_Create.t.sol	Updates creation fuzzing to vary `contestThreshold` instead of consensus/voting fields.
src/interfaces/ISafetyNet.sol	Renames/reshapes config and request fields/events/errors to reflect contest/veto instead of voting.
src/contracts/SafetyNet.sol	Implements contest counting + vetoing, removes voting state and vote execution path.
src/contracts/modification.txt	Adds an empty file under contracts (no content).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

src/interfaces/ISafetyNet.sol

src/contracts/SafetyNet.sol

test/unit/SafetyNetUnit.sol

test/invariants/fuzz/SafetyNetFuzz_RequestsContesting.t.sol

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

exo404 · 2026-03-14T20:08:46Z

@Fiuum1 please solve every copilot issue in a separate commit and replay to every comment with the commit ID

…t hardcoded 33%

…d ofmemory for safetyNet struct

… than 100 when tested [now 150]

Fiuum1 · 2026-03-15T15:23:29Z

@exo404 All done! Lmk if I need to do anything else.

exo404 · 2026-03-17T17:24:44Z

lgtm @RonTuretzky @Fiuum1

Fiuum1 · 2026-03-17T17:28:17Z

lgtm @RonTuretzky

src/contracts/SafetyNet.sol

…t WithdContested()

…re veto

Copilot

Pull request overview

This PR updates the SafetyNet withdrawal flow to match Issue #14 by removing the voting phase and instead vetoing a withdrawal request if more than a configured percentage of members contest it (defaulted to 33% in tests).

Changes:

Replaced request voting (yes/no votes + voting window) with a contest counter and veto logic based on contestThreshold.
Updated unit + fuzz/invariant tests to exercise contest/veto behavior and removed the prior voting-oriented fuzz suite.
Updated the public interface (ISafetyNet) to reflect the new request fields and events.

Reviewed changes

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

Show a summary per file

File	Description
`src/contracts/SafetyNet.sol`	Implements `contestCount` + veto threshold logic; removes voting; introduces `isVetoed` and `hasContested`.
`src/interfaces/ISafetyNet.sol`	Updates structs/events/errors to remove voting and add veto/contesting semantics.
`test/unit/SafetyNetUnit.sol`	Updates unit tests for new contest/veto behavior and request struct shape.
`test/invariants/fuzz/SafetyNetFuzzBase.t.sol`	Updates fuzz defaults to use `contestThreshold` and removes voting window config.
`test/invariants/fuzz/SafetyNetFuzz_DepositWithdraw.t.sol`	Adjusts request destructuring/assertions for `contestCount`.
`test/invariants/fuzz/SafetyNetFuzz_Create.t.sol`	Renames params and switches fuzz configuration to `contestThreshold`.
`test/invariants/fuzz/SafetyNetFuzz_RequestsVoting.t.sol`	Removed (legacy voting-focused fuzz tests).
`test/invariants/fuzz/SafetyNetFuzz_RequestsContesting.t.sol`	Added fuzz tests for contest threshold boundary + sybil/double-contest + timeout behavior.
`src/contracts/modification.txt`	Empty file added/modified.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/contracts/SafetyNet.sol

src/interfaces/ISafetyNet.sol

test/unit/SafetyNetUnit.sol

src/contracts/SafetyNet.sol

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

- Fix contest threshold integer division rounding: multiply contestCount by PERCENTAGE_BASE instead of dividing the right-hand side, preventing truncation from silently lowering the effective veto threshold - Change _deduct visibility from private to internal for consistency with other helper functions - Fix gendered language in AlreadyContestedByMember NatSpec - Remove empty src/contracts/modification.txt

RonTuretzky · 2026-03-19T20:59:20Z

@Fiuum1 please review https://github.com/Fiuum1/safety-net/pull/1/changes

fix: review feedback on PR BreadchainCoop#67

…requested

Fiuum1 · 2026-03-23T20:05:45Z

Hey @RonTuretzky,

I have completed all the fixes. Please let me know if there is anything else I should do.

Copilot

Pull request overview

This PR updates the SafetyNet withdrawal flow to remove request voting and instead support a contest-based veto: withdrawals above autoThreshold are auto-executed after the contest window unless contests exceed a configurable percentage threshold, in which case the request is vetoed.

Changes:

Replaced vote-based approval with contestCount-based veto logic using a contestThreshold percentage.
Updated ISafetyNet and SafetyNet to reflect the new request struct/events/errors and removed the voting window API surface.
Refactored and replaced fuzz/unit tests to cover contest threshold boundary behavior and veto behavior.

Reviewed changes

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

Show a summary per file

File	Description
`src/contracts/SafetyNet.sol`	Implements per-member contest tracking and veto threshold logic; removes voting execution path.
`src/interfaces/ISafetyNet.sol`	Updates structs/events/errors for contest/veto model and removes voting-related declarations.
`test/unit/SafetyNetUnit.sol`	Updates unit tests for new contest/veto semantics and request struct shape.
`test/invariants/fuzz/SafetyNetFuzzBase.t.sol`	Updates fuzz default config constants/struct fields to use `contestThreshold` and removes voting window config.
`test/invariants/fuzz/SafetyNetFuzz_RequestsVoting.t.sol`	Removes voting-focused fuzz tests that no longer apply.
`test/invariants/fuzz/SafetyNetFuzz_RequestsContesting.t.sol`	Adds new fuzz coverage for contest threshold boundaries, veto behavior, and contest window behavior.
`test/invariants/fuzz/SafetyNetFuzz_DepositWithdraw.t.sol`	Updates request destructuring and invariants to validate `contestCount` initialization.
`test/invariants/fuzz/SafetyNetFuzz_Create.t.sol`	Updates creation fuzzing to randomize `contestThreshold` instead of consensus threshold.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/unit/SafetyNetUnit.sol

src/contracts/SafetyNet.sol

src/interfaces/ISafetyNet.sol

Copilot · 2026-03-23T22:15:51Z

test/unit/SafetyNetUnit.sol

+  function test_CreateWhenContestThresholdIsZero() external {
    _allowToken(address(_token));
    ISafetyNet.SafetyNet memory _safetyNet = _defaultSafetyNet(address(_token));
-    _safetyNet.consensusThreshold = 0;
+    _safetyNet.contestThreshold = 0;
    uint256 id = _sn.create(_safetyNet);
    assertEq(id, 0);
  }

-  function test_CreateWhenConsensusThresholdIsGreaterThan100() external {
+  function test_CreateWhenContestThresholdIsGreaterThan100() external {
    _allowToken(address(_token));
    ISafetyNet.SafetyNet memory _safetyNet = _defaultSafetyNet(address(_token));
-    _safetyNet.consensusThreshold = 150;
+    _safetyNet.contestThreshold = 150;
    uint256 id = _sn.create(_safetyNet);
    assertEq(id, 0);


These tests are named as if contestThreshold values of 0 or >100 are invalid, but they don’t assert any failure mode (they only check id == 0, which will be true for the first create() call even if creation succeeds). Either add an explicit expectation (e.g., revert / no state changes) and enforce the constraint in create(), or rename the tests to reflect the actual intended behavior for out-of-range thresholds.

@RonTuretzky This tests are modeled based on the previous one created on vote() function, they were already like this...

test/invariants/fuzz/SafetyNetFuzz_RequestsContesting.t.sol

…et.sol Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fiuum1 added 2 commits March 13, 2026 15:45

feat: replace voting system with 33% contest veto threshold (Issue Br…

4790344

…eadchainCoop#14)

test: update unit and fuzz tests for new contest mechanism

b59f039

RonTuretzky requested a review from Copilot March 13, 2026 19:42

Copilot started reviewing on behalf of RonTuretzky March 13, 2026 19:42 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

Potential fix for pull request finding

516429e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Fiuum1 added 6 commits March 15, 2026 16:18

fix: resolve Copilot comment regarding parameter contestCount

e8ae8ab

fix: resolve Copilot comment regarding event RequestEnded [removed]

a8c2f5c

fix: resolve Copilot comment regarding a typo and the inline note tha…

c5a40a3

…t hardcoded 33%

fix: resolve Copilot comment regarding using storage reference instea…

141a41d

…d ofmemory for safetyNet struct

fix: resolve Cop comment regarding not having contestTreshold greater…

d7fea23

… than 100 when tested [now 150]

fix: resolve Copilot comment regarding a useless vm.prank()

f95bc9f

exo404 requested review from RonTuretzky and exo404 March 17, 2026 17:25

exo404 approved these changes Mar 17, 2026

View reviewed changes

exo404 assigned Fiuum1 Mar 17, 2026

exo404 added the enhancement New feature or request label Mar 17, 2026

RonTuretzky requested changes Mar 17, 2026

View reviewed changes

Fiuum1 added 3 commits March 18, 2026 12:21

fix: resolve RonTuretzky comment on misleading/misplaced comments

a373d6d

fix: resolve RonTuretzky comment on magic number 100 & comment on emi…

ca0c1a9

…t WithdContested()

test: increase contestThreshold in Sybil fuzz test to prevent prematu…

5dc3b1c

…re veto

Fiuum1 force-pushed the fix-issue-14 branch from 24ea09c to 5dc3b1c Compare March 19, 2026 13:49

RonTuretzky requested a review from Copilot March 19, 2026 18:52

Copilot started reviewing on behalf of RonTuretzky March 19, 2026 18:53 View session

Copilot AI reviewed Mar 19, 2026

View reviewed changes

Potential fix for pull request finding

3754986

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

RonTuretzky mentioned this pull request Mar 19, 2026

fix: review feedback on PR #67 Fiuum1/safety-net#1

Merged

3 tasks

RonTuretzky added 2 commits March 19, 2026 16:55

fix: typo in NatSpec — redeemeRatio → redeemRatio

827eef8

Fiuum1 and others added 3 commits March 20, 2026 21:08

Merge pull request #1 from BreadchainCoop/review-fixes

0454861

fix: review feedback on PR BreadchainCoop#67

fix: implementation of the last Copilot suggestions

94d8646

fix: removed WithdrawalApproved() event from ISafetyNet interface as …

cf5fac8

…requested

Fiuum1 requested a review from RonTuretzky March 23, 2026 20:05

RonTuretzky requested a review from Copilot March 23, 2026 22:10

Copilot started reviewing on behalf of RonTuretzky March 23, 2026 22:11 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

Fiuum1 and others added 2 commits March 25, 2026 11:31

fix: clarifying description of parameter contestThreshold in ISafetyN…

552a316

…et.sol Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix: modification of test logic of Fuzz_RequestContesting

456f1a9

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Conversation

Fiuum1 commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SafetyNet.sol:

ISafetyNet.sol:

Uh oh!

Fiuum1 commented Mar 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

exo404 commented Mar 14, 2026

Uh oh!

Fiuum1 commented Mar 15, 2026

Uh oh!

exo404 commented Mar 17, 2026

Uh oh!

Fiuum1 commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RonTuretzky commented Mar 19, 2026

Uh oh!

Fiuum1 commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Fiuum1 Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fiuum1 commented Mar 13, 2026 •

edited

Loading