From db865abae623680862ac5ae14f7a2df983027d3a Mon Sep 17 00:00:00 2001 From: Gas <86567384+gas1cent@users.noreply.github.com> Date: Mon, 27 Nov 2023 15:59:58 +0400 Subject: [PATCH] refactor: pull funds in `ERC20ResolutionModule` (#21) --- .../resolution/ERC20ResolutionModule.sol | 36 +++--- .../resolution/IERC20ResolutionModule.sol | 16 +++ .../resolution/ERC20ResolutionModule.t.sol | 107 ++++++++++++++---- solidity/test/utils/Helpers.sol | 20 ++++ 4 files changed, 143 insertions(+), 36 deletions(-) diff --git a/solidity/contracts/modules/resolution/ERC20ResolutionModule.sol b/solidity/contracts/modules/resolution/ERC20ResolutionModule.sol index ae1cf75e..c7a40ff6 100644 --- a/solidity/contracts/modules/resolution/ERC20ResolutionModule.sol +++ b/solidity/contracts/modules/resolution/ERC20ResolutionModule.sol @@ -72,7 +72,7 @@ contract ERC20ResolutionModule is Module, IERC20ResolutionModule { _voters[_disputeId].add(msg.sender); escalations[_disputeId].totalVotes += _numberOfVotes; - _params.votingToken.safeTransferFrom(msg.sender, address(this), _numberOfVotes); + _params.accountingExtension.bond(msg.sender, _dispute.requestId, _params.votingToken, _numberOfVotes); emit VoteCast(msg.sender, _disputeId, _numberOfVotes); } @@ -83,25 +83,23 @@ contract ERC20ResolutionModule is Module, IERC20ResolutionModule { IOracle.Response calldata _response, IOracle.Dispute calldata _dispute ) external onlyOracle { - // 0. Check disputeId actually exists and that it isn't resolved already + // Check disputeId actually exists and that it isn't resolved already if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated) { revert ERC20ResolutionModule_AlreadyResolved(); } - // 1. Check that the dispute is actually escalated + // Check that the dispute is actually escalated Escalation memory _escalation = escalations[_disputeId]; if (_escalation.startTime == 0) revert ERC20ResolutionModule_DisputeNotEscalated(); - // 2. Check that voting deadline is over + // Check that voting deadline is over RequestParameters memory _params = decodeRequestData(_request.resolutionModuleData); uint256 _deadline = _escalation.startTime + _params.timeUntilDeadline; if (block.timestamp < _deadline) revert ERC20ResolutionModule_OnGoingVotingPhase(); uint256 _quorumReached = _escalation.totalVotes >= _params.minVotesForQuorum ? 1 : 0; - address[] memory __voters = _voters[_disputeId].values(); - - // 5. Update status + // Update status if (_quorumReached == 1) { ORACLE.updateDisputeStatus(_request, _response, _dispute, IOracle.DisputeStatus.Won); emit DisputeResolved(_dispute.requestId, _disputeId, IOracle.DisputeStatus.Won); @@ -109,17 +107,23 @@ contract ERC20ResolutionModule is Module, IERC20ResolutionModule { ORACLE.updateDisputeStatus(_request, _response, _dispute, IOracle.DisputeStatus.Lost); emit DisputeResolved(_dispute.requestId, _disputeId, IOracle.DisputeStatus.Lost); } + } - uint256 _votersLength = __voters.length; + /// @inheritdoc IERC20ResolutionModule + function claimVote(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external { + bytes32 _disputeId = _getId(_dispute); + Escalation memory _escalation = escalations[_disputeId]; - // 6. Return tokens - for (uint256 _i; _i < _votersLength;) { - address _voter = __voters[_i]; - _params.votingToken.safeTransfer(_voter, votes[_disputeId][_voter]); - unchecked { - ++_i; - } - } + // Check that voting deadline is over + RequestParameters memory _params = decodeRequestData(_request.resolutionModuleData); + uint256 _deadline = _escalation.startTime + _params.timeUntilDeadline; + if (block.timestamp < _deadline) revert ERC20ResolutionModule_OnGoingVotingPhase(); + + // Transfer the tokens back to the voter + uint256 _amount = votes[_disputeId][msg.sender]; + _params.accountingExtension.release(msg.sender, _dispute.requestId, _params.votingToken, _amount); + + emit VoteClaimed(msg.sender, _disputeId, _amount); } /// @inheritdoc IERC20ResolutionModule diff --git a/solidity/interfaces/modules/resolution/IERC20ResolutionModule.sol b/solidity/interfaces/modules/resolution/IERC20ResolutionModule.sol index 95cbe46f..96d0c38c 100644 --- a/solidity/interfaces/modules/resolution/IERC20ResolutionModule.sol +++ b/solidity/interfaces/modules/resolution/IERC20ResolutionModule.sol @@ -6,6 +6,8 @@ import {IResolutionModule} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/modules/resolution/IResolutionModule.sol'; import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; +import {IAccountingExtension} from '../../extensions/IAccountingExtension.sol'; + /** * @title ERC20ResolutionModule * @notice This contract allows for disputes to be resolved by a voting process. @@ -34,6 +36,11 @@ interface IERC20ResolutionModule is IResolutionModule { */ event VotingPhaseStarted(uint256 _startTime, bytes32 _disputeId); + /** + * @notice Emitted when the voter gets back their bond + */ + event VoteClaimed(address _voter, bytes32 _disputeId, uint256 _amount); + /*/////////////////////////////////////////////////////////////// ERRORS //////////////////////////////////////////////////////////////*/ @@ -79,6 +86,7 @@ interface IERC20ResolutionModule is IResolutionModule { * @param timeUntilDeadline The time until the voting phase ends */ struct RequestParameters { + IAccountingExtension accountingExtension; IERC20 votingToken; uint256 minVotesForQuorum; uint256 timeUntilDeadline; @@ -165,6 +173,14 @@ interface IERC20ResolutionModule is IResolutionModule { IOracle.Dispute calldata _dispute ) external; + /** + * @notice Releases the voter's bond + * + * @param _request The request for which the dispute was created + * @param _dispute The resolved dispute + */ + function claimVote(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external; + /** * @notice Gets the voters of a dispute * diff --git a/solidity/test/unit/modules/resolution/ERC20ResolutionModule.t.sol b/solidity/test/unit/modules/resolution/ERC20ResolutionModule.t.sol index 304a55b3..b18e4fcd 100644 --- a/solidity/test/unit/modules/resolution/ERC20ResolutionModule.t.sol +++ b/solidity/test/unit/modules/resolution/ERC20ResolutionModule.t.sol @@ -16,6 +16,8 @@ import { IERC20ResolutionModule } from '../../../../contracts/modules/resolution/ERC20ResolutionModule.sol'; +import {IAccountingExtension} from '../../../../interfaces/extensions/IAccountingExtension.sol'; + contract ForTest_ERC20ResolutionModule is ERC20ResolutionModule { using EnumerableSet for EnumerableSet.AddressSet; @@ -43,9 +45,11 @@ contract BaseTest is Test, Helpers { // The target contract ForTest_ERC20ResolutionModule public module; // A mock oracle - IOracle public oracle; + IOracle public oracle = IOracle(_mockContract('Oracle')); // A mock token - IERC20 public token; + IERC20 public token = IERC20(_mockContract('Token')); + // Mock accounting extension + IAccountingExtension public accountingExtension = IAccountingExtension(_mockContract('AccountingExtension')); uint256 public votingTimeWindow = 40_000; @@ -53,17 +57,12 @@ contract BaseTest is Test, Helpers { event VoteCast(address _voter, bytes32 _disputeId, uint256 _numberOfVotes); event VotingPhaseStarted(uint256 _startTime, bytes32 _disputeId); event DisputeResolved(bytes32 indexed _requestId, bytes32 indexed _disputeId, IOracle.DisputeStatus _status); + event VoteClaimed(address _voter, bytes32 _disputeId, uint256 _amount); /** * @notice Deploy the target and mock oracle extension */ - function setUp() public { - oracle = IOracle(makeAddr('Oracle')); - vm.etch(address(oracle), hex'069420'); - - token = IERC20(makeAddr('ERC20')); - vm.etch(address(token), hex'069420'); - + function setUp() public virtual { module = new ForTest_ERC20ResolutionModule(oracle); } @@ -95,12 +94,13 @@ contract ERC20ResolutionModule_Unit_ModuleData is BaseTest { uint256 _votingTimeWindow ) public { // Mock data - bytes memory _requestData = abi.encode(_token, _minVotesForQuorum, _votingTimeWindow); + bytes memory _requestData = abi.encode(address(accountingExtension), _token, _minVotesForQuorum, _votingTimeWindow); // Test: decode the given request data IERC20ResolutionModule.RequestParameters memory _params = module.decodeRequestData(_requestData); // Check: decoded values match original values? + assertEq(address(_params.accountingExtension), address(accountingExtension)); assertEq(address(_params.votingToken), _token); assertEq(_params.minVotesForQuorum, _minVotesForQuorum); assertEq(_params.timeUntilDeadline, _votingTimeWindow); @@ -145,6 +145,7 @@ contract ERC20ResolutionModule_Unit_CastVote is BaseTest { mockRequest.resolutionModuleData = abi.encode( IERC20ResolutionModule.RequestParameters({ + accountingExtension: accountingExtension, votingToken: token, minVotesForQuorum: _minVotesForQuorum, timeUntilDeadline: votingTimeWindow @@ -156,9 +157,13 @@ contract ERC20ResolutionModule_Unit_CastVote is BaseTest { // Store mock escalation data with startTime 100_000 module.forTest_setStartTime(_disputeId, 100_000); - // Mock and expect IERC20.transferFrom to be called + // Mock and expect the bond to be placed _mockAndExpect( - address(token), abi.encodeCall(IERC20.transferFrom, (_voter, address(module), _amountOfVotes)), abi.encode() + address(accountingExtension), + abi.encodeWithSignature( + 'bond(address,bytes32,address,uint256)', _voter, mockDispute.requestId, token, _amountOfVotes + ), + abi.encode() ); _mockAndExpect( @@ -201,6 +206,7 @@ contract ERC20ResolutionModule_Unit_CastVote is BaseTest { function test_revertIfAlreadyResolved(uint256 _amountOfVotes, uint256 _votingTimeWindow) public { mockRequest.resolutionModuleData = abi.encode( IERC20ResolutionModule.RequestParameters({ + accountingExtension: accountingExtension, votingToken: token, minVotesForQuorum: _amountOfVotes, timeUntilDeadline: _votingTimeWindow @@ -230,6 +236,7 @@ contract ERC20ResolutionModule_Unit_CastVote is BaseTest { mockRequest.resolutionModuleData = abi.encode( IERC20ResolutionModule.RequestParameters({ + accountingExtension: accountingExtension, votingToken: token, minVotesForQuorum: _minVotesForQuorum, timeUntilDeadline: votingTimeWindow @@ -262,6 +269,7 @@ contract ERC20ResolutionModule_Unit_ResolveDispute is BaseTest { function test_resolveDispute(uint16 _minVotesForQuorum) public { mockRequest.resolutionModuleData = abi.encode( IERC20ResolutionModule.RequestParameters({ + accountingExtension: accountingExtension, votingToken: token, minVotesForQuorum: _minVotesForQuorum, timeUntilDeadline: votingTimeWindow @@ -284,14 +292,6 @@ contract ERC20ResolutionModule_Unit_ResolveDispute is BaseTest { // Warp to resolving phase vm.warp(150_000); - // Mock and expect token transfers (should happen always) - for (uint256 _i = 1; _i <= _votersAmount;) { - _mockAndExpect(address(token), abi.encodeCall(IERC20.transfer, (vm.addr(_i), 100)), abi.encode()); - unchecked { - ++_i; - } - } - _mockAndExpect( address(oracle), abi.encodeCall(IOracle.disputeStatus, (_disputeId)), abi.encode(IOracle.DisputeStatus.Escalated) ); @@ -330,6 +330,7 @@ contract ERC20ResolutionModule_Unit_ResolveDispute is BaseTest { mockRequest.resolutionModuleData = abi.encode( IERC20ResolutionModule.RequestParameters({ + accountingExtension: accountingExtension, votingToken: token, minVotesForQuorum: _minVotesForQuorum, timeUntilDeadline: _votingTimeWindow @@ -354,6 +355,72 @@ contract ERC20ResolutionModule_Unit_ResolveDispute is BaseTest { } } +contract ERC20ResolutionModule_Unit_ClaimVote is BaseTest { + /** + * @notice Reverts if the vote is still ongoing + */ + function test_revertIfVoteIsOnGoing(address _voter, uint256 _amount) public { + mockRequest.resolutionModuleData = abi.encode( + IERC20ResolutionModule.RequestParameters({ + accountingExtension: accountingExtension, + votingToken: token, + minVotesForQuorum: 1, + timeUntilDeadline: 1000 + }) + ); + + mockDispute.requestId = _getId(mockRequest); + bytes32 _disputeId = _getId(mockDispute); + module.forTest_setStartTime(_getId(mockDispute), block.timestamp); + + // Expect an error to be thrown + vm.expectRevert(IERC20ResolutionModule.ERC20ResolutionModule_OnGoingVotingPhase.selector); + + // Claim the refund + vm.prank(_voter); + module.claimVote(mockRequest, mockDispute); + } + + /** + * @notice Releases the funds + */ + function test_releasesFunds(address _voter, uint256 _amount) public { + mockRequest.resolutionModuleData = abi.encode( + IERC20ResolutionModule.RequestParameters({ + accountingExtension: accountingExtension, + votingToken: token, + minVotesForQuorum: 1, + timeUntilDeadline: 1 + }) + ); + + // Prepare the dispute + mockDispute.requestId = _getId(mockRequest); + module.forTest_setStartTime(_getId(mockDispute), block.timestamp); + module.forTest_setVotes(_getId(mockDispute), _voter, _amount); + + // Expect the bond to be released + _mockAndExpect( + address(accountingExtension), + abi.encodeCall(accountingExtension.release, (_voter, mockDispute.requestId, token, _amount)), + abi.encode() + ); + + vm.warp(block.timestamp + 1000); + + bytes32 _disputeId = _getId(mockDispute); + module.forTest_setVotes(_disputeId, _voter, _amount); + + // Expect the event to be emitted + _expectEmit(address(module)); + emit VoteClaimed(_voter, _disputeId, _amount); + + // Claim the refund + vm.prank(_voter); + module.claimVote(mockRequest, mockDispute); + } +} + contract ERC20ResolutionModule_Unit_GetVoters is BaseTest { /** * @notice Test that `getVoters` returns an array of addresses of users that have voted. diff --git a/solidity/test/utils/Helpers.sol b/solidity/test/utils/Helpers.sol index 26adab6e..4096dec5 100644 --- a/solidity/test/utils/Helpers.sol +++ b/solidity/test/utils/Helpers.sol @@ -97,4 +97,24 @@ contract Helpers is DSTestPlus, TestConstants { function _getId(IOracle.Dispute memory _dispute) internal pure returns (bytes32 _id) { _id = keccak256(abi.encode(_dispute)); } + + /** + * @notice Creates a mock contract, labels it and erases the bytecode + * + * @param _label The label to use for the mock contract + * @return _contract The address of the mock contract + */ + function _mockContract(string memory _label) internal returns (address _contract) { + _contract = makeAddr(_label); + vm.etch(_contract, hex'69'); + } + + /** + * @notice Sets an expectation for an event to be emitted + * + * @param _contract The contract to expect the event on + */ + function _expectEmit(address _contract) internal { + vm.expectEmit(true, true, true, true, _contract); + } }