fix: wond 5 (#55)

* fix: wond-5 * fix: wond-5 * feat: dispute and response validation in accounting extension * fix: remove attack test * test: bond escalation * test: attack now does not work * fix: linter * fix: linter * fix: linter * fix: remove repeated checks
defi-wonderland · Aug 5, 2024 · 1197c32 · 1197c32
1 parent f8f22eb
commit 1197c32
Show file tree

Hide file tree

Showing 18 changed files with 520 additions and 354 deletions.
diff --git a/package.json b/package.json
@@ -38,7 +38,7 @@
     "package.json": "sort-package-json"
   },
   "dependencies": {
-    "@defi-wonderland/prophet-core-contracts": "0.0.0-c25103ea",
+    "@defi-wonderland/prophet-core-contracts": "0.0.0-d01bc1a0",
     "@openzeppelin/contracts": "4.9.5",
     "solmate": "https://github.com/transmissions11/solmate.git#bfc9c25865a274a7827fea5abf6e4fb64fc64e6c"
   },

diff --git a/solidity/contracts/extensions/AccountingExtension.sol b/solidity/contracts/extensions/AccountingExtension.sol
@@ -6,15 +6,14 @@ import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol';
 import {SafeERC20} from '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol';
 import {EnumerableSet} from '@openzeppelin/contracts/utils/structs/EnumerableSet.sol';
 
+import {Validator} from '@defi-wonderland/prophet-core-contracts/solidity/contracts/Validator.sol';
+
 import {IAccountingExtension} from '../../interfaces/extensions/IAccountingExtension.sol';
 
-contract AccountingExtension is IAccountingExtension {
+contract AccountingExtension is Validator, IAccountingExtension {
   using SafeERC20 for IERC20;
   using EnumerableSet for EnumerableSet.AddressSet;
 
-  /// @inheritdoc IAccountingExtension
-  IOracle public immutable ORACLE;
-
   /// @inheritdoc IAccountingExtension
   mapping(address _bonder => mapping(IERC20 _token => uint256 _balance)) public balanceOf;
 
@@ -27,9 +26,7 @@ contract AccountingExtension is IAccountingExtension {
    */
   mapping(address _bonder => EnumerableSet.AddressSet _modules) internal _approvals;
 
-  constructor(IOracle _oracle) {
-    ORACLE = _oracle;
-  }
+  constructor(IOracle _oracle) Validator(_oracle) {}
 
   /**
    * @notice Checks that the caller is an allowed module used in the request.

diff --git a/solidity/contracts/extensions/BondEscalationAccounting.sol b/solidity/contracts/extensions/BondEscalationAccounting.sol
@@ -24,11 +24,15 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
   /// @inheritdoc IBondEscalationAccounting
   function pledge(
     address _pledger,
-    bytes32 _requestId,
-    bytes32 _disputeId,
+    IOracle.Request calldata _request,
+    IOracle.Dispute calldata _dispute,
     IERC20 _token,
     uint256 _amount
-  ) external onlyAllowedModule(_requestId) {
+  ) external {
+    bytes32 _requestId = _getId(_request);
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
+
+    if (!ORACLE.allowedModule(_requestId, msg.sender)) revert AccountingExtension_UnauthorizedModule();
     if (balanceOf[_pledger][_token] < _amount) revert BondEscalationAccounting_InsufficientFunds();
 
     pledges[_disputeId][_token] += _amount;
@@ -42,12 +46,17 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
 
   /// @inheritdoc IBondEscalationAccounting
   function onSettleBondEscalation(
-    bytes32 _requestId,
-    bytes32 _disputeId,
+    IOracle.Request calldata _request,
+    IOracle.Dispute calldata _dispute,
     IERC20 _token,
     uint256 _amountPerPledger,
     uint256 _winningPledgersLength
-  ) external onlyAllowedModule(_requestId) {
+  ) external {
+    bytes32 _requestId = _getId(_request);
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
+
+    if (!ORACLE.allowedModule(_requestId, msg.sender)) revert AccountingExtension_UnauthorizedModule();
+
     if (pledges[_disputeId][_token] < _amountPerPledger * _winningPledgersLength) {
       revert BondEscalationAccounting_InsufficientFunds();
     }
@@ -113,12 +122,17 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
 
   /// @inheritdoc IBondEscalationAccounting
   function releasePledge(
-    bytes32 _requestId,
-    bytes32 _disputeId,
+    IOracle.Request calldata _request,
+    IOracle.Dispute calldata _dispute,
     address _pledger,
     IERC20 _token,
     uint256 _amount
-  ) external onlyAllowedModule(_requestId) {
+  ) external {
+    bytes32 _requestId = _getId(_request);
+    bytes32 _disputeId = _validateDispute(_request, _dispute);
+
+    if (!ORACLE.allowedModule(_requestId, msg.sender)) revert AccountingExtension_UnauthorizedModule();
+
     if (pledges[_disputeId][_token] < _amount) revert BondEscalationAccounting_InsufficientFunds();
 
     balanceOf[_pledger][_token] += _amount;

diff --git a/solidity/contracts/modules/dispute/BondEscalationModule.sol b/solidity/contracts/modules/dispute/BondEscalationModule.sol
@@ -102,8 +102,8 @@ contract BondEscalationModule is Module, IBondEscalationModule {
 
         if (_escalation.amountOfPledgesForDispute + _escalation.amountOfPledgesAgainstDispute > 0) {
           _params.accountingExtension.onSettleBondEscalation({
-            _requestId: _dispute.requestId,
-            _disputeId: _disputeId,
+            _request: _request,
+            _dispute: _dispute,
             _token: _params.bondToken,
             _amountPerPledger: _params.bondSize,
             _winningPledgersLength: _escalation.amountOfPledgesForDispute + _escalation.amountOfPledgesAgainstDispute
@@ -133,8 +133,8 @@ contract BondEscalationModule is Module, IBondEscalationModule {
               + FixedPointMathLib.mulDivDown(_pledgesForDispute, _params.bondSize, _pledgesAgainstDispute);
 
           _params.accountingExtension.onSettleBondEscalation({
-            _requestId: _dispute.requestId,
-            _disputeId: _escalation.disputeId,
+            _request: _request,
+            _dispute: _dispute,
             _token: _params.bondToken,
             _amountPerPledger: _amountToPay,
             _winningPledgersLength: _won ? _pledgesForDispute : _pledgesAgainstDispute
@@ -217,8 +217,8 @@ contract BondEscalationModule is Module, IBondEscalationModule {
     pledgesForDispute[_dispute.requestId][msg.sender] += 1;
     _params.accountingExtension.pledge({
       _pledger: msg.sender,
-      _requestId: _dispute.requestId,
-      _disputeId: _disputeId,
+      _request: _request,
+      _dispute: _dispute,
       _token: _params.bondToken,
       _amount: _params.bondSize
     });
@@ -235,8 +235,8 @@ contract BondEscalationModule is Module, IBondEscalationModule {
     pledgesAgainstDispute[_dispute.requestId][msg.sender] += 1;
     _params.accountingExtension.pledge({
       _pledger: msg.sender,
-      _requestId: _dispute.requestId,
-      _disputeId: _disputeId,
+      _request: _request,
+      _dispute: _dispute,
       _token: _params.bondToken,
       _amount: _params.bondSize
     });

diff --git a/solidity/contracts/modules/resolution/BondEscalationResolutionModule.sol b/solidity/contracts/modules/resolution/BondEscalationResolutionModule.sol
@@ -140,8 +140,8 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
       _reward = FixedPointMathLib.mulDivDown(_escalation.pledgesAgainst, _pledgerProportion, BASE);
       _amountToRelease = _reward + _pledgerBalanceBefore;
       _claimPledge({
-        _requestId: _dispute.requestId,
-        _disputeId: _disputeId,
+        _request: _request,
+        _dispute: _dispute,
         _amountToRelease: _amountToRelease,
         _resolution: _escalation.resolution,
         _params: _params
@@ -153,8 +153,8 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
       _reward = FixedPointMathLib.mulDivDown(_escalation.pledgesFor, _pledgerProportion, BASE);
       _amountToRelease = _reward + _pledgerBalanceBefore;
       _claimPledge({
-        _requestId: _dispute.requestId,
-        _disputeId: _disputeId,
+        _request: _request,
+        _dispute: _dispute,
         _amountToRelease: _amountToRelease,
         _resolution: _escalation.resolution,
         _params: _params
@@ -166,8 +166,8 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
       if (_pledgerBalanceFor > 0) {
         pledgesForDispute[_disputeId][msg.sender] -= _pledgerBalanceFor;
         _claimPledge({
-          _requestId: _dispute.requestId,
-          _disputeId: _disputeId,
+          _request: _request,
+          _dispute: _dispute,
           _amountToRelease: _pledgerBalanceFor,
           _resolution: _escalation.resolution,
           _params: _params
@@ -177,8 +177,8 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
       if (_pledgerBalanceAgainst > 0) {
         pledgesAgainstDispute[_disputeId][msg.sender] -= _pledgerBalanceAgainst;
         _claimPledge({
-          _requestId: _dispute.requestId,
-          _disputeId: _disputeId,
+          _request: _request,
+          _dispute: _dispute,
           _amountToRelease: _pledgerBalanceAgainst,
           _resolution: _escalation.resolution,
           _params: _params
@@ -220,8 +220,8 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
 
     _params.accountingExtension.pledge({
       _pledger: msg.sender,
-      _requestId: _dispute.requestId,
-      _disputeId: _disputeId,
+      _request: _request,
+      _dispute: _dispute,
       _token: _params.bondToken,
       _amount: _pledgeAmount
     });
@@ -308,30 +308,30 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu
   /**
    * @notice Releases the pledged funds to the pledger
    *
-   * @param _requestId The ID of the request
-   * @param _disputeId The ID of the dispute
+   * @param _request The request
+   * @param _dispute The dispute
    * @param _amountToRelease The amount to release
    * @param _resolution The resolution of the dispute
    * @param _params The request parameters
    */
   function _claimPledge(
-    bytes32 _requestId,
-    bytes32 _disputeId,
+    IOracle.Request calldata _request,
+    IOracle.Dispute calldata _dispute,
     uint256 _amountToRelease,
     Resolution _resolution,
     RequestParameters memory _params
   ) internal {
     _params.accountingExtension.releasePledge({
-      _requestId: _requestId,
-      _disputeId: _disputeId,
+      _request: _request,
+      _dispute: _dispute,
       _pledger: msg.sender,
       _token: _params.bondToken,
       _amount: _amountToRelease
     });
 
     emit PledgeClaimed({
-      _requestId: _requestId,
-      _disputeId: _disputeId,
+      _requestId: _getId(_request),
+      _disputeId: _getId(_dispute),
       _pledger: msg.sender,
       _token: _params.bondToken,
       _pledgeReleased: _amountToRelease,

diff --git a/solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol b/solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol
@@ -53,7 +53,6 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule {
   /// @inheritdoc IPrivateERC20ResolutionModule
   function commitVote(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute, bytes32 _commitment) public {
     bytes32 _disputeId = _validateDispute(_request, _dispute);
-    if (ORACLE.disputeCreatedAt(_disputeId) == 0) revert PrivateERC20ResolutionModule_NonExistentDispute();
     if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated) {
       revert PrivateERC20ResolutionModule_AlreadyResolved();
     }
@@ -113,7 +112,6 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule {
     IOracle.Response calldata _response,
     IOracle.Dispute calldata _dispute
   ) external onlyOracle {
-    if (ORACLE.disputeCreatedAt(_disputeId) == 0) revert PrivateERC20ResolutionModule_NonExistentDispute();
     if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated) {
       revert PrivateERC20ResolutionModule_AlreadyResolved();
     }

diff --git a/solidity/interfaces/extensions/IAccountingExtension.sol b/solidity/interfaces/extensions/IAccountingExtension.sol
@@ -96,12 +96,6 @@ interface IAccountingExtension {
                             VARIABLES
   //////////////////////////////////////////////////////////////*/
 
-  /**
-   * @notice Returns the interface for the Oracle contract
-   * @return _oracle The Oracle address
-   */
-  function ORACLE() external view returns (IOracle _oracle);
-
   /**
    * @notice Returns the amount of a token a user has bonded
    * @param _user The address of the user with bonded tokens

diff --git a/solidity/interfaces/extensions/IBondEscalationAccounting.sol b/solidity/interfaces/extensions/IBondEscalationAccounting.sol
@@ -3,6 +3,8 @@ pragma solidity ^0.8.19;
 
 import {IBondEscalationModule} from '../modules/dispute/IBondEscalationModule.sol';
 import {IAccountingExtension} from './IAccountingExtension.sol';
+
+import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol';
 import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol';
 
 /**
@@ -170,26 +172,32 @@ interface IBondEscalationAccounting is IAccountingExtension {
    * @dev This function must be called by an allowed module
    *
    * @param _pledger           Address of the pledger
-   * @param _requestId         The ID of the bond-escalated request
-   * @param _disputeId         The ID of the bond-escalated dispute
+   * @param _request           The bond-escalated request
+   * @param _dispute           The bond-escalated dispute
    * @param _token             Address of the token being paid as a reward for winning the bond escalation
    * @param _amount            Amount of token to pledge
    */
-  function pledge(address _pledger, bytes32 _requestId, bytes32 _disputeId, IERC20 _token, uint256 _amount) external;
+  function pledge(
+    address _pledger,
+    IOracle.Request calldata _request,
+    IOracle.Dispute calldata _dispute,
+    IERC20 _token,
+    uint256 _amount
+  ) external;
 
   /**
    * @notice Updates the accounting of the given dispute to reflect the result of the bond escalation
    * @dev This function must be called by an allowed module
    *
-   * @param _requestId              The ID of the bond-escalated request
-   * @param _disputeId              The ID of the bond-escalated dispute
+   * @param _request                The bond-escalated request
+   * @param _dispute                The bond-escalated dispute
    * @param _token                  Address of the token being paid as a reward for winning the bond escalation
    * @param _amountPerPledger       Amount of `_token` to be rewarded to each of the winning pledgers
    * @param _winningPledgersLength  Amount of pledges that won the dispute
    */
   function onSettleBondEscalation(
-    bytes32 _requestId,
-    bytes32 _disputeId,
+    IOracle.Request calldata _request,
+    IOracle.Dispute calldata _dispute,
     IERC20 _token,
     uint256 _amountPerPledger,
     uint256 _winningPledgersLength
@@ -200,15 +208,15 @@ interface IBondEscalationAccounting is IAccountingExtension {
    *
    * @dev This function must be called by an allowed module
    *
-   * @param _requestId         The ID of the bond-escalated request
-   * @param _disputeId         The ID of the bond-escalated dispute
+   * @param _request           The bond-escalated request
+   * @param _dispute           The bond-escalated dispute
    * @param _pledger           Address of the pledger
    * @param _token             Address of the token to be released
    * @param _amount            Amount of `_token` to be released to the pledger
    */
   function releasePledge(
-    bytes32 _requestId,
-    bytes32 _disputeId,
+    IOracle.Request calldata _request,
+    IOracle.Dispute calldata _dispute,
     address _pledger,
     IERC20 _token,
     uint256 _amount

diff --git a/solidity/interfaces/modules/resolution/IPrivateERC20ResolutionModule.sol b/solidity/interfaces/modules/resolution/IPrivateERC20ResolutionModule.sol
@@ -70,11 +70,6 @@ interface IPrivateERC20ResolutionModule is IResolutionModule {
    */
   error PrivateERC20ResolutionModule_OnGoingRevealingPhase();
 
-  /**
-   * @notice Thrown when trying to resolve a dispute that does not exist
-   */
-  error PrivateERC20ResolutionModule_NonExistentDispute();
-
   /**
    * @notice Thrown when trying to commit an empty commitment
    */