feat: resolve TODOs (#34)

* fix: typo * feat: remove TODOs * feat: update linter * style: fix linter warnings * fix: put back the import * fix: a typo * feat: install solidity utils as dev dependency
defi-wonderland · Dec 14, 2023 · a66e42d · a66e42d
1 parent 89d0ba0
commit a66e42d
Show file tree

Hide file tree

Showing 18 changed files with 102 additions and 104 deletions.
diff --git a/build-docs.sh b/build-docs.sh
@@ -16,18 +16,6 @@ FOUNDRY_PROFILE=docs forge doc --out "$temp_folder"
 rm -rf ./solidity/contracts/core
 rm -rf ./solidity/interfaces/core
 
-# edit generated summary not to have container pages
-# - [jobs](solidity/interfaces/jobs/README.md)
-# should become
-# - [jobs]()
-# TODO
-
-# edit generated summary titles to start with an uppercase letter
-# - [jobs]()
-# should become
-# - [Jobs]()
-# TODO
-
 # edit the SUMMARY after the Interfaces section
 # https://stackoverflow.com/questions/67086574/no-such-file-or-directory-when-using-sed-in-combination-with-find
 if [[ "$OSTYPE" == "darwin"* ]]; then

diff --git a/package.json b/package.json
@@ -39,13 +39,13 @@
   },
   "dependencies": {
     "@defi-wonderland/prophet-core-contracts": "0.0.0-49fc8fa6",
-    "@defi-wonderland/solidity-utils": "0.0.0-3e9c8e8b",
     "@openzeppelin/contracts": "^4.9.3",
     "solmate": "https://github.com/transmissions11/solmate.git#bfc9c25865a274a7827fea5abf6e4fb64fc64e6c"
   },
   "devDependencies": {
     "@commitlint/cli": "17.0.3",
     "@commitlint/config-conventional": "17.0.3",
+    "@defi-wonderland/solidity-utils": "0.0.0-3e9c8e8b",
     "@typechain/ethers-v6": "0.3.0",
     "@typechain/truffle-v5": "8.0.2",
     "@typechain/web3-v1": "6.0.2",
@@ -57,7 +57,7 @@
     "lint-staged": "13.2.2",
     "pinst": "3.0.0",
     "solhint": "3.5.1",
-    "solhint-plugin-defi-wonderland": "1.1.0",
+    "solhint-plugin-defi-wonderland": "1.1.2",
     "solidity-docgen": "0.6.0-beta.35",
     "sort-package-json": "2.4.1",
     "standard-version": "9.5.0",

diff --git a/solidity/contracts/extensions/BondEscalationAccounting.sol b/solidity/contracts/extensions/BondEscalationAccounting.sol
@@ -49,7 +49,6 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
     uint256 _amountPerPledger,
     uint256 _winningPledgersLength
   ) external onlyAllowedModule(_requestId) {
-    // TODO: check that flooring at _amountPerPledger calculation doesn't mess with this check
     if (pledges[_disputeId][_token] < _amountPerPledger * _winningPledgersLength) {
       revert BondEscalationAccounting_InsufficientFunds();
     }

diff --git a/solidity/contracts/modules/dispute/CircuitResolverModule.sol b/solidity/contracts/modules/dispute/CircuitResolverModule.sol
@@ -29,14 +29,10 @@ contract CircuitResolverModule is Module, ICircuitResolverModule {
     IOracle.Response calldata _response,
     IOracle.Dispute calldata _dispute
   ) external onlyOracle {
-    // TODO: Call `disputeStatus` to check the current status instead of reading from `_correctResponses`
     RequestParameters memory _params = decodeRequestData(_request.disputeModuleData);
+    IOracle.DisputeStatus _status = ORACLE.disputeStatus(_disputeId);
 
-    bytes memory _correctResponse = _correctResponses[_dispute.requestId];
-    bool _won = _response.response.length != _correctResponse.length
-      || keccak256(_response.response) != keccak256(_correctResponse);
-
-    if (_won) {
+    if (_status == IOracle.DisputeStatus.Won) {
       _params.accountingExtension.pay({
         _requestId: _dispute.requestId,
         _payer: _dispute.proposer,
@@ -45,8 +41,11 @@ contract CircuitResolverModule is Module, ICircuitResolverModule {
         _amount: _params.bondSize
       });
 
-      IOracle.Response memory _newResponse =
-        IOracle.Response({requestId: _dispute.requestId, proposer: _dispute.disputer, response: _correctResponse});
+      IOracle.Response memory _newResponse = IOracle.Response({
+        requestId: _dispute.requestId,
+        proposer: _dispute.disputer,
+        response: _correctResponses[_dispute.requestId]
+      });
 
       emit DisputeStatusChanged({_disputeId: _disputeId, _dispute: _dispute, _status: IOracle.DisputeStatus.Won});
 

diff --git a/solidity/contracts/modules/dispute/RootVerificationModule.sol b/solidity/contracts/modules/dispute/RootVerificationModule.sol
@@ -35,13 +35,10 @@ contract RootVerificationModule is Module, IRootVerificationModule {
     IOracle.Response calldata _response,
     IOracle.Dispute calldata _dispute
   ) external onlyOracle {
-    // TODO: Call `disputeStatus` to check the current status
     RequestParameters memory _params = decodeRequestData(_request.disputeModuleData);
+    IOracle.DisputeStatus _status = ORACLE.disputeStatus(_disputeId);
 
-    bytes32 _correctRoot = _correctRoots[_dispute.requestId];
-    bool _won = abi.decode(_response.response, (bytes32)) != _correctRoot;
-
-    if (_won) {
+    if (_status == IOracle.DisputeStatus.Won) {
       _params.accountingExtension.pay({
         _requestId: _dispute.requestId,
         _payer: _dispute.proposer,
@@ -53,7 +50,7 @@ contract RootVerificationModule is Module, IRootVerificationModule {
       IOracle.Response memory _newResponse = IOracle.Response({
         requestId: _dispute.requestId,
         proposer: _dispute.disputer,
-        response: abi.encode(_correctRoot)
+        response: abi.encode(_correctRoots[_dispute.requestId])
       });
 
       emit DisputeStatusChanged({_disputeId: _disputeId, _dispute: _dispute, _status: IOracle.DisputeStatus.Won});

diff --git a/solidity/contracts/modules/resolution/BondEscalationResolutionModule.sol b/solidity/contracts/modules/resolution/BondEscalationResolutionModule.sol
@@ -9,6 +9,7 @@ import {FixedPointMathLib} from 'solmate/utils/FixedPointMathLib.sol';
 // solhint-disable-next-line no-unused-import
 import {IResolutionModule} from
   '@defi-wonderland/prophet-core-contracts/solidity/interfaces/modules/resolution/IResolutionModule.sol';
+// solhint-disable-next-line no-unused-import
 import {Module, IModule} from '@defi-wonderland/prophet-core-contracts/solidity/contracts/Module.sol';
 import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol';
 

diff --git a/solidity/contracts/modules/response/BondedResponseModule.sol b/solidity/contracts/modules/response/BondedResponseModule.sol
@@ -40,8 +40,8 @@ contract BondedResponseModule is Module, IBondedResponseModule {
       // Allowing one undisputed response at a time
       if (_disputeId == bytes32(0)) revert BondedResponseModule_AlreadyResponded();
       IOracle.DisputeStatus _status = ORACLE.disputeStatus(_disputeId);
-      // TODO: leaving a note here to re-check this check if a new status is added
-      // If the dispute was lost, we assume the proposed answer was correct. DisputeStatus.None should not be reachable due to the previous check.
+      // If the dispute was lost, we assume the proposed answer was correct.
+      // DisputeStatus.None should not be reachable due to the previous check.
       if (_status == IOracle.DisputeStatus.Lost) revert BondedResponseModule_AlreadyResponded();
     }
 
@@ -64,7 +64,6 @@ contract BondedResponseModule is Module, IBondedResponseModule {
   ) external override(IBondedResponseModule, Module) onlyOracle {
     RequestParameters memory _params = decodeRequestData(_request.responseModuleData);
 
-    // TODO: If deadline has passed, we can skip the caller validation
     bool _isModule = ORACLE.allowedModule(_response.requestId, _finalizer);
 
     if (!_isModule && block.number < _params.deadline) {

diff --git a/solidity/interfaces/extensions/IBondEscalationAccounting.sol b/solidity/interfaces/extensions/IBondEscalationAccounting.sol
@@ -89,26 +89,6 @@ interface IBondEscalationAccounting is IAccountingExtension {
     bytes32 indexed _requestId, bytes32 indexed _disputeId, address indexed _pledger, IERC20 _token, uint256 _amount
   );
 
-  /*///////////////////////////////////////////////////////////////
-                              STRUCTS
-  //////////////////////////////////////////////////////////////*/
-
-  /**
-   * @notice Contains the data of the result of an escalation. Is used by users to claim their pledges
-   * @param requestId         The ID of the bond-escalated request
-   * @param forVotesWon       Whether the for votes won the dispute
-   * @param token             The address of the token being paid out
-   * @param amountPerPledger  The amount of token paid to each of the winning pledgers
-   * @param bondEscalationModule The address of the bond escalation module that was used
-   */
-  struct EscalationResult {
-    bytes32 requestId;
-    bool forVotesWon;
-    IERC20 token;
-    uint256 amountPerPledger;
-    IBondEscalationModule bondEscalationModule;
-  }
-
   /*///////////////////////////////////////////////////////////////
                               ERRORS
   //////////////////////////////////////////////////////////////*/
@@ -133,6 +113,26 @@ interface IBondEscalationAccounting is IAccountingExtension {
    */
   error BondEscalationAccounting_AlreadySettled();
 
+  /*///////////////////////////////////////////////////////////////
+                              STRUCTS
+  //////////////////////////////////////////////////////////////*/
+
+  /**
+   * @notice Contains the data of the result of an escalation. Is used by users to claim their pledges
+   * @param requestId         The ID of the bond-escalated request
+   * @param forVotesWon       Whether the for votes won the dispute
+   * @param token             The address of the token being paid out
+   * @param amountPerPledger  The amount of token paid to each of the winning pledgers
+   * @param bondEscalationModule The address of the bond escalation module that was used
+   */
+  struct EscalationResult {
+    bytes32 requestId;
+    bool forVotesWon;
+    IERC20 token;
+    uint256 amountPerPledger;
+    IBondEscalationModule bondEscalationModule;
+  }
+
   /*///////////////////////////////////////////////////////////////
                               VARIABLES
   //////////////////////////////////////////////////////////////*/

diff --git a/solidity/interfaces/modules/dispute/ICircuitResolverModule.sol b/solidity/interfaces/modules/dispute/ICircuitResolverModule.sol
@@ -20,6 +20,15 @@ import {IAccountingExtension} from '../../extensions/IAccountingExtension.sol';
  * and the request is finalized.
  */
 interface ICircuitResolverModule is IDisputeModule {
+  /*///////////////////////////////////////////////////////////////
+                              ERRORS
+  //////////////////////////////////////////////////////////////*/
+
+  /**
+   * @notice Thrown when the verification of a response fails
+   */
+  error CircuitResolverModule_VerificationFailed();
+
   /*///////////////////////////////////////////////////////////////
                               STRUCTS
   //////////////////////////////////////////////////////////////*/
@@ -41,15 +50,6 @@ interface ICircuitResolverModule is IDisputeModule {
     uint256 bondSize;
   }
 
-  /*///////////////////////////////////////////////////////////////
-                              ERRORS
-  //////////////////////////////////////////////////////////////*/
-
-  /**
-   * @notice Thrown when the verification of a response fails
-   */
-  error CircuitResolverModule_VerificationFailed();
-
   /*///////////////////////////////////////////////////////////////
                               LOGIC
   //////////////////////////////////////////////////////////////*/

diff --git a/solidity/interfaces/modules/request/IHttpRequestModule.sol b/solidity/interfaces/modules/request/IHttpRequestModule.sol
@@ -12,6 +12,18 @@ import {IAccountingExtension} from '../../../interfaces/extensions/IAccountingEx
   * @notice Module allowing users to request HTTP calls
   */
 interface IHttpRequestModule is IRequestModule {
+  /*///////////////////////////////////////////////////////////////
+                              ENUMS
+  //////////////////////////////////////////////////////////////*/
+
+  /**
+   * @notice Available HTTP methods
+   */
+  enum HttpMethod {
+    GET,
+    POST
+  }
+
   /*///////////////////////////////////////////////////////////////
                               STRUCTS
   //////////////////////////////////////////////////////////////*/
@@ -34,18 +46,6 @@ interface IHttpRequestModule is IRequestModule {
     uint256 paymentAmount;
   }
 
-  /*///////////////////////////////////////////////////////////////
-                              ENUMS
-  //////////////////////////////////////////////////////////////*/
-
-  /**
-   * @notice Available HTTP methods
-   */
-  enum HttpMethod {
-    GET,
-    POST
-  }
-
   /*///////////////////////////////////////////////////////////////
                               LOGIC
   //////////////////////////////////////////////////////////////*/

diff --git a/solidity/interfaces/modules/resolution/IArbitratorModule.sol b/solidity/interfaces/modules/resolution/IArbitratorModule.sol
@@ -29,17 +29,6 @@ interface IArbitratorModule is IResolutionModule {
    */
   error ArbitratorModule_InvalidResolutionStatus();
 
-  /*///////////////////////////////////////////////////////////////
-                              STRUCTS
-  //////////////////////////////////////////////////////////////*/
-  /**
-   * @notice Parameters of the request as stored in the module
-   * @param arbitrator The address of the arbitrator
-   */
-  struct RequestParameters {
-    address arbitrator;
-  }
-
   /*///////////////////////////////////////////////////////////////
                               ENUMS
   //////////////////////////////////////////////////////////////*/
@@ -53,6 +42,17 @@ interface IArbitratorModule is IResolutionModule {
     Resolved // The arbitration process is resolved
   }
 
+  /*///////////////////////////////////////////////////////////////
+                              STRUCTS
+  //////////////////////////////////////////////////////////////*/
+  /**
+   * @notice Parameters of the request as stored in the module
+   * @param arbitrator The address of the arbitrator
+   */
+  struct RequestParameters {
+    address arbitrator;
+  }
+
   /*///////////////////////////////////////////////////////////////
                               LOGIC
   //////////////////////////////////////////////////////////////*/

diff --git a/solidity/test/unit/extensions/AccountingExtension.t.sol b/solidity/test/unit/extensions/AccountingExtension.t.sol
@@ -5,7 +5,7 @@ import 'forge-std/Test.sol';
 
 import {Helpers} from '../../utils/Helpers.sol';
 
-import {Oracle, IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/contracts/Oracle.sol';
+import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/contracts/Oracle.sol';
 import {
   AccountingExtension, IAccountingExtension, IERC20
 } from '../../../contracts/extensions/AccountingExtension.sol';

diff --git a/solidity/test/unit/modules/dispute/BondedDisputeModule.t.sol b/solidity/test/unit/modules/dispute/BondedDisputeModule.t.sol
@@ -27,7 +27,13 @@ contract BaseTest is Test, Helpers {
   IOracle public oracle;
 
   event DisputeStatusChanged(bytes32 indexed _disputeId, IOracle.Dispute _dispute, IOracle.DisputeStatus _status);
-  // TODO: event ResponseDisputed(bytes32 indexed _requestId, bytes32 indexed _responseId, IOracle.Dispute _dispute, uint256 _blockNumber);
+  event ResponseDisputed(
+    bytes32 indexed _requestId,
+    bytes32 indexed _responseId,
+    bytes32 indexed _disputeId,
+    IOracle.Dispute _dispute,
+    uint256 _blockNumber
+  );
 
   /**
    * @notice Deploy the target and mock oracle
@@ -224,7 +230,9 @@ contract BondedResponseModule_Unit_DisputeResponse is BaseTest {
     mockRequest.disputeModuleData =
       abi.encode(IBondedDisputeModule.RequestParameters(accountingExtension, _token, _bondSize));
     bytes32 _requestId = _getId(mockRequest);
+    mockResponse.requestId = _requestId;
     mockDispute.requestId = _requestId;
+    mockDispute.responseId = _getId(mockResponse);
 
     // Mock and expect the call to the accounting extension, initiating the bond
     _mockAndExpect(
@@ -235,6 +243,10 @@ contract BondedResponseModule_Unit_DisputeResponse is BaseTest {
       abi.encode()
     );
 
+    // Expect the event
+    vm.expectEmit(true, true, true, true, address(bondedDisputeModule));
+    emit ResponseDisputed(_requestId, _getId(mockResponse), _getId(mockDispute), mockDispute, block.number);
+
     // Test: call disputeResponse
     vm.prank(address(oracle));
     bondedDisputeModule.disputeResponse(mockRequest, mockResponse, mockDispute);

diff --git a/solidity/test/unit/modules/dispute/CircuitResolverModule.t.sol b/solidity/test/unit/modules/dispute/CircuitResolverModule.t.sol
@@ -259,7 +259,7 @@ contract CircuitResolverModule_Unit_OnDisputeStatusChange is BaseTest {
     IERC20 _randomToken,
     uint256 _bondSize,
     bytes memory _callData
-  ) public {
+  ) public assumeFuzzable(address(_accountingExtension)) {
     mockRequest.disputeModuleData = abi.encode(
       ICircuitResolverModule.RequestParameters({
         callData: _callData,
@@ -279,15 +279,18 @@ contract CircuitResolverModule_Unit_OnDisputeStatusChange is BaseTest {
     mockResponse.response = _encodedCorrectResponse;
     mockResponse.proposer = makeAddr('proposer');
 
-    // Mock and expect the call to the oracle, finalizing the request
-    _mockAndExpect(address(oracle), abi.encodeCall(IOracle.finalize, (mockRequest, mockResponse)), abi.encode());
-
     // Populate the mock dispute with the correct values
     mockDispute.responseId = _getId(mockResponse);
     mockDispute.requestId = _requestId;
     bytes32 _disputeId = _getId(mockDispute);
     IOracle.DisputeStatus _status = IOracle.DisputeStatus.Lost;
 
+    // Mock and expect the call to the oracle, getting the dispute status
+    _mockAndExpect(address(oracle), abi.encodeCall(IOracle.disputeStatus, (_disputeId)), abi.encode(_status));
+
+    // Mock and expect the call to the oracle, finalizing the request
+    _mockAndExpect(address(oracle), abi.encodeCall(IOracle.finalize, (mockRequest, mockResponse)), abi.encode());
+
     // Check: is the event emitted?
     vm.expectEmit(true, true, true, true, address(circuitResolverModule));
     emit DisputeStatusChanged(_disputeId, mockDispute, _status);
@@ -301,7 +304,7 @@ contract CircuitResolverModule_Unit_OnDisputeStatusChange is BaseTest {
     IERC20 _randomToken,
     uint256 _bondSize,
     bytes memory _callData
-  ) public {
+  ) public assumeFuzzable(address(_accountingExtension)) {
     mockRequest.disputeModuleData = abi.encode(
       ICircuitResolverModule.RequestParameters({
         callData: _callData,
@@ -327,6 +330,9 @@ contract CircuitResolverModule_Unit_OnDisputeStatusChange is BaseTest {
     bytes32 _disputeId = _getId(mockDispute);
     IOracle.DisputeStatus _status = IOracle.DisputeStatus.Won;
 
+    // Mock and expect the call to the oracle, getting the dispute status
+    _mockAndExpect(address(oracle), abi.encodeCall(IOracle.disputeStatus, (_disputeId)), abi.encode(_status));
+
     // Mock and expect the call to the accounting extension, paying the disputer
     _mockAndExpect(
       address(_accountingExtension),