From 8e9f7a591d9a35c90a93148bdbc2d86f19e7e752 Mon Sep 17 00:00:00 2001 From: Didi Date: Wed, 21 Feb 2024 15:41:55 +0100 Subject: [PATCH] [ETHEREUM-CONTRACTS] CFASuperAppBase renamed, made compatible with factory and proxy patterns (#1838) * SuperAppBaseFlow renamed to CFASuperAppBase and made compatible with factory and proxy pattern * added comment * added clarification about what 'breaking' means in this context * fixed changelog text --- packages/ethereum-contracts/.solcover.js | 2 +- packages/ethereum-contracts/CHANGELOG.md | 8 +++ ...perAppBaseFlow.sol => CFASuperAppBase.sol} | 56 ++++++++++++++----- ...owTester.sol => CFASuperAppBaseTester.sol} | 15 +++-- .../contracts/mocks/CrossStreamSuperApp.sol | 7 ++- .../ops-scripts/gov-authorize-app-deployer.js | 5 +- ...ppBaseFlow.t.sol => CFASuperAppBase.t.sol} | 34 ++++++----- .../apps/SuperAppTester/FlowSplitter.sol | 7 ++- 8 files changed, 92 insertions(+), 42 deletions(-) rename packages/ethereum-contracts/contracts/apps/{SuperAppBaseFlow.sol => CFASuperAppBase.sol} (79%) rename packages/ethereum-contracts/contracts/mocks/{SuperAppBaseFlowTester.sol => CFASuperAppBaseTester.sol} (85%) rename packages/ethereum-contracts/test/foundry/apps/{SuperAppBaseFlow.t.sol => CFASuperAppBase.t.sol} (89%) diff --git a/packages/ethereum-contracts/.solcover.js b/packages/ethereum-contracts/.solcover.js index b78b382805..9c1c98fa12 100644 --- a/packages/ethereum-contracts/.solcover.js +++ b/packages/ethereum-contracts/.solcover.js @@ -9,7 +9,7 @@ module.exports = { // we skip the coverage for the SuperAppBase contracts because // we override the functions in child contracts "apps/SuperAppBase.sol", - "apps/SuperAppBaseFlow.sol", + "apps/CFASuperAppBase.sol", "apps/SuperfluidLoaderLibrary.sol", // we skip the coverage for these contracts because they are diff --git a/packages/ethereum-contracts/CHANGELOG.md b/packages/ethereum-contracts/CHANGELOG.md index 566ae2b0b2..fdc1c8be97 100644 --- a/packages/ethereum-contracts/CHANGELOG.md +++ b/packages/ethereum-contracts/CHANGELOG.md @@ -5,6 +5,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## Unreleased +### Breaking + +- The abstract base contract`SuperAppBaseFlow` was renamed to `CFASuperAppBase`. +Initialization is now split between constructor and a method `_initialize`, with self-registration +made optional. +This allows the contract to be used with a SuperApp factory pattern (disable self-registration on networks with permissioned SuperApps) and for logic contracts in the context of the proxy pattern. +Note: this will NOT break any deployed contracts, only affects undeployed Super Apps in case the ethereum-contracts dependency is updated. + ### Added - New utility: MacroForwarder - a trusted forwarder extensible with permission-less macro contracts. diff --git a/packages/ethereum-contracts/contracts/apps/SuperAppBaseFlow.sol b/packages/ethereum-contracts/contracts/apps/CFASuperAppBase.sol similarity index 79% rename from packages/ethereum-contracts/contracts/apps/SuperAppBaseFlow.sol rename to packages/ethereum-contracts/contracts/apps/CFASuperAppBase.sol index 827d75a8c5..faed7bb89c 100644 --- a/packages/ethereum-contracts/contracts/apps/SuperAppBaseFlow.sol +++ b/packages/ethereum-contracts/contracts/apps/CFASuperAppBase.sol @@ -4,7 +4,18 @@ pragma solidity >= 0.8.11; import { ISuperfluid, ISuperToken, ISuperApp, SuperAppDefinitions } from "../interfaces/superfluid/ISuperfluid.sol"; import { SuperTokenV1Library } from "./SuperTokenV1Library.sol"; -abstract contract SuperAppBaseFlow is ISuperApp { +/** + * @title abstract base contract for SuperApps using CFA callbacks + * @author Superfluid + * @dev This contract provides a more convenient API for implementing CFA callbacks. + * It allows to write more concise and readable SuperApps when the full flexibility + * of the low-level agreement callbacks isn't needed. + * The API is tailored for the most common use cases, with the "beforeX" and "afterX" callbacks being + * abstrated into a single "onX" callback for create|update|delete flows. + * For use cases requiring more flexibility (specifically if more data needs to be provided by the before callbacks) + * it's recommended to implement the low-level callbacks directly instead of using this base contract. + */ +abstract contract CFASuperAppBase is ISuperApp { using SuperTokenV1Library for ISuperToken; bytes32 public constant CFAV1_TYPE = keccak256("org.superfluid-finance.agreements.ConstantFlowAgreement.v1"); @@ -21,36 +32,53 @@ abstract contract SuperAppBaseFlow is ISuperApp { error NotAcceptedSuperToken(); /** - * @dev Initializes the contract by setting the expected Superfluid Host. - * and register which callbacks the Host can engage when appropriate. + * @dev Creates the contract and ties it to a Superfluid Host. + * @notice You also need to call `_initialize()` after construction. */ - constructor( - ISuperfluid host_, + constructor(ISuperfluid host_) { + HOST = host_; + } + + /** + * @dev Initializes the SuperApp with the provided settings + * @param activateOnCreated activates the callbacks for `createFlow` + * @param activateOnUpdated activates the callbacks for `updateFlow` + * @param activateOnDeleted activates the callbacks for `deleteFlow` + * @param selfRegister if true, the App shall register itself with the host. + * If false, the caller is reposible for calling `registerApp` on the host contract. + * No callbacks will be received as long as the App is not registered. + * @return configWord the `configWord` used or to be used in the `registerApp` call + * + * Note that if the App self-registers on a network with permissioned SuperApp registration, + * the tx.origin needs to be whitelisted for that transaction to succeed. + * Fore more details, see https://github.com/superfluid-finance/protocol-monorepo/wiki/Super-App-White-listing-Guide + */ + function _initialize( bool activateOnCreated, bool activateOnUpdated, bool activateOnDeleted, - string memory registrationKey - ) { - HOST = host_; - - uint256 callBackDefinitions = SuperAppDefinitions.APP_LEVEL_FINAL + bool selfRegister + ) internal returns (uint256 configWord) { + configWord = SuperAppDefinitions.APP_LEVEL_FINAL | SuperAppDefinitions.BEFORE_AGREEMENT_CREATED_NOOP; if (!activateOnCreated) { - callBackDefinitions |= SuperAppDefinitions.AFTER_AGREEMENT_CREATED_NOOP; + configWord |= SuperAppDefinitions.AFTER_AGREEMENT_CREATED_NOOP; } if (!activateOnUpdated) { - callBackDefinitions |= SuperAppDefinitions.BEFORE_AGREEMENT_UPDATED_NOOP + configWord |= SuperAppDefinitions.BEFORE_AGREEMENT_UPDATED_NOOP | SuperAppDefinitions.AFTER_AGREEMENT_UPDATED_NOOP; } if (!activateOnDeleted) { - callBackDefinitions |= SuperAppDefinitions.BEFORE_AGREEMENT_TERMINATED_NOOP + configWord |= SuperAppDefinitions.BEFORE_AGREEMENT_TERMINATED_NOOP | SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP; } - host_.registerAppWithKey(callBackDefinitions, registrationKey); + if (selfRegister) { + HOST.registerApp(configWord); + } } /** diff --git a/packages/ethereum-contracts/contracts/mocks/SuperAppBaseFlowTester.sol b/packages/ethereum-contracts/contracts/mocks/CFASuperAppBaseTester.sol similarity index 85% rename from packages/ethereum-contracts/contracts/mocks/SuperAppBaseFlowTester.sol rename to packages/ethereum-contracts/contracts/mocks/CFASuperAppBaseTester.sol index 8fcc03d0c3..c4aafb7aed 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperAppBaseFlowTester.sol +++ b/packages/ethereum-contracts/contracts/mocks/CFASuperAppBaseTester.sol @@ -2,10 +2,10 @@ pragma solidity 0.8.23; import { ISuperfluid, ISuperToken } from "../interfaces/superfluid/ISuperfluid.sol"; -import { SuperAppBaseFlow } from "../apps/SuperAppBaseFlow.sol"; +import { CFASuperAppBase } from "../apps/CFASuperAppBase.sol"; import { SuperTokenV1Library } from "../apps/SuperTokenV1Library.sol"; -contract SuperAppBaseFlowTester is SuperAppBaseFlow { +contract CFASuperAppBaseTester is CFASuperAppBase { using SuperTokenV1Library for ISuperToken; int96 public oldFlowRateHolder; @@ -17,9 +17,16 @@ contract SuperAppBaseFlowTester is SuperAppBaseFlow { // irreversibly set to true once the setter is invoked bool internal _restrictAcceptedSuperTokens; - constructor(ISuperfluid host, bool activateOnCreated, bool activateOnUpdated, bool activateOnDeleted) - SuperAppBaseFlow(host, activateOnCreated, activateOnUpdated, activateOnDeleted, "") + constructor( + ISuperfluid host, + bool activateOnCreated, + bool activateOnUpdated, + bool activateOnDeleted, + bool selfRegister + ) + CFASuperAppBase(host) { + _initialize(activateOnCreated, activateOnUpdated, activateOnDeleted, selfRegister); lastUpdateHolder = 0; // appeasing linter } diff --git a/packages/ethereum-contracts/contracts/mocks/CrossStreamSuperApp.sol b/packages/ethereum-contracts/contracts/mocks/CrossStreamSuperApp.sol index 55907ab9eb..6f804718d7 100644 --- a/packages/ethereum-contracts/contracts/mocks/CrossStreamSuperApp.sol +++ b/packages/ethereum-contracts/contracts/mocks/CrossStreamSuperApp.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.23; import { ISuperfluid, ISuperToken } from "../interfaces/superfluid/ISuperfluid.sol"; -import { SuperAppBaseFlow } from "../apps/SuperAppBaseFlow.sol"; +import { CFASuperAppBase } from "../apps/CFASuperAppBase.sol"; import { SuperTokenV1Library } from "../apps/SuperTokenV1Library.sol"; using SuperTokenV1Library for ISuperToken; @@ -12,12 +12,13 @@ using SuperTokenV1Library for ISuperToken; /// @dev A super app used for testing "cross-stream" flows in callbacks /// and its behavior surrounding the internal protocol accounting. /// That is, two senders sending a flow to the super app -contract CrossStreamSuperApp is SuperAppBaseFlow { +contract CrossStreamSuperApp is CFASuperAppBase { address public flowRecipient; address public prevSender; int96 public prevFlowRate; - constructor(ISuperfluid host_, address z_) SuperAppBaseFlow(host_, true, true, true, "") { + constructor(ISuperfluid host_, address z_) CFASuperAppBase(host_) { + _initialize(true, true, true, true); flowRecipient = z_; } diff --git a/packages/ethereum-contracts/ops-scripts/gov-authorize-app-deployer.js b/packages/ethereum-contracts/ops-scripts/gov-authorize-app-deployer.js index 3c90444485..b496395da1 100644 --- a/packages/ethereum-contracts/ops-scripts/gov-authorize-app-deployer.js +++ b/packages/ethereum-contracts/ops-scripts/gov-authorize-app-deployer.js @@ -43,8 +43,9 @@ module.exports = eval(`(${S.toString()})({ console.log("Expiration timestamp", expirationTs); console.log("Expiration date", new Date(expirationTs * 1000)); // print human readable } - // for historical reasons, we have "registration keys" and now hardcode those to "k1" - const registrationKey = "k1"; + // for historical reasons, we have "registration keys" and now hardcode those to "k1" by default + const registrationKey = process.env.REGISTRATION_KEY !== undefined ? process.env.REGISTRATION_KEY : "k1"; + console.log("Registration key", registrationKey); const deployer = args.pop(); console.log("Deployer", deployer); diff --git a/packages/ethereum-contracts/test/foundry/apps/SuperAppBaseFlow.t.sol b/packages/ethereum-contracts/test/foundry/apps/CFASuperAppBase.t.sol similarity index 89% rename from packages/ethereum-contracts/test/foundry/apps/SuperAppBaseFlow.t.sol rename to packages/ethereum-contracts/test/foundry/apps/CFASuperAppBase.t.sol index a7a1ab311d..84dcf51bec 100644 --- a/packages/ethereum-contracts/test/foundry/apps/SuperAppBaseFlow.t.sol +++ b/packages/ethereum-contracts/test/foundry/apps/CFASuperAppBase.t.sol @@ -3,8 +3,8 @@ pragma solidity 0.8.23; import "forge-std/console.sol"; import "../FoundrySuperfluidTester.sol"; -import { SuperAppBaseFlow } from "../../../contracts/apps/SuperAppBaseFlow.sol"; -import { SuperAppBaseFlowTester } from "../../../contracts/mocks/SuperAppBaseFlowTester.sol"; +import { CFASuperAppBase } from "../../../contracts/apps/CFASuperAppBase.sol"; +import { CFASuperAppBaseTester } from "../../../contracts/mocks/CFASuperAppBaseTester.sol"; import { ISuperToken, ISuperApp, @@ -13,11 +13,11 @@ import { import { IConstantFlowAgreementV1 } from "../../../contracts/interfaces/agreements/IConstantFlowAgreementV1.sol"; import { SuperTokenV1Library } from "../../../contracts/apps/SuperTokenV1Library.sol"; -contract SuperAppBaseFlowTest is FoundrySuperfluidTester { +contract CFASuperAppBaseTest is FoundrySuperfluidTester { using SuperTokenV1Library for SuperToken; using SuperTokenV1Library for ISuperToken; - SuperAppBaseFlowTester superApp; + CFASuperAppBaseTester superApp; address superAppAddress; ISuperToken otherSuperToken; @@ -26,7 +26,7 @@ contract SuperAppBaseFlowTest is FoundrySuperfluidTester { function setUp() public virtual override { super.setUp(); vm.startPrank(admin); - superApp = new SuperAppBaseFlowTester(sf.host, true, true, true); + superApp = new CFASuperAppBaseTester(sf.host, true, true, true, true); superAppAddress = address(superApp); otherSuperToken = sfDeployer.deployPureSuperToken("FTT", "FTT", 1e27); otherSuperToken.transfer(alice, 1e21); @@ -58,20 +58,24 @@ contract SuperAppBaseFlowTest is FoundrySuperfluidTester { return callBackDefinitions; } - function _deploySuperAppAndGetConfig(bool activateOnCreated, bool activateOnUpdated, bool activateOnDeleted) + function _deploySuperAppAndGetConfig(bool activateOnCreated, bool activateOnUpdated, bool activateOnDeleted, bool selfRegister) internal - returns (SuperAppBaseFlowTester, uint256 configWord) + returns (CFASuperAppBaseTester, uint256 configWord) { - SuperAppBaseFlowTester mySuperApp = - new SuperAppBaseFlowTester(sf.host, activateOnCreated, activateOnUpdated, activateOnDeleted); + CFASuperAppBaseTester mySuperApp = + new CFASuperAppBaseTester(sf.host, activateOnCreated, activateOnUpdated, activateOnDeleted, selfRegister); uint256 appConfig = _genManifest(activateOnCreated, activateOnUpdated, activateOnDeleted); return (mySuperApp, appConfig); } - function testOnFlagsSetAppManifest(bool activateOnCreated, bool activateOnUpdated, bool activateOnDeleted) public { + function testOnFlagsSetAppManifest(bool activateOnCreated, bool activateOnUpdated, bool activateOnDeleted, bool selfRegister) public { //all onOperations - (SuperAppBaseFlowTester mySuperApp, uint256 configWord) = - _deploySuperAppAndGetConfig(activateOnCreated, activateOnUpdated, activateOnDeleted); + (CFASuperAppBaseTester mySuperApp, uint256 configWord) = + _deploySuperAppAndGetConfig(activateOnCreated, activateOnUpdated, activateOnDeleted, selfRegister); + if (!selfRegister) { + // this would revert if already registered + sf.host.registerApp(mySuperApp, configWord); + } (bool isSuperApp,, uint256 noopMask) = sf.host.getAppManifest(ISuperApp(mySuperApp)); configWord = configWord & SuperAppDefinitions.AGREEMENT_CALLBACK_NOOP_BITMASKS; assertTrue(isSuperApp, "SuperAppBase: is superApp incorrect"); @@ -103,10 +107,10 @@ contract SuperAppBaseFlowTest is FoundrySuperfluidTester { function testUnauthorizedHost() public { vm.startPrank(eve); - vm.expectRevert(SuperAppBaseFlow.UnauthorizedHost.selector); + vm.expectRevert(CFASuperAppBase.UnauthorizedHost.selector); superApp.afterAgreementCreated(superToken, address(sf.cfa), "0x", "0x", "0x", "0x"); - vm.expectRevert(SuperAppBaseFlow.UnauthorizedHost.selector); + vm.expectRevert(CFASuperAppBase.UnauthorizedHost.selector); superApp.afterAgreementUpdated(superToken, address(sf.cfa), "0x", "0x", "0x", "0x"); // termination callback doesn't revert, but should have no side effects @@ -229,7 +233,7 @@ contract SuperAppBaseFlowTest is FoundrySuperfluidTester { vm.startPrank(alice); // enable the filter superApp.setAcceptedSuperToken(superToken, true); - vm.expectRevert(SuperAppBaseFlow.NotAcceptedSuperToken.selector); + vm.expectRevert(CFASuperAppBase.NotAcceptedSuperToken.selector); sf.host.callAgreement( sf.cfa, abi.encodeCall(sf.cfa.createFlow, (otherSuperToken, address(superApp), int96(69), new bytes(0))), diff --git a/packages/ethereum-contracts/test/foundry/apps/SuperAppTester/FlowSplitter.sol b/packages/ethereum-contracts/test/foundry/apps/SuperAppTester/FlowSplitter.sol index 064b9f05d0..40c5e08084 100644 --- a/packages/ethereum-contracts/test/foundry/apps/SuperAppTester/FlowSplitter.sol +++ b/packages/ethereum-contracts/test/foundry/apps/SuperAppTester/FlowSplitter.sol @@ -8,13 +8,13 @@ pragma solidity 0.8.23; // import "hardhat/console.sol"; import { SuperTokenV1Library } from "@superfluid-finance/ethereum-contracts/contracts/apps/SuperTokenV1Library.sol"; -import { SuperAppBaseFlow } from "@superfluid-finance/ethereum-contracts/contracts/apps/SuperAppBaseFlow.sol"; +import { CFASuperAppBase } from "@superfluid-finance/ethereum-contracts/contracts/apps/CFASuperAppBase.sol"; import { ISuperfluid, ISuperToken } from "@superfluid-finance/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluid.sol"; -contract FlowSplitter is SuperAppBaseFlow { +contract FlowSplitter is CFASuperAppBase { using SuperTokenV1Library for ISuperToken; /// @dev Account that ought to be routed the majority of the inflows @@ -36,7 +36,8 @@ contract FlowSplitter is SuperAppBaseFlow { int96 _sideReceiverPortion, ISuperToken _acceptedSuperToken, ISuperfluid _host - ) SuperAppBaseFlow(_host, true, true, true, "") { + ) CFASuperAppBase(_host) { + _initialize(true, true, true, true); mainReceiver = _mainReceiver; sideReceiver = _sideReceiver; sideReceiverPortion = _sideReceiverPortion;