Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

a bug in agreement library causing non super app callbacks in IDA case #246

Merged
merged 5 commits into from
Feb 8, 2021
Merged

a bug in agreement library causing non super app callbacks in IDA case #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
89cfcf9
a bug in agreement library causing non super app callbacks in IDA case
hellwolf Feb 7, 2021
ffd93fe
Merge branch 'dev' into bug_callback_to_non_superapp
hellwolf Feb 7, 2021
b6821b4
Merge remote-tracking branch 'origin/dev' into bug_callback_to_non_su…
hellwolf Feb 8, 2021
25d2f70
bump ethereum-contracts version
hellwolf Feb 8, 2021
c5906da
skip-on-coverage on superfluid#testcase@6.24
hellwolf Feb 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 58 additions & 48 deletions packages/ethereum-contracts/contracts/agreements/AgreementLibrary.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ library AgreementLibrary {
*************************************************************************/

struct CallbackInputs {
bool isSuperApp;
uint256 noopMask;
ISuperfluidToken token;
address account;
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/ethereum-contracts/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
2 changes: 1 addition & 1 deletion packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion packages/js-sdk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down