bosonprotocol / contracts

[DEPRECATED] Boson Protocol v1
GNU Lesser General Public License v3.0
69 stars 17 forks source link

Sub-Optimal Modifier Enforcement #327

Closed zajck closed 2 years ago

zajck commented 2 years ago

CAS-01C: Sub-Optimal Modifier Enforcement

Type Severity Location
Gas Optimization Informational Cashier.sol:L219-L220

Description:

The refactoring of the withdraw function lead to its protective modifiers being relocated to the internal distributeAndWithdraw function which will redundantly invoke them three times during the executino of a normal withdraw course.

Example:

 /**
  * @notice Trigger withdrawals of what funds are releasable
  * The caller of this function triggers transfers to all involved entities (pool, issuer, token holder), also paying for gas.
  * @param _tokenIdVoucher  ID of a voucher token (ERC-721) to try withdraw funds from
  */
 function withdraw(uint256 _tokenIdVoucher)
     external
     override
{        bool released = distributeAndWithdraw(_tokenIdVoucher, Entity.ISSUER);
         released = distributeAndWithdraw(_tokenIdVoucher, Entity.HOLDER) || released;
         released = distributeAndWithdraw(_tokenIdVoucher, Entity.POOL) || released;
         require (released, "NOTHING_TO_WITHDRAW");

     }

 /**
  * @notice Trigger withdrawals of what funds are releasable for chosen entity {ISSUER, HOLDER, POOL}
  * @param _tokenIdVoucher  ID of a voucher token (ERC-721) to try withdraw funds from
  * @param _to               recipient, one of {ISSUER, HOLDER, POOL}
  */
  function withdrawSingle(uint256 _tokenIdVoucher, Entity _to)
     external
     override
  {
     require (distributeAndWithdraw(_tokenIdVoucher, _to),
         "NOTHING_TO_WITHDRAW");

 }

 /**
  * @notice Calcuate how much should entity receive and transfer funds to its address
  * @param _tokenIdVoucher  ID of a voucher token (ERC-721) to try withdraw funds from
  * @param _to recipient, one of {ISSUER, HOLDER, POOL}
  */
 function distributeAndWithdraw(uint256 _tokenIdVoucher, Entity _to)
     internal
     nonReentrant
     whenNotPaused
     returns (bool)
 {

Recommendation:

We advise them to be applied at the top-level call instead (i.e. withdraw and withdrawSingle) to optimize the withdraw function's gas cost.