From b905896f6269c9c60fe35e5722c7b738c6eebdd2 Mon Sep 17 00:00:00 2001 From: Didi Date: Fri, 5 Jul 2024 10:08:20 +0200 Subject: [PATCH] [ETHEREUM-CONTRACTS] per chain configurable app callback gas limit (#1982) * update forge-std * [ETHEREUM-CONTRACTS] CallbackUtils replaces SafeGasLibrary * update forge-std to v1.8.2 * [ETHEREUM-CONTRACTS] fix compiler warnings * [ETHEREUM-CONTRACTS] update changelog * revert lib/forge-std update * [ETHEREUM-CONTRACTS] relax solidity pragma ^0.8.23 * update forge-std to v1.9.1 * configurable app callback gas limit * amended changelog * add comments * [ETHEREUM-CONTRACTS] fast SuperAppMocks._burnGas * [workflows] actions/upload-artifact@v4 * [ethereum-contracts] some comments to SuperAppMocks._burnGas * [solidity-semantic-money] relax solidity pragma ^0.8.23 * [solidity-semantic-money] adapt to latet forge-std * ci.canary.yml: upgrade-contracts disabled * to use solc 0.8.26 * [ethereum-contracts] use 0.8.26 * fix forge command * fix build * fix build * sdk-core: fix build * [ethereum-contracts] move mocks for foundry to foundry folder * fix build * fix build * fix solidity pragma * [ethereum-contracts] updated changelog * [ethereum-contracts] skip-on-coverage superfluid#6.24 again * [ethereum-contracts] update CHANGELOG * [ethereum-contracts] fix Superfluid#6.24 gasLowerBound * [ethereum-contracts] fix Superfluid#6.24 gasLowerBound * [ethereum-contracts] make testOutOfCbGasZone pass for coverage test --------- Co-authored-by: Miao, ZhiCheng Co-authored-by: Miao ZhiCheng --- packages/ethereum-contracts/CHANGELOG.md | 1 + .../contracts/mocks/SuperfluidMock.t.sol | 12 +++++-- .../contracts/superfluid/Superfluid.sol | 13 ++++--- .../SuperfluidFrameworkDeploymentSteps.sol | 12 +++++-- .../ops-scripts/deploy-framework.js | 18 ++++++++-- .../ops-scripts/libs/getConfig.js | 35 ++++++++++++++++++- .../contracts/superfluid/Superfluid.test.ts | 3 ++ .../test/ops-scripts/deployment.test.js | 1 + 8 files changed, 82 insertions(+), 13 deletions(-) diff --git a/packages/ethereum-contracts/CHANGELOG.md b/packages/ethereum-contracts/CHANGELOG.md index 49ffbdf3e2..9dd9ba725b 100644 --- a/packages/ethereum-contracts/CHANGELOG.md +++ b/packages/ethereum-contracts/CHANGELOG.md @@ -23,6 +23,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed +- increase SuperApp callback gas limit on some chains, to be queried with `host.CALLBACK_GAS_LIMIT()` - Remove try/catch in PoolNFT callbacks. - upgrade flake locked foundry: 0.2.0 (20b3da1 2024-07-02T00:18:52.435480726Z). - relax pragram solidity with "^0.8.23". diff --git a/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.t.sol b/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.t.sol index 0b9e2a9d91..caa1fb6955 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.t.sol +++ b/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.t.sol @@ -11,7 +11,8 @@ import { CallUtils } from "../libs/CallUtils.sol"; contract SuperfluidUpgradabilityTester is Superfluid { - constructor() Superfluid(false, false, address(0)) + // 3_000_000 is the min callback gas limit used in a prod deployment + constructor() Superfluid(false, false, 3_000_000, address(0)) // solhint-disable-next-line no-empty-blocks { } @@ -129,8 +130,13 @@ contract SuperfluidUpgradabilityTester is Superfluid { contract SuperfluidMock is Superfluid { - constructor(bool nonUpgradable, bool appWhiteListingEnabled, address dmzForwarder) - Superfluid(nonUpgradable, appWhiteListingEnabled, dmzForwarder) + constructor( + bool nonUpgradable, + bool appWhiteListingEnabled, + uint64 callbackGasLimit, + address dmzForwarder + ) + Superfluid(nonUpgradable, appWhiteListingEnabled, callbackGasLimit, dmzForwarder) // 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 8094b0da95..9528c99748 100644 --- a/packages/ethereum-contracts/contracts/superfluid/Superfluid.sol +++ b/packages/ethereum-contracts/contracts/superfluid/Superfluid.sol @@ -52,6 +52,8 @@ contract Superfluid is // solhint-disable-next-line var-name-mixedcase bool immutable public APP_WHITE_LISTING_ENABLED; + uint64 immutable public CALLBACK_GAS_LIMIT; + DMZForwarder immutable public DMZ_FORWARDER; /** @@ -64,9 +66,6 @@ contract Superfluid is // solhint-disable-next-line var-name-mixedcase uint constant public MAX_APP_CALLBACK_LEVEL = 1; - // solhint-disable-next-line var-name-mixedcase - uint64 constant public CALLBACK_GAS_LIMIT = 3000000; - uint32 constant public MAX_NUM_AGREEMENTS = 256; /* WARNING: NEVER RE-ORDER VARIABLES! Always double-check that new @@ -98,9 +97,15 @@ contract Superfluid is /// function in its respective mock contract to ensure that it doesn't break anything or lead to unexpected /// behaviors/layout when upgrading - constructor(bool nonUpgradable, bool appWhiteListingEnabled, address dmzForwarderAddress) { + constructor( + bool nonUpgradable, + bool appWhiteListingEnabled, + uint64 callbackGasLimit, + address dmzForwarderAddress + ) { NON_UPGRADABLE_DEPLOYMENT = nonUpgradable; APP_WHITE_LISTING_ENABLED = appWhiteListingEnabled; + CALLBACK_GAS_LIMIT = callbackGasLimit; DMZ_FORWARDER = DMZForwarder(dmzForwarderAddress); } diff --git a/packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.sol b/packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.sol index 943e1f0e45..bef16401a6 100644 --- a/packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.sol +++ b/packages/ethereum-contracts/contracts/utils/SuperfluidFrameworkDeploymentSteps.sol @@ -156,7 +156,8 @@ contract SuperfluidFrameworkDeploymentSteps { } else if (step == 1) { // CORE CONTRACT: Superfluid (Host) DMZForwarder dmzForwarder = SuperfluidDMZForwarderDeployerLibrary.deploy(); // Deploy Host and initialize the test governance. - host = SuperfluidHostDeployerLibrary.deploy(true, false, address(dmzForwarder)); + // 3_000_000 is the min callback gas limit used in a prod deployment + host = SuperfluidHostDeployerLibrary.deploy(true, false, 3_000_000, address(dmzForwarder)); dmzForwarder.transferOwnership(address(host)); host.initialize(testGovernance); @@ -357,10 +358,15 @@ library SuperfluidDMZForwarderDeployerLibrary { } library SuperfluidHostDeployerLibrary { - function deploy(bool _nonUpgradable, bool _appWhiteListingEnabled, address dmzForwarderAddress) + function deploy( + bool _nonUpgradable, + bool _appWhiteListingEnabled, + uint64 callbackGasLimit, + address dmzForwarderAddress + ) external returns (Superfluid) { - return new Superfluid(_nonUpgradable, _appWhiteListingEnabled, dmzForwarderAddress); + return new Superfluid(_nonUpgradable, _appWhiteListingEnabled, callbackGasLimit, dmzForwarderAddress); } } diff --git a/packages/ethereum-contracts/ops-scripts/deploy-framework.js b/packages/ethereum-contracts/ops-scripts/deploy-framework.js index fe2b89e681..e758e07f7d 100644 --- a/packages/ethereum-contracts/ops-scripts/deploy-framework.js +++ b/packages/ethereum-contracts/ops-scripts/deploy-framework.js @@ -122,6 +122,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( useMocks, nonUpgradable, appWhiteListing, + appCallbackGasLimit, protocolReleaseVersion, outputFile, newSuperfluidLoader, @@ -149,6 +150,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( console.log("chain ID: ", chainId); console.log("deployer: ", deployerAddr); const config = getConfig(chainId); + if (config.isTestnet) { output += "IS_TESTNET=1\n"; } @@ -177,6 +179,10 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( appWhiteListing || config.gov_enableAppWhiteListing || !!process.env.ENABLE_APP_WHITELISTING; + appCallbackGasLimit = + appCallbackGasLimit || + config.appCallbackGasLimit || + !!process.env.APP_CALLBACK_GAS_LIMIT; newSuperfluidLoader = newSuperfluidLoader || !!process.env.NEW_SUPERFLUID_LOADER; console.log("app whitelisting enabled:", appWhiteListing); @@ -357,7 +363,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( const superfluidLogic = await web3tx( SuperfluidLogic.new, "SuperfluidLogic.new" - )(nonUpgradable, appWhiteListing, dmzForwarder.address); + )(nonUpgradable, appWhiteListing, appCallbackGasLimit, dmzForwarder.address); console.log( `Superfluid new code address ${superfluidLogic.address}` ); @@ -829,6 +835,14 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( } ); + // get previous callback gas limit, make sure we don't decrease it + const prevCallbackGasLimit = await superfluid.CALLBACK_GAS_LIMIT(); + if (prevCallbackGasLimit.toNumber() > appCallbackGasLimit) { + throw new Error("Cannot decrease app callback gas limit"); + } else if (prevCallbackGasLimit.toNumber() !== appCallbackGasLimit) { + console.log(` !!! CHANGING APP CALLBACK GAS LIMIT FROM ${prevCallbackGasLimit} to ${appCallbackGasLimit} !!!`); + } + // deploy new superfluid host logic superfluidNewLogicAddress = await deployContractIfCodeChanged( web3, @@ -841,7 +855,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function ( const superfluidLogic = await web3tx( SuperfluidLogic.new, "SuperfluidLogic.new" - )(nonUpgradable, appWhiteListing, dmzForwarderAddress); + )(nonUpgradable, appWhiteListing, appCallbackGasLimit, dmzForwarderAddress); output += `SUPERFLUID_HOST_LOGIC=${superfluidLogic.address}\n`; return superfluidLogic.address; } diff --git a/packages/ethereum-contracts/ops-scripts/libs/getConfig.js b/packages/ethereum-contracts/ops-scripts/libs/getConfig.js index 98bb0cb0da..8aef920ee3 100644 --- a/packages/ethereum-contracts/ops-scripts/libs/getConfig.js +++ b/packages/ethereum-contracts/ops-scripts/libs/getConfig.js @@ -22,10 +22,42 @@ module.exports = function getConfig(chainId) { trustedForwarders: ["0x3075b4dc7085C48A14A5A39BBa68F58B19545971"], }, + // persistent chains + + // eth-mainnet + 1: { + // we keep it low because mainnet is expensive and we don't want solvency risks + appCallbackGasLimit: 3000000, + }, + + // xdai-mainnet + 100: { + // half of the block gas limit of 17M + appCallbackGasLimit: 8500000, + }, + + // avalanche-fuji + 43113: { + // half of the block gas limit of 15M + appCallbackGasLimit: 7500000, + }, + // avalanche-c + 43114: { + // half of the block gas limit of 15M + appCallbackGasLimit: 7500000, + }, + // Celo Mainnet 42220: { gov_enableAppWhiteListing: false, - } + }, + + // scroll-mainnet + 534352: { + // we keep it low in order to minimize the chance of "proof overflows" + // see https://docs.scroll.io/en/technology/sequencer/execution-node/#circuit-capacity-checker + appCallbackGasLimit: 3000000, + }, }; const sfNw = sfMetadata.getNetworkByChainId(chainId); @@ -51,6 +83,7 @@ module.exports = function getConfig(chainId) { metadata: sfNw, resolverAddress: global?.process.env.RESOLVER_ADDRESS || sfNw?.contractsV1?.resolver, trustedForwarders: sfNw?.trustedForwarders, + appCallbackGasLimit: 15000000, ...EXTRA_CONFIG[chainId] }; }; diff --git a/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.ts b/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.ts index c727456791..2573dd827e 100644 --- a/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.ts +++ b/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.ts @@ -113,11 +113,13 @@ describe("Superfluid Host Contract", function () { const mock1 = await sfMockFactory.deploy( false /* nonUpgradable */, false /* appWhiteListingEnabled */, + 3000000 /* callbackGasLimit */, ZERO_ADDRESS /* dmzForwader */ ); const mock2 = await sfMockFactory.deploy( true /* nonUpgradable */, false /* appWhiteListingEnabled */, + 3000000 /* callbackGasLimit */, ZERO_ADDRESS /* dmzForwader */ ); await governance.updateContracts( @@ -2702,6 +2704,7 @@ describe("Superfluid Host Contract", function () { const mock1 = await mock1Factory.deploy( false /* nonUpgradable */, false /* appWhiteListingEnabled */, + 3000000 /* callbackGasLimit */, ZERO_ADDRESS /* dmzForwader */ ); await expectCustomError( diff --git a/packages/ethereum-contracts/test/ops-scripts/deployment.test.js b/packages/ethereum-contracts/test/ops-scripts/deployment.test.js index 9bbb62b166..cc934187fa 100644 --- a/packages/ethereum-contracts/test/ops-scripts/deployment.test.js +++ b/packages/ethereum-contracts/test/ops-scripts/deployment.test.js @@ -125,6 +125,7 @@ contract("Embedded deployment scripts", (accounts) => { const a1 = await web3tx(Superfluid.new, "Superfluid.new 1")( true, // nonUpgradable false, // appWhiteListingEnabled + 3000000, // callbackGasLimit ZERO_ADDRESS // dmzForwader ); assert.isFalse(await codeChanged(web3, Superfluid, a1.address));