[ETHEREUM-CONTRACTS] fix simple forwarder (alternative solution) (#2045)

* deploy SimpleForwarder the same way as ERC2771Forwarder
* chore: fix some typos in comment (#2041)

packages/automation-contracts/autowrap/package.json

@@ -4,7 +4,7 @@
     "version": "0.3.0",
     "devDependencies": {
         "@openzeppelin/contracts": "^4.9.6",
-        "@superfluid-finance/ethereum-contracts": "^1.12.0",
+        "@superfluid-finance/ethereum-contracts": "^1.12.1",
         "@superfluid-finance/metadata": "^1.5.2"
     },
     "license": "MIT",

packages/automation-contracts/scheduler/contracts/interface/IVestingScheduler.sol

@@ -59,7 +59,7 @@ interface IVestingScheduler {
      * @param superToken SuperToken to be vested
      * @param receiver Vesting receiver
      * @param startDate Timestamp when the vesting should start
-     * @param cliffDate Timestamp of cliff exectution - if 0, startDate acts as cliff
+     * @param cliffDate Timestamp of cliff execution - if 0, startDate acts as cliff
      * @param flowRate The flowRate for the stream
      * @param cliffAmount The amount to be transferred at the cliff
      * @param endDate The timestamp when the stream should stop.

packages/automation-contracts/scheduler/contracts/interface/IVestingSchedulerV2.sol

@@ -45,7 +45,7 @@ interface IVestingSchedulerV2 {
      * @param receiver Vesting receiver
      * @param startDate Timestamp when the vesting should start
      * @param claimValidityDate Date before which the claimable schedule must be claimed
-     * @param cliffDate Timestamp of cliff exectution - if 0, startDate acts as cliff
+     * @param cliffDate Timestamp of cliff execution - if 0, startDate acts as cliff
      * @param flowRate The flowRate for the stream
      * @param cliffAmount The amount to be transferred at the cliff
      * @param endDate The timestamp when the stream should stop.
@@ -98,7 +98,7 @@ interface IVestingSchedulerV2 {
      * @param superToken SuperToken to be vested
      * @param receiver Vesting receiver
      * @param startDate Timestamp when the vesting should start
-     * @param cliffDate Timestamp of cliff exectution - if 0, startDate acts as cliff
+     * @param cliffDate Timestamp of cliff execution - if 0, startDate acts as cliff
      * @param flowRate The flowRate for the stream
      * @param cliffAmount The amount to be transferred at the cliff
      * @param endDate The timestamp when the stream should stop.

packages/automation-contracts/scheduler/package.json

@@ -4,7 +4,7 @@
     "version": "1.3.0",
     "devDependencies": {
         "@openzeppelin/contracts": "^4.9.6",
-        "@superfluid-finance/ethereum-contracts": "^1.12.0",
+        "@superfluid-finance/ethereum-contracts": "^1.12.1",
         "@superfluid-finance/metadata": "^1.5.2"
     },
     "license": "MIT",

packages/ethereum-contracts/CHANGELOG.md

@@ -3,12 +3,15 @@ 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).
 
-## UNRELEASED
+## [v1.12.1]
 
 ### Added
 - Superfluid Pools now implement `IERC20Metadata`, thus going forward have a name, symbol and decimals
 - `ISuperfluidPool.createPoolWithCustomERC20Metadata` for creating pools with custom ERC20 metadata
 
+### Changed
+- Fixed deployment of SimpleForwarder (solved an issue which caused batch operation `OPERATION_TYPE_SIMPLE_FORWARD_CALL` to always revert)
+
 ## [v1.12.0]
 
 ### Added

packages/ethereum-contracts/contracts/mocks/SuperfluidMock.t.sol

@@ -12,7 +12,7 @@ import { CallUtils } from "../libs/CallUtils.sol";
 contract SuperfluidUpgradabilityTester is Superfluid {
 
     // 3_000_000 is the min callback gas limit used in a prod deployment
-    constructor() Superfluid(false, false, 3_000_000, address(0))
+    constructor() Superfluid(false, false, 3_000_000, address(0), address(0))
     // solhint-disable-next-line no-empty-blocks
     {
     }
@@ -134,9 +134,12 @@ contract SuperfluidMock is Superfluid {
         bool nonUpgradable,
         bool appWhiteListingEnabled,
         uint64 callbackGasLimit,
-        address erc2771Forwarder
+        address simpleForwarderAddress,
+        address erc2771ForwarderAddress
     )
-        Superfluid(nonUpgradable, appWhiteListingEnabled, callbackGasLimit, erc2771Forwarder)
+        Superfluid(
+            nonUpgradable, appWhiteListingEnabled, callbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress
+        )
     // solhint-disable-next-line no-empty-blocks
     {
     }

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

@@ -56,7 +56,6 @@ contract Superfluid is
     uint64 immutable public CALLBACK_GAS_LIMIT;
 
     // simple forwarder contract used to relay arbitrary calls for batch operations
-    // Note: address will change with every contract upgrade
     SimpleForwarder immutable public SIMPLE_FORWARDER;
     ERC2771Forwarder immutable internal _ERC2771_FORWARDER;
 
@@ -105,13 +104,14 @@ contract Superfluid is
         bool nonUpgradable,
         bool appWhiteListingEnabled,
         uint64 callbackGasLimit,
+        address simpleForwarderAddress,
         address erc2771ForwarderAddress
     ) {
         NON_UPGRADABLE_DEPLOYMENT = nonUpgradable;
         APP_WHITE_LISTING_ENABLED = appWhiteListingEnabled;
         CALLBACK_GAS_LIMIT = callbackGasLimit;
+        SIMPLE_FORWARDER = SimpleForwarder(simpleForwarderAddress);
         _ERC2771_FORWARDER = ERC2771Forwarder(erc2771ForwarderAddress);
-        SIMPLE_FORWARDER = new SimpleForwarder();
     }
 
     ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol

@@ -29,6 +29,7 @@ import { UUPSProxy } from "../upgradability/UUPSProxy.sol";
 import { BatchLiquidator } from "./BatchLiquidator.sol";
 import { TOGA } from "./TOGA.sol";
 import { IResolver } from "../interfaces/utils/IResolver.sol";
+import { SimpleForwarder } from "../utils/SimpleForwarder.sol";
 import { ERC2771Forwarder } from "../utils/ERC2771Forwarder.sol";
 import { MacroForwarder } from "../utils/MacroForwarder.sol";
 
@@ -140,10 +141,14 @@ contract SuperfluidFrameworkDeploymentSteps {
             testGovernance = SuperfluidGovDeployerLibrary.deployTestGovernance();
             SuperfluidGovDeployerLibrary.transferOwnership(testGovernance, address(this));
         } else if (step == 1) { // CORE CONTRACT: Superfluid (Host)
-            ERC2771Forwarder erc2771Forwarder = SuperfluidERC2771ForwarderDeployerLibrary.deploy();
+            SimpleForwarder simpleForwarder = new SimpleForwarder();
+            ERC2771Forwarder erc2771Forwarder = new ERC2771Forwarder();
             // Deploy Host and initialize the test governance.
             // 3_000_000 is the min callback gas limit used in a prod deployment
-            host = SuperfluidHostDeployerLibrary.deploy(true, false, 3_000_000, address(erc2771Forwarder));
+            host = SuperfluidHostDeployerLibrary.deploy(
+                true, false, 3_000_000, address(simpleForwarder), address(erc2771Forwarder)
+            );
+            simpleForwarder.transferOwnership(address(host));
             erc2771Forwarder.transferOwnership(address(host));
 
             host.initialize(testGovernance);
@@ -308,23 +313,19 @@ library SuperfluidGovDeployerLibrary {
     }
 }
 
-library SuperfluidERC2771ForwarderDeployerLibrary {
-    // After deploying, you may want to transfer ownership to the host
-    function deploy() external returns (ERC2771Forwarder) {
-        return new ERC2771Forwarder();
-    }
-}
-
 library SuperfluidHostDeployerLibrary {
     function deploy(
         bool _nonUpgradable,
         bool _appWhiteListingEnabled,
         uint64 callbackGasLimit,
+        address simpleForwarderAddress,
         address erc2771ForwarderAddress
     )
         external returns (Superfluid)
     {
-        return new Superfluid(_nonUpgradable, _appWhiteListingEnabled, callbackGasLimit, erc2771ForwarderAddress);
+        return new Superfluid(
+            _nonUpgradable, _appWhiteListingEnabled, callbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress
+        );
     }
 }
 

packages/ethereum-contracts/dev-scripts/deploy-test-framework.js

@@ -18,7 +18,6 @@ const SuperTokenFactoryDeployerLibraryArtifact = require("@superfluid-finance/et
 const SuperfluidFrameworkDeployerArtifact = require("@superfluid-finance/ethereum-contracts/build/hardhat/contracts/utils/SuperfluidFrameworkDeployer.t.sol/SuperfluidFrameworkDeployer.json");
 const SlotsBitmapLibraryArtifact = require("@superfluid-finance/ethereum-contracts/build/hardhat/contracts/libs/SlotsBitmapLibrary.sol/SlotsBitmapLibrary.json");
 const TokenDeployerLibraryArtifact = require("@superfluid-finance/ethereum-contracts/build/hardhat/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol/TokenDeployerLibrary.json");
-const SuperfluidERC2771ForwarderDeployerLibraryArtifact = require("@superfluid-finance/ethereum-contracts/build/hardhat/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol/SuperfluidERC2771ForwarderDeployerLibrary.json");
 
 const ERC1820Registry = require("../dev-scripts/artifacts/ERC1820Registry.json");
 
@@ -225,12 +224,6 @@ const _deployTestFramework = async (provider, signer) => {
         TokenDeployerLibraryArtifact,
         signer
     );
-    const SuperfluidERC2771ForwarderDeployerLibrary =
-        await _getFactoryAndReturnDeployedContract(
-            "SuperfluidERC2771ForwarderDeployerLibrary",
-            SuperfluidERC2771ForwarderDeployerLibraryArtifact,
-            signer
-        );
 
     const sfDeployer = await _getFactoryAndReturnDeployedContract(
         "SuperfluidFrameworkDeployer",
@@ -276,9 +269,6 @@ const _deployTestFramework = async (provider, signer) => {
                     SuperTokenFactoryDeployerLibrary
                 ),
                 TokenDeployerLibrary: getContractAddress(TokenDeployerLibrary),
-                SuperfluidERC2771ForwarderDeployerLibrary: getContractAddress(
-                    SuperfluidERC2771ForwarderDeployerLibrary
-                ),
             },
         }
     );

packages/ethereum-contracts/ops-scripts/deploy-framework.js

@@ -232,6 +232,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
         "PoolAdminNFT",
         "PoolMemberNFT",
         "IAccessControlEnumerable",
+        "SimpleForwarder",
         "ERC2771Forwarder",
     ];
     const mockContracts = [
@@ -270,6 +271,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
         PoolAdminNFT,
         PoolMemberNFT,
         IAccessControlEnumerable,
+        SimpleForwarder,
         ERC2771Forwarder,
     } = await SuperfluidSDK.loadContracts({
         ...extractWeb3Options(options),
@@ -351,6 +353,10 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
         `Superfluid.${protocolReleaseVersion}`,
         async (contractAddress) => !(await hasCode(web3, contractAddress)),
         async () => {
+            const simpleForwarder = await web3tx(SimpleForwarder.new, "SimpleForwarder.new")();
+            console.log("SimpleForwarder address:", simpleForwarder.address);
+            output += `SIMPLE_FORWARDER=${simpleForwarder.address}\n`;
+
             const erc2771Forwarder = await web3tx(ERC2771Forwarder.new, "ERC2771Forwarder.new")();
             console.log("ERC2771Forwarder address:", erc2771Forwarder.address);
             output += `ERC2771_FORWARDER=${erc2771Forwarder.address}\n`;
@@ -359,15 +365,12 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
             const superfluidLogic = await web3tx(
                 SuperfluidLogic.new,
                 "SuperfluidLogic.new"
-            )(nonUpgradable, appWhiteListing, appCallbackGasLimit, erc2771Forwarder.address);
+            )(nonUpgradable, appWhiteListing, appCallbackGasLimit, simpleForwarder.address, erc2771Forwarder.address);
             console.log(
                 `Superfluid new code address ${superfluidLogic.address}`
             );
             output += `SUPERFLUID_HOST_LOGIC=${superfluidLogic.address}\n`;
-            // get the address of SimpleForwarder (deployed in constructor) for verification
-            const simpleForwarderAddr = await superfluidLogic.SIMPLE_FORWARDER();
-            console.log("SimpleForwarder address", simpleForwarderAddr);
-            output += `SIMPLE_FORWARDER=${simpleForwarderAddr}\n`;
+
             if (!nonUpgradable) {
                 const proxy = await web3tx(
                     UUPSProxy.new,
@@ -383,10 +386,15 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
                 superfluidAddress = superfluidLogic.address;
             }
             const superfluid = await Superfluid.at(superfluidAddress);
+            await web3tx(
+                simpleForwarder.transferOwnership,
+                "simpleForwarder.transferOwnership"
+            )(superfluid.address);
             await web3tx(
                 erc2771Forwarder.transferOwnership,
                 "erc2771Forwarder.transferOwnership"
             )(superfluid.address);
+
             await web3tx(
                 superfluid.initialize,
                 "Superfluid.initialize"
@@ -784,33 +792,60 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
             throw new Error("Superfluid is not upgradable");
         }
 
-        async function getPrevERC2771ForwarderAddr() {
-            try {
-                return await superfluid.getERC2771Forwarder();
-            } catch (err) {
-                console.error("### Error getting ERC2771Forwarder address, likely not yet deployed");
-                return ZERO_ADDRESS; // fallback
+        async function getOrDeployForwarder(
+            superfluid,
+            ForwarderContract,
+            getPrevAddrFn,
+            outputKey
+        ) {
+            let prevAddr = await getPrevAddrFn().catch(_err => {
+                console.error(`### Error getting ${ForwarderContract.contractName} address, likely not yet deployed`);
+                return ZERO_ADDRESS;
+            });
+
+            {
+                // TEMPORARY FIX - can be removed after applied
+                // we found a previous deployment. Now verify it has the host as owner.
+                // the first mainnet deployment didn't have this for SimpleForwarder, thus needs a redeployment.
+                const ownerAddr = await (await Ownable.at(prevAddr)).owner();
+                if (ownerAddr != superfluid.address) {
+                    console.log(`  !!! ${outputKey} has wrong owner, needs re-deployment`);
+                    prevAddr = ZERO_ADDRESS; // by setting zero, we force a re-deployment
+                }
             }
+
+            const newAddress = await deployContractIfCodeChanged(
+                web3,
+                ForwarderContract,
+                prevAddr,
+                async () => {
+                    const forwarder = await web3tx(ForwarderContract.new, `${ForwarderContract.contractName}.new`)();
+                    await web3tx(
+                        forwarder.transferOwnership,
+                        "forwarder.transferOwnership"
+                    )(superfluid.address);
+
+                    output += `${outputKey}=${forwarder.address}\n`;
+                    return forwarder.address;
+                }
+            );
+
+            return newAddress !== ZERO_ADDRESS ? newAddress : prevAddr;
         }
-        const prevERC2771ForwarderAddr = await getPrevERC2771ForwarderAddr();
 
-        const erc2771ForwarderNewAddress = await deployContractIfCodeChanged(
-            web3,
+        const simpleForwarderAddress = await getOrDeployForwarder(
+            superfluid,
+            SimpleForwarder,
+            () => superfluid.SIMPLE_FORWARDER(),
+            "SIMPLE_FORWARDER"
+        );
+
+        const erc2771ForwarderAddress = await getOrDeployForwarder(
+            superfluid,
             ERC2771Forwarder,
-            prevERC2771ForwarderAddr,
-            async () => {
-                const erc2771Forwarder = await web3tx(ERC2771Forwarder.new, "ERC2771Forwarder.new")();
-                await web3tx(
-                    erc2771Forwarder.transferOwnership,
-                    "erc2771Forwarder.transferOwnership"
-                )(superfluid.address);
-                output += `ERC2771_FORWARDER=${erc2771Forwarder.address}\n`;
-                return erc2771Forwarder.address;
-            }
+            () => superfluid.getERC2771Forwarder(),
+            "ERC2771_FORWARDER"
         );
-        const erc2771ForwarderAddress = erc2771ForwarderNewAddress !== ZERO_ADDRESS
-            ? erc2771ForwarderNewAddress
-            : prevERC2771ForwarderAddr;
 
         // get previous callback gas limit, make sure we don't decrease it
         const prevCallbackGasLimit = await superfluid.CALLBACK_GAS_LIMIT();
@@ -820,13 +855,6 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
             console.log(` !!! CHANGING APP CALLBACK GAS LIMIT FROM ${prevCallbackGasLimit} to ${appCallbackGasLimit} !!!`);
         }
 
-        // get prev SimpleForwarder addr (only for replacements for codeChanged check)
-        let simpleForwarderAddr;
-        try {
-            simpleForwarderAddr = await superfluid.SIMPLE_FORWARDER();
-        } catch (error) {
-            simpleForwarderAddr = ZERO_ADDRESS;
-        }
         // deploy new superfluid host logic
         superfluidNewLogicAddress = await deployContractIfCodeChanged(
             web3,
@@ -839,13 +867,13 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
                 const superfluidLogic = await web3tx(
                     SuperfluidLogic.new,
                     "SuperfluidLogic.new"
-                )(nonUpgradable, appWhiteListing, appCallbackGasLimit, erc2771ForwarderAddress);
+                )(nonUpgradable, appWhiteListing, appCallbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress);
                 output += `SUPERFLUID_HOST_LOGIC=${superfluidLogic.address}\n`;
                 return superfluidLogic.address;
             },
             [
                 ap(erc2771ForwarderAddress),
-                ap(simpleForwarderAddr),
+                ap(simpleForwarderAddress),
                 appCallbackGasLimit.toString(16).padStart(64, "0")
             ],
         );

packages/ethereum-contracts/package.json

@@ -1,7 +1,7 @@
 {
     "name": "@superfluid-finance/ethereum-contracts",
     "description": " Ethereum contracts implementation for the Superfluid Protocol",
-    "version": "1.12.0",
+    "version": "1.12.1",
     "dependencies": {
         "@decentral.ee/web3-helpers": "0.5.3",
         "@nomiclabs/hardhat-ethers": "2.2.3",