refactor: validate dispute through Module

solidity/contracts/modules/dispute/BondEscalationModule.sol

@@ -211,7 +211,7 @@ contract BondEscalationModule is Module, IBondEscalationModule {
   /// @inheritdoc IBondEscalationModule
   function pledgeForDispute(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external {
     bytes32 _disputeId = _getId(_dispute);
-    RequestParameters memory _params = _pledgeChecks(_disputeId, _request, _dispute, true);
+    RequestParameters memory _params = _pledgeChecks(_request, _dispute, true);
 
     _escalations[_dispute.requestId].amountOfPledgesForDispute += 1;
     pledgesForDispute[_dispute.requestId][msg.sender] += 1;
@@ -229,7 +229,7 @@ contract BondEscalationModule is Module, IBondEscalationModule {
   /// @inheritdoc IBondEscalationModule
   function pledgeAgainstDispute(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external {
     bytes32 _disputeId = _getId(_dispute);
-    RequestParameters memory _params = _pledgeChecks(_disputeId, _request, _dispute, false);
+    RequestParameters memory _params = _pledgeChecks(_request, _dispute, false);
 
     _escalations[_dispute.requestId].amountOfPledgesAgainstDispute += 1;
     pledgesAgainstDispute[_dispute.requestId][msg.sender] += 1;
@@ -281,22 +281,18 @@ contract BondEscalationModule is Module, IBondEscalationModule {
 
   /**
    * @notice Checks the necessary conditions for pledging
-   * @param _disputeId The ID of the dispute to pledge for or against
    * @param _request The request data
    * @param _dispute The dispute data
    * @param _forDispute Whether the pledge is for or against the dispute
    * @return _params The decoded parameters for the request
    */
   function _pledgeChecks(
-    bytes32 _disputeId,
     IOracle.Request calldata _request,
     IOracle.Dispute calldata _dispute,
     bool _forDispute
   ) internal view returns (RequestParameters memory _params) {
-    bytes32 _requestId = _getId(_request);
-    if (_requestId != _dispute.requestId) revert BondEscalationModule_InvalidRequestId();
-
-    BondEscalation memory _escalation = _escalations[_requestId];
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
+    BondEscalation memory _escalation = _escalations[_dispute.requestId];
 
     if (_disputeId != _escalation.disputeId) {
       revert BondEscalationModule_InvalidDispute();

solidity/contracts/modules/resolution/BondEscalationResolutionModule.sol

@@ -122,10 +122,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
 
   /// @inheritdoc IBondEscalationResolutionModule
   function claimPledge(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external {
-    bytes32 _requestId = _getId(_request);
-    if (_requestId != _dispute.requestId) revert BondEscalationResolutionModule_InvalidRequestId();
-
-    bytes32 _disputeId = _getId(_dispute);
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
     Escalation storage _escalation = escalations[_disputeId];
 
     if (_escalation.resolution == Resolution.Unresolved) revert BondEscalationResolutionModule_NotResolved();
@@ -143,7 +140,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
       _reward = FixedPointMathLib.mulDivDown(_escalation.pledgesAgainst, _pledgerProportion, BASE);
       _amountToRelease = _reward + _pledgerBalanceBefore;
       _claimPledge({
-        _requestId: _requestId,
+        _requestId: _dispute.requestId,
         _disputeId: _disputeId,
         _amountToRelease: _amountToRelease,
         _resolution: _escalation.resolution,
@@ -156,7 +153,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
       _reward = FixedPointMathLib.mulDivDown(_escalation.pledgesFor, _pledgerProportion, BASE);
       _amountToRelease = _reward + _pledgerBalanceBefore;
       _claimPledge({
-        _requestId: _requestId,
+        _requestId: _dispute.requestId,
         _disputeId: _disputeId,
         _amountToRelease: _amountToRelease,
         _resolution: _escalation.resolution,
@@ -169,7 +166,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
       if (_pledgerBalanceFor > 0) {
         pledgesForDispute[_disputeId][msg.sender] -= _pledgerBalanceFor;
         _claimPledge({
-          _requestId: _requestId,
+          _requestId: _dispute.requestId,
           _disputeId: _disputeId,
           _amountToRelease: _pledgerBalanceFor,
           _resolution: _escalation.resolution,
@@ -180,7 +177,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
       if (_pledgerBalanceAgainst > 0) {
         pledgesAgainstDispute[_disputeId][msg.sender] -= _pledgerBalanceAgainst;
         _claimPledge({
-          _requestId: _requestId,
+          _requestId: _dispute.requestId,
           _disputeId: _disputeId,
           _amountToRelease: _pledgerBalanceAgainst,
           _resolution: _escalation.resolution,
@@ -204,10 +201,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
     uint256 _pledgeAmount,
     bool _pledgingFor
   ) internal {
-    bytes32 _requestId = _getId(_request);
-    if (_requestId != _dispute.requestId) revert BondEscalationResolutionModule_InvalidRequestId();
-
-    bytes32 _disputeId = _getId(_dispute);
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
     Escalation storage _escalation = escalations[_disputeId];
 
     if (_escalation.startTime == 0) revert BondEscalationResolutionModule_NotEscalated();
@@ -226,7 +220,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
 
     _params.accountingExtension.pledge({
       _pledger: msg.sender,
-      _requestId: _requestId,
+      _requestId: _dispute.requestId,
       _disputeId: _disputeId,
       _token: _params.bondToken,
       _amount: _pledgeAmount
@@ -239,15 +233,15 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
 
       _escalation.pledgesFor += _pledgeAmount;
       pledgesForDispute[_disputeId][msg.sender] += _pledgeAmount;
-      emit PledgedForDispute(msg.sender, _requestId, _disputeId, _pledgeAmount);
+      emit PledgedForDispute(msg.sender, _dispute.requestId, _disputeId, _pledgeAmount);
     } else {
       if (_inequalityData.inequalityStatus == InequalityStatus.ForTurnToEqualize) {
         revert BondEscalationResolutionModule_ForTurnToEqualize();
       }
 
       _escalation.pledgesAgainst += _pledgeAmount;
       pledgesAgainstDispute[_disputeId][msg.sender] += _pledgeAmount;
-      emit PledgedAgainstDispute(msg.sender, _requestId, _disputeId, _pledgeAmount);
+      emit PledgedAgainstDispute(msg.sender, _dispute.requestId, _disputeId, _pledgeAmount);
     }
 
     if (_escalation.pledgesFor + _escalation.pledgesAgainst >= _params.pledgeThreshold) {

solidity/contracts/modules/resolution/ERC20ResolutionModule.sol

@@ -56,10 +56,7 @@ contract ERC20ResolutionModule is Module, IERC20ResolutionModule {
     IOracle.Dispute calldata _dispute,
     uint256 _numberOfVotes
   ) public {
-    bytes32 _requestId = _getId(_request);
-    if (_requestId != _dispute.requestId) revert ERC20ResolutionModule_InvalidRequestId();
-
-    bytes32 _disputeId = _getId(_dispute);
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
     Escalation memory _escalation = escalations[_disputeId];
     if (_escalation.startTime == 0) revert ERC20ResolutionModule_DisputeNotEscalated();
     if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated) {
@@ -75,7 +72,7 @@ contract ERC20ResolutionModule is Module, IERC20ResolutionModule {
     _voters[_disputeId].add(msg.sender);
     escalations[_disputeId].totalVotes += _numberOfVotes;
 
-    _params.accountingExtension.bond(msg.sender, _requestId, _params.votingToken, _numberOfVotes);
+    _params.accountingExtension.bond(msg.sender, _dispute.requestId, _params.votingToken, _numberOfVotes);
     emit VoteCast(msg.sender, _disputeId, _numberOfVotes);
   }
 
@@ -114,10 +111,7 @@ contract ERC20ResolutionModule is Module, IERC20ResolutionModule {
 
   /// @inheritdoc IERC20ResolutionModule
   function claimVote(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external {
-    bytes32 _requestId = _getId(_request);
-    if (_requestId != _dispute.requestId) revert ERC20ResolutionModule_InvalidRequestId();
-
-    bytes32 _disputeId = _getId(_dispute);
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
     Escalation memory _escalation = escalations[_disputeId];
 
     // Check that voting deadline is over
@@ -127,7 +121,7 @@ contract ERC20ResolutionModule is Module, IERC20ResolutionModule {
 
     // Transfer the tokens back to the voter
     uint256 _amount = votes[_disputeId][msg.sender];
-    _params.accountingExtension.release(msg.sender, _requestId, _params.votingToken, _amount);
+    _params.accountingExtension.release(msg.sender, _dispute.requestId, _params.votingToken, _amount);
 
     emit VoteClaimed(msg.sender, _disputeId, _amount);
   }

solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol

@@ -52,10 +52,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule {
 
   /// @inheritdoc IPrivateERC20ResolutionModule
   function commitVote(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute, bytes32 _commitment) public {
-    bytes32 _requestId = _getId(_request);
-    if (_requestId != _dispute.requestId) revert PrivateERC20ResolutionModule_InvalidRequestId();
-
-    bytes32 _disputeId = _getId(_dispute);
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
     if (ORACLE.createdAt(_disputeId) == 0) revert PrivateERC20ResolutionModule_NonExistentDispute();
     if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated) {
       revert PrivateERC20ResolutionModule_AlreadyResolved();
@@ -81,10 +78,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule {
     uint256 _numberOfVotes,
     bytes32 _salt
   ) public {
-    bytes32 _requestId = _getId(_request);
-    if (_requestId != _dispute.requestId) revert PrivateERC20ResolutionModule_InvalidRequestId();
-
-    bytes32 _disputeId = _getId(_dispute);
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
     Escalation memory _escalation = escalations[_disputeId];
     if (_escalation.startTime == 0) revert PrivateERC20ResolutionModule_DisputeNotEscalated();
 

solidity/interfaces/modules/dispute/IBondEscalationModule.sol

@@ -50,10 +50,6 @@ interface IBondEscalationModule is IDisputeModule {
                               ERRORS
   //////////////////////////////////////////////////////////////*/
 
-  /**
-   * @notice Thrown when a request does not match a request id.
-   */
-  error BondEscalationModule_InvalidRequestId();
   /**
    * @notice Thrown when trying to escalate a dispute going through the bond escalation module before its deadline.
    */

solidity/interfaces/modules/resolution/IBondEscalationResolutionModule.sol

@@ -73,11 +73,6 @@ interface IBondEscalationResolutionModule is IResolutionModule {
                               ERRORS
   //////////////////////////////////////////////////////////////*/
 
-  /**
-   * @notice Thrown when a request does not match a request id.
-   */
-  error BondEscalationResolutionModule_InvalidRequestId();
-
   /**
    * @notice Thrown when the user tries to resolve a dispute that has already been resolved.
    */

solidity/interfaces/modules/resolution/IERC20ResolutionModule.sol

@@ -48,16 +48,6 @@ interface IERC20ResolutionModule is IResolutionModule {
                               ERRORS
   //////////////////////////////////////////////////////////////*/
 
-  /**
-   * @notice Throws if the caller is not the dispute module
-   */
-  error ERC20ResolutionModule_OnlyDisputeModule();
-
-  /**
-   * @notice Thrown when a request does not match a request id
-   */
-  error ERC20ResolutionModule_InvalidRequestId();
-
   /**
    * @notice Throws if the dispute doesn't exist or has not been escalated
    */

solidity/interfaces/modules/resolution/IPrivateERC20ResolutionModule.sol

@@ -45,11 +45,6 @@ interface IPrivateERC20ResolutionModule is IResolutionModule {
                               ERRORS
   //////////////////////////////////////////////////////////////*/
 
-  /**
-   * @notice Thrown when a request does not match a request id
-   */
-  error PrivateERC20ResolutionModule_InvalidRequestId();
-
   /**
    * @notice Thrown when the dispute has not been escalated
    */

solidity/test/unit/modules/dispute/BondEscalationModule.t.sol

@@ -933,20 +933,18 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest {
 
 contract BondEscalationModule_Unit_PledgeForDispute is BaseTest {
   /**
-   * @notice Tests that pledgeForDispute reverts if request ids do not match.
+   * @notice Tests that pledgeForDispute reverts if the dispute body is invalid.
    */
-  function test_revertIfInvalidRequestId() public {
-    // Check: does it revert if request ids do not match?
-    vm.expectRevert(IBondEscalationModule.BondEscalationModule_InvalidRequestId.selector);
+  function test_revertIfInvalidDisputeBody() public {
+    // Check: does it revert if the dispute body is invalid?
+    vm.expectRevert(IModule.Module_InvalidDisputeBody.selector);
     bondEscalationModule.pledgeForDispute(mockRequest, mockDispute);
   }
 
   /**
    * @notice Tests that pledgeForDispute reverts if the dispute is not going through the bond escalation mechanism.
    */
-  function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess(bytes32 _disputeId) public {
-    vm.assume(_disputeId > 0);
-
+  function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess() public {
     bytes32 _requestId = _getId(mockRequest);
     mockDispute.requestId = _requestId;
 
@@ -1121,20 +1119,18 @@ contract BondEscalationModule_Unit_PledgeForDispute is BaseTest {
 
 contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest {
   /**
-   * @notice Tests that pledgeAgainstDispute reverts if request ids do not match.
+   * @notice Tests that pledgeAgainstDispute reverts if the dispute body is invalid.
    */
-  function test_revertIfInvalidRequestId() public {
-    // Check: does it revert if request ids do not match?
-    vm.expectRevert(IBondEscalationModule.BondEscalationModule_InvalidRequestId.selector);
+  function test_revertIfInvalidDisputeBody() public {
+    // Check: does it revert if the dispute body is invalid?
+    vm.expectRevert(IModule.Module_InvalidDisputeBody.selector);
     bondEscalationModule.pledgeAgainstDispute(mockRequest, mockDispute);
   }
 
   /**
    * @notice Tests that pledgeAgainstDispute reverts if the dispute is not going through the bond escalation mechanism.
    */
-  function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess(bytes32 _disputeId) public {
-    vm.assume(_disputeId > 0);
-
+  function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess() public {
     bytes32 _requestId = _getId(mockRequest);
     mockDispute.requestId = _requestId;
 

solidity/test/unit/modules/resolution/BondEscalationResolutionModule.t.sol

@@ -255,8 +255,8 @@ contract BondEscalationResolutionModule_Unit_PledgeForDispute is BaseTest {
     uint256 _pledgeAmount,
     IBondEscalationResolutionModule.RequestParameters memory _params
   ) public assumeFuzzable(address(_params.accountingExtension)) {
-    // Check: does it revert if request ids do not match?
-    vm.expectRevert(IBondEscalationResolutionModule.BondEscalationResolutionModule_InvalidRequestId.selector);
+    // Check: does it revert if the dispute body is invalid?
+    vm.expectRevert(IModule.Module_InvalidDisputeBody.selector);
     module.pledgeForDispute(mockRequest, mockDispute, _pledgeAmount);
 
     // 1. BondEscalationResolutionModule_NotEscalated
@@ -540,8 +540,8 @@ contract BondEscalationResolutionModule_Unit_PledgeAgainstDispute is BaseTest {
     uint256 _pledgeAmount,
     IBondEscalationResolutionModule.RequestParameters memory _params
   ) public assumeFuzzable(address(_params.accountingExtension)) {
-    // Check: does it revert if request ids do not match?
-    vm.expectRevert(IBondEscalationResolutionModule.BondEscalationResolutionModule_InvalidRequestId.selector);
+    // Check: does it revert if the dispute body is invalid?
+    vm.expectRevert(IModule.Module_InvalidDisputeBody.selector);
     module.pledgeAgainstDispute(mockRequest, mockDispute, _pledgeAmount);
 
     // 1. BondEscalationResolutionModule_NotEscalated
@@ -1013,8 +1013,8 @@ contract BondEscalationResolutionModule_Unit_ClaimPledge is BaseTest {
     address _randomPledger,
     IBondEscalationResolutionModule.RequestParameters memory _params
   ) public assumeFuzzable(address(_params.accountingExtension)) {
-    // Check: does it revert if request ids do not match?
-    vm.expectRevert(IBondEscalationResolutionModule.BondEscalationResolutionModule_InvalidRequestId.selector);
+    // Check: does it revert if the dispute body is invalid?
+    vm.expectRevert(IModule.Module_InvalidDisputeBody.selector);
     module.claimPledge(mockRequest, mockDispute);
 
     (,, bytes32 _disputeId) = _setResolutionModuleData(_params);

solidity/test/unit/modules/resolution/ERC20ResolutionModule.t.sol

@@ -203,11 +203,11 @@ contract ERC20ResolutionModule_Unit_CastVote is BaseTest {
   }
 
   /**
-   * @notice Test that `castVote` reverts if request ids do not match.
+   * @notice Test that `castVote` reverts if the dispute body is invalid.
    */
-  function test_revertIfInvalidRequestId(uint256 _numberOfVotes) public {
-    // Check: does it revert if request ids do not match?
-    vm.expectRevert(IERC20ResolutionModule.ERC20ResolutionModule_InvalidRequestId.selector);
+  function test_revertIfInvalidDisputeBody(uint256 _numberOfVotes) public {
+    // Check: does it revert if the dispute body is invalid?
+    vm.expectRevert(IModule.Module_InvalidDisputeBody.selector);
     module.castVote(mockRequest, mockDispute, _numberOfVotes);
   }
 
@@ -388,11 +388,11 @@ contract ERC20ResolutionModule_Unit_ResolveDispute is BaseTest {
 
 contract ERC20ResolutionModule_Unit_ClaimVote is BaseTest {
   /**
-   * @notice Reverts if request ids do not match
+   * @notice Reverts if the dispute body is invalid
    */
-  function test_revertIfInvalidRequestId() public {
-    // Check: does it revert if request ids do not match?
-    vm.expectRevert(IERC20ResolutionModule.ERC20ResolutionModule_InvalidRequestId.selector);
+  function test_revertIfInvalidDisputeBody() public {
+    // Check: does it revert if the dispute body is invalid?
+    vm.expectRevert(IModule.Module_InvalidDisputeBody.selector);
     module.claimVote(mockRequest, mockDispute);
   }
 

solidity/test/unit/modules/resolution/PrivateERC20ResolutionModule.t.sol

@@ -226,11 +226,11 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest {
   }
 
   /**
-   * @notice Test that `commitVote` reverts if request ids do not match.
+   * @notice Test that `commitVote` reverts if the dispute body is invalid.
    */
-  function test_revertIfInvalidRequestId(bytes32 _commitment) public {
-    // Check: does it revert if request ids do not match?
-    vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_InvalidRequestId.selector);
+  function test_revertIfInvalidDisputeBody(bytes32 _commitment) public {
+    // Check: does it revert if the dispute body is invalid?
+    vm.expectRevert(IModule.Module_InvalidDisputeBody.selector);
     module.commitVote(mockRequest, mockDispute, _commitment);
   }
 
@@ -455,11 +455,11 @@ contract PrivateERC20ResolutionModule_Unit_RevealVote is BaseTest {
   }
 
   /**
-   * @notice Test that `revealVote` reverts if request ids do not match.
+   * @notice Test that `revealVote` reverts if the dispute body is invalid.
    */
-  function test_revertIfInvalidRequestId(uint256 _numberOfVotes, bytes32 _salt) public {
-    // Check: does it revert if request ids do not match?
-    vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_InvalidRequestId.selector);
+  function test_revertIfInvalidDisputeBody(uint256 _numberOfVotes, bytes32 _salt) public {
+    // Check: does it revert if the dispute body is invalid?
+    vm.expectRevert(IModule.Module_InvalidDisputeBody.selector);
     module.revealVote(mockRequest, mockDispute, _numberOfVotes, _salt);
   }