[ETHEREUM-CONTRACTS|SDK-CORE] 1544 new flow operator updates (#1559)

* add new update functions
  - add increaseFlowAllowanceWithPermissions and decreaseFlowAllowanceWithPermissions
  - refactor code slightly to use reuse validation logic for
  - add to SuperTokenV1Library
  - modify default tdd dev script + update readme

packages/ethereum-contracts/.gitignore

@@ -11,3 +11,4 @@
 /.gas-snapshot
 /lib
 /packages
+/testing-benchmark.json

packages/ethereum-contracts/CHANGELOG.md

@@ -5,6 +5,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## Unreleased
 
+### Added
+- `increaseFlowRateAllowanceWithPermissions` and `decreaseFlowRateAllowanceWithPermissions` added to `ConstantFlowAgreementV1.sol`
+
 ### Changed
 - `SuperToken.sol` made external and public methods virtual to facilitate creation of customized implementations.
 - Explicitly set EVM target to "paris" because EIP-3855 isn't yet supported on all chains with Superfluid deployment.

packages/ethereum-contracts/README.md

@@ -313,14 +313,14 @@ You should choose the tests relevant for what you're working on using the [only
 You can put the `only` keyword at any level between whole test suites (`only` appended to a top level `describe`, e.g. `describe.only`) and individual testcases (`it`, e.g. `it.only`).
 With the testing scope defined, run:
 ```
-yarn dev
+yarn dev-hardhat
 ```
 This has [testsuites/all-contracts.js](testsuites/all-contracts.js) as its entrypoint, but if there's an `only` keyword at any nesting level in any of the tests traversed, only that selected subset of tests will be executed.
 The selected test(s) will run once when starting the session and re-run everytime you save changes in a relevant file.
 
-You may also focus on a specific testsuite with yarn dev:
+You may also focus on a specific testsuite with yarn dev-hardhat:
 ```
-yarn dev test/contracts/libs/CallUtils.test.js
+yarn dev-hardhat test/contracts/libs/CallUtils.test.js
 ```
 
 After finishing the session, you can stop the hardhat instance you started in the first step (Ctrl-C).
@@ -337,15 +337,15 @@ python3 testDataToCharts.py output
 ```
 
 #### Foundry
-On the other hand, you can run the development session with foundry with the command: `yarn dev-forge`.
+On the other hand, you can run the development session with foundry with the command: `yarn dev-foundry`.
 
 ### Other Useful Commands
 
 ```
 yarn run-hardhat # run hardhat
 yarn run-truffle # run truffle
-yarn run-forge # run foundry forge
-yarn run-nodemon forge test # use nodemon to run forge test
+yarn run-foundry # run foundry forge
+yarn run-nodemon forge test # use nodemon to run foundry test
 ```
 
 ### Troubleshooting

packages/ethereum-contracts/contracts/agreements/AgreementLibrary.sol

@@ -41,6 +41,8 @@ library AgreementLibrary {
     {
         require(token.getHost() == msg.sender, "unauthorized host");
         require(ISuperfluid(msg.sender).isCtxValid(ctx), "invalid ctx");
+        // [SECURITY] NOTE: we are holding the assumption here that the decoded ctx is correct
+        // at this point.
         return ISuperfluid(msg.sender).decodeCtx(ctx);
     }
 

packages/ethereum-contracts/contracts/agreements/ConstantFlowAgreementV1.sol

@@ -734,7 +734,7 @@ contract ConstantFlowAgreementV1 is
                 ? flowRateAllowance
                 : flowRateAllowance - flowRate;
             if (updatedFlowRateAllowance < 0) revert CFA_ACL_FLOW_RATE_ALLOWANCE_EXCEEDED();
-            _updateFlowRateAllowance(token, flowOperatorId, permissions, updatedFlowRateAllowance);
+            _updateFlowOperatorData(token, flowOperatorId, permissions, updatedFlowRateAllowance);
         }
         {
             _StackVars_createOrUpdateFlow memory flowVars;
@@ -783,7 +783,7 @@ contract ConstantFlowAgreementV1 is
                 ? flowRateAllowance
                 : flowRateAllowance - (flowRate - oldFlowData.flowRate);
             if (updatedFlowRateAllowance < 0) revert CFA_ACL_FLOW_RATE_ALLOWANCE_EXCEEDED();
-            _updateFlowRateAllowance(token, flowOperatorId, permissions, updatedFlowRateAllowance);
+            _updateFlowOperatorData(token, flowOperatorId, permissions, updatedFlowRateAllowance);
         }
 
         {
@@ -826,21 +826,37 @@ contract ConstantFlowAgreementV1 is
         newCtx = _deleteFlow(flowVars, hasPermissions, ctx, currentContext);
     }
 
+    /// @dev IConstantFlowAgreementV1.increaseFlowRateAllowance implementation
+    function increaseFlowRateAllowance(
+        ISuperfluidToken token,
+        address flowOperator,
+        int96 addedFlowRateAllowance, // flowRateBudget
+        bytes calldata ctx
+    ) public override returns (bytes memory newCtx) {
+        newCtx = increaseFlowRateAllowanceWithPermissions(token, flowOperator, 0, addedFlowRateAllowance, ctx);
+    }
+
+    /// @dev IConstantFlowAgreementV1.decreaseFlowRateAllowance implementation
+    function decreaseFlowRateAllowance(
+        ISuperfluidToken token,
+        address flowOperator,
+        int96 subtractedFlowRateAllowance, // flowRateBudget
+        bytes calldata ctx
+    ) public override returns (bytes memory newCtx) {
+        newCtx = decreaseFlowRateAllowanceWithPermissions(token, flowOperator, 0, subtractedFlowRateAllowance, ctx);
+    }
+
     /// @dev IConstantFlowAgreementV1.updateFlowOperatorPermissions implementation
     function updateFlowOperatorPermissions(
         ISuperfluidToken token,
         address flowOperator,
         uint8 permissions,
         int96 flowRateAllowance, // flowRateBudget
         bytes calldata ctx
-    ) public override returns(bytes memory newCtx) {
+    ) public override returns (bytes memory newCtx) {
         newCtx = ctx;
-        if (!FlowOperatorDefinitions.isPermissionsClean(permissions)) revert CFA_ACL_UNCLEAN_PERMISSIONS();
-        ISuperfluid.Context memory currentContext = AgreementLibrary.authorizeTokenAccess(token, ctx);
-        // [SECURITY] NOTE: we are holding the assumption here that ctx is correct and we validate it with
-        // authorizeTokenAccess:
-        if (currentContext.msgSender == flowOperator) revert CFA_ACL_NO_SENDER_FLOW_OPERATOR();
-        if (flowRateAllowance < 0) revert CFA_ACL_NO_NEGATIVE_ALLOWANCE();
+        ISuperfluid.Context memory currentContext =
+            _validateAndAuthorizeUpdateFlowOperatorDataInput(token, flowOperator, permissions, flowRateAllowance, ctx);
         FlowOperatorData memory flowOperatorData;
         flowOperatorData.permissions = permissions;
         flowOperatorData.flowRateAllowance = flowRateAllowance;
@@ -850,90 +866,98 @@ contract ConstantFlowAgreementV1 is
         emit FlowOperatorUpdated(token, currentContext.msgSender, flowOperator, permissions, flowRateAllowance);
     }
 
-    /// @dev IConstantFlowAgreementV1.increaseFlowRateAllowance implementation
-    function increaseFlowRateAllowance(
+    /// @dev IConstantFlowAgreementV1.increaseFlowRateAllowanceWithPermissions implementation
+    function increaseFlowRateAllowanceWithPermissions(
         ISuperfluidToken token,
         address flowOperator,
-        int96 addedFlowRateAllowance, // flowRateBudget
+        uint8 permissionsToAdd,
+        int96 addedFlowRateAllowance,
         bytes calldata ctx
     ) public override returns (bytes memory newCtx) {
         newCtx = ctx;
+        ISuperfluid.Context memory currentContext = _validateAndAuthorizeUpdateFlowOperatorDataInput(
+            token, flowOperator, permissionsToAdd, addedFlowRateAllowance, ctx
+        );
 
-        // [SECURITY] NOTE: we are holding the assumption here that ctx is correct and we validate it with
-        // authorizeTokenAccess:
-        ISuperfluid.Context memory currentContext = AgreementLibrary.authorizeTokenAccess(token, ctx);
-
-        if (currentContext.msgSender == flowOperator) revert CFA_ACL_NO_SENDER_FLOW_OPERATOR();
-        if (addedFlowRateAllowance < 0) revert CFA_ACL_NO_NEGATIVE_ALLOWANCE();
-
-        (
-            bytes32 flowOperatorId,
-            uint8 oldPermissions,
-            int96 oldFlowRateAllowance
-        ) = getFlowOperatorData(token, currentContext.msgSender, flowOperator);
+        (bytes32 flowOperatorId, uint8 oldPermissions, int96 oldFlowRateAllowance) =
+            getFlowOperatorData(token, currentContext.msgSender, flowOperator);
 
         // @note this will revert if it overflows
         int96 newFlowRateAllowance = oldFlowRateAllowance + addedFlowRateAllowance;
-        _updateFlowRateAllowance(
-            token,
-            flowOperatorId,
-            oldPermissions,
-            newFlowRateAllowance
+        uint8 permissions = addPermissions(oldPermissions, permissionsToAdd);
+        _updateFlowOperatorData(
+            token, flowOperatorId, permissions, newFlowRateAllowance
         );
 
         emit FlowOperatorUpdated(
-            token,
-            currentContext.msgSender,
-            flowOperator,
-            oldPermissions,
-            newFlowRateAllowance
+            token, currentContext.msgSender, flowOperator, permissions, newFlowRateAllowance
         );
     }
 
-    /// @dev IConstantFlowAgreementV1.decreaseFlowRateAllowance implementation
-    function decreaseFlowRateAllowance(
+    /// @dev IConstantFlowAgreementV1.decreaseFlowRateAllowanceWithPermissions implementation
+    function decreaseFlowRateAllowanceWithPermissions(
         ISuperfluidToken token,
         address flowOperator,
-        int96 subtractedFlowRateAllowance, // flowRateBudget
+        uint8 permissionsToRemove,
+        int96 subtractedFlowRateAllowance,
         bytes calldata ctx
     ) public override returns (bytes memory newCtx) {
         newCtx = ctx;
+        ISuperfluid.Context memory currentContext = _validateAndAuthorizeUpdateFlowOperatorDataInput(
+            token, flowOperator, permissionsToRemove, subtractedFlowRateAllowance, ctx
+        );
 
-        // [SECURITY] NOTE: we are holding the assumption here that ctx is correct and we validate it with
-        // authorizeTokenAccess:
-        ISuperfluid.Context memory currentContext = AgreementLibrary
-            .authorizeTokenAccess(token, ctx);
+        (bytes32 flowOperatorId, uint8 oldPermissions, int96 oldFlowRateAllowance) =
+            getFlowOperatorData(token, currentContext.msgSender, flowOperator);
 
-        if (currentContext.msgSender == flowOperator) revert CFA_ACL_NO_SENDER_FLOW_OPERATOR();
-        if (subtractedFlowRateAllowance < 0) revert CFA_ACL_NO_NEGATIVE_ALLOWANCE();
-
-        (
-            bytes32 flowOperatorId,
-            uint8 oldPermissions,
-            int96 oldFlowRateAllowance
-        ) = getFlowOperatorData(token, currentContext.msgSender, flowOperator);
-
+        uint8 permissions = removePermissions(oldPermissions, permissionsToRemove);
         int96 newFlowRateAllowance = oldFlowRateAllowance - subtractedFlowRateAllowance;
 
         // @note this defends against negative allowance
         if (newFlowRateAllowance < 0) revert CFA_ACL_NO_NEGATIVE_ALLOWANCE();
-
-        _updateFlowRateAllowance(
-            token,
-            flowOperatorId,
-            oldPermissions,
-            newFlowRateAllowance
+        _updateFlowOperatorData(
+            token, flowOperatorId, permissions, newFlowRateAllowance
         );
 
         emit FlowOperatorUpdated(
-            token,
-            currentContext.msgSender,
-            flowOperator,
-            oldPermissions,
-            newFlowRateAllowance
+            token, currentContext.msgSender, flowOperator, permissions, newFlowRateAllowance
         );
     }
 
+    function addPermissions(uint8 existingPermissions, uint8 permissionDelta)
+        public
+        pure
+        returns (uint8)
+    {
+        return existingPermissions | permissionDelta;
+    }
+
+    function removePermissions(uint8 existingPermissions, uint8 permissionDelta)
+        public
+        pure
+        returns (uint8)
+    {
+        return existingPermissions & (~permissionDelta);
+    }
+
+    /// @dev This function ensures:
+    /// - token access is authorized
+    /// - passed permissions are "clean"
+    /// - no sender flow operator
+    /// - no negative allowance
+    function _validateAndAuthorizeUpdateFlowOperatorDataInput(
+        ISuperfluidToken token,
+        address flowOperator,
+        uint8 permissions,
+        int96 flowAllowance,
+        bytes calldata ctx
+    ) internal returns (ISuperfluid.Context memory currentContext) {
+        if (!FlowOperatorDefinitions.isPermissionsClean(permissions)) revert CFA_ACL_UNCLEAN_PERMISSIONS();
+        currentContext = AgreementLibrary.authorizeTokenAccess(token, ctx);
+        if (currentContext.msgSender == flowOperator) revert CFA_ACL_NO_SENDER_FLOW_OPERATOR();
+        if (flowAllowance < 0) revert CFA_ACL_NO_NEGATIVE_ALLOWANCE();
+    }
+
     /// @dev IConstantFlowAgreementV1.authorizeFlowOperatorWithFullControl implementation
     function authorizeFlowOperatorWithFullControl(
         ISuperfluidToken token,
@@ -1040,17 +1064,19 @@ contract ConstantFlowAgreementV1 is
         return _decodeFlowOperatorData(uint256(data[0]));
     }
 
-    function _updateFlowRateAllowance
+    // @dev This function updates the flow operator data
+    // for `flowOperatorId` and sets it in token storage
+    function _updateFlowOperatorData
     (
         ISuperfluidToken token,
         bytes32 flowOperatorId,
-        uint8 existingPermissions,
+        uint8 updatedPermissions,
         int96 updatedFlowRateAllowance
     )
         private
     {
         FlowOperatorData memory flowOperatorData;
-        flowOperatorData.permissions = existingPermissions;
+        flowOperatorData.permissions = updatedPermissions;
         flowOperatorData.flowRateAllowance = updatedFlowRateAllowance;
         token.updateAgreementData(flowOperatorId, _encodeFlowOperatorData(flowOperatorData));
     }