diff --git a/Makefile b/Makefile index 0c6358fb..6fc807c0 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ start-forkedMainnet: test-forked: @echo " > \033[32mTesting contracts... \033[0m " - truffle test --stacktrace testUnderForked/* + npx truffle test --stacktrace testUnderForked/* start-geth: @echo " > \033[32mStarting geth... \033[0m " diff --git a/contracts/handlers/fee/dynamic/TwapERC20NativeFeeHandler.sol b/contracts/handlers/fee/dynamic/TwapERC20NativeFeeHandler.sol index 9f0aeaa2..f5915fe2 100644 --- a/contracts/handlers/fee/dynamic/TwapERC20NativeFeeHandler.sol +++ b/contracts/handlers/fee/dynamic/TwapERC20NativeFeeHandler.sol @@ -15,11 +15,13 @@ contract TwapERC20NativeFeeHandler is TwapNativeTokenFeeHandler { @param bridgeAddress Contract address of previously deployed Bridge. @param feeHandlerRouterAddress Contract address of previously deployed FeeHandlerRouter. @param gasUsed Default gas used for proposal execution in the destination. + @param recoverGas Gas used for an optional call fallback. */ constructor( address bridgeAddress, address feeHandlerRouterAddress, - uint32 gasUsed - ) TwapNativeTokenFeeHandler(bridgeAddress, feeHandlerRouterAddress, gasUsed) { + uint32 gasUsed, + uint32 recoverGas + ) TwapNativeTokenFeeHandler(bridgeAddress, feeHandlerRouterAddress, gasUsed, recoverGas) { } } diff --git a/contracts/handlers/fee/dynamic/TwapFeeHandler.sol b/contracts/handlers/fee/dynamic/TwapFeeHandler.sol index b15d31da..7ed418f2 100644 --- a/contracts/handlers/fee/dynamic/TwapFeeHandler.sol +++ b/contracts/handlers/fee/dynamic/TwapFeeHandler.sol @@ -26,6 +26,7 @@ abstract contract TwapFeeHandler is IFeeHandler, AccessControl { ProtocolFeeType public protocolFeeType; uint32 public _gasUsed; + uint32 public _recoverGas; mapping(uint8 => address) public destinationNativeCoinWrap; mapping(uint8 => Fee) public destinationFee; @@ -43,7 +44,8 @@ abstract contract TwapFeeHandler is IFeeHandler, AccessControl { } event FeeOracleAddressSet(TwapOracle feeOracleAddress); - event FeePropertySet(uint32 gasUsed); + event GasUsedSet(uint32 gasUsed); + event RecoverGasSet(uint32 recoverGas); event GasPriceSet(uint8 destinationDomainID, uint256 gasPrice); event WrapTokenAddressSet(uint8 destinationDomainID, address wrapTokenAddress); @@ -136,16 +138,29 @@ abstract contract TwapFeeHandler is IFeeHandler, AccessControl { } /** - @notice Sets the fee properties. + @notice Sets the recover gas property. + @param recoverGas Gas used for an optional call fallback. + */ + function setRecoverGas(uint32 recoverGas) external onlyAdmin { + _setRecoverGas(recoverGas); + } + + function _setRecoverGas(uint32 recoverGas) internal { + _recoverGas = recoverGas; + emit RecoverGasSet(recoverGas); + } + + /** + @notice Sets the gas used property. @param gasUsed Default gas used for proposal execution in the destination. */ - function setFeeProperties(uint32 gasUsed) external onlyAdmin { - _setFeeProperties(gasUsed); + function setGasUsed(uint32 gasUsed) external onlyAdmin { + _setGasUsed(gasUsed); } - function _setFeeProperties(uint32 gasUsed) internal { + function _setGasUsed(uint32 gasUsed) internal { _gasUsed = gasUsed; - emit FeePropertySet(gasUsed); + emit GasUsedSet(gasUsed); } /** diff --git a/contracts/handlers/fee/dynamic/TwapNativeTokenFeeHandler.sol b/contracts/handlers/fee/dynamic/TwapNativeTokenFeeHandler.sol index 0a5fa34c..73d0ebf7 100644 --- a/contracts/handlers/fee/dynamic/TwapNativeTokenFeeHandler.sol +++ b/contracts/handlers/fee/dynamic/TwapNativeTokenFeeHandler.sol @@ -15,13 +15,16 @@ contract TwapNativeTokenFeeHandler is TwapFeeHandler { @param bridgeAddress Contract address of previously deployed Bridge. @param feeHandlerRouterAddress Contract address of previously deployed FeeHandlerRouter. @param gasUsed Default gas used for proposal execution in the destination. + @param recoverGas Gas used for an optional call fallback. */ constructor( address bridgeAddress, address feeHandlerRouterAddress, - uint32 gasUsed + uint32 gasUsed, + uint32 recoverGas ) TwapFeeHandler(bridgeAddress, feeHandlerRouterAddress) { - _setFeeProperties(gasUsed); + _setGasUsed(gasUsed); + _setRecoverGas(recoverGas); } /** @@ -38,6 +41,7 @@ contract TwapNativeTokenFeeHandler is TwapFeeHandler { if (depositData.length > (pointer + 64)) { uint256 gas = abi.decode(depositData[pointer:], (uint256)); maxFee += gas; + maxFee += uint256(_recoverGas); } uint256 destinationCoinPrice = twapOracle.getPrice(destinationNativeCoinWrap[destinationDomainID]); if (destinationCoinPrice == 0) revert IncorrectPrice(); diff --git a/testUnderForked/admin.js b/testUnderForked/admin.js index a3e05149..17f9ef92 100644 --- a/testUnderForked/admin.js +++ b/testUnderForked/admin.js @@ -87,6 +87,7 @@ contract("TwapFeeHandler - [admin]", async (accounts) => { DynamicFeeHandlerInstance = await DynamicFeeHandlerContract.new( BridgeInstance.address, FeeHandlerRouterInstance.address, + 0, 0 ); ADMIN_ROLE = await DynamicFeeHandlerInstance.DEFAULT_ADMIN_ROLE(); @@ -119,21 +120,40 @@ contract("TwapFeeHandler - [admin]", async (accounts) => { ); }); - it("should set fee properties and emit 'FeePropertySet' event", async () => { + it("should set gas used property and emit 'GasUsedSet' event", async () => { assert.equal(await DynamicFeeHandlerInstance._gasUsed.call(), "0"); - const setFeeOraclePropertiesTx = await DynamicFeeHandlerInstance.setFeeProperties(gasUsed); + const setFeeOraclePropertiesTx = await DynamicFeeHandlerInstance.setGasUsed(gasUsed); assert.equal(await DynamicFeeHandlerInstance._gasUsed.call(), gasUsed); - TruffleAssert.eventEmitted(setFeeOraclePropertiesTx, "FeePropertySet", (event) => { + TruffleAssert.eventEmitted(setFeeOraclePropertiesTx, "GasUsedSet", (event) => { return ( event.gasUsed.toNumber() === gasUsed ); }); }); - it("should require admin role to change fee properties", async () => { + it("should require admin role to set gas used", async () => { await assertOnlyAdmin( - DynamicFeeHandlerInstance.setFeeProperties, + DynamicFeeHandlerInstance.setGasUsed, + gasUsed + ); + }); + + it("should set revert gas property and emit 'RecoverGasSet' event", async () => { + assert.equal(await DynamicFeeHandlerInstance._recoverGas.call(), "0"); + const setRecoverGasTx = await DynamicFeeHandlerInstance.setRecoverGas(gasUsed); + assert.equal(await DynamicFeeHandlerInstance._recoverGas.call(), gasUsed); + + TruffleAssert.eventEmitted(setRecoverGasTx, "RecoverGasSet", (event) => { + return ( + event.recoverGas.toNumber() === gasUsed + ); + }); + }); + + it("should require admin role to set recover gas", async () => { + await assertOnlyAdmin( + DynamicFeeHandlerInstance.setRecoverGas, gasUsed ); }); diff --git a/testUnderForked/calculateFeeERC20EVM.js b/testUnderForked/calculateFeeERC20EVM.js index 42628b69..2d725f72 100644 --- a/testUnderForked/calculateFeeERC20EVM.js +++ b/testUnderForked/calculateFeeERC20EVM.js @@ -102,6 +102,7 @@ contract("TwapFeeHandler - [calculateFee]", async (accounts) => { DynamicFeeHandlerInstance = await DynamicFeeHandlerContract.new( BridgeInstance.address, FeeHandlerRouterInstance.address, + 0, 0 ); FeeHandlerRouterInstance.adminSetResourceHandler( @@ -117,7 +118,7 @@ contract("TwapFeeHandler - [calculateFee]", async (accounts) => { fixedProtocolFee ); await DynamicFeeHandlerInstance.setWrapTokenAddress(destinationDomainID, MATIC_ADDRESS); - await DynamicFeeHandlerInstance.setFeeProperties(gasUsed); + await DynamicFeeHandlerInstance.setGasUsed(gasUsed); depositData = Helpers.createERCDepositData( 100, diff --git a/testUnderForked/collectFeeERC20EVM.js b/testUnderForked/collectFeeERC20EVM.js index 35e971b5..208bf582 100644 --- a/testUnderForked/collectFeeERC20EVM.js +++ b/testUnderForked/collectFeeERC20EVM.js @@ -81,6 +81,7 @@ contract("TwapNativeTokenFeeHandler - [collectFee]", async (accounts) => { DynamicFeeHandlerInstance = await DynamicFeeHandlerContract.new( BridgeInstance.address, FeeHandlerRouterInstance.address, + 0, 0 ); @@ -127,7 +128,7 @@ contract("TwapNativeTokenFeeHandler - [collectFee]", async (accounts) => { fixedProtocolFee ); await DynamicFeeHandlerInstance.setWrapTokenAddress(destinationDomainID, MATIC_ADDRESS); - await DynamicFeeHandlerInstance.setFeeProperties(gasUsed); + await DynamicFeeHandlerInstance.setGasUsed(gasUsed); await BridgeInstance.adminSetResource( ERC20HandlerInstance.address, @@ -221,7 +222,7 @@ contract("TwapNativeTokenFeeHandler - [collectFee]", async (accounts) => { const fee = Ethers.BigNumber.from(expectedFee.toString()).div(2); const errorValues = await Helpers.expectToRevertWithCustomError( - BridgeInstance.deposit( + BridgeInstance.deposit.call( destinationDomainID, resourceID, depositData, @@ -242,7 +243,7 @@ contract("TwapNativeTokenFeeHandler - [collectFee]", async (accounts) => { await TwapOracleInstance.setPrice(MATIC_ADDRESS, 0); await Helpers.expectToRevertWithCustomError( - BridgeInstance.deposit( + BridgeInstance.deposit.call( destinationDomainID, resourceID, depositData, diff --git a/testUnderForked/collectFeeGenericEVM.js b/testUnderForked/collectFeeGenericEVM.js index 65046eb7..e049a9bf 100644 --- a/testUnderForked/collectFeeGenericEVM.js +++ b/testUnderForked/collectFeeGenericEVM.js @@ -217,7 +217,7 @@ contract("TwapGenericFeeHandler - [collectFee]", async (accounts) => { const fee = Ethers.BigNumber.from(expectedFee.toString()).div(2); const errorValues = await Helpers.expectToRevertWithCustomError( - BridgeInstance.deposit( + BridgeInstance.deposit.call( destinationDomainID, resourceID, depositData, @@ -238,7 +238,7 @@ contract("TwapGenericFeeHandler - [collectFee]", async (accounts) => { await TwapOracleInstance.setPrice(MATIC_ADDRESS, 0); await Helpers.expectToRevertWithCustomError( - BridgeInstance.deposit( + BridgeInstance.deposit.call( destinationDomainID, resourceID, depositData, diff --git a/testUnderForked/optionalContractCall/calculateFeeERC20EVM.js b/testUnderForked/optionalContractCall/calculateFeeERC20EVM.js index 55e1648f..11551013 100644 --- a/testUnderForked/optionalContractCall/calculateFeeERC20EVM.js +++ b/testUnderForked/optionalContractCall/calculateFeeERC20EVM.js @@ -49,6 +49,7 @@ contract("TwapFeeHandler - [calculateFee]", async (accounts) => { const higherExecutionGasAmount = 30000000; const lowerExecutionGasAmount = 3000000; const feeData = "0x"; + const feePercentage = 1000; // 10% let UniswapFactoryInstance; let TwapOracleInstance; @@ -107,6 +108,7 @@ contract("TwapFeeHandler - [calculateFee]", async (accounts) => { DynamicFeeHandlerInstance = await DynamicFeeHandlerContract.new( BridgeInstance.address, FeeHandlerRouterInstance.address, + 0, 0 ); FeeHandlerRouterInstance.adminSetResourceHandler( @@ -122,7 +124,7 @@ contract("TwapFeeHandler - [calculateFee]", async (accounts) => { fixedProtocolFee ); await DynamicFeeHandlerInstance.setWrapTokenAddress(destinationDomainID, MATIC_ADDRESS); - await DynamicFeeHandlerInstance.setFeeProperties(gasUsed); + await DynamicFeeHandlerInstance.setGasUsed(gasUsed); }); it("should return higher fee for higher execution amount", async () => { @@ -175,4 +177,56 @@ contract("TwapFeeHandler - [calculateFee]", async (accounts) => { expect(higherExecutionGasAmountRes.fee.toNumber()).to.be.gt(lowerExecutionGasAmountRes.fee.toNumber()) }); + + it("[percentage protocol fee] should calculate in recover gas for tx cost", async () => { + const mintableERC20Iface = new Ethers.utils.Interface(["function mint(address to, uint256 amount)"]); + const actions = [{ + nativeValue: 0, + callTo: ERC20MintableInstance.address, + approveTo: Ethers.constants.AddressZero, + tokenSend: Ethers.constants.AddressZero, + tokenReceive: Ethers.constants.AddressZero, + data: mintableERC20Iface.encodeFunctionData("mint", [recipientAddress, "20"]), + }] + const message = Helpers.createMessageCallData( + transactionId, + actions, + recipientAddress + ); + + const depositData = Helpers.createOptionalContractCallDepositData( + transferredAmount, + Ethers.constants.AddressZero, + higherExecutionGasAmount, + message + ); + + await DynamicFeeHandlerInstance.setGasPrice( + destinationDomainID, + gasPrice, // Polygon gas price is 200 Gwei + ProtocolFeeType.Percentage, + feePercentage + ); + + const resWithoutRecoverGas = await FeeHandlerRouterInstance.calculateFee.call( + sender, + originDomainID, + destinationDomainID, + resourceID, + depositData, + "0x00" + ); + + await DynamicFeeHandlerInstance.setRecoverGas(gasUsed); + + const resWithRecoverGas = await FeeHandlerRouterInstance.calculateFee.call( + sender, + originDomainID, + destinationDomainID, + resourceID, + depositData, + "0x00" + ); + expect(resWithRecoverGas.fee.toNumber()).to.be.gt(resWithoutRecoverGas.fee.toNumber()); + }); });