diff --git a/contracts/handlers/PermissionlessGenericHandler.sol b/contracts/handlers/PermissionlessGenericHandler.sol index c95861be..c69fdba7 100644 --- a/contracts/handlers/PermissionlessGenericHandler.sol +++ b/contracts/handlers/PermissionlessGenericHandler.sol @@ -10,6 +10,8 @@ import "../interfaces/IHandler.sol"; @notice This contract is intended to be used with the Bridge contract. */ contract PermissionlessGenericHandler is IHandler { + uint256 public constant MAX_FEE = 1000000; + address public immutable _bridgeAddress; modifier onlyBridge() { @@ -87,16 +89,19 @@ contract PermissionlessGenericHandler is IHandler { function deposit(bytes32 resourceID, address depositor, bytes calldata data) external view returns (bytes memory) { require(data.length >= 76, "Incorrect data length"); // 32 + 2 + 1 + 1 + 20 + 20 + uint256 maxFee; uint16 lenExecuteFuncSignature; uint8 lenExecuteContractAddress; uint8 lenExecutionDataDepositor; address executionDataDepositor; + maxFee = uint256(bytes32(data[:32])); lenExecuteFuncSignature = uint16(bytes2(data[32:34])); lenExecuteContractAddress = uint8(bytes1(data[34 + lenExecuteFuncSignature:35 + lenExecuteFuncSignature])); lenExecutionDataDepositor = uint8(bytes1(data[35 + lenExecuteFuncSignature + lenExecuteContractAddress:36 + lenExecuteFuncSignature + lenExecuteContractAddress])); executionDataDepositor = address(uint160(bytes20(data[36 + lenExecuteFuncSignature + lenExecuteContractAddress:36 + lenExecuteFuncSignature + lenExecuteContractAddress + lenExecutionDataDepositor]))); + require(maxFee < MAX_FEE, 'requested fee too large'); require(depositor == executionDataDepositor, 'incorrect depositor in deposit data'); } @@ -143,6 +148,7 @@ contract PermissionlessGenericHandler is IHandler { executeFuncSignature(address executionDataDepositor, uint[] uintArray, address addr) */ function executeProposal(bytes32 resourceID, bytes calldata data) external onlyBridge returns (bytes memory) { + uint256 maxFee; uint16 lenExecuteFuncSignature; bytes4 executeFuncSignature; uint8 lenExecuteContractAddress; @@ -151,6 +157,7 @@ contract PermissionlessGenericHandler is IHandler { address executionDataDepositor; bytes memory executionData; + maxFee = uint256(bytes32(data[0:32])); lenExecuteFuncSignature = uint16(bytes2(data[32:34])); executeFuncSignature = bytes4(data[34:34 + lenExecuteFuncSignature]); lenExecuteContractAddress = uint8(bytes1(data[34 + lenExecuteFuncSignature:35 + lenExecuteFuncSignature])); @@ -160,7 +167,7 @@ contract PermissionlessGenericHandler is IHandler { executionData = bytes(data[36 + lenExecuteFuncSignature + lenExecuteContractAddress + lenExecutionDataDepositor:]); bytes memory callData = abi.encodePacked(executeFuncSignature, abi.encode(executionDataDepositor), executionData); - (bool success, bytes memory returndata) = executeContractAddress.call(callData); + (bool success, bytes memory returndata) = executeContractAddress.call{gas: maxFee}(callData); return abi.encode(success, returndata); } } diff --git a/test/handlers/fee/dynamic/calculateFeeGenericEVM.js b/test/handlers/fee/dynamic/calculateFeeGenericEVM.js index 49999f3d..53c5b24b 100644 --- a/test/handlers/fee/dynamic/calculateFeeGenericEVM.js +++ b/test/handlers/fee/dynamic/calculateFeeGenericEVM.js @@ -18,7 +18,7 @@ contract("DynamicGenericFeeHandlerEVM - [calculateFee]", async (accounts) => { const sender = accounts[0]; const depositorAddress = accounts[1]; const emptySetResourceData = "0x"; - const destinationMaxFee = 2000000; + const destinationMaxFee = 900000; const msgGasLimit = 2300000; const ter = 0; // Not used const feeDataAmount = 0; // Not used diff --git a/test/handlers/fee/dynamic/collectFeeGenericEVM.js b/test/handlers/fee/dynamic/collectFeeGenericEVM.js index 5179b778..98076740 100644 --- a/test/handlers/fee/dynamic/collectFeeGenericEVM.js +++ b/test/handlers/fee/dynamic/collectFeeGenericEVM.js @@ -20,7 +20,7 @@ contract("DynamicGenericFeeHandlerEVM - [collectFee]", async (accounts) => { const depositorAddress = accounts[1]; const emptySetResourceData = "0x"; - const destinationMaxFee = 2000000; + const destinationMaxFee = 900000; const msgGasLimit = 2300000; const hashOfTestStore = Ethers.utils.keccak256("0xc0ffee"); const fee = Ethers.utils.parseEther("0.000036777"); diff --git a/test/handlers/generic/permissionlessDeposit.js b/test/handlers/generic/permissionlessDeposit.js index 4fc326aa..e7dc5492 100644 --- a/test/handlers/generic/permissionlessDeposit.js +++ b/test/handlers/generic/permissionlessDeposit.js @@ -20,7 +20,7 @@ contract("PermissionlessGenericHandler - [deposit]", async (accounts) => { const depositorAddress = accounts[1]; const feeData = "0x"; - const destinationMaxFee = 2000000; + const destinationMaxFee = 900000; const hashOfTestStore = Ethers.utils.keccak256("0xc0ffee"); const emptySetResourceData = "0x"; @@ -152,4 +152,27 @@ contract("PermissionlessGenericHandler - [deposit]", async (accounts) => { "incorrect depositor in deposit data" ); }); + + + it("should revert if max fee exceeds 1000000", async () => { + const invalidMaxFee = 1000001; + const invalidDepositData = Helpers.createPermissionlessGenericDepositData( + depositFunctionSignature, + TestStoreInstance.address, + invalidMaxFee , + depositorAddress, + hashOfTestStore + ); + + await TruffleAssert.reverts( + BridgeInstance.deposit( + destinationDomainID, + resourceID, + invalidDepositData, + feeData, + {from: depositorAddress} + ), + "requested fee too large" + ); + }); }); diff --git a/test/handlers/generic/permissionlessExecuteProposal.js b/test/handlers/generic/permissionlessExecuteProposal.js index f6cfc380..30e2e62d 100644 --- a/test/handlers/generic/permissionlessExecuteProposal.js +++ b/test/handlers/generic/permissionlessExecuteProposal.js @@ -24,7 +24,7 @@ contract( const invalidExecutionContractAddress = accounts[4]; const feeData = "0x"; - const destinationMaxFee = 2000000; + const destinationMaxFee = 900000; const hashOfTestStore = Ethers.utils.keccak256("0xc0ffee"); const handlerResponseLength = 64; const contractCallReturndata = Ethers.constants.HashZero; @@ -226,6 +226,74 @@ contract( ); }); + it("ProposalExecution should be emitted even if gas specified too small", async () => { + const num = 6; + const addresses = [BridgeInstance.address, TestStoreInstance.address]; + const message = Ethers.utils.hexlify(Ethers.utils.toUtf8Bytes("message")); + const executionData = Helpers.abiEncode(["uint", "address[]", "bytes"], [num, addresses, message]); + + // If the target function accepts (address depositor, bytes executionData) + // then this helper can be used + const preparedExecutionData = await TestDepositInstance.prepareDepositData(executionData); + const depositFunctionSignature = Helpers.getFunctionSignature( + TestDepositInstance, + "executePacked" + ); + const tooSmallGas = 500; + const depositData = Helpers.createPermissionlessGenericDepositData( + depositFunctionSignature, + TestDepositInstance.address, + tooSmallGas, + depositorAddress, + preparedExecutionData + ); + const proposal = { + originDomainID: originDomainID, + depositNonce: expectedDepositNonce, + data: depositData, + resourceID: resourceID, + }; + const proposalSignedData = await Helpers.signTypedProposal( + BridgeInstance.address, + [proposal] + ); + + // relayer1 executes the proposal + const executeTx = await BridgeInstance.executeProposal( + proposal, + proposalSignedData, + {from: relayer1Address} + ); + // check that ProposalExecution event is emitted + TruffleAssert.eventEmitted(executeTx, "ProposalExecution", (event) => { + return ( + event.originDomainID.toNumber() === originDomainID && + event.depositNonce.toNumber() === expectedDepositNonce + ); + }); + + // check that deposit nonce isn't unmarked as used in bitmap + assert.isTrue( + await BridgeInstance.isProposalExecuted( + originDomainID, + expectedDepositNonce + ) + ); + + const internalTx = await TruffleAssert.createTransactionResult( + TestDepositInstance, + executeTx.tx + ); + TruffleAssert.eventNotEmitted(internalTx, "TestExecute", (event) => { + return ( + event.depositor === depositorAddress && + event.num.toNumber() === num && + event.addr === TestStoreInstance.address && + event.message === message + ); + }); + }); + it("call with packed depositData should be successful", async () => { const num = 5; const addresses = [BridgeInstance.address, TestStoreInstance.address];