diff --git a/packages/ethereum-contracts/CHANGELOG.md b/packages/ethereum-contracts/CHANGELOG.md index 913170e221..78dc039b06 100644 --- a/packages/ethereum-contracts/CHANGELOG.md +++ b/packages/ethereum-contracts/CHANGELOG.md @@ -9,6 +9,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `SuperToken` now implements [EIP-2612](https://eips.ethereum.org/EIPS/eip-2612) (permit extension for EIP-20 signed approvals). - `SuperfluidPool` now has additional methods `increaseMemberUnits` and `decreaseMemberUnits` which allow the pool admin to change member units parameterized with delta amounts. +### Changed +- Curation of the SuperApp registration allowlist can now be delegated by governance to a newly added `ACL` contract. + ### Breaking - `SuperfluidPool` does no longer mint and burn EIP-721 tokens (NFTs) on member unit updates. The gas overhead of this operation caused friction for integrations with other protocols (e.g. Uniswap V4). diff --git a/packages/ethereum-contracts/contracts/apps/SuperTokenV1Library.sol b/packages/ethereum-contracts/contracts/apps/SuperTokenV1Library.sol index 4c51580392..529ec77d57 100644 --- a/packages/ethereum-contracts/contracts/apps/SuperTokenV1Library.sol +++ b/packages/ethereum-contracts/contracts/apps/SuperTokenV1Library.sol @@ -47,7 +47,7 @@ import { * `expectRevert` expects a revert in the next call. * If a revert is triggered by library code itself (vs by a call), `expectRevert` will thus not _see_ that. * Possible mitigations: - * - avoid higher-level library methods which can themselves trigger reverts in tests where this is is an issue + * - avoid higher-level library methods which can themselves trigger reverts in tests where this is an issue * - wrap the method invocation into an external helper method which you then invoke with `this.helperMethod()`, * which makes it an external call * Also be aware of other limitations, see diff --git a/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluid.sol b/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluid.sol index 97d60cc6e3..d04c83ed95 100644 --- a/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluid.sol +++ b/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluid.sol @@ -18,6 +18,7 @@ import { /// Note: CustomSuperTokenBase is not included for people building CustomSuperToken. import { IERC20, IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import { IERC777 } from "@openzeppelin/contracts/token/ERC777/IERC777.sol"; +import { IAccessControl } from "@openzeppelin/contracts/access/IAccessControl.sol"; import { ISuperfluidToken } from "./ISuperfluidToken.sol"; import { ISuperToken } from "./ISuperToken.sol"; import { ISuperTokenFactory } from "./ISuperTokenFactory.sol"; @@ -29,10 +30,10 @@ import { IPoolMemberNFT } from "../agreements/gdav1/IPoolMemberNFT.sol"; import { ISuperAgreement } from "./ISuperAgreement.sol"; import { IConstantFlowAgreementV1 } from "../agreements/IConstantFlowAgreementV1.sol"; import { IInstantDistributionAgreementV1 } from "../agreements/IInstantDistributionAgreementV1.sol"; -import { - IGeneralDistributionAgreementV1, - PoolConfig, - PoolERC20Metadata +import { + IGeneralDistributionAgreementV1, + PoolConfig, + PoolERC20Metadata } from "../agreements/gdav1/IGeneralDistributionAgreementV1.sol"; import { ISuperfluidPool } from "../agreements/gdav1/ISuperfluidPool.sol"; /// Superfluid App interfaces: @@ -658,6 +659,19 @@ interface ISuperfluid { // solhint-disable func-name-mixedcase function getERC2771Forwarder() external view returns(address); + // solhint-disable max-line-length + /** + * @dev returns the SimpleACL contract (currently used for SuperApp registration permissioning). + * That contract implements the interface [IAccessControl](https://docs.openzeppelin.com/contracts/4.x/api/access#IAccessControl), + * which provides the following functions: + * - [hasRole(role, account)](https://docs.openzeppelin.com/contracts/4.x/api/access#IAccessControl-hasRole-bytes32-address-) + * - [getRoleAdmin(role)](https://docs.openzeppelin.com/contracts/4.x/api/access#IAccessControl-getRoleAdmin-bytes32-) + * - [grantRole(role, account)](https://docs.openzeppelin.com/contracts/4.x/api/access#IAccessControl-grantRole-bytes32-address-) + * - [revokeRole(role, account)](https://docs.openzeppelin.com/contracts/4.x/api/access#IAccessControl-revokeRole-bytes32-address-) + * - [renounceRole(role, account)](https://docs.openzeppelin.com/contracts/4.x/api/access#IAccessControl-renounceRole-bytes32-address-) + */ + function getSimpleACL() external view returns(IAccessControl); + /************************************************************************** * Function modifiers for access control and parameter validations * diff --git a/packages/ethereum-contracts/contracts/mocks/CFAAppMocks.t.sol b/packages/ethereum-contracts/contracts/mocks/CFAAppMocks.t.sol index ef7a92ce30..87c223f6d6 100644 --- a/packages/ethereum-contracts/contracts/mocks/CFAAppMocks.t.sol +++ b/packages/ethereum-contracts/contracts/mocks/CFAAppMocks.t.sol @@ -36,7 +36,7 @@ contract ExclusiveInflowTestApp is SuperAppBase { // | SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP ; - _host.registerAppWithKey(configWord, ""); + _host.registerApp(configWord); } function afterAgreementCreated( @@ -121,7 +121,7 @@ contract NonClosableOutflowTestApp is SuperAppBase { // | SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP ; - _host.registerAppWithKey(configWord, ""); + _host.registerApp(configWord); } function setupOutflow( @@ -210,7 +210,7 @@ contract SelfDeletingFlowTestApp is SuperAppBase { | SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP ; - _host.registerAppWithKey(configWord, ""); + _host.registerApp(configWord); } function afterAgreementCreated( @@ -267,7 +267,7 @@ contract ClosingOnUpdateFlowTestApp is SuperAppBase { | SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP ; - _host.registerAppWithKey(configWord, ""); + _host.registerApp(configWord); } function afterAgreementUpdated( @@ -326,7 +326,7 @@ contract FlowExchangeTestApp is SuperAppBase { | SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP ; - _host.registerAppWithKey(configWord, ""); + _host.registerApp(configWord); } function afterAgreementCreated( diff --git a/packages/ethereum-contracts/contracts/mocks/IDASuperAppTester.t.sol b/packages/ethereum-contracts/contracts/mocks/IDASuperAppTester.t.sol index ac9d2a5488..362e881929 100644 --- a/packages/ethereum-contracts/contracts/mocks/IDASuperAppTester.t.sol +++ b/packages/ethereum-contracts/contracts/mocks/IDASuperAppTester.t.sol @@ -26,7 +26,7 @@ contract IDASuperAppTester is ISuperApp { uint32 indexId) { _host = host; - _host.registerAppWithKey(configWord, ""); + _host.registerApp(configWord); _ida = ida; _token = token; _indexId = indexId; diff --git a/packages/ethereum-contracts/contracts/mocks/MultiFlowTesterApp.t.sol b/packages/ethereum-contracts/contracts/mocks/MultiFlowTesterApp.t.sol index 53476772f7..b97dc79af6 100644 --- a/packages/ethereum-contracts/contracts/mocks/MultiFlowTesterApp.t.sol +++ b/packages/ethereum-contracts/contracts/mocks/MultiFlowTesterApp.t.sol @@ -41,7 +41,7 @@ contract MultiFlowTesterApp is SuperAppBase { SuperAppDefinitions.BEFORE_AGREEMENT_CREATED_NOOP | SuperAppDefinitions.BEFORE_AGREEMENT_TERMINATED_NOOP; - _host.registerAppWithKey(configWord, ""); + _host.registerApp(configWord); } function _parseUserData( diff --git a/packages/ethereum-contracts/contracts/mocks/StreamRedirector.t.sol b/packages/ethereum-contracts/contracts/mocks/StreamRedirector.t.sol index 8f1bfd106e..6a88672e17 100644 --- a/packages/ethereum-contracts/contracts/mocks/StreamRedirector.t.sol +++ b/packages/ethereum-contracts/contracts/mocks/StreamRedirector.t.sol @@ -33,7 +33,7 @@ contract StreamRedirector is SuperAppBase { token = _token; receiver = _receiver; - host.registerAppWithKey(configWord, ""); + host.registerApp(configWord); } /** diff --git a/packages/ethereum-contracts/contracts/mocks/SuperAppMocks.t.sol b/packages/ethereum-contracts/contracts/mocks/SuperAppMocks.t.sol index 3c92cf017c..17791b9ca2 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperAppMocks.t.sol +++ b/packages/ethereum-contracts/contracts/mocks/SuperAppMocks.t.sol @@ -52,15 +52,14 @@ contract SuperAppMock is ISuperApp { constructor(ISuperfluid host, uint256 configWord, bool doubleRegistration) { _host = host; - _host.registerAppWithKey(configWord, ""); + _host.registerApp(configWord); if (doubleRegistration) { - _host.registerAppWithKey(configWord, ""); + _host.registerApp(configWord); } _aux = new SuperAppMockAux(); } function tryRegisterApp(uint256 configWord) external { - // @note this is deprecated keeping this here for testing/coverage _host.registerApp(configWord); } @@ -472,7 +471,7 @@ contract SuperAppMockReturningEmptyCtx { constructor(ISuperfluid host) { _host = host; - _host.registerAppWithKey(SuperAppDefinitions.APP_LEVEL_FINAL, ""); + _host.registerApp(SuperAppDefinitions.APP_LEVEL_FINAL); } function beforeAgreementCreated( @@ -533,7 +532,7 @@ contract SuperAppMockReturningInvalidCtx { constructor(ISuperfluid host) { _host = host; - _host.registerAppWithKey(SuperAppDefinitions.APP_LEVEL_FINAL, ""); + _host.registerApp(SuperAppDefinitions.APP_LEVEL_FINAL); } function afterAgreementCreated( @@ -574,7 +573,7 @@ contract SuperAppMock2ndLevel { constructor(ISuperfluid host, SuperAppMock app, AgreementMock agreement) { _host = host; - _host.registerAppWithKey(SuperAppDefinitions.APP_LEVEL_SECOND, ""); + _host.registerApp(SuperAppDefinitions.APP_LEVEL_SECOND); _app = app; _agreement = agreement; } @@ -615,10 +614,9 @@ contract SuperAppMockWithRegistrationKey { } } -// An Super App that uses registerAppWithKey +// An Super App that self-registers contract SuperAppMockUsingRegisterApp { constructor(ISuperfluid host, uint256 configWord) { - // @note this is deprecated keeping this here for testing/coverage host.registerApp(configWord); } } @@ -630,6 +628,7 @@ contract SuperAppMockNotSelfRegistering { } // Factory which allows anybody to deploy arbitrary contracts as app (do NOT allow this in a real factory!) contract SuperAppFactoryMock { function registerAppWithHost(ISuperfluid host, ISuperApp app, uint256 configWord) external { + // @note this way of registering is DEPREACTED! host.registerAppByFactory(app, configWord); } } diff --git a/packages/ethereum-contracts/contracts/mocks/SuperTokenLibraryV1Mock.t.sol b/packages/ethereum-contracts/contracts/mocks/SuperTokenLibraryV1Mock.t.sol index b41452d371..c2464582cb 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperTokenLibraryV1Mock.t.sol +++ b/packages/ethereum-contracts/contracts/mocks/SuperTokenLibraryV1Mock.t.sol @@ -384,7 +384,7 @@ contract SuperTokenLibraryCFASuperAppMock is SuperAppBase { SuperAppDefinitions.BEFORE_AGREEMENT_TERMINATED_NOOP | SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP; - host.registerAppWithKey(configWord, ""); + host.registerApp(configWord); } function createFlow(ISuperToken token) external { diff --git a/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.t.sol b/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.t.sol index 83206208e1..9ed7ac4183 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.t.sol +++ b/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), address(0)) + constructor() Superfluid(false, false, 3_000_000, address(0), address(0), address(0)) // solhint-disable-next-line no-empty-blocks { } @@ -135,10 +135,12 @@ contract SuperfluidMock is Superfluid { bool appWhiteListingEnabled, uint64 callbackGasLimit, address simpleForwarderAddress, - address erc2771ForwarderAddress + address erc2771ForwarderAddress, + address simpleAclAddress ) Superfluid( - nonUpgradable, appWhiteListingEnabled, callbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress + nonUpgradable, appWhiteListingEnabled, callbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress, + simpleAclAddress ) // solhint-disable-next-line no-empty-blocks { diff --git a/packages/ethereum-contracts/contracts/superfluid/Superfluid.sol b/packages/ethereum-contracts/contracts/superfluid/Superfluid.sol index 258a1f4856..7df746f332 100644 --- a/packages/ethereum-contracts/contracts/superfluid/Superfluid.sol +++ b/packages/ethereum-contracts/contracts/superfluid/Superfluid.sol @@ -16,7 +16,8 @@ import { SuperfluidGovernanceConfigs, ISuperfluidToken, ISuperToken, - ISuperTokenFactory + ISuperTokenFactory, + IAccessControl } from "../interfaces/superfluid/ISuperfluid.sol"; import { GeneralDistributionAgreementV1 } from "../agreements/gdav1/GeneralDistributionAgreementV1.sol"; import { SuperfluidUpgradeableBeacon } from "../upgradability/SuperfluidUpgradeableBeacon.sol"; @@ -25,6 +26,7 @@ import { CallbackUtils } from "../libs/CallbackUtils.sol"; import { BaseRelayRecipient } from "../libs/BaseRelayRecipient.sol"; import { SimpleForwarder } from "../utils/SimpleForwarder.sol"; import { ERC2771Forwarder } from "../utils/ERC2771Forwarder.sol"; +import { SimpleACL } from "../utils/SimpleACL.sol"; /** * @dev The Superfluid host implementation. @@ -59,6 +61,9 @@ contract Superfluid is SimpleForwarder immutable public SIMPLE_FORWARDER; ERC2771Forwarder immutable internal _ERC2771_FORWARDER; + // ACL (for superapp registration) + SimpleACL immutable internal _SIMPLE_ACL; + /** * @dev Maximum number of level of apps can be composed together * @@ -71,6 +76,8 @@ contract Superfluid is uint32 constant public MAX_NUM_AGREEMENTS = 256; + bytes32 constant public ACL_SUPERAPP_REGISTRATION_ROLE = keccak256("ACL_SUPERAPP_REGISTRATION_ROLE"); + /* WARNING: NEVER RE-ORDER VARIABLES! Always double-check that new variables are added APPEND-ONLY. Re-ordering variables can permanently BREAK the deployed proxy contract. */ @@ -105,13 +112,15 @@ contract Superfluid is bool appWhiteListingEnabled, uint64 callbackGasLimit, address simpleForwarderAddress, - address erc2771ForwarderAddress + address erc2771ForwarderAddress, + address simpleAclAddress ) { NON_UPGRADABLE_DEPLOYMENT = nonUpgradable; APP_WHITE_LISTING_ENABLED = appWhiteListingEnabled; CALLBACK_GAS_LIMIT = callbackGasLimit; SIMPLE_FORWARDER = SimpleForwarder(simpleForwarderAddress); _ERC2771_FORWARDER = ERC2771Forwarder(erc2771ForwarderAddress); + _SIMPLE_ACL = SimpleACL(simpleAclAddress); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -375,8 +384,16 @@ contract Superfluid is _registerApp(ISuperApp(msg.sender), configWord); } - // internally we keep using the gov config method with key + // Checks if the deployer account has permission to register SuperApps, reverts if not. + // New method: lookup in the SimpleACL contract. + // Legacy/fallback method: lookup in the governance contract. function _enforceAppRegistrationPermissioning(string memory registrationKey, address deployer) internal view { + // new method: check if the deployer is granted permission in the ACL + if (_SIMPLE_ACL.hasRole(ACL_SUPERAPP_REGISTRATION_ROLE, deployer)) { + return; + } + + // legacy/fallback method: check if permission is given by gov bytes32 configKey = SuperfluidGovernanceConfigs.getAppRegistrationConfigKey( // solhint-disable-next-line avoid-tx-origin deployer, @@ -958,6 +975,10 @@ contract Superfluid is return address(_ERC2771_FORWARDER); } + function getSimpleACL() external view override returns(IAccessControl) { + return _SIMPLE_ACL; + } + /************************************************************************** * Internal **************************************************************************/ diff --git a/packages/ethereum-contracts/contracts/utils/SimpleACL.sol b/packages/ethereum-contracts/contracts/utils/SimpleACL.sol new file mode 100644 index 0000000000..05fb553429 --- /dev/null +++ b/packages/ethereum-contracts/contracts/utils/SimpleACL.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.23; + +import { AccessControl } from "@openzeppelin/contracts/access/AccessControl.sol"; + +contract SimpleACL is AccessControl { + constructor() { + _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); + } + + /// Allows the default admin to set the admin role for a given role. + /// This gives the flexibility to set up sophisticated permissioning schemes in the future. + function setRoleAdmin(bytes32 role, bytes32 adminRole) external onlyRole(DEFAULT_ADMIN_ROLE) { + _setRoleAdmin(role, adminRole); + } +} \ No newline at end of file diff --git a/packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol b/packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol index 4a7052e69e..0276d399f0 100644 --- a/packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol +++ b/packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.t.sol @@ -31,6 +31,7 @@ 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 { SimpleACL } from "../utils/SimpleACL.sol"; import { MacroForwarder } from "../utils/MacroForwarder.sol"; /// @title Superfluid Framework Deployment Steps @@ -143,10 +144,11 @@ contract SuperfluidFrameworkDeploymentSteps { } else if (step == 1) { // CORE CONTRACT: Superfluid (Host) SimpleForwarder simpleForwarder = new SimpleForwarder(); ERC2771Forwarder erc2771Forwarder = new ERC2771Forwarder(); + SimpleACL simpleAcl = new SimpleACL(); // 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(simpleForwarder), address(erc2771Forwarder) + true, false, 3_000_000, address(simpleForwarder), address(erc2771Forwarder), address(simpleAcl) ); simpleForwarder.transferOwnership(address(host)); erc2771Forwarder.transferOwnership(address(host)); @@ -319,12 +321,14 @@ library SuperfluidHostDeployerLibrary { bool _appWhiteListingEnabled, uint64 callbackGasLimit, address simpleForwarderAddress, - address erc2771ForwarderAddress + address erc2771ForwarderAddress, + address simpleAclAddress ) external returns (Superfluid) { return new Superfluid( - _nonUpgradable, _appWhiteListingEnabled, callbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress + _nonUpgradable, _appWhiteListingEnabled, callbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress, + simpleAclAddress ); } } diff --git a/packages/ethereum-contracts/ops-scripts/deploy-framework.js b/packages/ethereum-contracts/ops-scripts/deploy-framework.js index 2c6709af45..850c977500 100644 --- a/packages/ethereum-contracts/ops-scripts/deploy-framework.js +++ b/packages/ethereum-contracts/ops-scripts/deploy-framework.js @@ -234,6 +234,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( "IAccessControlEnumerable", "SimpleForwarder", "ERC2771Forwarder", + "SimpleACL", ]; const mockContracts = [ "SuperfluidMock", @@ -273,6 +274,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( IAccessControlEnumerable, SimpleForwarder, ERC2771Forwarder, + SimpleACL, } = await SuperfluidSDK.loadContracts({ ...extractWeb3Options(options), additionalContracts: contracts.concat(useMocks ? mockContracts : []), @@ -361,11 +363,15 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( console.log("ERC2771Forwarder address:", erc2771Forwarder.address); output += `ERC2771_FORWARDER=${erc2771Forwarder.address}\n`; + const simpleAcl = await web3tx(SimpleACL.new, "SimpleACL.new")(); + console.log("SimpleACL address:", simpleAcl.address); + output += `SIMPLE_ACL=${simpleAcl.address}\n`; + let superfluidAddress; const superfluidLogic = await web3tx( SuperfluidLogic.new, "SuperfluidLogic.new" - )(nonUpgradable, appWhiteListing, appCallbackGasLimit, simpleForwarder.address, erc2771Forwarder.address); + )(nonUpgradable, appWhiteListing, appCallbackGasLimit, simpleForwarder.address, erc2771Forwarder.address, simpleAcl.address); console.log( `Superfluid new code address ${superfluidLogic.address}` ); @@ -803,17 +809,6 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( return ZERO_ADDRESS; }); - if (prevAddr !== 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, @@ -833,6 +828,25 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( return newAddress !== ZERO_ADDRESS ? newAddress : prevAddr; } + async function getOrDeployHelper( + Contract, + getPrevAddrFn, + outputKey + ) { + let prevAddr = await getPrevAddrFn().catch(_err => { + console.error(`### Error getting ${Contract.contractName} address, likely not yet deployed`); + return ZERO_ADDRESS; + }); + + if (prevAddr !== ZERO_ADDRESS) { + return prevAddr; + } + + const instance = await web3tx(Contract.new, `${Contract.contractName}.new`)(); + output += `${outputKey}=${instance.address}\n`; + return instance.address; + } + const simpleForwarderAddress = await getOrDeployForwarder( superfluid, SimpleForwarder, @@ -847,6 +861,13 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( "ERC2771_FORWARDER" ); + const simpleAclAddress = await getOrDeployHelper( + SimpleACL, + () => superfluid.getSimpleACL(), + "SIMPLE_ACL" + ); + console.log("SimpleACL address", simpleAclAddress); + // get previous callback gas limit, make sure we don't decrease it const prevCallbackGasLimit = await superfluid.CALLBACK_GAS_LIMIT(); if (prevCallbackGasLimit.toNumber() > appCallbackGasLimit) { @@ -867,13 +888,14 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( const superfluidLogic = await web3tx( SuperfluidLogic.new, "SuperfluidLogic.new" - )(nonUpgradable, appWhiteListing, appCallbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress); + )(nonUpgradable, appWhiteListing, appCallbackGasLimit, simpleForwarderAddress, erc2771ForwarderAddress, simpleAclAddress); output += `SUPERFLUID_HOST_LOGIC=${superfluidLogic.address}\n`; return superfluidLogic.address; }, [ ap(erc2771ForwarderAddress), ap(simpleForwarderAddress), + ap(simpleAclAddress), appCallbackGasLimit.toString(16).padStart(64, "0") ], ); diff --git a/packages/ethereum-contracts/tasks/bundled-abi-contracts-list.json b/packages/ethereum-contracts/tasks/bundled-abi-contracts-list.json index dcb99646a4..2657511050 100644 --- a/packages/ethereum-contracts/tasks/bundled-abi-contracts-list.json +++ b/packages/ethereum-contracts/tasks/bundled-abi-contracts-list.json @@ -21,5 +21,6 @@ "IPureSuperToken", "IResolver", "Resolver", "TestResolver", "SuperfluidLoader", - "IUserDefinedMacro" + "IUserDefinedMacro", + "SimpleACL" ] diff --git a/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.ts b/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.ts index 6a453c81dc..16cac2cdd0 100644 --- a/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.ts +++ b/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.ts @@ -115,14 +115,16 @@ describe("Superfluid Host Contract", function () { false /* appWhiteListingEnabled */, 3000000 /* callbackGasLimit */, ZERO_ADDRESS /* simpleForwarder */, - ZERO_ADDRESS /* erc2771Forwarder */ + ZERO_ADDRESS /* erc2771Forwarder */, + ZERO_ADDRESS /* simpleAcl */ ); const mock2 = await sfMockFactory.deploy( true /* nonUpgradable */, false /* appWhiteListingEnabled */, 3000000 /* callbackGasLimit */, ZERO_ADDRESS /* simpleForwarder */, - ZERO_ADDRESS /* erc2771Forwarder */ + ZERO_ADDRESS /* erc2771Forwarder */, + ZERO_ADDRESS /* simpleAcl */ ); await governance.updateContracts( superfluid.address, @@ -2696,7 +2698,8 @@ describe("Superfluid Host Contract", function () { false /* appWhiteListingEnabled */, 3000000 /* callbackGasLimit */, ZERO_ADDRESS /* simpleForwarder */, - ZERO_ADDRESS /* erc2771Forwarder */ + ZERO_ADDRESS /* erc2771Forwarder */, + ZERO_ADDRESS /* simpleAcl */ ); await expectCustomError( governance.updateContracts( diff --git a/packages/ethereum-contracts/test/foundry/superfluid/Superfluid.t.sol b/packages/ethereum-contracts/test/foundry/superfluid/Superfluid.t.sol index e36ac75b84..9721897ca6 100644 --- a/packages/ethereum-contracts/test/foundry/superfluid/Superfluid.t.sol +++ b/packages/ethereum-contracts/test/foundry/superfluid/Superfluid.t.sol @@ -6,8 +6,11 @@ import { UUPSProxiable } from "../../../contracts/upgradability/UUPSProxiable.so import { SuperToken } from "../../../contracts/superfluid/SuperToken.sol"; import { SuperTokenV1Library } from "../../../contracts/apps/SuperTokenV1Library.sol"; import { ISuperAgreement } from "../../../contracts/interfaces/superfluid/ISuperAgreement.sol"; -import { ISuperfluid } from "../../../contracts/interfaces/superfluid/ISuperfluid.sol"; +import { ISuperfluid, SuperAppDefinitions } from "../../../contracts/interfaces/superfluid/ISuperfluid.sol"; +import { ISuperApp } from "../../../contracts/interfaces/superfluid/ISuperApp.sol"; import { AgreementMock } from "../../../contracts/mocks/AgreementMock.t.sol"; +import { SuperAppMockNotSelfRegistering } from "../../../contracts/mocks/SuperAppMocks.t.sol"; +import { SimpleACL } from "../../../contracts/utils/SimpleACL.sol"; contract SuperfluidIntegrationTest is FoundrySuperfluidTester { using SuperTokenV1Library for SuperToken; @@ -31,7 +34,9 @@ contract SuperfluidIntegrationTest is FoundrySuperfluidTester { vm.startPrank(sf.governance.owner()); sf.governance.registerAgreementClass(sf.host, address(agreementMock)); vm.stopPrank(); - agreementMock = sf.host.NON_UPGRADABLE_DEPLOYMENT() ? agreementMock : AgreementMock(address(sf.host.getAgreementClass(id))); + agreementMock = sf.host.NON_UPGRADABLE_DEPLOYMENT() + ? agreementMock + : AgreementMock(address(sf.host.getAgreementClass(id))); mocks[i + _NUM_AGREEMENTS] = ISuperAgreement(address(agreementMock)); } @@ -83,4 +88,66 @@ contract SuperfluidIntegrationTest is FoundrySuperfluidTester { sf.host.changeSuperTokenAdmin(superToken, newAdmin); vm.stopPrank(); } + + function testSuperAppRegistrationViaSimpleACL() public { + SimpleACL simpleAcl = new SimpleACL(); + Superfluid hostWithSimpleACL = new Superfluid( + true, true, 3_000_000, address(0), address(0), address(simpleAcl) + ); + ISuperApp mockSuperApp1 = ISuperApp(address(new SuperAppMockNotSelfRegistering())); + ISuperApp mockSuperApp2 = ISuperApp(address(new SuperAppMockNotSelfRegistering())); + + hostWithSimpleACL.initialize(sf.governance); + + bytes32 aclSuperAppRegRole = hostWithSimpleACL.ACL_SUPERAPP_REGISTRATION_ROLE(); + + // first, give permission to alice + simpleAcl.grantRole(aclSuperAppRegRole, alice); + + // as bob, try to register a superapp - should revert + vm.startPrank(bob); + vm.expectRevert(); + hostWithSimpleACL.registerApp(mockSuperApp1, SuperAppDefinitions.APP_LEVEL_FINAL); + + // as alice, try to register a superapp - should succeed + vm.startPrank(alice); + hostWithSimpleACL.registerApp(mockSuperApp1, SuperAppDefinitions.APP_LEVEL_FINAL); + vm.stopPrank(); + vm.assertTrue(hostWithSimpleACL.isApp(mockSuperApp1)); + + // revoke permission from alice + simpleAcl.revokeRole(aclSuperAppRegRole, alice); + + // as alice, try to register a superapp - should revert + vm.startPrank(alice); + vm.expectRevert(); + hostWithSimpleACL.registerApp(mockSuperApp2, SuperAppDefinitions.APP_LEVEL_FINAL); + vm.stopPrank(); + + // nobody else can grant permission to register superapps + vm.startPrank(eve); + vm.expectRevert(); + simpleAcl.grantRole(aclSuperAppRegRole, bob); + vm.stopPrank(); + + // nobody can define admin roles ... + bytes32 dedicatedAdminRole = keccak256("SUPER_APP_REGISTRATION_ADMIN"); + vm.startPrank(eve); + vm.expectRevert(); + simpleAcl.setRoleAdmin(aclSuperAppRegRole, dedicatedAdminRole); + vm.stopPrank(); + + // ... except the default admin + // (This is to be done in a possible future where we start using this ACL for other purposes too + // and want to have a more sophisticated permissioning scheme.) + simpleAcl.setRoleAdmin(aclSuperAppRegRole, dedicatedAdminRole); + + // grant heidi the new admin role + simpleAcl.grantRole(dedicatedAdminRole, heidi); + + // now heidi can manage permissions for superapp deployers + vm.startPrank(heidi); + simpleAcl.grantRole(aclSuperAppRegRole, bob); + vm.stopPrank(); + } } diff --git a/packages/ethereum-contracts/test/ops-scripts/deployment.test.js b/packages/ethereum-contracts/test/ops-scripts/deployment.test.js index 5a0e7da488..d026d6f47d 100644 --- a/packages/ethereum-contracts/test/ops-scripts/deployment.test.js +++ b/packages/ethereum-contracts/test/ops-scripts/deployment.test.js @@ -128,7 +128,8 @@ contract("Embedded deployment scripts", (accounts) => { false, // appWhiteListingEnabled callbackGasLimit, // callbackGasLimit ZERO_ADDRESS, // simpleForwarder - ZERO_ADDRESS // erc2771Forwarder + ZERO_ADDRESS, // erc2771Forwarder + ZERO_ADDRESS // simpleAcl ); assert.isFalse( await codeChanged(web3, Superfluid, a1.address, [ diff --git a/packages/ethereum-contracts/truffle-config.js b/packages/ethereum-contracts/truffle-config.js index 5e247d18fd..d89b4a0e67 100644 --- a/packages/ethereum-contracts/truffle-config.js +++ b/packages/ethereum-contracts/truffle-config.js @@ -275,7 +275,7 @@ const E = (module.exports = { ...createNetworkDefaultConfiguration("base-sepolia"), network_id: 84532, maxPriorityFeePerGas: 1e6, // 0.001 gwei - even 0 may do - maxFeePerGas: 1e9, // 1 gwei + maxFeePerGas: 1e8, // 0.1 gwei }, //