From 88fcb4639e7818fb92a501e1e4ab43050e760654 Mon Sep 17 00:00:00 2001 From: zajck <64400885+zajck@users.noreply.github.com> Date: Mon, 14 Nov 2022 18:03:51 +0100 Subject: [PATCH] Allowlist metatx methods (#461) --- contracts/domain/BosonConstants.sol | 2 +- .../events/IBosonMetaTransactionsEvents.sol | 2 + .../IBosonMetaTransactionsHandler.sol | 33 +- .../facets/MetaTransactionsHandlerFacet.sol | 72 +++- contracts/protocol/libs/ProtocolLib.sol | 2 + scripts/config/facet-deploy.js | 56 ++- scripts/config/facet-upgrade.js | 20 +- scripts/config/revert-reasons.js | 2 +- scripts/deploy-suite.js | 18 +- scripts/upgrade-facets.js | 19 +- scripts/util/diamond-utils.js | 25 ++ test/protocol/MetaTransactionsHandlerTest.js | 359 +++++++++++++++++- 12 files changed, 539 insertions(+), 71 deletions(-) diff --git a/contracts/domain/BosonConstants.sol b/contracts/domain/BosonConstants.sol index fc016eb82..f8de52e6b 100644 --- a/contracts/domain/BosonConstants.sol +++ b/contracts/domain/BosonConstants.sol @@ -144,10 +144,10 @@ string constant NATIVE_NOT_ALLOWED = "Transfer of native currency not allowed"; // Revert Reasons: Meta-Transactions related string constant NONCE_USED_ALREADY = "Nonce used already"; string constant FUNCTION_CALL_NOT_SUCCESSFUL = "Function call not successful"; -string constant INVALID_FUNCTION_SIGNATURE = "functionSignature can not be of executeMetaTransaction method"; string constant SIGNER_AND_SIGNATURE_DO_NOT_MATCH = "Signer and signature do not match"; string constant INVALID_FUNCTION_NAME = "Invalid function name"; string constant INVALID_SIGNATURE = "Invalid signature"; +string constant FUNCTION_NOT_ALLOWLISTED = "Function can not be executed via meta transaction"; // Revert Reasons: Dispute related string constant DISPUTE_PERIOD_HAS_ELAPSED = "Dispute period has already elapsed"; diff --git a/contracts/interfaces/events/IBosonMetaTransactionsEvents.sol b/contracts/interfaces/events/IBosonMetaTransactionsEvents.sol index c2388a064..b98a4cb12 100644 --- a/contracts/interfaces/events/IBosonMetaTransactionsEvents.sol +++ b/contracts/interfaces/events/IBosonMetaTransactionsEvents.sol @@ -13,4 +13,6 @@ interface IBosonMetaTransactionsEvents { string indexed functionName, uint256 nonce ); + + event FunctionsAllowlisted(bytes32[] functionNameHashes, bool isAllowlisted, address indexed executedBy); } diff --git a/contracts/interfaces/handlers/IBosonMetaTransactionsHandler.sol b/contracts/interfaces/handlers/IBosonMetaTransactionsHandler.sol index 06ee872fb..c6a362995 100644 --- a/contracts/interfaces/handlers/IBosonMetaTransactionsHandler.sol +++ b/contracts/interfaces/handlers/IBosonMetaTransactionsHandler.sol @@ -9,7 +9,7 @@ import { IBosonMetaTransactionsEvents } from "../events/IBosonMetaTransactionsEv * * @notice Manages incoming meta-transactions in the protocol. * - * The ERC-165 identifier for this interface is: 0xd25fcdc1 + * The ERC-165 identifier for this interface is: 0xb3e4e803 */ interface IBosonMetaTransactionsHandler is IBosonMetaTransactionsEvents { /** @@ -27,7 +27,7 @@ interface IBosonMetaTransactionsHandler is IBosonMetaTransactionsEvents { * Reverts if: * - The meta-transactions region of protocol is paused * - Nonce is already used by the msg.sender for another transaction - * - Function signature matches executeMetaTransaction + * - Function is not allowlisted to be called using metatransactions * - Function name does not match the bytes4 version of the function signature * - sender does not match the recovered signer * - Any code executed in the signed transaction reverts @@ -50,4 +50,33 @@ interface IBosonMetaTransactionsHandler is IBosonMetaTransactionsEvents { bytes32 _sigS, uint8 _sigV ) external payable returns (bytes memory); + + /** + * @notice Manages allow list of functions that can be executed using metatransactions. + * + * Emits a FunctionsAllowlisted event if successful. + * + * Reverts if: + * - Caller is not a protocol admin + * + * @param _functionNameHashes - a list of hashed function names (keccak256) + * @param _isAllowlisted - new allowlist status + */ + function setAllowlistedFunctions(bytes32[] calldata _functionNameHashes, bool _isAllowlisted) external; + + /** + * @notice Tells if function can be executed as meta transaction or not. + * + * @param _functionNameHash - hashed function name (keccak256) + * @return isAllowlisted - allowlist status + */ + function isFunctionAllowlisted(bytes32 _functionNameHash) external view returns (bool isAllowlisted); + + /** + * @notice Tells if function can be executed as meta transaction or not. + * + * @param _functionName - function name + * @return isAllowlisted - allowlist status + */ + function isFunctionAllowlisted(string calldata _functionName) external view returns (bool isAllowlisted); } diff --git a/contracts/protocol/facets/MetaTransactionsHandlerFacet.sol b/contracts/protocol/facets/MetaTransactionsHandlerFacet.sol index c58f0b89a..16ca83cdc 100644 --- a/contracts/protocol/facets/MetaTransactionsHandlerFacet.sol +++ b/contracts/protocol/facets/MetaTransactionsHandlerFacet.sol @@ -21,7 +21,10 @@ contract MetaTransactionsHandlerFacet is IBosonMetaTransactionsHandler, Protocol * @notice Initializes Facet. * This function is callable only once. */ - function initialize() public onlyUnInitialized(type(IBosonMetaTransactionsHandler).interfaceId) { + function initialize(bytes32[] calldata _functionNameHashes) + public + onlyUnInitialized(type(IBosonMetaTransactionsHandler).interfaceId) + { DiamondLib.addSupportedInterface(type(IBosonMetaTransactionsHandler).interfaceId); // Set types for special metatxs @@ -47,6 +50,8 @@ contract MetaTransactionsHandlerFacet is IBosonMetaTransactionsHandler, Protocol META_TX_DISPUTE_RESOLUTIONS_TYPEHASH, hashDisputeResolutionDetails ); + + setAllowlistedFunctions(_functionNameHashes, true); } /** @@ -170,7 +175,7 @@ contract MetaTransactionsHandlerFacet is IBosonMetaTransactionsHandler, Protocol * * Reverts if: * - Nonce is already used by the msg.sender for another transaction - * - Function signature matches executeMetaTransaction + * - Function is not allowlisted to be called using metatransactions * - Function name does not match the bytes4 version of the function signature * * @param _functionName - the function name that we want to execute @@ -183,12 +188,18 @@ contract MetaTransactionsHandlerFacet is IBosonMetaTransactionsHandler, Protocol uint256 _nonce, address _userAddress ) internal view { - require(!protocolMetaTxInfo().usedNonce[_userAddress][_nonce], NONCE_USED_ALREADY); + ProtocolLib.ProtocolMetaTxInfo storage pmti = protocolMetaTxInfo(); - bytes4 destinationFunctionSig = convertBytesToBytes4(_functionSignature); - require(destinationFunctionSig != msg.sig, INVALID_FUNCTION_SIGNATURE); + // Nonce should be unused + require(!pmti.usedNonce[_userAddress][_nonce], NONCE_USED_ALREADY); + + // Function must be allowlisted + bytes32 functionNameHash = keccak256(abi.encodePacked(_functionName)); + require(pmti.isAllowlisted[functionNameHash], FUNCTION_NOT_ALLOWLISTED); - bytes4 functionNameSig = bytes4(keccak256(abi.encodePacked(_functionName))); + // Function name must correspond to selector + bytes4 destinationFunctionSig = convertBytesToBytes4(_functionSignature); + bytes4 functionNameSig = bytes4(functionNameHash); require(destinationFunctionSig == functionNameSig, INVALID_FUNCTION_NAME); } @@ -259,7 +270,7 @@ contract MetaTransactionsHandlerFacet is IBosonMetaTransactionsHandler, Protocol * Reverts if: * - The meta-transactions region of protocol is paused * - Nonce is already used by the msg.sender for another transaction - * - Function signature matches executeMetaTransaction + * - Function is not allowlisted to be called using metatransactions * - Function name does not match the bytes4 version of the function signature * - sender does not match the recovered signer * - Any code executed in the signed transaction reverts @@ -305,4 +316,51 @@ contract MetaTransactionsHandlerFacet is IBosonMetaTransactionsHandler, Protocol return executeTx(_userAddress, _functionName, _functionSignature, _nonce); } + + /** + * @notice Manages allow list of functions that can be executed using metatransactions. + * + * Emits a FunctionsAllowlisted event if successful. + * + * Reverts if: + * - Caller is not a protocol admin + * + * @param _functionNameHashes - a list of hashed function names (keccak256) + * @param _isAllowlisted - new allowlist status + */ + function setAllowlistedFunctions(bytes32[] calldata _functionNameHashes, bool _isAllowlisted) + public + override + onlyRole(ADMIN) + { + ProtocolLib.ProtocolMetaTxInfo storage pmti = protocolMetaTxInfo(); + + // set new values + for (uint256 i = 0; i < _functionNameHashes.length; i++) { + pmti.isAllowlisted[_functionNameHashes[i]] = _isAllowlisted; + } + + // Notify external observers + emit FunctionsAllowlisted(_functionNameHashes, _isAllowlisted, msgSender()); + } + + /** + * @notice Tells if function can be executed as meta transaction or not. + * + * @param _functionNameHash - hashed function name (keccak256) + * @return isAllowlisted - allowlist status + */ + function isFunctionAllowlisted(bytes32 _functionNameHash) external view override returns (bool isAllowlisted) { + return protocolMetaTxInfo().isAllowlisted[_functionNameHash]; + } + + /** + * @notice Tells if function can be executed as meta transaction or not. + * + * @param _functionName - function name + * @return isAllowlisted - allowlist status + */ + function isFunctionAllowlisted(string calldata _functionName) external view override returns (bool isAllowlisted) { + return protocolMetaTxInfo().isAllowlisted[keccak256(abi.encodePacked(_functionName))]; + } } diff --git a/contracts/protocol/libs/ProtocolLib.sol b/contracts/protocol/libs/ProtocolLib.sol index aedc45934..8bc364585 100644 --- a/contracts/protocol/libs/ProtocolLib.sol +++ b/contracts/protocol/libs/ProtocolLib.sol @@ -214,6 +214,8 @@ library ProtocolLib { mapping(string => BosonTypes.MetaTxInputType) inputType; // map input type => hash info mapping(BosonTypes.MetaTxInputType => BosonTypes.HashInfo) hashInfo; + // Can function be executed using meta transactions + mapping(bytes32 => bool) isAllowlisted; } // Individual facet initialization states diff --git a/scripts/config/facet-deploy.js b/scripts/config/facet-deploy.js index ce87a5121..a63b6fb01 100644 --- a/scripts/config/facet-deploy.js +++ b/scripts/config/facet-deploy.js @@ -1,6 +1,10 @@ +const { getStateModifyingFunctionsHashes } = require("../../scripts/util/diamond-utils.js"); + /** * Config file used to deploy the facets * + * Function getFacets() returns the object that is used by the deploy script. To specify custom deployment parameters, modify return value. + * Returned value should have the following fields: * - noArgFacets: list of facet names that don't expect any argument passed into initializer * - argFacets: object that specify facet names and arguments that needs to be passed into initializer in format object {facetName: initializerArguments} * @@ -13,23 +17,35 @@ } * */ -module.exports = { - noArgFacets: [ - "AccountHandlerFacet", - "SellerHandlerFacet", - "BuyerHandlerFacet", - "DisputeResolverHandlerFacet", - "AgentHandlerFacet", - "BundleHandlerFacet", - "DisputeHandlerFacet", - "ExchangeHandlerFacet", - "FundsHandlerFacet", - "GroupHandlerFacet", - "OfferHandlerFacet", - "OrchestrationHandlerFacet", - "TwinHandlerFacet", - "PauseHandlerFacet", - "MetaTransactionsHandlerFacet", - ], - argFacets: {}, -}; + +const noArgFacetNames = [ + "AccountHandlerFacet", + "SellerHandlerFacet", + "BuyerHandlerFacet", + "DisputeResolverHandlerFacet", + "AgentHandlerFacet", + "BundleHandlerFacet", + "DisputeHandlerFacet", + "ExchangeHandlerFacet", + "FundsHandlerFacet", + "GroupHandlerFacet", + "OfferHandlerFacet", + "OrchestrationHandlerFacet", + "TwinHandlerFacet", + "PauseHandlerFacet", +]; + +async function getFacets() { + // metaTransactionsHandlerFacet initializer arguments. + const MetaTransactionsHandlerFacetInitArgs = await getStateModifyingFunctionsHashes( + [...noArgFacetNames, "MetaTransactionsHandlerFacet"], + ["executeMetaTransaction(address,string,bytes,uint256,bytes32,bytes32,uint8)"] + ); + + return { + noArgFacets: noArgFacetNames, + argFacets: { MetaTransactionsHandlerFacet: [MetaTransactionsHandlerFacetInitArgs] }, + }; +} + +exports.getFacets = getFacets; diff --git a/scripts/config/facet-upgrade.js b/scripts/config/facet-upgrade.js index 1c322cb6b..2794a5f21 100644 --- a/scripts/config/facet-upgrade.js +++ b/scripts/config/facet-upgrade.js @@ -8,7 +8,7 @@ * Skip does not apply to facets that are completely removed. * - initArgs: if facet initializer expects arguments, provide them here. For no-arg initializers you don't have to specify anything. * - skipInit": list of facets for which you want to skip initialization call. - * + * * Example: { addOrUpgrade: ["Facet1", "Facet2"], @@ -21,10 +21,14 @@ skipInit: ["Facet2"], } */ -exports.Facets = { - addOrUpgrade: [], - remove: [], - skipSelectors: {}, - initArgs: {}, - skipInit: [], -}; +async function getFacets() { + return { + addOrUpgrade: [], + remove: [], + skipSelectors: {}, + initArgs: {}, + skipInit: [], + }; +} + +exports.getFacets = getFacets; diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index b3ba076f1..f1010962f 100644 --- a/scripts/config/revert-reasons.js +++ b/scripts/config/revert-reasons.js @@ -147,10 +147,10 @@ exports.RevertReasons = { // Meta-Transactions related NONCE_USED_ALREADY: "Nonce used already", FUNCTION_CALL_NOT_SUCCESSFUL: "Function call not successful", - INVALID_FUNCTION_SIGNATURE: "functionSignature can not be of executeMetaTransaction method", INVALID_SIGNATURE: "Invalid signature", SIGNER_AND_SIGNATURE_DO_NOT_MATCH: "Signer and signature do not match", INVALID_FUNCTION_NAME: "Invalid function name", + FUNCTION_NOT_ALLOWLISTED: "Function can not be executed via meta transaction", // Dispute related DISPUTE_PERIOD_HAS_ELAPSED: "Dispute period has already elapsed", diff --git a/scripts/deploy-suite.js b/scripts/deploy-suite.js index d6d3c4ce5..59e99a6b6 100644 --- a/scripts/deploy-suite.js +++ b/scripts/deploy-suite.js @@ -9,7 +9,7 @@ const maxPriorityFeePerGas = ethers.BigNumber.from(tipSuggestion).mul(tipMultipl const protocolConfig = require("./config/protocol-parameters"); const authTokenAddresses = require("./config/auth-token-addresses"); -const facets = require("./config/facet-deploy"); +const { getFacets } = require("./config/facet-deploy"); const Role = require("./domain/Role"); const { deployProtocolDiamond } = require("./util/deploy-protocol-diamond.js"); @@ -62,15 +62,15 @@ function getAuthTokenContracts() { /** * Get a list of no-arg initializer facet names to be cut into the Diamond */ -function getNoArgFacetNames() { - return facets.noArgFacets; +async function getNoArgFacetNames() { + return (await getFacets()).noArgFacets; } /** * Get a list of facet names to be cut into the Diamond */ -function getArgFacetNames() { - return facets.argFacets; +async function getArgFacetNames() { + return (await getFacets()).argFacets; } async function main(env) { @@ -148,7 +148,11 @@ async function main(env) { console.log(`\nšŸ’Ž Deploying and initializing protocol handler facets...`); // Deploy and cut facets - const deployedFacets = await deployProtocolHandlerFacets(protocolDiamond, getNoArgFacetNames(), maxPriorityFeePerGas); + const deployedFacets = await deployProtocolHandlerFacets( + protocolDiamond, + await getNoArgFacetNames(), + maxPriorityFeePerGas + ); for (const deployedFacet of deployedFacets) { deploymentComplete( deployedFacet.name, @@ -161,7 +165,7 @@ async function main(env) { const deployedFacetsWithArgs = await deployProtocolHandlerFacetsWithArgs( protocolDiamond, - getArgFacetNames(), + await getArgFacetNames(), maxPriorityFeePerGas ); for (const deployedFacet of deployedFacetsWithArgs) { diff --git a/scripts/upgrade-facets.js b/scripts/upgrade-facets.js index 636fabe74..8d5e2eb16 100644 --- a/scripts/upgrade-facets.js +++ b/scripts/upgrade-facets.js @@ -1,7 +1,7 @@ const hre = require("hardhat"); const ethers = hre.ethers; const network = hre.network.name; -const { Facets } = require("./config/facet-upgrade"); +const { getFacets } = require("./config/facet-upgrade"); const environments = require("../environments"); const confirmations = network == "hardhat" ? 1 : environments.confirmations; const tipMultiplier = ethers.BigNumber.from(environments.tipMultiplier); @@ -111,10 +111,13 @@ async function main(env) { process.exit(1); } + // Get facets to upgrade + const facets = await getFacets(); + // Deploy new facets const deployedFacets = await deployProtocolHandlerFacets( protocolAddress, - Facets.addOrUpgrade, + facets.addOrUpgrade, maxPriorityFeePerGas, false ); @@ -150,14 +153,14 @@ async function main(env) { deploymentComplete(newFacet.name, newFacet.contract.address, [], newFacetInterfaceId, contracts); // Determine calldata. Depends on whether initialize accepts args or not - const callData = Facets.initArgs[newFacet.name] - ? newFacet.contract.interface.encodeFunctionData("initialize", Facets.initArgs[newFacet.name]) + const callData = facets.initArgs[newFacet.name] + ? newFacet.contract.interface.encodeFunctionData("initialize", facets.initArgs[newFacet.name]) : noArgCallData; // Get new selectors from compiled contract const selectors = getSelectors(newFacet.contract, true); let newSelectors; - if (!Facets.skipInit.includes(newFacet.name)) { + if (!facets.skipInit.includes(newFacet.name)) { newSelectors = selectors.selectors.remove([callData.slice(0, 10)]); } else { newSelectors = selectors.selectors; @@ -169,7 +172,7 @@ async function main(env) { let selectorsToAdd = newSelectors.filter((value) => !selectorsToReplace.includes(value)); // unique new selectors // Skip selectors if set in config - let selectorsToSkip = Facets.skipSelectors[newFacet.name] ? Facets.skipSelectors[newFacet.name] : []; + let selectorsToSkip = facets.skipSelectors[newFacet.name] ? facets.skipSelectors[newFacet.name] : []; selectorsToReplace = removeSelectors(selectorsToReplace, selectorsToSkip); selectorsToRemove = removeSelectors(selectorsToRemove, selectorsToSkip); selectorsToAdd = removeSelectors(selectorsToAdd, selectorsToSkip); @@ -204,7 +207,7 @@ async function main(env) { // Diamond cut - add or replace let transactionResponse; - if (Facets.skipInit.includes(newFacet.name)) { + if (facets.skipInit.includes(newFacet.name)) { // Without initialization transactionResponse = await diamondCutFacet .connect(adminSigner) @@ -279,7 +282,7 @@ async function main(env) { } // manage facets that are being completely removed - for (const facetToRemove of Facets.remove) { + for (const facetToRemove of facets.remove) { // Get currently registered selectors const oldFacet = contracts.find((i) => i.name === facetToRemove); diff --git a/scripts/util/diamond-utils.js b/scripts/util/diamond-utils.js index b618991d6..59d0f36e4 100644 --- a/scripts/util/diamond-utils.js +++ b/scripts/util/diamond-utils.js @@ -1,5 +1,7 @@ const hre = require("hardhat"); const ethers = hre.ethers; +const keccak256 = ethers.utils.keccak256; +const toUtf8Bytes = ethers.utils.toUtf8Bytes; /** * Utilities for testing and interacting with Diamond @@ -110,6 +112,27 @@ function getFacetRemoveCut(facet, omitFunctions = []) { return [facet.address, FacetCutAction.Remove, selectors]; } +async function getStateModifyingFunctions(facetNames) { + let stateModifyingFunctions = []; + for (const facetName of facetNames) { + let FacetContractFactory = await ethers.getContractFactory(facetName); + const functions = FacetContractFactory.interface.functions; + const functionNames = Object.keys(functions); + const facetStateModifyingFunctions = functionNames.filter( + (fn) => fn != "initialize()" && functions[fn].stateMutability != "view" + ); + stateModifyingFunctions.push(...facetStateModifyingFunctions); + } + return stateModifyingFunctions; +} + +async function getStateModifyingFunctionsHashes(facetNames, omitFunctions = []) { + // Allowlist contract methods + const stateModifyingFunctions = await getStateModifyingFunctions(facetNames); + const smf = stateModifyingFunctions.filter((fn) => !omitFunctions.includes(fn)); + return smf.map((smf) => keccak256(toUtf8Bytes(smf))); +} + exports.getSelectors = getSelectors; exports.getSelector = getSelector; exports.FacetCutAction = FacetCutAction; @@ -120,3 +143,5 @@ exports.getFacetAddCut = getFacetAddCut; exports.getFacetReplaceCut = getFacetReplaceCut; exports.getFacetRemoveCut = getFacetRemoveCut; exports.getInterfaceId = getInterfaceId; +exports.getStateModifyingFunctions = getStateModifyingFunctions; +exports.getStateModifyingFunctionsHashes = getStateModifyingFunctionsHashes; diff --git a/test/protocol/MetaTransactionsHandlerTest.js b/test/protocol/MetaTransactionsHandlerTest.js index 311dd7b6d..ad8aa6b3a 100644 --- a/test/protocol/MetaTransactionsHandlerTest.js +++ b/test/protocol/MetaTransactionsHandlerTest.js @@ -1,5 +1,7 @@ const hre = require("hardhat"); const ethers = hre.ethers; +const keccak256 = ethers.utils.keccak256; +const toUtf8Bytes = ethers.utils.toUtf8Bytes; const { expect, assert } = require("chai"); const Buyer = require("../../scripts/domain/Buyer"); @@ -12,7 +14,10 @@ const { DisputeResolverFee } = require("../../scripts/domain/DisputeResolverFee" const { getInterfaceIds } = require("../../scripts/config/supported-interfaces.js"); const { RevertReasons } = require("../../scripts/config/revert-reasons.js"); const { deployProtocolDiamond } = require("../../scripts/util/deploy-protocol-diamond.js"); -const { deployProtocolHandlerFacets } = require("../../scripts/util/deploy-protocol-handler-facets.js"); +const { + deployProtocolHandlerFacets, + deployProtocolHandlerFacetsWithArgs, +} = require("../../scripts/util/deploy-protocol-handler-facets.js"); const { deployProtocolConfigFacet } = require("../../scripts/util/deploy-protocol-config-facet.js"); const { deployMockTokens } = require("../../scripts/util/deploy-mock-tokens"); const { prepareDataSignatureParameters, setNextBlockTimestamp } = require("../util/utils.js"); @@ -28,7 +33,12 @@ const { mockExchange, } = require("../util/mock"); const { oneWeek, oneMonth, maxPriorityFeePerGas } = require("../util/constants"); -const { getSelectors, FacetCutAction } = require("../../scripts/util/diamond-utils.js"); +const { + getSelectors, + FacetCutAction, + getStateModifyingFunctions, + getStateModifyingFunctionsHashes, +} = require("../../scripts/util/diamond-utils.js"); /** * Test the Boson Meta transactions Handler interface @@ -99,6 +109,7 @@ describe("IBosonMetaTransactionsHandler", function () { let voucherInitValues; let emptyAuthToken; let agentId; + let stateModifyingFunctionsHashes; before(async function () { // get interface Ids @@ -127,20 +138,28 @@ describe("IBosonMetaTransactionsHandler", function () { await accessController.grantRole(Role.PAUSER, pauser.address); // Cut the protocol handler facets into the Diamond - await deployProtocolHandlerFacets( + const facetNames = [ + "SellerHandlerFacet", + "DisputeResolverHandlerFacet", + "FundsHandlerFacet", + "ExchangeHandlerFacet", + "OfferHandlerFacet", + "TwinHandlerFacet", + "DisputeHandlerFacet", + "PauseHandlerFacet", + "BuyerHandlerFacet", + ]; + + // Get input arguments for MetaTransactionsHandlerFacet + stateModifyingFunctionsHashes = await getStateModifyingFunctionsHashes( + [...facetNames, "MetaTransactionsHandlerFacet"], + ["executeMetaTransaction(address,string,bytes,uint256,bytes32,bytes32,uint8)"] + ); + + await deployProtocolHandlerFacets(protocolDiamond, [...facetNames], maxPriorityFeePerGas); + await deployProtocolHandlerFacetsWithArgs( protocolDiamond, - [ - "SellerHandlerFacet", - "DisputeResolverHandlerFacet", - "FundsHandlerFacet", - "ExchangeHandlerFacet", - "OfferHandlerFacet", - "TwinHandlerFacet", - "DisputeHandlerFacet", - "MetaTransactionsHandlerFacet", - "PauseHandlerFacet", - "BuyerHandlerFacet", - ], + { MetaTransactionsHandlerFacet: [stateModifyingFunctionsHashes] }, maxPriorityFeePerGas ); @@ -376,6 +395,184 @@ describe("IBosonMetaTransactionsHandler", function () { }); }); + context("šŸ‘‰ setAllowlistedFunctions()", async function () { + let functionHashList; + beforeEach(async function () { + // A list of random functions + const functionList = [ + "testFunction1(uint256)", + "testFunction2(uint256)", + "testFunction3((uint256,address,bool))", + "testFunction4(uint256[])", + ]; + + functionHashList = functionList.map((func) => keccak256(toUtf8Bytes(func))); + + // Grant UPGRADER role to admin account + await accessController.grantRole(Role.ADMIN, admin.address); + }); + + it("should emit a FunctionsAllowlisted event", async function () { + // Enable functions + await expect(metaTransactionsHandler.connect(admin).setAllowlistedFunctions(functionHashList, true)) + .to.emit(metaTransactionsHandler, "FunctionsAllowlisted") + .withArgs(functionHashList, true, admin.address); + + // Disable functions + await expect(metaTransactionsHandler.connect(admin).setAllowlistedFunctions(functionHashList, false)) + .to.emit(metaTransactionsHandler, "FunctionsAllowlisted") + .withArgs(functionHashList, false, admin.address); + }); + + it("should update state", async function () { + // Functions should be disabled by default + for (const func of functionHashList) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(bytes32)"](func)).to.be.false; + } + + // Enable functions + await metaTransactionsHandler.connect(admin).setAllowlistedFunctions(functionHashList, true); + + // Functions should be enabled + for (const func of functionHashList) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(bytes32)"](func)).to.be.true; + } + + // Disable functions + await metaTransactionsHandler.connect(admin).setAllowlistedFunctions(functionHashList, false); + + // Functions should be disabled + for (const func of functionHashList) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(bytes32)"](func)).to.be.false; + } + }); + + context("šŸ’” Revert Reasons", async function () { + it("caller is not the admin", async function () { + // Attempt to set new max offer per group, expecting revert + await expect( + metaTransactionsHandler.connect(rando).setAllowlistedFunctions(functionHashList, true) + ).to.revertedWith(RevertReasons.ACCESS_DENIED); + }); + }); + }); + + context("šŸ‘‰ isFunctionAllowlisted(bytes32)", async function () { + let functionHashList; + beforeEach(async function () { + // A list of random functions + const functionList = [ + "testFunction1(uint256)", + "testFunction2(uint256)", + "testFunction3((uint256,address,bool))", + "testFunction4(uint256[])", + ]; + + functionHashList = functionList.map((func) => keccak256(toUtf8Bytes(func))); + + // Grant UPGRADER role to admin account + await accessController.grantRole(Role.ADMIN, admin.address); + }); + + it("after initialization all state modifying functions should be allowlisted", async function () { + // Functions should be enabled + for (const func of stateModifyingFunctionsHashes) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(bytes32)"](func)).to.be.true; + } + }); + + it("should return correct value", async function () { + // Functions should be disabled by default + for (const func of functionHashList) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(bytes32)"](func)).to.be.false; + } + + // Enable functions + await metaTransactionsHandler.connect(admin).setAllowlistedFunctions(functionHashList, true); + + // Functions should be enabled + for (const func of functionHashList) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(bytes32)"](func)).to.be.true; + } + + // Disable functions + await metaTransactionsHandler.connect(admin).setAllowlistedFunctions(functionHashList, false); + + // Functions should be disabled + for (const func of functionHashList) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(bytes32)"](func)).to.be.false; + } + }); + }); + + context("šŸ‘‰ isFunctionAllowlisted(string)", async function () { + let functionList, functionHashList; + beforeEach(async function () { + // A list of random functions + functionList = [ + "testFunction1(uint256)", + "testFunction2(uint256)", + "testFunction3((uint256,address,bool))", + "testFunction4(uint256[])", + ]; + + functionHashList = functionList.map((func) => keccak256(toUtf8Bytes(func))); + + // Grant UPGRADER role to admin account + await accessController.grantRole(Role.ADMIN, admin.address); + }); + + it("after initialization all state modifying functions should be allowlisted", async function () { + // Cut the protocol handler facets into the Diamond + const facetNames = [ + "SellerHandlerFacet", + "DisputeResolverHandlerFacet", + "FundsHandlerFacet", + "ExchangeHandlerFacet", + "OfferHandlerFacet", + "TwinHandlerFacet", + "DisputeHandlerFacet", + "PauseHandlerFacet", + "BuyerHandlerFacet", + ]; + + // Get list of state modifying functions + const stateModifyingFunctions = (await getStateModifyingFunctions(facetNames)).filter( + (fn) => fn != "executeMetaTransaction(address,string,bytes,uint256,bytes32,bytes32,uint8)" + ); + console.log(stateModifyingFunctions); + + // Functions should be enabled + getStateModifyingFunctions(); + for (const func of stateModifyingFunctions) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(string)"](func)).to.be.true; + } + }); + + it("should return correct value", async function () { + // Functions should be disabled by default + for (const func of functionList) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(string)"](func)).to.be.false; + } + + // Enable functions + await metaTransactionsHandler.connect(admin).setAllowlistedFunctions(functionHashList, true); + + // Functions should be enabled + for (const func of functionList) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(string)"](func)).to.be.true; + } + + // Disable functions + await metaTransactionsHandler.connect(admin).setAllowlistedFunctions(functionHashList, false); + + // Functions should be disabled + for (const func of functionList) { + expect(await metaTransactionsHandler["isFunctionAllowlisted(string)"](func)).to.be.false; + } + }); + }); + context("šŸ‘‰ executeMetaTransaction()", async function () { beforeEach(async function () { // Set the message Type @@ -637,8 +834,85 @@ describe("IBosonMetaTransactionsHandler", function () { ).to.revertedWith(RevertReasons.REGION_PAUSED); }); + it("Should fail when function name is not allowlisted", async function () { + // Remove function from allowlist + await metaTransactionsHandler.setAllowlistedFunctions( + [keccak256(toUtf8Bytes(message.functionName))], + false + ); + + // Prepare the function signature for the facet function. + functionSignature = accountHandler.interface.encodeFunctionData("createSeller", [ + seller, + emptyAuthToken, + voucherInitValues, + ]); + + // Prepare the message + message.functionSignature = functionSignature; + + // Collect the signature components + let { r, s, v } = await prepareDataSignatureParameters( + operator, + customTransactionType, + "MetaTransaction", + message, + metaTransactionsHandler.address + ); + + // Execute meta transaction, expecting revert. + await expect( + metaTransactionsHandler.executeMetaTransaction( + operator.address, + message.functionName, + functionSignature, + nonce, + r, + s, + v + ) + ).to.revertedWith(RevertReasons.FUNCTION_NOT_ALLOWLISTED); + }); + + it("Should fail when function name is not allowlisted - incorrect name", async function () { + let incorrectFunctionName = "createSeller"; // function with this name does not exist (argument types are missing) + + // Prepare the function signature for the facet function. + functionSignature = accountHandler.interface.encodeFunctionData("createSeller", [ + seller, + emptyAuthToken, + voucherInitValues, + ]); + + // Prepare the message + message.functionName = incorrectFunctionName; + message.functionSignature = functionSignature; + + // Collect the signature components + let { r, s, v } = await prepareDataSignatureParameters( + operator, + customTransactionType, + "MetaTransaction", + message, + metaTransactionsHandler.address + ); + + // Execute meta transaction, expecting revert. + await expect( + metaTransactionsHandler.executeMetaTransaction( + operator.address, + message.functionName, + functionSignature, + nonce, + r, + s, + v + ) + ).to.revertedWith(RevertReasons.FUNCTION_NOT_ALLOWLISTED); + }); + it("Should fail when function name is incorrect", async function () { - let incorrectFunctionName = "createSeller"; // there are no function argument types here. + let incorrectFunctionName = "redeemVoucher(uint256)"; // function name is allowlisted, but different than what we encode in next step // Prepare the function signature for the facet function. functionSignature = accountHandler.interface.encodeFunctionData("createSeller", [ @@ -674,6 +948,49 @@ describe("IBosonMetaTransactionsHandler", function () { ).to.revertedWith(RevertReasons.INVALID_FUNCTION_NAME); }); + it("Should fail when function name is incorrect, even if selector is correct [collision]", async function () { + // Prepare a function, which selector collide with another funtion selector + // In this case certain bytes are appended to redeemVoucher so it gets the same selector as cancelVoucher + const fn = `redeemVoucher(uint256)`; + const fnBytes = ethers.utils.toUtf8Bytes(fn); + const collisionBytes = "0a7f0f031e"; + const collisionBytesBuffer = Buffer.from(collisionBytes, "hex"); + const fnCollision = Buffer.concat([fnBytes, collisionBytesBuffer]); + const sigCollision = ethers.utils.keccak256(fnCollision).slice(0, 10); + + // Prepare the function signature for the facet function. + functionSignature = exchangeHandler.interface.encodeFunctionData("cancelVoucher", [1]); + + // Make sure that collision actually exists + assert.equal(sigCollision, functionSignature.slice(0, 10)); + + // Prepare the message + message.functionName = fnCollision.toString(); // malicious function name + message.functionSignature = functionSignature; // true function signature + + // Collect the signature components + let { r, s, v } = await prepareDataSignatureParameters( + operator, + customTransactionType, + "MetaTransaction", + message, + metaTransactionsHandler.address + ); + + // Execute meta transaction, expecting revert. + await expect( + metaTransactionsHandler.executeMetaTransaction( + operator.address, + message.functionName, + functionSignature, + nonce, + r, + s, + v + ) + ).to.revertedWith(RevertReasons.FUNCTION_NOT_ALLOWLISTED); + }); + it("Should fail when replaying a transaction", async function () { // Prepare the function signature for the facet function. functionSignature = accountHandler.interface.encodeFunctionData("createSeller", [ @@ -902,6 +1219,14 @@ describe("IBosonMetaTransactionsHandler", function () { }); context("šŸ’” Revert Reasons", async function () { + beforeEach(async function () { + // Prepare the message + message = {}; + message.nonce = parseInt(nonce); + message.from = operator.address; + message.contractAddress = metaTransactionsHandler.address; + }); + it("Should fail when try to call executeMetaTransaction method itself", async function () { // Function signature for executeMetaTransaction function. functionSignature = metaTransactionsHandler.interface.encodeFunctionData("executeMetaTransaction", [ @@ -939,7 +1264,7 @@ describe("IBosonMetaTransactionsHandler", function () { s, v ) - ).to.revertedWith(RevertReasons.INVALID_FUNCTION_SIGNATURE); + ).to.revertedWith(RevertReasons.FUNCTION_NOT_ALLOWLISTED); }); context("Reentrancy guard", async function () {