[ETHEREUM-CONTRACTS] 1.14.1: backport GDA patch, adjust truffle verification pipeline vor etherscan v2 (#2121)

Author: d10r

packages/automation-contracts/autowrap/package.json

@@ -3,7 +3,7 @@
     "description": "Open contracts that allow upgrading underlying token to supertokens based on running stream",
     "version": "0.3.0",
     "devDependencies": {
-        "@superfluid-finance/ethereum-contracts": "^1.14.0",
+        "@superfluid-finance/ethereum-contracts": "^1.14.1",
         "@superfluid-finance/metadata": "^1.6.2"
     },
     "license": "MIT",

packages/automation-contracts/scheduler/package.json

@@ -3,7 +3,7 @@
     "description": "Open contracts that allow scheduling streams and vestings onchain",
     "version": "1.3.0",
     "devDependencies": {
-        "@superfluid-finance/ethereum-contracts": "^1.14.0",
+        "@superfluid-finance/ethereum-contracts": "^1.14.1",
         "@superfluid-finance/metadata": "^1.6.2"
     },
     "license": "MIT",

packages/ethereum-contracts/CHANGELOG.md

@@ -3,7 +3,7 @@ All notable changes to the ethereum-contracts will be documented in this file.
 
 This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [v1.14.0]
+## [v1.14.1]
 
 ### Added
 - GDA _autoconnect_ feature: now any account can connect pool members using `tryConnectPoolFor()` as long as they have less than 4 connection slots occupied for that Super Token. This allows for smoother onboarding of new users, allowing Apps to make sure tokens distributed via GDA immediately show up in user's wallets. Accounts can opt out of this by using `setConnectPermission()`, this is mainly supposed to be used by contracts.
@@ -15,7 +15,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   - `GDAv1StorageWriter` contains functions for writing agreement data to the token contract. This can only be used by the GDA contract itself.
 - bump solc to "0.8.30".
 - Changed EVM target from `paris` to `shanghai` because now all networks with supported Superfluid deployment support it.
-- Emit ERC20 `Transfer` events (with amount 0) on CFA and GDA actions potentially leading to account balance changes. This shall help indexers to keep track of SuperToken holders and account balances.
 - Don't emit ERC20 `Approval` events on `transferFrom` operations. This is consistent with the OpenZeppelin ERC20 implementation from v5 onwards. Change effective only for SuperTokens using the latest logic.
 
 ### Fixed
@@ -47,6 +46,10 @@ subtask(TASK_COMPILE_GET_REMAPPINGS).setAction(
 - `CFASuperAppBase`: added `flowRate` argument to `onFlowCreated` and `onFlowUpdated`.
 - PoolMemberNFT pruning: `IPoolMemberNFT` and `PoolMemberNFT` removed, `POOL_MEMBER_NFT()` removed from `ISuperToken`.
 
+## [v1.14.0]
+
+Defect release, don't use!
+
 ## [v1.13.0]
 
 ### Added

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

@@ -453,8 +453,6 @@ contract ConstantFlowAgreementV1 is
                 ctx, currentContext);
         }
 
-        flowVars.token.emitPseudoTransfer(flowVars.sender, flowVars.receiver);
-
         _requireAvailableBalance(flowVars.token, flowVars.sender, currentContext);
     }
 
@@ -595,7 +593,6 @@ contract ConstantFlowAgreementV1 is
             }
         }
 
-        flowVars.token.emitPseudoTransfer(flowVars.sender, flowVars.receiver);
     }
 
     /**************************************************************************

packages/ethereum-contracts/contracts/agreements/gdav1/GeneralDistributionAgreementV1.sol

@@ -279,34 +279,49 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
     }
 
     /// @inheritdoc IGeneralDistributionAgreementV1
-    function updateMemberUnits(ISuperfluidPool pool, address memberAddress, uint128 newUnits, bytes calldata ctx)
+    function updateMemberUnits(
+        ISuperfluidPool untrustedPool,
+        address memberAddress,
+        uint128 newUnits,
+        bytes calldata ctx
+    )
         external
         override
         returns (bytes memory newCtx)
     {
+        ISuperfluidToken token = untrustedPool.superToken();
+        address msgSender = AgreementLibrary.authorizeTokenAccess(token, ctx).msgSender;
+
         // Only the admin can update member units here
-        if (AgreementLibrary.authorizeTokenAccess(pool.superToken(), ctx).msgSender != pool.admin()) {
+        if (msgSender != untrustedPool.admin()) {
             revert GDA_NOT_POOL_ADMIN();
         }
         newCtx = ctx;
 
-        pool.updateMemberUnits(memberAddress, newUnits);
+        // NOTE: In GDA.appendIndexUpdateByPool, it checks whether pool is created by the token.
+        untrustedPool.updateMemberUnits(memberAddress, newUnits);
     }
 
     /// @inheritdoc IGeneralDistributionAgreementV1
-    function claimAll(ISuperfluidPool pool, address memberAddress, bytes calldata ctx)
+    function claimAll(ISuperfluidPool untrustedPool, address memberAddress, bytes calldata ctx)
         external
         override
         returns (bytes memory newCtx)
     {
-        AgreementLibrary.authorizeTokenAccess(pool.superToken(), ctx);
+        ISuperfluidToken token = untrustedPool.superToken();
+        AgreementLibrary.authorizeTokenAccess(token, ctx);
         newCtx = ctx;
 
-        pool.claimAll(memberAddress);
+        // NOTE: In GDA.poolSettleClaim, it checks whether pool is created by the token.
+        untrustedPool.claimAll(memberAddress);
     }
 
     /// @inheritdoc IGeneralDistributionAgreementV1
-    function connectPool(ISuperfluidPool pool, bytes calldata ctx) external override returns (bytes memory newCtx) {
+    function connectPool(ISuperfluidPool pool, bytes calldata ctx)
+        external
+        override
+        returns (bytes memory newCtx)
+    {
         newCtx = ctx;
         _setPoolConnectionFor(pool, address(0), true /* doConnect */, ctx);
     }
@@ -319,6 +334,7 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
     {
         newCtx = ctx;
 
+        // NOTE: We do not allow a pool to connect to another pool.
         if (memberAddr == address(0) || pool.superToken().isPool(this, memberAddr)) {
             revert GDA_CANNOT_CONNECT_POOL();
         }
@@ -358,6 +374,10 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
         returns (bool success)
     {
         ISuperfluidToken token = pool.superToken();
+        // TODO: convert to modifier `poolIsTrustedByItsSuperToken(pool)`
+        if (!token.isPool(this, address(pool))) {
+            revert GDA_ONLY_SUPER_TOKEN_POOL();
+        }
         ISuperfluid.Context memory currentContext = AgreementLibrary.authorizeTokenAccess(token, ctx);
 
         bool autoConnectForOtherMember = false;
@@ -401,14 +421,22 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
                 )
             );
 
+            // NOTE: similar to Transfer, we cannot tell if it is done through tryConnect or regular connect.
             emit PoolConnectionUpdated(token, pool, memberAddr, doConnect, currentContext.userData);
         }
 
         return true;
     }
 
     /// @inheritdoc IGeneralDistributionAgreementV1
-    function isMemberConnected(ISuperfluidPool pool, address member) external view override returns (bool) {
+    function isMemberConnected(ISuperfluidPool pool, address member)
+        external view override
+        returns (bool)
+    {
+        // NOTE: this function is total, in that even for invalid pools, it will always return false.
+        //
+        // Retrospectively, it may be more helpful to the developers if this function is non-total, and always revert
+        // on invalid pool.
         return pool.superToken().isPoolMemberConnected(this, pool, member);
     }
 
@@ -424,9 +452,12 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
 
         newCtx = ctx;
 
-        if (token.isPool(this, address(pool)) == false ||
+        // TODO: convert to modifier `poolIsTrustedByItsSuperToken(pool)`
+        if (
+            token.isPool(this, address(pool)) == false ||
             // Note: we do not support multi-tokens pools
-            pool.superToken() != token) {
+            pool.superToken() != token)
+        {
             revert GDA_ONLY_SUPER_TOKEN_POOL();
         }
 
@@ -490,9 +521,12 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
         int96 requestedFlowRate,
         bytes calldata ctx
     ) external override returns (bytes memory newCtx) {
-        if (token.isPool(this, address(pool)) == false ||
+        // TODO: convert to modifier `poolIsTrustedByItsSuperToken(pool)`
+        if (
+            token.isPool(this, address(pool)) == false ||
             // Note: we do not support multi-tokens pools
-            pool.superToken() != token) {
+            pool.superToken() != token)
+        {
             revert GDA_ONLY_SUPER_TOKEN_POOL();
         }
         if (requestedFlowRate < 0) {
@@ -599,6 +633,8 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
         )
         internal
     {
+        // NOTE: the caller to guarantee that the token and pool are mutually trusted.
+
         // not using oldFlowRate in this model
         // surprising effect: reducing flow rate may require more buffer when liquidation_period adjusted upward
         ISuperfluidGovernance gov = ISuperfluidGovernance(ISuperfluid(_host).getGovernance());
@@ -774,39 +810,51 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
 
     //
     // Pool-only operations
-    //
+    // Can only be called (`msg.sender`) by legitimate pool contracts.
+    // If `token` is legitimate, `token.isPool()` can return true only if the pool was created by this agreement.
+    // "false positives" (does not revert for illegitimate caller) could occur if `token`:
+    // 1. is lying (claims the pool was registered by this agreement when it was not)
+    // or
+    // 2. is not associated to the same host (and agreements).
+    // In both cases, pre-conditions are not met and no state this agreement is responsible for can be manipulated.
 
     function appendIndexUpdateByPool(ISuperfluidToken token, BasicParticle memory p, Time t)
         external
         returns (bool)
     {
-        if (token.isPool(this, msg.sender) == false) {
+        address poolAddress = msg.sender;
+
+        // TODO: convert to modifier `poolIsTrustedByItsSuperToken(pool)`
+        if (
+            token.isPool(this, msg.sender) == false ||
+            ISuperfluidPool(poolAddress).superToken() != token
+        ) {
             revert GDA_ONLY_SUPER_TOKEN_POOL();
         }
+
         bytes memory eff = abi.encode(token);
-        _setUIndex(eff, msg.sender, _getUIndex(eff, msg.sender).mappend(p));
-        _setPoolAdjustmentFlowRate(eff, msg.sender, true, /* doShift? */ p.flow_rate(), t);
+        _setUIndex(eff, msg.sender, _getUIndex(eff, poolAddress).mappend(p));
+        _setPoolAdjustmentFlowRate(eff, poolAddress, true, /* doShift? */ p.flow_rate(), t);
         return true;
     }
 
-    function poolSettleClaim(ISuperfluidToken superToken, address claimRecipient, int256 amount)
+    // succeeds only if `msg.sender` is a pool trusted by `token`
+    function poolSettleClaim(ISuperfluidToken token, address claimRecipient, int256 amount)
         external
         returns (bool)
     {
-        if (superToken.isPool(this, msg.sender) == false) {
-            revert GDA_ONLY_SUPER_TOKEN_POOL();
-        }
+        address poolAddress = msg.sender;
 
-        _doShift(abi.encode(superToken), msg.sender, claimRecipient, Value.wrap(amount));
-        return true;
-    }
-
-    function tokenEmitPseudoTransfer(ISuperfluidToken superToken, address from, address to) external {
-        if (superToken.isPool(this, msg.sender) == false) {
+        // TODO: convert to modifier `poolIsTrustedByItsSuperToken(pool)`
+        if (
+            token.isPool(this, msg.sender) == false ||
+            ISuperfluidPool(poolAddress).superToken() != token
+        ) {
             revert GDA_ONLY_SUPER_TOKEN_POOL();
         }
 
-        superToken.emitPseudoTransfer(from, to);
+        _doShift(abi.encode(token), poolAddress, claimRecipient, Value.wrap(amount));
+        return true;
     }
 
     //////////////////////////////////////////////////////////////////////////////////////////////////////

packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol

@@ -454,10 +454,6 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
             assert(GDA.appendIndexUpdateByPool(superToken, p, t));
         }
 
-        if ((oldUnits == 0 || newUnits == 0) && oldUnits != newUnits) {
-            GDA.tokenEmitPseudoTransfer(superToken, address(this), memberAddr);
-        }
-
         emit MemberUnitsUpdated(superToken, memberAddr, oldUnits, newUnits);
     }
 

packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluidToken.sol

@@ -431,11 +431,4 @@ interface ISuperfluidToken {
         uint256 rewardAmount,
         uint256 bailoutAmount
     );
-
-    /**
-     * @dev Emit an ERC20.Transfer event with zero amount, helps indexers track token holders.
-     * @param from The address from which the transfer is happening
-     * @param to The address to which the transfer is happening
-     */
-    function emitPseudoTransfer(address from, address to) external;
 }

packages/ethereum-contracts/contracts/superfluid/SuperfluidToken.sol

@@ -376,10 +376,6 @@ abstract contract SuperfluidToken is ISuperfluidToken
         );
     }
 
-    function emitPseudoTransfer(address from, address to) external onlyAgreement {
-        emit IERC20.Transfer(from, to, 0);
-    }
-
     /**************************************************************************
     * Modifiers
     *************************************************************************/

packages/ethereum-contracts/package.json

@@ -1,7 +1,7 @@
 {
     "name": "@superfluid-finance/ethereum-contracts",
     "description": " Ethereum contracts implementation for the Superfluid Protocol",
-    "version": "1.14.0",
+    "version": "1.14.1",
     "dependencies": {
         "@decentral.ee/web3-helpers": "0.5.3",
         "@nomiclabs/hardhat-ethers": "2.2.3",
@@ -11,7 +11,7 @@
         "hardhat": "2.26.1"
     },
     "devDependencies": {
-        "@d10r/truffle-plugin-verify": "^0.6.11",
+        "@d10r/truffle-plugin-verify": "^0.7.2",
         "@nomiclabs/hardhat-truffle5": "^2.1.0",
         "@safe-global/safe-core-sdk": "^3.3.5",
         "@safe-global/safe-service-client": "^2.0.3",

packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol

@@ -1105,6 +1105,19 @@ contract GeneralDistributionAgreementV1IntegrationTest is FoundrySuperfluidTeste
         vm.stopPrank();
     }
 
+    function testConnectUnauthorizedPool() public {
+        FakePool pool = new FakePool(address(superToken));
+
+        vm.startPrank(eve);
+        vm.expectRevert(IGeneralDistributionAgreementV1.GDA_ONLY_SUPER_TOKEN_POOL.selector);
+        sf.host.callAgreement(
+            sf.gda,
+            abi.encodeWithSelector(IGeneralDistributionAgreementV1.connectPool.selector, pool, ""),
+            new bytes(0)
+        );
+        vm.stopPrank();
+    }
+
     /*//////////////////////////////////////////////////////////////////////////
                                     Assertion Functions
     //////////////////////////////////////////////////////////////////////////*/
@@ -1213,3 +1226,24 @@ contract GeneralDistributionAgreementV1IntegrationTest is FoundrySuperfluidTeste
         assertEq(flowRatesSum, 0, "GDAv1.t: flowRatesSum != 0");
     }
 }
+
+/// Fake pool with `operatorConnectMember` not calling back into the GDA as expected
+contract FakePool {
+    address internal immutable _SUPER_TOKEN;
+
+    constructor(address superToken_) {
+        _SUPER_TOKEN = superToken_;
+    }
+
+    function superToken() external view returns (address) {
+        return _SUPER_TOKEN;
+    }
+
+    function getClaimable(address, uint32) public pure returns (int256) {
+        return type(int256).max;
+    }
+
+    function operatorConnectMember(address, bool, uint32) external pure returns (bool) {
+        return true;
+    }
+}