From d758a6732492a49cbb0b322fa24919cacf0acf42 Mon Sep 17 00:00:00 2001 From: Miao ZhiCheng Date: Wed, 27 Jan 2021 20:59:57 +0200 Subject: [PATCH] Testing 195 additional testings (#220) * added validateContextStructLayout * validate decoded ctx * test noops masks --- .../contracts/mocks/AgreementMock.sol | 114 ++++++++++++---- .../contracts/mocks/SuperAppMock.sol | 23 +++- .../contracts/mocks/SuperTokenFactoryMock.sol | 2 + .../contracts/mocks/SuperTokenMock.sol | 13 +- .../contracts/mocks/SuperfluidMock.sol | 87 ++++++++++-- .../contracts/superfluid/Superfluid.test.js | 128 ++++++++++++------ 6 files changed, 278 insertions(+), 89 deletions(-) diff --git a/packages/ethereum-contracts/contracts/mocks/AgreementMock.sol b/packages/ethereum-contracts/contracts/mocks/AgreementMock.sol index 596229b7d9..2c23d4c7cd 100644 --- a/packages/ethereum-contracts/contracts/mocks/AgreementMock.sol +++ b/packages/ethereum-contracts/contracts/mocks/AgreementMock.sol @@ -173,45 +173,107 @@ contract AgreementMock is AgreementBase { return ctx; } - event AppBeforeCallbackResult(bytes cbdata); - - function callAppBeforeAgreementCreatedCallback( + event AppBeforeCallbackResult( + uint8 appLevel, + uint8 callType, + bytes4 agreementSelector, + bytes cbdata); + + event AppAfterCallbackResult( + uint8 appLevel, + uint8 callType, + bytes4 agreementSelector); + + function _callAppBeforeCallback( ISuperApp app, + uint256 noopBit, bytes calldata ctx ) - external - validCtx(ctx) - returns (bytes memory newCtx) + private { + ISuperfluid.Context memory context = ISuperfluid(msg.sender).decodeCtx(ctx); AgreementLibrary.CallbackInputs memory cbStates = AgreementLibrary.createCallbackInputs( ISuperfluidToken(address(0)) /* token */, address(app) /* account */, 0 /* agreementId */, "" /* agreementData */ ); - cbStates.noopBit = SuperAppDefinitions.BEFORE_AGREEMENT_CREATED_NOOP; + cbStates.noopBit = noopBit; bytes memory cbdata = AgreementLibrary.callAppBeforeCallback(cbStates, ctx); - emit AppBeforeCallbackResult(cbdata); - newCtx = ctx; + emit AppBeforeCallbackResult( + context.appLevel, + context.callType, + context.agreementSelector, + cbdata); } - function callAppAfterAgreementCreatedCallback( + function _callAppAfterAgreementCallback( ISuperApp app, + uint256 noopBit, bytes calldata ctx ) - external - validCtx(ctx) - returns (bytes memory newCtx) + private returns (bytes memory newCtx) { + ISuperfluid.Context memory context = ISuperfluid(msg.sender).decodeCtx(ctx); AgreementLibrary.CallbackInputs memory cbStates = AgreementLibrary.createCallbackInputs( ISuperfluidToken(address(0)) /* token */, address(app) /* account */, 0 /* agreementId */, "" /* agreementData */ ); - cbStates.noopBit = SuperAppDefinitions.AFTER_AGREEMENT_CREATED_NOOP; + cbStates.noopBit = noopBit; (, newCtx) = AgreementLibrary.callAppAfterCallback(cbStates, "", ctx); require(ISuperfluid(msg.sender).isCtxValid(newCtx), "AgreementMock: ctx not valid after"); + emit AppAfterCallbackResult( + context.appLevel, + context.callType, + context.agreementSelector); + } + + function callAppBeforeAgreementCreatedCallback( + ISuperApp app, + bytes calldata ctx + ) + external + validCtx(ctx) + returns (bytes memory newCtx) + { + _callAppBeforeCallback(app, SuperAppDefinitions.BEFORE_AGREEMENT_CREATED_NOOP, ctx); + return ctx; + } + + function callAppAfterAgreementCreatedCallback( + ISuperApp app, + bytes calldata ctx + ) + external + validCtx(ctx) + returns (bytes memory newCtx) + { + return _callAppAfterAgreementCallback(app, SuperAppDefinitions.AFTER_AGREEMENT_CREATED_NOOP, ctx); + } + + function callAppBeforeAgreementUpdatedCallback( + ISuperApp app, + bytes calldata ctx + ) + external + validCtx(ctx) + returns (bytes memory newCtx) + { + _callAppBeforeCallback(app, SuperAppDefinitions.BEFORE_AGREEMENT_UPDATED_NOOP, ctx); + return ctx; + } + + function callAppAfterAgreementUpdatedCallback( + ISuperApp app, + bytes calldata ctx + ) + external + validCtx(ctx) + returns (bytes memory newCtx) + { + return _callAppAfterAgreementCallback(app, SuperAppDefinitions.AFTER_AGREEMENT_UPDATED_NOOP, ctx); } function callAppBeforeAgreementTerminatedCallback( @@ -222,15 +284,7 @@ contract AgreementMock is AgreementBase { validCtx(ctx) returns (bytes memory newCtx) { - AgreementLibrary.CallbackInputs memory cbStates = AgreementLibrary.createCallbackInputs( - ISuperfluidToken(address(0)) /* token */, - address(app) /* account */, - 0 /* agreementId */, - "" /* agreementData */ - ); - cbStates.noopBit = SuperAppDefinitions.BEFORE_AGREEMENT_TERMINATED_NOOP; - bytes memory cbdata = AgreementLibrary.callAppBeforeCallback(cbStates, ctx); - emit AppBeforeCallbackResult(cbdata); + _callAppBeforeCallback(app, SuperAppDefinitions.BEFORE_AGREEMENT_TERMINATED_NOOP, ctx); newCtx = ctx; } @@ -242,15 +296,21 @@ contract AgreementMock is AgreementBase { validCtx(ctx) returns (bytes memory newCtx) { + /* ISuperfluid.Context memory context = ISuperfluid(msg.sender).decodeCtx(ctx); AgreementLibrary.CallbackInputs memory cbStates = AgreementLibrary.createCallbackInputs( - ISuperfluidToken(address(0)) /* token */, - address(app) /* account */, - 0 /* agreementId */, - "" /* agreementData */ + ISuperfluidToken(address(0)) // token, + address(app) // account, + 0 // agreementId, + "" // agreementData ); cbStates.noopBit = SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP; (, newCtx) = AgreementLibrary.callAppAfterCallback(cbStates, "", ctx); require(ISuperfluid(msg.sender).isCtxValid(newCtx), "AgreementMock: ctx not valid after"); + emit AppAfterCallbackResult( + context.appLevel, + context.callType, + context.agreementSelector); */ + return _callAppAfterAgreementCallback(app, SuperAppDefinitions.AFTER_AGREEMENT_TERMINATED_NOOP, ctx); } modifier validCtx(bytes memory ctx) { diff --git a/packages/ethereum-contracts/contracts/mocks/SuperAppMock.sol b/packages/ethereum-contracts/contracts/mocks/SuperAppMock.sol index c4046342da..0c588ca374 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperAppMock.sol +++ b/packages/ethereum-contracts/contracts/mocks/SuperAppMock.sol @@ -36,10 +36,17 @@ contract SuperAppMock is ISuperApp { * Test App Actions **************************************************************************/ - event NoopEvent(); + event NoopEvent( + uint8 appLevel, + uint8 callType, + bytes4 agreementSelector); function actionNoop(bytes calldata ctx) external validCtx(ctx) returns (bytes memory newCtx) { - emit NoopEvent(); + ISuperfluid.Context memory context = ISuperfluid(msg.sender).decodeCtx(ctx); + emit NoopEvent( + context.appLevel, + context.callType, + context.agreementSelector); return ctx; } @@ -50,7 +57,10 @@ contract SuperAppMock is ISuperApp { { ISuperfluid.Context memory context = ISuperfluid(msg.sender).decodeCtx(ctx); assert(context.msgSender == expectedMsgSender); - emit NoopEvent(); + emit NoopEvent( + context.appLevel, + context.callType, + context.agreementSelector); return ctx; } @@ -235,7 +245,6 @@ contract SuperAppMock is ISuperApp { returns (bytes memory cbdata) { if (_nextCallbackAction.actionType == NextCallbackActionType.Noop) { - //emit NoopEvent(); return "Noop"; } else if (_nextCallbackAction.actionType == NextCallbackActionType.Assert) { assert(false); @@ -254,8 +263,12 @@ contract SuperAppMock is ISuperApp { private returns (bytes memory newCtx) { + ISuperfluid.Context memory context = ISuperfluid(msg.sender).decodeCtx(ctx); if (_nextCallbackAction.actionType == NextCallbackActionType.Noop) { - emit NoopEvent(); + emit NoopEvent( + context.appLevel, + context.callType, + context.agreementSelector); return ctx; } else if (_nextCallbackAction.actionType == NextCallbackActionType.Assert) { assert(false); diff --git a/packages/ethereum-contracts/contracts/mocks/SuperTokenFactoryMock.sol b/packages/ethereum-contracts/contracts/mocks/SuperTokenFactoryMock.sol index fb3c1cf647..680fad95a6 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperTokenFactoryMock.sol +++ b/packages/ethereum-contracts/contracts/mocks/SuperTokenFactoryMock.sol @@ -17,6 +17,7 @@ contract SuperTokenFactoryStorageLayoutTester is SuperTokenFactoryBase { { } + // @dev Make sure the storage layout never change over the course of the development function validateStorageLayout() external pure { uint256 slot; uint256 offset; @@ -27,6 +28,7 @@ contract SuperTokenFactoryStorageLayoutTester is SuperTokenFactoryBase { require (slot == 0 && offset == 2, "_superTokenLogic changed location"); } + // dummy impl function createSuperTokenLogic(ISuperfluid) external pure override returns (address) diff --git a/packages/ethereum-contracts/contracts/mocks/SuperTokenMock.sol b/packages/ethereum-contracts/contracts/mocks/SuperTokenMock.sol index bba83bd45d..5280d49260 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperTokenMock.sol +++ b/packages/ethereum-contracts/contracts/mocks/SuperTokenMock.sol @@ -16,14 +16,7 @@ contract SuperTokenStorageLayoutTester is SuperToken { { } - function getLastSuperTokenStorageSlot() external pure returns (uint slot) { - assembly { slot:= _reserve19.slot } - } - - /** - * Upgradability - */ - + // @dev Make sure the storage layout never change over the course of the development function validateStorageLayout() external pure { uint256 slot; uint256 offset; @@ -74,6 +67,10 @@ contract SuperTokenStorageLayoutTester is SuperToken { assembly { slot:= _reserve19.slot offset := _reserve19.offset } require (slot == 31 && offset == 0, "_reserve19 changed location"); } + + function getLastSuperTokenStorageSlot() external pure returns (uint slot) { + assembly { slot:= _reserve19.slot } + } } contract SuperTokenMock is SuperToken { diff --git a/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.sol b/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.sol index d726ecad4a..89d3822973 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.sol +++ b/packages/ethereum-contracts/contracts/mocks/SuperfluidMock.sol @@ -10,19 +10,14 @@ import { import { CallUtils } from "../utils/CallUtils.sol"; -contract SuperfluidMock is Superfluid { - +contract SuperfluidUpgradabilityTester is Superfluid { - constructor(bool nonUpgradable) - Superfluid(nonUpgradable) + constructor() Superfluid(false) // solhint-disable-next-line no-empty-blocks { } - /** - * Upgradability - */ - + // @dev Make sure the storage layout never change over the course of the development function validateStorageLayout() external pure { @@ -51,6 +46,82 @@ contract SuperfluidMock is Superfluid { require (slot == 6 && offset == 0, "_ctxStamp changed location"); } + // @dev Make sure the context struct layout never change over the course of the development + function validateContextStructLayout() + external pure + { + // context.appLevel + { + Context memory context; + assembly { mstore(add(context, mul(32, 0)), 42) } + require(context.appLevel == 42, "appLevel changed location"); + } + // context.callType + { + Context memory context; + assembly { mstore(add(context, mul(32, 1)), 42) } + require(context.callType == 42, "callType changed location"); + } + // context.timestamp + { + Context memory context; + assembly { mstore(add(context, mul(32, 2)), 42) } + require(context.timestamp == 42, "timestamp changed location"); + } + // context.msgSender + { + Context memory context; + assembly { mstore(add(context, mul(32, 3)), 42) } + require(context.msgSender == address(42), "msgSender changed location"); + } + // context.agreementSelector + { + Context memory context; + // be aware of the bytes4 endianness + assembly { mstore(add(context, mul(32, 4)), shl(224, 0xdeadbeef)) } + require(context.agreementSelector == bytes4(uint32(0xdeadbeef)), "agreementSelector changed location"); + } + // context.userData + { + Context memory context; + context.userData = new bytes(42); + uint256 dataOffset; + assembly { dataOffset := mload(add(context, mul(32, 5))) } + require(dataOffset != 0, "userData offset is zero"); + uint256 dataLen; + assembly { dataLen := mload(dataOffset) } + require(dataLen == 42, "userData changed location"); + } + // context.appAllowanceGranted + { + Context memory context; + assembly { mstore(add(context, mul(32, 6)), 42) } + require(context.appAllowanceGranted == 42, "appAllowanceGranted changed location"); + } + // context.appAllowanceGranted + { + Context memory context; + assembly { mstore(add(context, mul(32, 7)), 42) } + require(context.appAllowanceWanted == 42, "appAllowanceWanted changed location"); + } + // context.appAllowanceGranted + { + Context memory context; + assembly { mstore(add(context, mul(32, 8)), 42) } + require(context.appAllowanceUsed == 42, "appAllowanceUsed changed location"); + } + } +} + +contract SuperfluidMock is Superfluid { + + + constructor(bool nonUpgradable) + Superfluid(nonUpgradable) + // solhint-disable-next-line no-empty-blocks + { + } + function ctxFunc1(uint256 n, bytes calldata ctx) external pure returns (uint256, bytes memory) diff --git a/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.js b/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.js index 357e6aa28f..be2f5a4b82 100644 --- a/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.js +++ b/packages/ethereum-contracts/test/contracts/superfluid/Superfluid.test.js @@ -33,7 +33,9 @@ contract("Superfluid Host Contract", accounts => { describe("#1 upgradability", () => { it("#1.1 storage layout", async () => { - await superfluid.validateStorageLayout.call(); + const T = artifacts.require("SuperfluidUpgradabilityTester"); + const tester = await T.new(); + await tester.validateStorageLayout.call(); }); it("#1.2 proxiable info", async () => { @@ -83,6 +85,12 @@ contract("Superfluid Host Contract", accounts => { "SF: cannot downgrade to non upgradable" ); }); + + it("#1.6 context struct layout", async () => { + const T = artifacts.require("SuperfluidUpgradabilityTester"); + const tester = await T.new(); + await tester.validateContextStructLayout.call(); + }); }); describe("#2 Agreement Whitelisting", async () => { @@ -559,7 +567,7 @@ contract("Superfluid Host Contract", accounts => { }); }); - describe("#6 (WIP) Agreement Framework", () => { + describe("#6 Agreement Framework", () => { let agreement; let app; let gasLimit; @@ -677,7 +685,6 @@ contract("Superfluid Host Contract", accounts => { await expectRevert(mock.tryJailApp(superfluid.address), reason); }); - // TODO decode ctx it("#6.2 beforeAgreementCreated callback noop", async () => { await app.setNextCallbackAction(0 /* noop */, "0x"); const tx = await superfluid.callAgreement( @@ -695,6 +702,13 @@ contract("Superfluid Host Contract", accounts => { agreement.contract, "AppBeforeCallbackResult", { + appLevel: "0", + callType: "1" /* CALL_INFO_CALL_TYPE_AGREEMENT */, + agreementSelector: agreement.abi.filter( + i => + i.name === + "callAppBeforeAgreementCreatedCallback" + )[0].signature, cbdata: "0x" + Buffer.from("Noop").toString("hex") } ); @@ -750,7 +764,6 @@ contract("Superfluid Host Contract", accounts => { ); }); - // TODO decode ctx it("#6.4 afterAgreementCreated callback noop", async () => { await app.setNextCallbackAction(0 /* noop */, "0x"); const tx = await superfluid.callAgreement( @@ -760,10 +773,28 @@ contract("Superfluid Host Contract", accounts => { .encodeABI(), "0x" ); + const agreementSelector = agreement.abi.filter( + i => i.name === "callAppAfterAgreementCreatedCallback" + )[0].signature; await expectEvent.inTransaction( tx.tx, app.contract, - "NoopEvent" + "NoopEvent", + { + appLevel: "1", + callType: "3" /* CALL_INFO_CALL_TYPE_APP_CALLBACK */, + agreementSelector + } + ); + await expectEvent.inTransaction( + tx.tx, + agreement.contract, + "AppAfterCallbackResult", + { + appLevel: "0", + callType: "1" /* CALL_INFO_CALL_TYPE_AGREEMENT */, + agreementSelector + } ); }); @@ -836,10 +867,7 @@ contract("Superfluid Host Contract", accounts => { it("#6.7 beforeAgreementTerminated callback revert jail rule", async () => { await app.setNextCallbackAction(1 /* assert */, "0x"); - const tx = await web3tx( - superfluid.callAgreement, - "callAgreement" - )( + const tx = await superfluid.callAgreement( agreement.address, agreement.contract.methods .callAppBeforeAgreementTerminatedCallback( @@ -863,10 +891,7 @@ contract("Superfluid Host Contract", accounts => { it("#6.8 afterAgreementTerminated callback revert jail rule", async () => { await app.setNextCallbackAction(1 /* assert */, "0x"); - const tx = await web3tx( - superfluid.callAgreement, - "callAgreement" - )( + const tx = await superfluid.callAgreement( agreement.address, agreement.contract.methods .callAppAfterAgreementTerminatedCallback( @@ -890,10 +915,7 @@ contract("Superfluid Host Contract", accounts => { it("#6.9 afterAgreementTerminated callback readonly ctx jail rule", async () => { await app.setNextCallbackAction(4 /* AlteringCtx */, "0x"); - const tx = await web3tx( - superfluid.callAgreement, - "callAgreement" - )( + const tx = await superfluid.callAgreement( agreement.address, agreement.contract.methods .callAppAfterAgreementTerminatedCallback( @@ -918,7 +940,7 @@ contract("Superfluid Host Contract", accounts => { it("#6.11 callback will not be called for jailed apps", async () => { await superfluid.jailApp(app.address); await app.setNextCallbackAction(1 /* assert */, "0x"); - await web3tx(superfluid.callAgreement, "callAgreement")( + await superfluid.callAgreement( agreement.address, agreement.contract.methods .callAppAfterAgreementCreatedCallback(app.address, "0x") @@ -1138,10 +1160,7 @@ contract("Superfluid Host Contract", accounts => { ), "SF: APP_RULE_COMPOSITE_APP_IS_NOT_WHITELISTED" ); - await web3tx( - app3.allowCompositeApp, - "app3.allowCompositeApp(app)" - )(); + await app3.allowCompositeApp(); await expectRevert( superfluid.callAgreement( agreement.address, @@ -1192,10 +1211,7 @@ contract("Superfluid Host Contract", accounts => { it("#6.40 should give explicit error message in non-termination callbacks", async () => { const app2 = await SuperAppMock2.new(superfluid.address); await expectRevert( - web3tx( - superfluid.callAgreement, - "to SuperAppMockReturningEmptyCtx" - )( + superfluid.callAgreement( agreement.address, agreement.contract.methods .callAppAfterAgreementCreatedCallback( @@ -1210,10 +1226,7 @@ contract("Superfluid Host Contract", accounts => { const app3 = await SuperAppMock3.new(superfluid.address); await expectRevert( - web3tx( - superfluid.callAgreement, - "to SuperAppMockReturningInvalidCtx" - )( + superfluid.callAgreement( agreement.address, agreement.contract.methods .callAppAfterAgreementCreatedCallback( @@ -1280,8 +1293,39 @@ contract("Superfluid Host Contract", accounts => { }); }); - // TODO app allowance - // TODO app callback masks + it("#6.50 test noops masks", async () => { + const tests = [ + [0, "callAppBeforeAgreementCreatedCallback"], + [1, "callAppAfterAgreementCreatedCallback"], + [2, "callAppBeforeAgreementUpdatedCallback"], + [3, "callAppAfterAgreementUpdatedCallback"], + [4, "callAppBeforeAgreementTerminatedCallback"], + [5, "callAppAfterAgreementTerminatedCallback"] + ]; + for (let i = 0; i < tests.length; ++i) { + console.log("testing noop mask for", tests[i][1]); + let app2 = await SuperAppMock.new( + superfluid.address, + /* APP_TYPE_FINAL_LEVEL */ + toBN(1).add( + // *_NOOP: 1 << (32 + n) + toBN(1).shln(32 + tests[i][0]) + ), + false + ); + await app2.setNextCallbackAction(1 /* assert */, "0x"); + await superfluid.callAgreement( + agreement.address, + agreement.contract.methods[tests[i][1]]( + app2.address, + "0x" + ).encodeABI(), + "0x" + ); + } + }); + + // TODO app allowance functions }); describe("#7 callAgreement", () => { @@ -1329,10 +1373,10 @@ contract("Superfluid Host Contract", accounts => { web3.utils.sha3("MockAgreement"), 0 ); - await web3tx( - governance.registerAgreementClass, - "Registering mock agreement" - )(superfluid.address, agreement.address); + await governance.registerAgreementClass( + superfluid.address, + agreement.address + ); agreement = await AgreementMock.at( await superfluid.getAgreementClass( web3.utils.sha3("MockAgreement") @@ -1365,7 +1409,6 @@ contract("Superfluid Host Contract", accounts => { ); }); - // TODO decode ctx it("#8.2 actionNoop", async () => { const tx = await superfluid.callAppAction( app.address, @@ -1374,7 +1417,12 @@ contract("Superfluid Host Contract", accounts => { await expectEvent.inTransaction( tx.tx, app.contract, - "NoopEvent" + "NoopEvent", + { + appLevel: "0", + callType: "2" /* CALL_INFO_CALL_TYPE_APP_ACTION */, + agreementSelector: "0x00000000" + } ); }); @@ -1796,9 +1844,7 @@ contract("Superfluid Host Contract", accounts => { [ 202, // call app action app.address, - app.contract.methods - .actionExpectMsgSender(admin, "0x") - .encodeABI() + app.contract.methods.actionNoop("0x").encodeABI() ] ], {