From a74cde776d82fe29c84eda0b7975c37e0967f0c5 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Mon, 24 Jul 2023 11:27:30 -0300 Subject: [PATCH 01/14] test: start resolution --- .../unit/PrivateERC20ResolutionModule.t.sol | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 solidity/test/unit/PrivateERC20ResolutionModule.t.sol diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol new file mode 100644 index 00000000..0b93c5a5 --- /dev/null +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.19; + +// solhint-disable-next-line +import 'forge-std/Test.sol'; + +import { + PrivateERC20ResolutionModule, + IPrivateERC20ResolutionModule +} from '../../contracts/modules/PrivateERC20ResolutionModule.sol'; +import {IOracle} from '../../interfaces/IOracle.sol'; +import {IAccountingExtension} from '../../interfaces/extensions/IAccountingExtension.sol'; +import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; +import {IModule} from '../../interfaces/IModule.sol'; + +contract ForTest_PrivateERC20ResolutionModule is PrivateERC20ResolutionModule { + constructor(IOracle _oracle) PrivateERC20ResolutionModule(_oracle) {} + + function forTest_setRequestData(bytes32 _requestId, bytes memory _data) public { + requestData[_requestId] = _data; + } + + function forTest_setEscalationData( + bytes32 _disputeId, + PrivateERC20ResolutionModule.EscalationData calldata __escalationData + ) public { + escalationData[_disputeId] = __escalationData; + } +} + +contract PrivateERC20ResolutionModule_UnitTest is Test { + struct FakeDispute { + bytes32 requestId; + bytes32 test; + } + + struct FakeRequest { + address disputeModule; + } + + // The target contract + ForTest_PrivateERC20ResolutionModule public module; + + // A mock oracle + IOracle public oracle; + + // A mock accounting extension + IAccountingExtension public accounting; + + // A mock token + IERC20 public token; + + // Mock EOA proposer + address public proposer; + + // Mock EOA disputer + address public disputer; + + // Mock EOA pledgerFor + address public pledgerFor; + + // Mock EOA pledgerAgainst + address public pledgerAgainst; + + // Mock percentageDiff + uint256 percentageDiff; + + // Mock pledge threshold + uint256 pledgeThreshold; + + // Mock time until main deadline + uint256 timeUntilDeadline; + + // Mock time to break inequality + uint256 timeToBreakInequality; + + event CommitingPhaseStarted(uint128 _startTime, bytes32 _disputeId); + + /** + * @notice Deploy the target and mock oracle+accounting extension + */ + function setUp() public { + oracle = IOracle(makeAddr('Oracle')); + vm.etch(address(oracle), hex'069420'); + + accounting = IAccountingExtension(makeAddr('AccountingExtension')); + vm.etch(address(accounting), hex'069420'); + + token = IERC20(makeAddr('ERC20')); + vm.etch(address(token), hex'069420'); + + proposer = makeAddr('proposer'); + disputer = makeAddr('disputer'); + pledgerFor = makeAddr('pledgerFor'); + pledgerAgainst = makeAddr('pledgerAgainst'); + + // Avoid starting at 0 for time sensitive tests + vm.warp(123_456); + + module = new ForTest_PrivateERC20ResolutionModule(oracle); + } + + /** + * @notice Test that the moduleName function returns the correct name + */ + function test_moduleName() public { + assertEq(module.moduleName(), 'PrivateERC20ResolutionModule'); + } + + /** + * @notice Test that the startResolution is correctly called and the commiting phase is started + */ + function test_startResolution(bytes32 _disputeId, bytes32 _requestId, uint256 _disputerBondSize) public { + // Mock the dispute + IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); + + // Mock the oracle response for looking up a dispute + vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); + vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); + + // Store the request for decoding data + module.forTest_setRequestData( + _requestId, abi.encode(address(accounting), token, _disputerBondSize, uint256(1), uint256(1), uint256(1)) + ); + + // Check: does revert if called by address != oracle? + vm.expectRevert(IModule.Module_OnlyOracle.selector); + module.startResolution(_disputeId); + + // Check: emits CommitingPhaseStarted event? + vm.expectEmit(true, true, true, true); + emit CommitingPhaseStarted(uint128(block.timestamp), _disputeId); + + // Mock calls if disputerBondSize != 0 + if (_disputerBondSize != 0) { + vm.mockCall( + address(accounting), + abi.encodeCall(IAccountingExtension.pay, (_requestId, disputer, address(module), token, _disputerBondSize)), + abi.encode() + ); + vm.expectCall( + address(accounting), + abi.encodeCall(IAccountingExtension.pay, (_requestId, disputer, address(module), token, _disputerBondSize)) + ); + + vm.mockCall( + address(accounting), abi.encodeCall(IAccountingExtension.withdraw, (token, _disputerBondSize)), abi.encode() + ); + vm.expectCall(address(accounting), abi.encodeCall(IAccountingExtension.withdraw, (token, _disputerBondSize))); + } + + vm.prank(address(oracle)); + module.startResolution(_disputeId); + + (uint128 _startTime,,,) = module.escalationData(_disputeId); + + // Check: startTime is set to block.timestamp? + assertEq(_startTime, uint128(block.timestamp)); + } + + function _getMockDispute(bytes32 _requestId) internal view returns (IOracle.Dispute memory _dispute) { + _dispute = IOracle.Dispute({ + disputer: disputer, + responseId: bytes32('response'), + proposer: proposer, + requestId: _requestId, + status: IOracle.DisputeStatus.Active, + createdAt: block.timestamp + }); + } +} From b9f986ba82b811084342e6815033613db17baad4 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Mon, 24 Jul 2023 11:57:00 -0300 Subject: [PATCH 02/14] test: commit vote --- .../unit/PrivateERC20ResolutionModule.t.sol | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 0b93c5a5..39223e73 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -75,6 +75,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { uint256 timeToBreakInequality; event CommitingPhaseStarted(uint128 _startTime, bytes32 _disputeId); + event VoteCommited(address _voter, bytes32 _disputeId, bytes32 _commitment); /** * @notice Deploy the target and mock oracle+accounting extension @@ -158,6 +159,55 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { assertEq(_startTime, uint128(block.timestamp)); } + function test_commitVote( + bytes32 _requestId, + bytes32 _disputeId, + uint256 _amountOfVotes, + bytes32 _salt, + address _voter + ) public { + // Mock the dispute + IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); + + // Store mock escalation data with startTime 100_000 + module.forTest_setEscalationData( + _disputeId, + IPrivateERC20ResolutionModule.EscalationData({ + startTime: 100_000, + results: 0, // Escalated + disputerBond: uint256(0), // Set as zero for testing + totalVotes: 0 // Initial amount of votes + }) + ); + + // Store mock request data with 40_000 commiting time window + module.forTest_setRequestData( + _requestId, abi.encode(address(accounting), token, uint256(0), uint256(1), uint256(40_000), uint256(40_000)) + ); + + // Mock the oracle response for looking up a dispute + vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); + vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); + + // Set timestamp for valid commitingTimeWindow + vm.warp(123_456); + + // Compute commitment + vm.prank(_voter); + bytes32 _commitment = module.computeCommitment(_disputeId, _amountOfVotes, _salt); + + // Check: is event emitted? + vm.expectEmit(true, true, true, true); + emit VoteCommited(_voter, _disputeId, _commitment); + + // Compute and store commitment + vm.prank(_voter); + module.commitVote(_requestId, _disputeId, _commitment); + + // Check: commitment is stored? + assertEq(module.commitments(_disputeId, _voter), _commitment); + } + function _getMockDispute(bytes32 _requestId) internal view returns (IOracle.Dispute memory _dispute) { _dispute = IOracle.Dispute({ disputer: disputer, From add5f3cc93f7b0845ea24ff43d5524a76b285b17 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Mon, 24 Jul 2023 14:29:52 -0300 Subject: [PATCH 03/14] test: change commitment --- .../test/unit/PrivateERC20ResolutionModule.t.sol | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 39223e73..1152ecbb 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -193,21 +193,30 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { vm.warp(123_456); // Compute commitment - vm.prank(_voter); + vm.startPrank(_voter); bytes32 _commitment = module.computeCommitment(_disputeId, _amountOfVotes, _salt); // Check: is event emitted? vm.expectEmit(true, true, true, true); emit VoteCommited(_voter, _disputeId, _commitment); + // Revert if no commitment is given + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_EmptyCommitment.selector); + module.commitVote(_requestId, _disputeId, bytes32('')); + // Compute and store commitment - vm.prank(_voter); module.commitVote(_requestId, _disputeId, _commitment); // Check: commitment is stored? assertEq(module.commitments(_disputeId, _voter), _commitment); + + bytes32 _newComitment = module.computeCommitment(_disputeId, uint256(_salt), bytes32(_amountOfVotes)); + module.commitVote(_requestId, _disputeId, _newComitment); + vm.stopPrank(); } + function test_revealVote() public {} + function _getMockDispute(bytes32 _requestId) internal view returns (IOracle.Dispute memory _dispute) { _dispute = IOracle.Dispute({ disputer: disputer, From b256735b44ebe73527f757379ce350f47f77a357 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Mon, 24 Jul 2023 15:07:10 -0300 Subject: [PATCH 04/14] test: reveal vote --- .../unit/PrivateERC20ResolutionModule.t.sol | 66 ++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 1152ecbb..48286b14 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -26,6 +26,10 @@ contract ForTest_PrivateERC20ResolutionModule is PrivateERC20ResolutionModule { ) public { escalationData[_disputeId] = __escalationData; } + + function forTest_setCommitment(bytes32 _disputeId, address _voter, bytes32 _commitment) public { + commitments[_disputeId][_voter] = _commitment; + } } contract PrivateERC20ResolutionModule_UnitTest is Test { @@ -76,6 +80,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { event CommitingPhaseStarted(uint128 _startTime, bytes32 _disputeId); event VoteCommited(address _voter, bytes32 _disputeId, bytes32 _commitment); + event VoteRevealed(address _voter, bytes32 _disputeId, uint256 _numberOfVotes); /** * @notice Deploy the target and mock oracle+accounting extension @@ -153,10 +158,13 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { vm.prank(address(oracle)); module.startResolution(_disputeId); - (uint128 _startTime,,,) = module.escalationData(_disputeId); + (uint128 _startTime,, uint256 _disputerBond,) = module.escalationData(_disputeId); // Check: startTime is set to block.timestamp? assertEq(_startTime, uint128(block.timestamp)); + + // Check: disputerBond is set to _disputerBondSize? + assertEq(_disputerBond, _disputerBondSize); } function test_commitVote( @@ -215,7 +223,61 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { vm.stopPrank(); } - function test_revealVote() public {} + function test_revealVote( + bytes32 _requestId, + bytes32 _disputeId, + uint256 _amountOfVotes, + bytes32 _salt, + address _voter + ) public { + // Store mock escalation data with startTime 100_000 + module.forTest_setEscalationData( + _disputeId, + IPrivateERC20ResolutionModule.EscalationData({ + startTime: 100_000, + results: 0, // Escalated + disputerBond: uint256(0), // Set as zero for testing + totalVotes: 0 // Initial amount of votes + }) + ); + + // Store mock request data with 40_000 commiting time window + module.forTest_setRequestData( + _requestId, abi.encode(address(accounting), token, uint256(0), uint256(1), uint256(40_000), uint256(40_000)) + ); + + // Store commitment + vm.prank(_voter); + bytes32 _commitment = module.computeCommitment(_disputeId, _amountOfVotes, _salt); + module.forTest_setCommitment(_disputeId, _voter, _commitment); + + // Mock token transfer (user must have approved token spending) + vm.mockCall( + address(token), abi.encodeCall(IERC20.transferFrom, (_voter, address(module), _amountOfVotes)), abi.encode() + ); + vm.expectCall(address(token), abi.encodeCall(IERC20.transferFrom, (_voter, address(module), _amountOfVotes))); + + // Warp to revealing phase + vm.warp(150_000); + + // Check: is event emmited? + vm.expectEmit(true, true, true, true); + emit VoteRevealed(_voter, _disputeId, _amountOfVotes); + + vm.prank(_voter); + module.revealVote(_requestId, _disputeId, _amountOfVotes, _salt); + + (,,, uint256 _totalVotes) = module.escalationData(_disputeId); + // Check: totalVotes is updated? + assertEq(_totalVotes, _amountOfVotes); + + (address _userAddress, uint256 _userVotes) = module.votes(_disputeId, 0); + + // Check: user address is stored? + assertEq(_userAddress, _voter); + // Check: user votes is stored? + assertEq(_userVotes, _amountOfVotes); + } function _getMockDispute(bytes32 _requestId) internal view returns (IOracle.Dispute memory _dispute) { _dispute = IOracle.Dispute({ From 1144387a0cb88261c0adc08c7154c4b1261c04f9 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Mon, 24 Jul 2023 18:03:55 -0300 Subject: [PATCH 05/14] test: updated tests according to module changes --- .../modules/PrivateERC20ResolutionModule.sol | 2 +- .../unit/PrivateERC20ResolutionModule.t.sol | 114 +++++------------- 2 files changed, 34 insertions(+), 82 deletions(-) diff --git a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol index c9e8960b..f74658ac 100644 --- a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol +++ b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol @@ -18,7 +18,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule { // todo: this storage layout must be super optimizable. many disputeId mappings mapping(bytes32 _disputeId => EscalationData _escalationData) public escalationData; - mapping(bytes32 _disputeId => mapping(address _voter => VoterData)) private _votersData; + mapping(bytes32 _disputeId => mapping(address _voter => VoterData)) internal _votersData; mapping(bytes32 _disputeId => uint256 _numOfVotes) public totalNumberOfVotes; mapping(bytes32 _disputeId => EnumerableSet.AddressSet _votersSet) private _voters; diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 48286b14..d41d55de 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -27,21 +27,23 @@ contract ForTest_PrivateERC20ResolutionModule is PrivateERC20ResolutionModule { escalationData[_disputeId] = __escalationData; } - function forTest_setCommitment(bytes32 _disputeId, address _voter, bytes32 _commitment) public { - commitments[_disputeId][_voter] = _commitment; - } -} - -contract PrivateERC20ResolutionModule_UnitTest is Test { - struct FakeDispute { - bytes32 requestId; - bytes32 test; + function forTest_setVoterData( + bytes32 _disputeId, + address _voter, + IPrivateERC20ResolutionModule.VoterData memory _data + ) public { + _votersData[_disputeId][_voter] = _data; } - struct FakeRequest { - address disputeModule; + function forTest_getVoterData( + bytes32 _disputeId, + address _voter + ) public view returns (IPrivateERC20ResolutionModule.VoterData memory _data) { + _data = _votersData[_disputeId][_voter]; } +} +contract PrivateERC20ResolutionModule_UnitTest is Test { // The target contract ForTest_PrivateERC20ResolutionModule public module; @@ -60,24 +62,6 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { // Mock EOA disputer address public disputer; - // Mock EOA pledgerFor - address public pledgerFor; - - // Mock EOA pledgerAgainst - address public pledgerAgainst; - - // Mock percentageDiff - uint256 percentageDiff; - - // Mock pledge threshold - uint256 pledgeThreshold; - - // Mock time until main deadline - uint256 timeUntilDeadline; - - // Mock time to break inequality - uint256 timeToBreakInequality; - event CommitingPhaseStarted(uint128 _startTime, bytes32 _disputeId); event VoteCommited(address _voter, bytes32 _disputeId, bytes32 _commitment); event VoteRevealed(address _voter, bytes32 _disputeId, uint256 _numberOfVotes); @@ -97,8 +81,6 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { proposer = makeAddr('proposer'); disputer = makeAddr('disputer'); - pledgerFor = makeAddr('pledgerFor'); - pledgerAgainst = makeAddr('pledgerAgainst'); // Avoid starting at 0 for time sensitive tests vm.warp(123_456); @@ -116,17 +98,9 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { /** * @notice Test that the startResolution is correctly called and the commiting phase is started */ - function test_startResolution(bytes32 _disputeId, bytes32 _requestId, uint256 _disputerBondSize) public { - // Mock the dispute - IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); - - // Mock the oracle response for looking up a dispute - vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); - vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); - - // Store the request for decoding data - module.forTest_setRequestData( - _requestId, abi.encode(address(accounting), token, _disputerBondSize, uint256(1), uint256(1), uint256(1)) + function test_startResolution(bytes32 _disputeId) public { + module.forTest_setEscalationData( + _disputeId, IPrivateERC20ResolutionModule.EscalationData({startTime: 0, totalVotes: 0}) ); // Check: does revert if called by address != oracle? @@ -137,36 +111,18 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { vm.expectEmit(true, true, true, true); emit CommitingPhaseStarted(uint128(block.timestamp), _disputeId); - // Mock calls if disputerBondSize != 0 - if (_disputerBondSize != 0) { - vm.mockCall( - address(accounting), - abi.encodeCall(IAccountingExtension.pay, (_requestId, disputer, address(module), token, _disputerBondSize)), - abi.encode() - ); - vm.expectCall( - address(accounting), - abi.encodeCall(IAccountingExtension.pay, (_requestId, disputer, address(module), token, _disputerBondSize)) - ); - - vm.mockCall( - address(accounting), abi.encodeCall(IAccountingExtension.withdraw, (token, _disputerBondSize)), abi.encode() - ); - vm.expectCall(address(accounting), abi.encodeCall(IAccountingExtension.withdraw, (token, _disputerBondSize))); - } - vm.prank(address(oracle)); module.startResolution(_disputeId); - (uint128 _startTime,, uint256 _disputerBond,) = module.escalationData(_disputeId); + (uint256 _startTime,) = module.escalationData(_disputeId); // Check: startTime is set to block.timestamp? - assertEq(_startTime, uint128(block.timestamp)); - - // Check: disputerBond is set to _disputerBondSize? - assertEq(_disputerBond, _disputerBondSize); + assertEq(_startTime, block.timestamp); } + /** + * @notice Test that a user can store a vote commitment for a dispute + */ function test_commitVote( bytes32 _requestId, bytes32 _disputeId, @@ -182,15 +138,13 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { _disputeId, IPrivateERC20ResolutionModule.EscalationData({ startTime: 100_000, - results: 0, // Escalated - disputerBond: uint256(0), // Set as zero for testing totalVotes: 0 // Initial amount of votes }) ); // Store mock request data with 40_000 commiting time window module.forTest_setRequestData( - _requestId, abi.encode(address(accounting), token, uint256(0), uint256(1), uint256(40_000), uint256(40_000)) + _requestId, abi.encode(address(accounting), token, uint256(1), uint256(40_000), uint256(40_000)) ); // Mock the oracle response for looking up a dispute @@ -216,7 +170,8 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { module.commitVote(_requestId, _disputeId, _commitment); // Check: commitment is stored? - assertEq(module.commitments(_disputeId, _voter), _commitment); + IPrivateERC20ResolutionModule.VoterData memory _voterData = module.forTest_getVoterData(_disputeId, _voter); + assertEq(_voterData.commitment, _commitment); bytes32 _newComitment = module.computeCommitment(_disputeId, uint256(_salt), bytes32(_amountOfVotes)); module.commitVote(_requestId, _disputeId, _newComitment); @@ -235,21 +190,21 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { _disputeId, IPrivateERC20ResolutionModule.EscalationData({ startTime: 100_000, - results: 0, // Escalated - disputerBond: uint256(0), // Set as zero for testing totalVotes: 0 // Initial amount of votes }) ); // Store mock request data with 40_000 commiting time window module.forTest_setRequestData( - _requestId, abi.encode(address(accounting), token, uint256(0), uint256(1), uint256(40_000), uint256(40_000)) + _requestId, abi.encode(address(accounting), token, uint256(1), uint256(40_000), uint256(40_000)) ); // Store commitment vm.prank(_voter); bytes32 _commitment = module.computeCommitment(_disputeId, _amountOfVotes, _salt); - module.forTest_setCommitment(_disputeId, _voter, _commitment); + module.forTest_setVoterData( + _disputeId, _voter, IPrivateERC20ResolutionModule.VoterData({numOfVotes: 0, commitment: _commitment}) + ); // Mock token transfer (user must have approved token spending) vm.mockCall( @@ -267,16 +222,13 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { vm.prank(_voter); module.revealVote(_requestId, _disputeId, _amountOfVotes, _salt); - (,,, uint256 _totalVotes) = module.escalationData(_disputeId); + (, uint256 _totalVotes) = module.escalationData(_disputeId); // Check: totalVotes is updated? assertEq(_totalVotes, _amountOfVotes); - (address _userAddress, uint256 _userVotes) = module.votes(_disputeId, 0); - - // Check: user address is stored? - assertEq(_userAddress, _voter); - // Check: user votes is stored? - assertEq(_userVotes, _amountOfVotes); + // Check: voter data is updated? + IPrivateERC20ResolutionModule.VoterData memory _voterData = module.forTest_getVoterData(_disputeId, _voter); + assertEq(_voterData.numOfVotes, _amountOfVotes); } function _getMockDispute(bytes32 _requestId) internal view returns (IOracle.Dispute memory _dispute) { @@ -285,7 +237,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { responseId: bytes32('response'), proposer: proposer, requestId: _requestId, - status: IOracle.DisputeStatus.Active, + status: IOracle.DisputeStatus.None, createdAt: block.timestamp }); } From d85e309e0c57ee59b67db64d710e8961500b2b64 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Tue, 25 Jul 2023 10:00:46 -0300 Subject: [PATCH 06/14] test: resolve dispute --- .../modules/PrivateERC20ResolutionModule.sol | 2 +- .../unit/PrivateERC20ResolutionModule.t.sol | 86 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol index f74658ac..9669d7f8 100644 --- a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol +++ b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol @@ -20,7 +20,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule { mapping(bytes32 _disputeId => EscalationData _escalationData) public escalationData; mapping(bytes32 _disputeId => mapping(address _voter => VoterData)) internal _votersData; mapping(bytes32 _disputeId => uint256 _numOfVotes) public totalNumberOfVotes; - mapping(bytes32 _disputeId => EnumerableSet.AddressSet _votersSet) private _voters; + mapping(bytes32 _disputeId => EnumerableSet.AddressSet _votersSet) internal _voters; constructor(IOracle _oracle) Module(_oracle) {} diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index d41d55de..007cffec 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -12,6 +12,7 @@ import {IOracle} from '../../interfaces/IOracle.sol'; import {IAccountingExtension} from '../../interfaces/extensions/IAccountingExtension.sol'; import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; import {IModule} from '../../interfaces/IModule.sol'; +import {EnumerableSet} from '@openzeppelin/contracts/utils/structs/EnumerableSet.sol'; contract ForTest_PrivateERC20ResolutionModule is PrivateERC20ResolutionModule { constructor(IOracle _oracle) PrivateERC20ResolutionModule(_oracle) {} @@ -65,6 +66,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { event CommitingPhaseStarted(uint128 _startTime, bytes32 _disputeId); event VoteCommited(address _voter, bytes32 _disputeId, bytes32 _commitment); event VoteRevealed(address _voter, bytes32 _disputeId, uint256 _numberOfVotes); + event DisputeResolved(bytes32 _disputeId, IOracle.DisputeStatus _status); /** * @notice Deploy the target and mock oracle+accounting extension @@ -231,6 +233,90 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { assertEq(_voterData.numOfVotes, _amountOfVotes); } + function test_resolveDispute( + bytes32 _requestId, + bytes32 _disputeId, + uint256 _minVotesForQuorum, + uint8 _votersAmount + ) public { + IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); + + vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); + vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); + + module.forTest_setRequestData( + _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, uint256(40_000), uint256(40_000)) + ); + + module.forTest_setEscalationData( + _disputeId, IPrivateERC20ResolutionModule.EscalationData({startTime: 100_000, totalVotes: 0}) + ); + + uint256 _totalVotesCast = _populateVoters(_requestId, _disputeId, _votersAmount, 100); + + // Warp to resolving phase + vm.warp(190_000); + + for (uint256 i = 1; i < _votersAmount;) { + vm.mockCall(address(token), abi.encodeCall(IERC20.transfer, (vm.addr(i), 100)), abi.encode()); + vm.expectCall(address(token), abi.encodeCall(IERC20.transfer, (vm.addr(i), 100))); + unchecked { + ++i; + } + } + + if (_totalVotesCast >= _minVotesForQuorum) { + vm.mockCall( + address(oracle), + abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, IOracle.DisputeStatus.Won)), + abi.encode() + ); + vm.expectCall( + address(oracle), abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, IOracle.DisputeStatus.Won)) + ); + vm.expectEmit(true, true, true, true); + emit DisputeResolved(_disputeId, IOracle.DisputeStatus.Won); + } else { + vm.mockCall( + address(oracle), + abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, IOracle.DisputeStatus.Lost)), + abi.encode() + ); + vm.expectCall( + address(oracle), abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, IOracle.DisputeStatus.Lost)) + ); + vm.expectEmit(true, true, true, true); + emit DisputeResolved(_disputeId, IOracle.DisputeStatus.Lost); + } + + vm.prank(address(oracle)); + module.resolveDispute(_disputeId); + } + + function _populateVoters( + bytes32 _requestId, + bytes32 _disputeId, + uint8 _size, + uint8 _votes + ) internal returns (uint256 _totalVotesCast) { + for (uint256 i = 1; i < _size;) { + vm.warp(120_000); + vm.startPrank(vm.addr(i)); + bytes32 _commitment = module.computeCommitment(_disputeId, _votes, bytes32(i)); // index as salt + module.commitVote(_requestId, _disputeId, _commitment); + vm.warp(140_000); + vm.mockCall( + address(token), abi.encodeCall(IERC20.transferFrom, (vm.addr(i), address(module), _votes)), abi.encode() + ); + module.revealVote(_requestId, _disputeId, _votes, bytes32(i)); + vm.stopPrank(); + _totalVotesCast += _votes; + unchecked { + ++i; + } + } + } + function _getMockDispute(bytes32 _requestId) internal view returns (IOracle.Dispute memory _dispute) { _dispute = IOracle.Dispute({ disputer: disputer, From bb9612def2c83c2cb1604dc276569fcff58c764a Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Tue, 25 Jul 2023 10:14:29 -0300 Subject: [PATCH 07/14] test: optimization --- .../unit/PrivateERC20ResolutionModule.t.sol | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 007cffec..2bd8c627 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -233,31 +233,34 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { assertEq(_voterData.numOfVotes, _amountOfVotes); } - function test_resolveDispute( - bytes32 _requestId, - bytes32 _disputeId, - uint256 _minVotesForQuorum, - uint8 _votersAmount - ) public { + /** + * @notice Test that a dispute is resolved and the tokens are transferred back + */ + function test_resolveDispute(bytes32 _requestId, bytes32 _disputeId, uint16 _minVotesForQuorum) public { + // Store mock dispute and mock calls IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); + // Store request data module.forTest_setRequestData( _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, uint256(40_000), uint256(40_000)) ); + // Store escalation data with starttime 100_000 and votes 0 module.forTest_setEscalationData( _disputeId, IPrivateERC20ResolutionModule.EscalationData({startTime: 100_000, totalVotes: 0}) ); - uint256 _totalVotesCast = _populateVoters(_requestId, _disputeId, _votersAmount, 100); + // Make 100 addresses cast 20 votes each + uint256 _totalVotesCast = _populateVoters(_requestId, _disputeId, 20, 100); // Warp to resolving phase vm.warp(190_000); - for (uint256 i = 1; i < _votersAmount;) { + // Mock and expect token transfers (should happen always) + for (uint256 i = 1; i < 20;) { vm.mockCall(address(token), abi.encodeCall(IERC20.transfer, (vm.addr(i), 100)), abi.encode()); vm.expectCall(address(token), abi.encodeCall(IERC20.transfer, (vm.addr(i), 100))); unchecked { @@ -265,6 +268,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { } } + // If quorum reached, check for dispute status update and event emission if (_totalVotesCast >= _minVotesForQuorum) { vm.mockCall( address(oracle), @@ -289,6 +293,10 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { emit DisputeResolved(_disputeId, IOracle.DisputeStatus.Lost); } + // Check: does revert if called by address != oracle? + vm.expectRevert(IModule.Module_OnlyOracle.selector); + module.resolveDispute(_disputeId); + vm.prank(address(oracle)); module.resolveDispute(_disputeId); } From 9ede79dbc8fe238d042f46b0a18a8a5fd2326636 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Tue, 25 Jul 2023 10:40:46 -0300 Subject: [PATCH 08/14] fix: remove cast --- solidity/test/unit/PrivateERC20ResolutionModule.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 2bd8c627..7e7b95e2 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -63,7 +63,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { // Mock EOA disputer address public disputer; - event CommitingPhaseStarted(uint128 _startTime, bytes32 _disputeId); + event CommitingPhaseStarted(uint256 _startTime, bytes32 _disputeId); event VoteCommited(address _voter, bytes32 _disputeId, bytes32 _commitment); event VoteRevealed(address _voter, bytes32 _disputeId, uint256 _numberOfVotes); event DisputeResolved(bytes32 _disputeId, IOracle.DisputeStatus _status); @@ -111,7 +111,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { // Check: emits CommitingPhaseStarted event? vm.expectEmit(true, true, true, true); - emit CommitingPhaseStarted(uint128(block.timestamp), _disputeId); + emit CommitingPhaseStarted(block.timestamp, _disputeId); vm.prank(address(oracle)); module.startResolution(_disputeId); From df2bcdfb37934bf324adc2c606480e846f85cb29 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Tue, 25 Jul 2023 10:48:00 -0300 Subject: [PATCH 09/14] style: clutter cleaning --- .../unit/PrivateERC20ResolutionModule.t.sol | 30 +++++-------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 7e7b95e2..eb119ac0 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -269,29 +269,13 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { } // If quorum reached, check for dispute status update and event emission - if (_totalVotesCast >= _minVotesForQuorum) { - vm.mockCall( - address(oracle), - abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, IOracle.DisputeStatus.Won)), - abi.encode() - ); - vm.expectCall( - address(oracle), abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, IOracle.DisputeStatus.Won)) - ); - vm.expectEmit(true, true, true, true); - emit DisputeResolved(_disputeId, IOracle.DisputeStatus.Won); - } else { - vm.mockCall( - address(oracle), - abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, IOracle.DisputeStatus.Lost)), - abi.encode() - ); - vm.expectCall( - address(oracle), abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, IOracle.DisputeStatus.Lost)) - ); - vm.expectEmit(true, true, true, true); - emit DisputeResolved(_disputeId, IOracle.DisputeStatus.Lost); - } + + IOracle.DisputeStatus _newStatus = + _totalVotesCast >= _minVotesForQuorum ? IOracle.DisputeStatus.Won : IOracle.DisputeStatus.Lost; + vm.mockCall(address(oracle), abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, _newStatus)), abi.encode()); + vm.expectCall(address(oracle), abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, _newStatus))); + vm.expectEmit(true, true, true, true); + emit DisputeResolved(_disputeId, _newStatus); // Check: does revert if called by address != oracle? vm.expectRevert(IModule.Module_OnlyOracle.selector); From b6dce2f8220a9c87f0437501de892e5eb6af8c25 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Tue, 25 Jul 2023 14:08:02 -0300 Subject: [PATCH 10/14] test: revert cases --- foundry.toml | 1 + .../modules/PrivateERC20ResolutionModule.sol | 4 +- .../unit/PrivateERC20ResolutionModule.t.sol | 258 ++++++++++++++++-- 3 files changed, 242 insertions(+), 21 deletions(-) diff --git a/foundry.toml b/foundry.toml index b5b961d0..9b938c65 100644 --- a/foundry.toml +++ b/foundry.toml @@ -29,6 +29,7 @@ src = 'solidity/interfaces/' [fuzz] runs = 1000 +max_test_rejects = 200000 [rpc_endpoints] optimism = "${OPTIMISM_RPC}" diff --git a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol index a2e5b493..5113b237 100644 --- a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol +++ b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol @@ -16,7 +16,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule { // todo: this storage layout must be super optimizable. many disputeId mappings mapping(bytes32 _disputeId => EscalationData _escalationData) public escalationData; - mapping(bytes32 _disputeId => mapping(address _voter => VoterData)) internal _votersData; + mapping(bytes32 _disputeId => mapping(address _voter => VoterData)) public _votersData; mapping(bytes32 _disputeId => uint256 _numOfVotes) public totalNumberOfVotes; mapping(bytes32 _disputeId => EnumerableSet.AddressSet _votersSet) internal _voters; @@ -73,7 +73,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule { _escalationData.startTime + _commitingTimeWindow, _escalationData.startTime + _commitingTimeWindow + _revealingTimeWindow ); - if (block.timestamp < _revealStartTime) revert PrivateERC20ResolutionModule_OnGoingCommitingPhase(); + if (block.timestamp <= _revealStartTime) revert PrivateERC20ResolutionModule_OnGoingCommitingPhase(); if (block.timestamp >= _revealEndTime) revert PrivateERC20ResolutionModule_RevealingPhaseOver(); VoterData storage _voterData = _votersData[_disputeId][msg.sender]; diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index eb119ac0..32807f4d 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -84,9 +84,6 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { proposer = makeAddr('proposer'); disputer = makeAddr('disputer'); - // Avoid starting at 0 for time sensitive tests - vm.warp(123_456); - module = new ForTest_PrivateERC20ResolutionModule(oracle); } @@ -145,8 +142,12 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { ); // Store mock request data with 40_000 commiting time window + uint256 _minVotesForQuorum = 1; + uint256 _commitingTimeWindow = 40_000; + uint256 _revealingTimewindow = 40_000; + module.forTest_setRequestData( - _requestId, abi.encode(address(accounting), token, uint256(1), uint256(40_000), uint256(40_000)) + _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, _commitingTimeWindow, _revealingTimewindow) ); // Mock the oracle response for looking up a dispute @@ -171,13 +172,82 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { // Compute and store commitment module.commitVote(_requestId, _disputeId, _commitment); + // Check: reverts if empty commitment is given? + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_EmptyCommitment.selector); + module.commitVote(_requestId, _disputeId, bytes32('')); + // Check: commitment is stored? IPrivateERC20ResolutionModule.VoterData memory _voterData = module.forTest_getVoterData(_disputeId, _voter); assertEq(_voterData.commitment, _commitment); - bytes32 _newComitment = module.computeCommitment(_disputeId, uint256(_salt), bytes32(_amountOfVotes)); - module.commitVote(_requestId, _disputeId, _newComitment); + bytes32 _newCommitment = module.computeCommitment(_disputeId, uint256(_salt), bytes32(_amountOfVotes)); + module.commitVote(_requestId, _disputeId, _newCommitment); vm.stopPrank(); + + // Check: does update with new commitment? + IPrivateERC20ResolutionModule.VoterData memory _newVoterData = module.forTest_getVoterData(_disputeId, _voter); + assertEq(_newVoterData.commitment, _newCommitment); + } + + function test_revertCommitVote_AlreadyResolved(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { + // Mock dispute already resolved => DisputeStatus.Lost + IOracle.Dispute memory _mockDispute = IOracle.Dispute({ + disputer: disputer, + responseId: bytes32('response'), + proposer: proposer, + requestId: _requestId, + status: IOracle.DisputeStatus.Lost, + createdAt: block.timestamp + }); + + // Mock the oracle response for looking up a dispute + vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); + vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); + + // Check: reverts if dispute is already resolved? + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_AlreadyResolved.selector); + module.commitVote(_requestId, _disputeId, _commitment); + } + + function test_revertCommitVote_NotEscalated(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { + // Mock the oracle response for looking up a dispute + IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); + vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); + vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); + + // Check: reverts if dispute is not escalated? == no escalation data + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_DisputeNotEscalated.selector); + module.commitVote(_requestId, _disputeId, _commitment); + } + + function test_revertCommitVote_CommitingPhaseOver(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { + // Mock the oracle response for looking up a dispute + IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); + vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); + vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); + + module.forTest_setEscalationData( + _disputeId, + IPrivateERC20ResolutionModule.EscalationData({ + startTime: 100_000, + totalVotes: 0 // Initial amount of votes + }) + ); + + uint256 _minVotesForQuorum = 1; + uint256 _commitingTimeWindow = 40_000; + uint256 _revealingTimewindow = 40_000; + + module.forTest_setRequestData( + _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, _commitingTimeWindow, _revealingTimewindow) + ); + + // Warp to invalid timestamp for commitment + vm.warp(150_000); + + // Check: reverts if commiting phase is over? + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_CommitingPhaseOver.selector); + module.commitVote(_requestId, _disputeId, _commitment); } function test_revealVote( @@ -233,6 +303,112 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { assertEq(_voterData.numOfVotes, _amountOfVotes); } + /** + * @notice Call should revert if a dispute has not been escalated + */ + function test_revertRevealVote_NotEscalated( + bytes32 _requestId, + bytes32 _disputeId, + uint256 _numberOfVotes, + bytes32 _salt + ) public { + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_DisputeNotEscalated.selector); + module.revealVote(_requestId, _disputeId, _numberOfVotes, _salt); + } + + function test_revertRevealVote_InvalidPhase( + bytes32 _requestId, + bytes32 _disputeId, + uint256 _numberOfVotes, + bytes32 _salt, + uint256 _timestamp + ) public { + vm.assume(_timestamp >= 100_000 && (_timestamp <= 140_000 || _timestamp > 180_000)); + + module.forTest_setEscalationData( + _disputeId, + IPrivateERC20ResolutionModule.EscalationData({ + startTime: 100_000, + totalVotes: 0 // Initial amount of votes + }) + ); + + // Store request data + uint256 _minVotesForQuorum = 1; + uint256 _commitingTimeWindow = 40_000; + uint256 _revealingTimewindow = 40_000; + + module.forTest_setRequestData( + _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, _commitingTimeWindow, _revealingTimewindow) + ); + + // Jump to timestamp + vm.warp(_timestamp); + + if (_timestamp <= 140_000) { + // Check: reverts if trying to reveal during commiting phase? + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_OnGoingCommitingPhase.selector); + module.revealVote(_requestId, _disputeId, _numberOfVotes, _salt); + } else { + // Check: reverts if trying to reveal after revealing phase? + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_RevealingPhaseOver.selector); + module.revealVote(_requestId, _disputeId, _numberOfVotes, _salt); + } + } + + function test_revertRevealVote_FalseCommitment( + bytes32 _requestId, + bytes32 _disputeId, + uint256 _amountOfVotes, + uint256 _wrongAmountOfVotes, + bytes32 _salt, + bytes32 _wrongSalt, + address _voter, + address _wrongVoter + ) public { + vm.assume(_amountOfVotes != _wrongAmountOfVotes); + vm.assume(_salt != _wrongSalt); + vm.assume(_voter != _wrongVoter); + + module.forTest_setEscalationData( + _disputeId, + IPrivateERC20ResolutionModule.EscalationData({ + startTime: 100_000, + totalVotes: 0 // Initial amount of votes + }) + ); + + // Store request data + uint256 _minVotesForQuorum = 1; + uint256 _commitingTimeWindow = 40_000; + uint256 _revealingTimewindow = 40_000; + + module.forTest_setRequestData( + _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, _commitingTimeWindow, _revealingTimewindow) + ); + vm.warp(150_000); + + vm.startPrank(_voter); + bytes32 _commitment = module.computeCommitment(_disputeId, _amountOfVotes, _salt); + module.forTest_setVoterData( + _disputeId, _voter, IPrivateERC20ResolutionModule.VoterData({numOfVotes: 0, commitment: _commitment}) + ); + + // Check: reverts if commitment is not valid? (wrong salt) + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_WrongRevealData.selector); + module.revealVote(_requestId, _disputeId, _amountOfVotes, _wrongSalt); + + // Check: reverts if commitment is not valid? (wrong amount of votes) + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_WrongRevealData.selector); + module.revealVote(_requestId, _disputeId, _wrongAmountOfVotes, _salt); + vm.stopPrank(); + + // Check: reverts if commitment is not valid? (wrong voter) + vm.prank(_wrongVoter); + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_WrongRevealData.selector); + module.revealVote(_requestId, _disputeId, _amountOfVotes, _salt); + } + /** * @notice Test that a dispute is resolved and the tokens are transferred back */ @@ -244,8 +420,11 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); // Store request data + uint256 _commitingTimeWindow = 40_000; + uint256 _revealingTimewindow = 40_000; + module.forTest_setRequestData( - _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, uint256(40_000), uint256(40_000)) + _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, _commitingTimeWindow, _revealingTimewindow) ); // Store escalation data with starttime 100_000 and votes 0 @@ -253,14 +432,14 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { _disputeId, IPrivateERC20ResolutionModule.EscalationData({startTime: 100_000, totalVotes: 0}) ); - // Make 100 addresses cast 20 votes each - uint256 _totalVotesCast = _populateVoters(_requestId, _disputeId, 20, 100); + // Make 5 addresses cast 100 votes each + uint256 _totalVotesCast = _populateVoters(_requestId, _disputeId, 6, 100); // Warp to resolving phase vm.warp(190_000); // Mock and expect token transfers (should happen always) - for (uint256 i = 1; i < 20;) { + for (uint256 i = 1; i < 6;) { vm.mockCall(address(token), abi.encodeCall(IERC20.transfer, (vm.addr(i), 100)), abi.encode()); vm.expectCall(address(token), abi.encodeCall(IERC20.transfer, (vm.addr(i), 100))); unchecked { @@ -269,7 +448,6 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { } // If quorum reached, check for dispute status update and event emission - IOracle.DisputeStatus _newStatus = _totalVotesCast >= _minVotesForQuorum ? IOracle.DisputeStatus.Won : IOracle.DisputeStatus.Lost; vm.mockCall(address(oracle), abi.encodeCall(IOracle.updateDisputeStatus, (_disputeId, _newStatus)), abi.encode()); @@ -285,24 +463,66 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { module.resolveDispute(_disputeId); } + function test_revertResolveDispute_WrongPhase(bytes32 _requestId, bytes32 _disputeId, uint32 _timestamp) public { + vm.assume(_timestamp >= 100_000 && _timestamp < 180_000); + + // Store mock dispute and mock calls + IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); + + vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); + vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); + + module.forTest_setEscalationData( + _disputeId, + IPrivateERC20ResolutionModule.EscalationData({ + startTime: 100_000, + totalVotes: 0 // Initial amount of votes + }) + ); + + // Store request data + uint256 _minVotesForQuorum = 1; + uint256 _commitingTimeWindow = 40_000; + uint256 _revealingTimewindow = 40_000; + + module.forTest_setRequestData( + _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, _commitingTimeWindow, _revealingTimewindow) + ); + + // Jump to timestamp + vm.warp(_timestamp); + + if (_timestamp < 140_000) { + // Check: reverts if trying to resolve during commiting phase? + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_OnGoingCommitingPhase.selector); + vm.prank(address(oracle)); + module.resolveDispute(_disputeId); + } else { + // Check: reverts if trying to resolve during revealing phase? + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_OnGoingRevealingPhase.selector); + vm.prank(address(oracle)); + module.resolveDispute(_disputeId); + } + } + function _populateVoters( bytes32 _requestId, bytes32 _disputeId, - uint8 _size, - uint8 _votes + uint8 _amountOfVoters, + uint8 _amountOfVotes ) internal returns (uint256 _totalVotesCast) { - for (uint256 i = 1; i < _size;) { + for (uint256 i = 1; i < _amountOfVoters;) { vm.warp(120_000); vm.startPrank(vm.addr(i)); - bytes32 _commitment = module.computeCommitment(_disputeId, _votes, bytes32(i)); // index as salt + bytes32 _commitment = module.computeCommitment(_disputeId, _amountOfVotes, bytes32(i)); // index as salt module.commitVote(_requestId, _disputeId, _commitment); - vm.warp(140_000); + vm.warp(140_001); vm.mockCall( - address(token), abi.encodeCall(IERC20.transferFrom, (vm.addr(i), address(module), _votes)), abi.encode() + address(token), abi.encodeCall(IERC20.transferFrom, (vm.addr(i), address(module), _amountOfVotes)), abi.encode() ); - module.revealVote(_requestId, _disputeId, _votes, bytes32(i)); + module.revealVote(_requestId, _disputeId, _amountOfVotes, bytes32(i)); vm.stopPrank(); - _totalVotesCast += _votes; + _totalVotesCast += _amountOfVotes; unchecked { ++i; } From 770afe6e8c0a38050c9f151047e7ce28a121dd3f Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Tue, 25 Jul 2023 14:23:36 -0300 Subject: [PATCH 11/14] test: undo config file, fix test --- foundry.toml | 1 - .../unit/PrivateERC20ResolutionModule.t.sol | 18 ++++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/foundry.toml b/foundry.toml index 9b938c65..b5b961d0 100644 --- a/foundry.toml +++ b/foundry.toml @@ -29,7 +29,6 @@ src = 'solidity/interfaces/' [fuzz] runs = 1000 -max_test_rejects = 200000 [rpc_endpoints] optimism = "${OPTIMISM_RPC}" diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 32807f4d..39f3f39b 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -189,6 +189,11 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { assertEq(_newVoterData.commitment, _newCommitment); } + function test_revertCommitVote_Underflow(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { + vm.expectRevert(); + module.commitVote(_requestId, _disputeId, _commitment); + } + function test_revertCommitVote_AlreadyResolved(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { // Mock dispute already resolved => DisputeStatus.Lost IOracle.Dispute memory _mockDispute = IOracle.Dispute({ @@ -464,7 +469,8 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { } function test_revertResolveDispute_WrongPhase(bytes32 _requestId, bytes32 _disputeId, uint32 _timestamp) public { - vm.assume(_timestamp >= 100_000 && _timestamp < 180_000); + vm.assume(_timestamp > 1); + vm.assume(_timestamp < 1_000_000); // Store mock dispute and mock calls IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); @@ -475,24 +481,24 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { module.forTest_setEscalationData( _disputeId, IPrivateERC20ResolutionModule.EscalationData({ - startTime: 100_000, + startTime: 1, totalVotes: 0 // Initial amount of votes }) ); // Store request data uint256 _minVotesForQuorum = 1; - uint256 _commitingTimeWindow = 40_000; - uint256 _revealingTimewindow = 40_000; + uint256 _commitingTimeWindow = 500_000; + uint256 _revealingTimeWindow = 1_000_000; module.forTest_setRequestData( - _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, _commitingTimeWindow, _revealingTimewindow) + _requestId, abi.encode(address(accounting), token, _minVotesForQuorum, _commitingTimeWindow, _revealingTimeWindow) ); // Jump to timestamp vm.warp(_timestamp); - if (_timestamp < 140_000) { + if (_timestamp < 500_000) { // Check: reverts if trying to resolve during commiting phase? vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_OnGoingCommitingPhase.selector); vm.prank(address(oracle)); From 53dbb1dafa352e5d15e657a8d85d0f39b2d84ab4 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Wed, 26 Jul 2023 08:54:57 -0300 Subject: [PATCH 12/14] fix: flaky test --- solidity/test/unit/PrivateERC20ResolutionModule.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 39f3f39b..1e8abbae 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -498,7 +498,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { // Jump to timestamp vm.warp(_timestamp); - if (_timestamp < 500_000) { + if (_timestamp <= 500_000) { // Check: reverts if trying to resolve during commiting phase? vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_OnGoingCommitingPhase.selector); vm.prank(address(oracle)); From abc53cbcea07594664b01a3157a544eaa50cfdbf Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Wed, 26 Jul 2023 09:04:36 -0300 Subject: [PATCH 13/14] feat: reset commitment to prevent double voting --- solidity/contracts/modules/PrivateERC20ResolutionModule.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol index 5113b237..69f8708a 100644 --- a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol +++ b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol @@ -83,6 +83,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule { } _voterData.numOfVotes = _numberOfVotes; + _voterData.commitment = bytes32(''); _voters[_disputeId].add(msg.sender); escalationData[_disputeId].totalVotes += _numberOfVotes; From c2f98e495c41a89f31989c188a9693d721856110 Mon Sep 17 00:00:00 2001 From: moebius <0xmoebius@tutanota.com> Date: Wed, 26 Jul 2023 12:56:06 -0300 Subject: [PATCH 14/14] fix: addressed pr comments --- .../modules/PrivateERC20ResolutionModule.sol | 3 +- .../modules/IPrivateERC20ResolutionModule.sol | 1 - .../unit/PrivateERC20ResolutionModule.t.sol | 67 +++++++++++++++---- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol index 69f8708a..953ecada 100644 --- a/solidity/contracts/modules/PrivateERC20ResolutionModule.sol +++ b/solidity/contracts/modules/PrivateERC20ResolutionModule.sol @@ -17,7 +17,6 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule { // todo: this storage layout must be super optimizable. many disputeId mappings mapping(bytes32 _disputeId => EscalationData _escalationData) public escalationData; mapping(bytes32 _disputeId => mapping(address _voter => VoterData)) public _votersData; - mapping(bytes32 _disputeId => uint256 _numOfVotes) public totalNumberOfVotes; mapping(bytes32 _disputeId => EnumerableSet.AddressSet _votersSet) internal _voters; constructor(IOracle _oracle) Module(_oracle) {} @@ -74,7 +73,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule { _escalationData.startTime + _commitingTimeWindow + _revealingTimeWindow ); if (block.timestamp <= _revealStartTime) revert PrivateERC20ResolutionModule_OnGoingCommitingPhase(); - if (block.timestamp >= _revealEndTime) revert PrivateERC20ResolutionModule_RevealingPhaseOver(); + if (block.timestamp > _revealEndTime) revert PrivateERC20ResolutionModule_RevealingPhaseOver(); VoterData storage _voterData = _votersData[_disputeId][msg.sender]; diff --git a/solidity/interfaces/modules/IPrivateERC20ResolutionModule.sol b/solidity/interfaces/modules/IPrivateERC20ResolutionModule.sol index 098a4241..bcef2728 100644 --- a/solidity/interfaces/modules/IPrivateERC20ResolutionModule.sol +++ b/solidity/interfaces/modules/IPrivateERC20ResolutionModule.sol @@ -42,7 +42,6 @@ interface IPrivateERC20ResolutionModule is IResolutionModule { function escalationData(bytes32 _disputeId) external view returns (uint256 _startTime, uint256 _totalVotes); // TODO: create getter -- see if its possible to declare this // function votes(bytes32 _disputeId) external view returns (VoterData memory _voterData); - function totalNumberOfVotes(bytes32 _disputeId) external view returns (uint256 _numOfVotes); function commitVote(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) external; function revealVote(bytes32 _requestId, bytes32 _disputeId, uint256 _numberOfVotes, bytes32 _salt) external; function resolveDispute(bytes32 _disputeId) external; diff --git a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol index 1e8abbae..ac9ea429 100644 --- a/solidity/test/unit/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/PrivateERC20ResolutionModule.t.sol @@ -63,6 +63,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { // Mock EOA disputer address public disputer; + // Mocking module events event CommitingPhaseStarted(uint256 _startTime, bytes32 _disputeId); event VoteCommited(address _voter, bytes32 _disputeId, bytes32 _commitment); event VoteRevealed(address _voter, bytes32 _disputeId, uint256 _numberOfVotes); @@ -189,11 +190,30 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { assertEq(_newVoterData.commitment, _newCommitment); } - function test_revertCommitVote_Underflow(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { - vm.expectRevert(); + /** + * @notice Test that `commitVote` reverts if there is no dispute with the given`_disputeId` + */ + function test_revertCommitVote_NonExistentDispute(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { + IOracle.Dispute memory _mockDispute = IOracle.Dispute({ + disputer: address(0), + responseId: bytes32(0), + proposer: address(0), + requestId: bytes32(0), + status: IOracle.DisputeStatus.None, + createdAt: 0 + }); + + // Mock the oracle response for looking up a dispute + vm.mockCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId)), abi.encode(_mockDispute)); + vm.expectCall(address(oracle), abi.encodeCall(IOracle.getDispute, (_disputeId))); + + vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_NonExistentDispute.selector); module.commitVote(_requestId, _disputeId, _commitment); } + /** + * @notice Test that `commitVote` reverts if called with `_disputeId` of an already resolved dispute. + */ function test_revertCommitVote_AlreadyResolved(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { // Mock dispute already resolved => DisputeStatus.Lost IOracle.Dispute memory _mockDispute = IOracle.Dispute({ @@ -214,6 +234,9 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { module.commitVote(_requestId, _disputeId, _commitment); } + /** + * @notice Test that `commitVote` reverts if called with `_disputeId` of a non-escalated dispute. + */ function test_revertCommitVote_NotEscalated(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { // Mock the oracle response for looking up a dispute IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); @@ -225,6 +248,9 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { module.commitVote(_requestId, _disputeId, _commitment); } + /** + * @notice Test that `commitVote` reverts if called outside of the commiting time window. + */ function test_revertCommitVote_CommitingPhaseOver(bytes32 _requestId, bytes32 _disputeId, bytes32 _commitment) public { // Mock the oracle response for looking up a dispute IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); @@ -255,6 +281,9 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { module.commitVote(_requestId, _disputeId, _commitment); } + /** + * @notice Test revealing votes with proper timestamp, dispute status and commitment data. + */ function test_revealVote( bytes32 _requestId, bytes32 _disputeId, @@ -309,7 +338,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { } /** - * @notice Call should revert if a dispute has not been escalated + * @notice Test that `revealVote` reverts if called with `_disputeId` of a non-escalated dispute. */ function test_revertRevealVote_NotEscalated( bytes32 _requestId, @@ -321,6 +350,9 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { module.revealVote(_requestId, _disputeId, _numberOfVotes, _salt); } + /** + * @notice Test that `revealVote` reverts if called outside the revealing time window. + */ function test_revertRevealVote_InvalidPhase( bytes32 _requestId, bytes32 _disputeId, @@ -361,6 +393,10 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { } } + /** + * @notice Test that `revealVote` reverts if called with revealing parameters (`_disputeId`, `_numberOfVotes`, `_salt`) + * that do not compute to the stored commitment. + */ function test_revertRevealVote_FalseCommitment( bytes32 _requestId, bytes32 _disputeId, @@ -415,7 +451,7 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { } /** - * @notice Test that a dispute is resolved and the tokens are transferred back + * @notice Test that a dispute is resolved, the tokens are transferred back to the voters and the dispute status updated. */ function test_resolveDispute(bytes32 _requestId, bytes32 _disputeId, uint16 _minVotesForQuorum) public { // Store mock dispute and mock calls @@ -437,14 +473,16 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { _disputeId, IPrivateERC20ResolutionModule.EscalationData({startTime: 100_000, totalVotes: 0}) ); + uint256 _votersAmount = 5; + // Make 5 addresses cast 100 votes each - uint256 _totalVotesCast = _populateVoters(_requestId, _disputeId, 6, 100); + uint256 _totalVotesCast = _populateVoters(_requestId, _disputeId, _votersAmount, 100); // Warp to resolving phase vm.warp(190_000); // Mock and expect token transfers (should happen always) - for (uint256 i = 1; i < 6;) { + for (uint256 i = 1; i <= _votersAmount;) { vm.mockCall(address(token), abi.encodeCall(IERC20.transfer, (vm.addr(i), 100)), abi.encode()); vm.expectCall(address(token), abi.encodeCall(IERC20.transfer, (vm.addr(i), 100))); unchecked { @@ -468,9 +506,11 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { module.resolveDispute(_disputeId); } - function test_revertResolveDispute_WrongPhase(bytes32 _requestId, bytes32 _disputeId, uint32 _timestamp) public { - vm.assume(_timestamp > 1); - vm.assume(_timestamp < 1_000_000); + /** + * @notice Test that `resolveDispute` reverts if called during commiting or reavealing time window. + */ + function test_revertResolveDispute_WrongPhase(bytes32 _requestId, bytes32 _disputeId, uint256 _timestamp) public { + _timestamp = bound(_timestamp, 1, 1_000_000); // Store mock dispute and mock calls IOracle.Dispute memory _mockDispute = _getMockDispute(_requestId); @@ -511,13 +551,16 @@ contract PrivateERC20ResolutionModule_UnitTest is Test { } } + /** + * @dev Helper function to store commitments and reveal votes. + */ function _populateVoters( bytes32 _requestId, bytes32 _disputeId, - uint8 _amountOfVoters, - uint8 _amountOfVotes + uint256 _amountOfVoters, + uint256 _amountOfVotes ) internal returns (uint256 _totalVotesCast) { - for (uint256 i = 1; i < _amountOfVoters;) { + for (uint256 i = 1; i <= _amountOfVoters;) { vm.warp(120_000); vm.startPrank(vm.addr(i)); bytes32 _commitment = module.computeCommitment(_disputeId, _amountOfVotes, bytes32(i)); // index as salt