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

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,51 @@ 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 = null // optional parameter for writing to output string
+        ) {
+            const prevAddr = await getPrevAddrFn().catch(_err => {
+                console.error(`### Error getting ${ForwarderContract.contractName} address, likely not yet deployed`);
+                return ZERO_ADDRESS;
+            });
+
+            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);
+
+                    if (outputKey) {
+                        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()
+        );
+
+        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 +846,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 +858,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/test/contracts/superfluid/Superfluid.test.ts

@@ -114,13 +114,15 @@ describe("Superfluid Host Contract", function () {
                     false /* nonUpgradable */,
                     false /* appWhiteListingEnabled */,
                     3000000 /* callbackGasLimit */,
+                    ZERO_ADDRESS /* simpleForwarder */,
                     ZERO_ADDRESS /* erc2771Forwarder */
                 );
                 const mock2 = await sfMockFactory.deploy(
                     true /* nonUpgradable */,
                     false /* appWhiteListingEnabled */,
                     3000000 /* callbackGasLimit */,
-                    ZERO_ADDRESS /* erc2771Forwader */
+                    ZERO_ADDRESS /* simpleForwarder */,
+                    ZERO_ADDRESS /* erc2771Forwarder */
                 );
                 await governance.updateContracts(
                     superfluid.address,
@@ -2693,7 +2695,8 @@ describe("Superfluid Host Contract", function () {
                     false /* nonUpgradable */,
                     false /* appWhiteListingEnabled */,
                     3000000 /* callbackGasLimit */,
-                    ZERO_ADDRESS /* erc2771Forwader */
+                    ZERO_ADDRESS /* simpleForwarder */,
+                    ZERO_ADDRESS /* erc2771Forwarder */
                 );
                 await expectCustomError(
                     governance.updateContracts(

packages/ethereum-contracts/test/ops-scripts/deployment.test.js

@@ -127,17 +127,13 @@ contract("Embedded deployment scripts", (accounts) => {
                 false, // nonUpgradable
                 false, // appWhiteListingEnabled
                 callbackGasLimit, // callbackGasLimit
-                ZERO_ADDRESS // erc2771Forwader
+                ZERO_ADDRESS, // simpleForwarder
+                ZERO_ADDRESS // erc2771Forwarder
             );
-            const simpleForwarderAddr = await a1.SIMPLE_FORWARDER();
             assert.isFalse(
                 await codeChanged(web3, Superfluid, a1.address, [
                     // replace immutables with 0
                     callbackGasLimit.toString(16).padStart(64, "0"),
-                    simpleForwarderAddr
-                        .toLowerCase()
-                        .slice(2)
-                        .padStart(64, "0"),
                 ])
             );
         }