[ETHEREUM-CONTRACTS] SuperApp registerApp deprecation (#1706)

* register app * fix build * simplify app registration API * satisfy linter * adapt ops-scipts to new app registration methodology * simplify errors too * do check in right place * piggy-back verification script fix * cleanup - fix broken tests - remove unused app jail reasons from codebase * fix signature * updated changelog * Update CHANGELOG.md --------- Co-authored-by: didi <[email protected]>
superfluid-finance · Oct 9, 2023 · 7a4f969 · 7a4f969
1 parent 46f6d40
commit 7a4f969
Show file tree

Hide file tree

Showing 10 changed files with 103 additions and 182 deletions.
diff --git a/packages/ethereum-contracts/CHANGELOG.md b/packages/ethereum-contracts/CHANGELOG.md
@@ -28,6 +28,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Changed
 - Reuse config keys from `SuperfluidGovernanceConfigs` instead of duplicating them in `ConstantFlowAgreementV1`.
+- Deprecating `registerAppWithKey` and `registerAppByFactory`: DO NOT USE for new deployments
+  - Simplification of Super App registration: use `registerApp` in all cases going forward.
+  - Use `registerApp(uint256 configWord)` to be called by the super app in the constructor or `registerApp(ISuperApp app, uint256 configWord)` to be called by any address with a valid app registration config key
 
 ## [v1.8.1] - 2023-08-28
 

diff --git a/packages/ethereum-contracts/contracts/interfaces/superfluid/Definitions.sol b/packages/ethereum-contracts/contracts/interfaces/superfluid/Definitions.sol
@@ -48,9 +48,6 @@ library SuperAppDefinitions {
     /**************************************************************************
     / App Jail Reasons
     /**************************************************************************/
-
-    uint256 constant internal APP_RULE_REGISTRATION_ONLY_IN_CONSTRUCTOR = 1;
-    uint256 constant internal APP_RULE_NO_REGISTRATION_FOR_EOA = 2;
     uint256 constant internal APP_RULE_NO_REVERT_ON_TERMINATION_CALLBACK = 10;
     uint256 constant internal APP_RULE_NO_CRITICAL_SENDER_ACCOUNT = 11;
     uint256 constant internal APP_RULE_NO_CRITICAL_RECEIVER_ACCOUNT = 12;

diff --git a/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluid.sol b/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluid.sol
@@ -73,15 +73,13 @@ interface ISuperfluid {
     // uses SuperAppDefinitions' App Jail Reasons as _code
     error APP_RULE(uint256 _code);                              // 0xa85ba64f
 
-    error HOST_INVALID_OR_EXPIRED_SUPER_APP_REGISTRATION_KEY(); // 0x19ab84d1
     error HOST_NOT_A_SUPER_APP();                               // 0x163cbe43
-    error HOST_NO_APP_REGISTRATION_PERMISSIONS();               // 0x5b93ebf0
+    error HOST_NO_APP_REGISTRATION_PERMISSION();                // 0xb56455f0
     error HOST_RECEIVER_IS_NOT_SUPER_APP();                     // 0x96aa315e
     error HOST_SENDER_IS_NOT_SUPER_APP();                       // 0xbacfdc40
     error HOST_SOURCE_APP_NEEDS_HIGHER_APP_LEVEL();             // 0x44725270
     error HOST_SUPER_APP_IS_JAILED();                           // 0x02384b64
     error HOST_SUPER_APP_ALREADY_REGISTERED();                  // 0x01b0a935
-    error HOST_UNAUTHORIZED_SUPER_APP_FACTORY();                // 0x289533c5
 
     /**************************************************************************
      * Time
@@ -245,35 +243,39 @@ interface ISuperfluid {
      *************************************************************************/
 
     /**
-     * @dev Message sender (must be a contract) declares itself as a super app.
-     * @custom:deprecated you should use `registerAppWithKey` or `registerAppByFactory` instead,
-     * because app registration is currently governance permissioned on mainnets.
+     * @dev Message sender (must be a contract) registers itself as a super app.
      * @param configWord The super app manifest configuration, flags are defined in
      * `SuperAppDefinitions`
+     * @notice On some mainnet deployments, pre-authorization by governance may be needed for this to succeed.
+     * See https://github.com/superfluid-finance/protocol-monorepo/wiki/Super-App-White-listing-Guide
      */
     function registerApp(uint256 configWord) external;
+
+    /**
+     * @dev Registers an app (must be a contract) as a super app.
+     * @param app The super app address
+     * @param configWord The super app manifest configuration, flags are defined in
+     * `SuperAppDefinitions`
+     * @notice On some mainnet deployments, pre-authorization by governance may be needed for this to succeed.
+     * See https://github.com/superfluid-finance/protocol-monorepo/wiki/Super-App-White-listing-Guide
+     */
+    function registerApp(ISuperApp app, uint256 configWord) external;
+
     /**
      * @dev App registered event
      * @param app Address of jailed app
      */
     event AppRegistered(ISuperApp indexed app);
 
     /**
-     * @dev Message sender declares itself as a super app.
-     * @param configWord The super app manifest configuration, flags are defined in `SuperAppDefinitions`
-     * @param registrationKey The registration key issued by the governance, needed to register on a mainnet.
-     * @notice See https://github.com/superfluid-finance/protocol-monorepo/wiki/Super-App-White-listing-Guide
-     * On testnets or in dev environment, a placeholder (e.g. empty string) can be used.
-     * While the message sender must be the super app itself, the transaction sender (tx.origin)
-     * must be the deployer account the registration key was issued for.
+     * @dev DO NOT USE for new deployments
+     * @custom:deprecated you should use `registerApp(uint256 configWord) instead.
      */
     function registerAppWithKey(uint256 configWord, string calldata registrationKey) external;
 
     /**
-     * @dev Message sender (must be a contract) declares app as a super app
-     * @param configWord The super app manifest configuration, flags are defined in `SuperAppDefinitions`
-     * @notice On mainnet deployments, only factory contracts pre-authorized by governance can use this.
-     * See https://github.com/superfluid-finance/protocol-monorepo/wiki/Super-App-White-listing-Guide
+     * @dev DO NOT USE for new deployments
+     * @custom:deprecated you should use `registerApp(ISuperApp app, uint256 configWord) instead.
      */
     function registerAppByFactory(ISuperApp app, uint256 configWord) external;
 

diff --git a/packages/ethereum-contracts/contracts/mocks/SuperAppMocks.sol b/packages/ethereum-contracts/contracts/mocks/SuperAppMocks.sol
@@ -609,7 +609,7 @@ contract SuperAppMockWithRegistrationKey {
 }
 
 // An Super App that uses registerAppWithKey
-contract SuperAppMockUsingDeprecatedRegisterApp {
+contract SuperAppMockUsingRegisterApp {
     constructor(ISuperfluid host, uint256 configWord) {
         // @note this is deprecated keeping this here for testing/coverage
         host.registerApp(configWord);

diff --git a/packages/ethereum-contracts/contracts/superfluid/Superfluid.sol b/packages/ethereum-contracts/contracts/superfluid/Superfluid.sol
@@ -312,89 +312,81 @@ contract Superfluid is
     // App Registry
     ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-    /// @custom:deprecated
-    function registerApp(
-        uint256 configWord
-    )
-        external override
-    {
-        // check if whitelisting required
+    /// @inheritdoc ISuperfluid
+    function registerApp(uint256 configWord) external override {
         if (APP_WHITE_LISTING_ENABLED) {
-            revert HOST_NO_APP_REGISTRATION_PERMISSIONS();
+            // for historical reasons, we internal use "k1" as default registration key
+            // solhint-disable-next-line avoid-tx-origin
+            _enforceAppRegistrationPermissioning("k1", tx.origin);
         }
-        _registerApp(configWord, ISuperApp(msg.sender), true);
+        _registerApp(ISuperApp(msg.sender), configWord);
     }
 
-    function registerAppWithKey(uint256 configWord, string calldata registrationKey)
-        external override
-    {
+    /// @inheritdoc ISuperfluid
+    function registerApp(ISuperApp app, uint256 configWord) external override {
+        // Cannot register an EOA as SuperApp
+        if ((address(app)).code.length == 0) revert HOST_MUST_BE_CONTRACT();
         if (APP_WHITE_LISTING_ENABLED) {
-            bytes32 configKey = SuperfluidGovernanceConfigs.getAppRegistrationConfigKey(
-                // solhint-disable-next-line avoid-tx-origin
-                tx.origin,
-                registrationKey
-            );
-            // check if the key is valid and not expired
-            if (
-                _gov.getConfigAsUint256(
-                    this,
-                    ISuperfluidToken(address(0)),
-                    configKey
-                // solhint-disable-next-line not-rely-on-time
-                ) < block.timestamp) revert HOST_INVALID_OR_EXPIRED_SUPER_APP_REGISTRATION_KEY();
+            _enforceAppRegistrationPermissioning("k1", msg.sender);
         }
-        _registerApp(configWord, ISuperApp(msg.sender), true);
+        _registerApp(app, configWord);
     }
 
-    function registerAppByFactory(
-        ISuperApp app,
-        uint256 configWord
-    )
+    /// @custom:deprecated
+    function registerAppWithKey(uint256 configWord, string calldata registrationKey)
         external override
     {
-        // msg sender must be a contract
-        {
-            uint256 cs;
-            // solhint-disable-next-line no-inline-assembly
-            assembly { cs := extcodesize(caller()) }
-            if (cs == 0) revert HOST_MUST_BE_CONTRACT();
+        if (APP_WHITE_LISTING_ENABLED) {
+            // solhint-disable-next-line avoid-tx-origin
+            _enforceAppRegistrationPermissioning(registrationKey, tx.origin);
         }
+        _registerApp(ISuperApp(msg.sender), configWord);
+    }
 
-        if (APP_WHITE_LISTING_ENABLED) {
-            // check if msg sender is authorized to register
-            bytes32 configKey = SuperfluidGovernanceConfigs.getAppFactoryConfigKey(msg.sender);
-            bool isAuthorizedAppFactory = _gov.getConfigAsUint256(
+    // internally we keep using the gov config method with key
+    function _enforceAppRegistrationPermissioning(string memory registrationKey, address deployer) internal view {
+        bytes32 configKey = SuperfluidGovernanceConfigs.getAppRegistrationConfigKey(
+            // solhint-disable-next-line avoid-tx-origin
+            deployer,
+            registrationKey
+        );
+        // check if the key is valid and not expired
+        if (
+            _gov.getConfigAsUint256(
                 this,
                 ISuperfluidToken(address(0)),
-                configKey) == 1;
-
-            if (!isAuthorizedAppFactory) revert HOST_UNAUTHORIZED_SUPER_APP_FACTORY();
+                configKey
+            // solhint-disable-next-line not-rely-on-time
+            ) < block.timestamp)
+        {
+            revert HOST_NO_APP_REGISTRATION_PERMISSION();
         }
-        _registerApp(configWord, app, false);
     }
 
-    function _registerApp(uint256 configWord, ISuperApp app, bool checkIfInAppConstructor) private
-    {
-        // solhint-disable-next-line avoid-tx-origin
-        if (msg.sender == tx.origin) {
-            revert APP_RULE(SuperAppDefinitions.APP_RULE_NO_REGISTRATION_FOR_EOA);
+    /// @custom:deprecated
+    function registerAppByFactory(ISuperApp app, uint256 configWord) external override {
+        // Cannot register an EOA as SuperApp
+        if ((address(app)).code.length == 0) revert HOST_MUST_BE_CONTRACT();
+        if (APP_WHITE_LISTING_ENABLED) {
+            // enforce permissiniong with legacy gov config key for app factory
+            bytes32 configKey = SuperfluidGovernanceConfigs.getAppFactoryConfigKey(msg.sender);
+            bool isAuthorizedAppFactory = _gov.getConfigAsUint256(this, ISuperfluidToken(address(0)), configKey) == 1;
+            if (!isAuthorizedAppFactory) revert HOST_NO_APP_REGISTRATION_PERMISSION();
+            // We do not enforce any assumptions about what a "factory" is. It is whatever gov decided to.
         }
+        _registerApp(app, configWord);
+    }
 
-        if (checkIfInAppConstructor) {
-            uint256 cs;
-            // solhint-disable-next-line no-inline-assembly
-            assembly { cs := extcodesize(app) }
-            if (cs != 0) {
-                revert APP_RULE(SuperAppDefinitions.APP_RULE_REGISTRATION_ONLY_IN_CONSTRUCTOR);
-            }
-        }
+    function _registerApp(ISuperApp app, uint256 configWord) private {
+        // validate configWord
         if (
             !SuperAppDefinitions.isConfigWordClean(configWord) ||
             SuperAppDefinitions.getAppCallbackLevel(configWord) == 0 ||
             (configWord & SuperAppDefinitions.APP_JAIL_BIT) != 0
-            ) {
-                revert HOST_INVALID_CONFIG_WORD();
-            }
+        ) {
+            revert HOST_INVALID_CONFIG_WORD();
+        }
+
         if (_appManifests[ISuperApp(app)].configWord != 0) revert HOST_SUPER_APP_ALREADY_REGISTERED();
         _appManifests[ISuperApp(app)] = AppManifest(configWord);
         emit AppRegistered(app);

diff --git a/...ts/gov-create-new-app-registration-key.js → ...ops-scripts/gov-authorize-app-deployer.js b/...ts/gov-create-new-app-registration-key.js → ...ops-scripts/gov-authorize-app-deployer.js
@@ -7,46 +7,46 @@ const {
 } = require("./libs/common");
 
 /**
- * @dev Create a new super app registration key.
+ * @dev Authorizes an account to get Super Apps registered.
+ * For more details, see https://github.com/superfluid-finance/protocol-monorepo/wiki/Super-App-White-listing-Guide
  * @param {Array} argv Overriding command line arguments
  * @param {boolean} options.isTruffle Whether the script is used within native truffle framework
  * @param {Web3} options.web3  Injected web3 instance
  * @param {Address} options.from Address to deploy contracts from
  * @param {string} options.protocolReleaseVersion Specify the protocol release version to be used
  *
- * Note: the key itself doesn't have much meaning, it could be "stolen" from a broadcast tx anyway.
- * But since it's bound to a deployer address, that doesn't really matter.
- *
  * Usage:
- * npx truffle exec ops-scripts/gov-create-new-app-registration-key.js : {DEPLOYER} {REGISTRATION_KEY} [EXPIRATION_TS]
- *        EXPIRATION_TS is a Unix timestamp in seconds for when the key should expire.
- *        If not set, we default to 90 days in the future.
+ * npx truffle exec ops-scripts/gov-authorize-app-deployer.js : {DEPLOYER} [EXPIRATION_TS]
+ *        EXPIRATION_TS is a Unix timestamp in seconds for when the authorization should expire.
+ *        If not set, we default to (for practical purposed) no expiration.
  *        Hint: you may use https://www.unixtimestamp.com/ to calculate a timestamp
  */
 module.exports = eval(`(${S.toString()})({
     doNotPrintColonArgs: true
 })`)(async function (args, options = {}) {
-    console.log("======== Creating new app registration key ========");
+    console.log("======== Authorizing Super App Deployer ========");
     let {protocolReleaseVersion} = options;
 
     if (args.length > 3 || args.length < 2) {
         throw new Error("Wrong number of arguments");
     }
 
-    let expirationTs = Math.floor(Date.now() / 1000) + 3600 * 24 * 90; // 90 days from now
+    // default: 2^64 - 1 (far in the future - for practical purposes, never expiring)
+    let expirationTs = (BigInt(2) ** BigInt(64) - BigInt(1)).toString();
     if (args.length === 3) {
         const expTsStr = args.pop();
         const parsedExpTs = parseInt(expTsStr);
         if (parsedExpTs.toString() !== expTsStr) {
             throw new Error("EXPIRATON_TS not an integer");
         }
         expirationTs = parsedExpTs;
+        console.log("Expiration timestamp", expirationTs);
+        console.log("Expiration date", new Date(expirationTs * 1000)); // print human readable
     }
-    const registrationKey = args.pop();
+    // for historical reasons, we have "registration keys" and now hardcode those to "k1"
+    const registrationKey = "k1";
     const deployer = args.pop();
     console.log("Deployer", deployer);
-    console.log("Registration key", registrationKey);
-    console.log("Expiration timestamp", expirationTs);
 
     console.log("protocol release version:", protocolReleaseVersion);
 
@@ -63,8 +63,6 @@ module.exports = eval(`(${S.toString()})({
     });
     await sf.initialize();
 
-    console.log("Expiration date", new Date(expirationTs * 1000)); // print human readable
-
     await sendGovernanceAction(sf, (gov) =>
         gov.setAppRegistrationKey(
             sf.host.address,

diff --git a/packages/ethereum-contracts/ops-scripts/gov-create-new-factory-registration.js b/packages/ethereum-contracts/ops-scripts/gov-create-new-factory-registration.js
diff --git a/packages/ethereum-contracts/tasks/etherscan-verify-framework.sh b/packages/ethereum-contracts/tasks/etherscan-verify-framework.sh
@@ -8,7 +8,7 @@
 
 set -x
 
-CONTRACTS_DIR=build/truffle/contracts
+CONTRACTS_DIR=build/truffle
 
 TRUFFLE_NETWORK=$1
 ADDRESSES_VARS=$2