From 89cfcf9f07b10f1a99668b8fa9b729954f54a986 Mon Sep 17 00:00:00 2001 From: Miao ZhiCheng Date: Sun, 7 Feb 2021 20:33:02 +0200 Subject: [PATCH 1/3] a bug in agreement library causing non super app callbacks in IDA case 1. due to dividends rights token are no longer part of the test suite, this regression was not detected. 2. IDA full test suite should cover this --- .../contracts/agreements/AgreementLibrary.sol | 106 ++++++++++-------- 1 file changed, 58 insertions(+), 48 deletions(-) diff --git a/packages/ethereum-contracts/contracts/agreements/AgreementLibrary.sol b/packages/ethereum-contracts/contracts/agreements/AgreementLibrary.sol index cc6b5b9d9e..ffd41f6da7 100644 --- a/packages/ethereum-contracts/contracts/agreements/AgreementLibrary.sol +++ b/packages/ethereum-contracts/contracts/agreements/AgreementLibrary.sol @@ -49,6 +49,7 @@ library AgreementLibrary { *************************************************************************/ struct CallbackInputs { + bool isSuperApp; uint256 noopMask; ISuperfluidToken token; address account; @@ -73,9 +74,14 @@ library AgreementLibrary { inputs.account = account; inputs.agreementId = agreementId; inputs.agreementData = agreementData; - (bool isSuperApp, bool isJailed, uint256 noopMask) = host.getAppManifest(ISuperApp(account)); - // skip the callbacks if the app is already jailed - inputs.noopMask = isSuperApp && !isJailed ? noopMask : type(uint256).max; + { + bool isJailed; + (inputs.isSuperApp, isJailed, inputs.noopMask) = host.getAppManifest(ISuperApp(account)); + // skip the callbacks if the app is already jailed + if (isJailed) { + inputs.noopMask = type(uint256).max; + } + } } function callAppBeforeCallback( @@ -85,25 +91,27 @@ library AgreementLibrary { internal returns(bytes memory cbdata) { - // this will check composit app whitelisting, do not skip! - // otherwise an app could be trapped into an agreement - bytes memory appCtx = _pushCallbackStack(ctx, inputs); - if ((inputs.noopMask & inputs.noopBit) == 0) { - bytes memory callData = abi.encodeWithSelector( - _selectorFromNoopBit(inputs.noopBit), - inputs.token, - address(this) /* agreementClass */, - inputs.agreementId, - inputs.agreementData, - new bytes(0) // placeholder ctx - ); - cbdata = ISuperfluid(msg.sender).callAppBeforeCallback( - ISuperApp(inputs.account), - callData, - inputs.noopBit == SuperAppDefinitions.BEFORE_AGREEMENT_TERMINATED_NOOP, - appCtx); + if (inputs.isSuperApp) { + // this will check composit app whitelisting, do not skip! + // otherwise an app could be trapped into an agreement: + bytes memory appCtx = _pushCallbackStack(ctx, inputs); + if ((inputs.noopMask & inputs.noopBit) == 0) { + bytes memory callData = abi.encodeWithSelector( + _selectorFromNoopBit(inputs.noopBit), + inputs.token, + address(this) /* agreementClass */, + inputs.agreementId, + inputs.agreementData, + new bytes(0) // placeholder ctx + ); + cbdata = ISuperfluid(msg.sender).callAppBeforeCallback( + ISuperApp(inputs.account), + callData, + inputs.noopBit == SuperAppDefinitions.BEFORE_AGREEMENT_TERMINATED_NOOP, + appCtx); + } + _popCallbackStatck(ctx, 0); } - _popCallbackStatck(ctx, 0); } function callAppAfterCallback( @@ -114,33 +122,35 @@ library AgreementLibrary { internal returns (ISuperfluid.Context memory appContext, bytes memory appCtx) { - // this will check composit app whitelisting, do not skip! - // otherwise an app could be trapped into an agreement - appCtx = _pushCallbackStack(ctx, inputs); - if ((inputs.noopMask & inputs.noopBit) == 0) { - bytes memory callData = abi.encodeWithSelector( - _selectorFromNoopBit(inputs.noopBit), - inputs.token, - address(this) /* agreementClass */, - inputs.agreementId, - inputs.agreementData, - cbdata, - new bytes(0) // placeholder ctx - ); - appCtx = ISuperfluid(msg.sender).callAppAfterCallback( - ISuperApp(inputs.account), - callData, - inputs.noopBit == SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP, - appCtx); - - appContext = ISuperfluid(msg.sender).decodeCtx(appCtx); - - // adjust allowance used to the range [appAllowanceWanted..appAllowanceGranted] - appContext.appAllowanceUsed = max(0, min( - inputs.appAllowanceGranted.toInt256(), - max(appContext.appAllowanceWanted.toInt256(), appContext.appAllowanceUsed))); - - appCtx = _popCallbackStatck(ctx, appContext.appAllowanceUsed); + if (inputs.isSuperApp) { + // this will check composit app whitelisting, do not skip! + // otherwise an app could be trapped into an agreement + appCtx = _pushCallbackStack(ctx, inputs); + if ((inputs.noopMask & inputs.noopBit) == 0) { + bytes memory callData = abi.encodeWithSelector( + _selectorFromNoopBit(inputs.noopBit), + inputs.token, + address(this) /* agreementClass */, + inputs.agreementId, + inputs.agreementData, + cbdata, + new bytes(0) // placeholder ctx + ); + appCtx = ISuperfluid(msg.sender).callAppAfterCallback( + ISuperApp(inputs.account), + callData, + inputs.noopBit == SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP, + appCtx); + + appContext = ISuperfluid(msg.sender).decodeCtx(appCtx); + + // adjust allowance used to the range [appAllowanceWanted..appAllowanceGranted] + appContext.appAllowanceUsed = max(0, min( + inputs.appAllowanceGranted.toInt256(), + max(appContext.appAllowanceWanted.toInt256(), appContext.appAllowanceUsed))); + + appCtx = _popCallbackStatck(ctx, appContext.appAllowanceUsed); + } } } From 25d2f705ba96330b54ed49d9236b0ae53c0eb1fd Mon Sep 17 00:00:00 2001 From: Miao ZhiCheng Date: Mon, 8 Feb 2021 10:20:14 +0200 Subject: [PATCH 2/3] bump ethereum-contracts version --- packages/ethereum-contracts/package.json | 2 +- packages/js-sdk/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ethereum-contracts/package.json b/packages/ethereum-contracts/package.json index 48cabbb6fc..268c7a7b85 100644 --- a/packages/ethereum-contracts/package.json +++ b/packages/ethereum-contracts/package.json @@ -1,6 +1,6 @@ { "name": "@superfluid-finance/ethereum-contracts", - "version": "0.2.4", + "version": "0.2.5", "description": " Ethereum contracts implementation for the Superfluid Protocol", "repository": "github:superfluid-finance/protocol-monorepo", "license": "AGPL-3.0", diff --git a/packages/js-sdk/package.json b/packages/js-sdk/package.json index 942d9ca6c8..21418cbaa3 100644 --- a/packages/js-sdk/package.json +++ b/packages/js-sdk/package.json @@ -42,7 +42,7 @@ "@ethersproject/contracts": "^5.0.10", "@ethersproject/providers": "^5.0.21", "@openzeppelin/test-helpers": "0.5.10", - "@superfluid-finance/ethereum-contracts": "^0.2.4", + "@superfluid-finance/ethereum-contracts": "^0.2.5", "chai-as-promised": "7.1.1", "truffle": "5.1.49", "web3": "^1.3.3", From c5906da47dbe7d12a610db1dac337c731636f441 Mon Sep 17 00:00:00 2001 From: Miao ZhiCheng Date: Mon, 8 Feb 2021 11:05:58 +0200 Subject: [PATCH 3/3] skip-on-coverage on superfluid#testcase@6.24 --- .../test/contracts/superfluid/Superfluid.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.js b/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.js index be2f5a4b82..ac3d877d57 100644 --- a/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.js +++ b/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.js @@ -1055,7 +1055,7 @@ contract("Superfluid Host Contract", accounts => { ); }); - it("#6.24 beforeCreated try to burn just enough gas", async () => { + it("#6.24 beforeCreated try to burn just enough gas [ @skip-on-coverage ]", async () => { const actionOverhead = 20000; /* some action overhead */ const setNextAction = async () => { await app.setNextCallbackAction(