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.

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

feat: add revert gas #280

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
6 changes: 4 additions & 2 deletions contracts/handlers/fee/dynamic/TwapERC20NativeFeeHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
}
}
27 changes: 21 additions & 6 deletions contracts/handlers/fee/dynamic/TwapFeeHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand Down Expand Up @@ -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);
}

/**
Expand Down
8 changes: 6 additions & 2 deletions contracts/handlers/fee/dynamic/TwapNativeTokenFeeHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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();
Expand Down
30 changes: 25 additions & 5 deletions testUnderForked/admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
);
});
Expand Down
3 changes: 2 additions & 1 deletion testUnderForked/calculateFeeERC20EVM.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ contract("TwapFeeHandler - [calculateFee]", async (accounts) => {
DynamicFeeHandlerInstance = await DynamicFeeHandlerContract.new(
BridgeInstance.address,
FeeHandlerRouterInstance.address,
0,
0
);
FeeHandlerRouterInstance.adminSetResourceHandler(
Expand All @@ -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,
Expand Down
7 changes: 4 additions & 3 deletions testUnderForked/collectFeeERC20EVM.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ contract("TwapNativeTokenFeeHandler - [collectFee]", async (accounts) => {
DynamicFeeHandlerInstance = await DynamicFeeHandlerContract.new(
BridgeInstance.address,
FeeHandlerRouterInstance.address,
0,
0
);

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions testUnderForked/collectFeeGenericEVM.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
56 changes: 55 additions & 1 deletion testUnderForked/optionalContractCall/calculateFeeERC20EVM.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -107,6 +108,7 @@ contract("TwapFeeHandler - [calculateFee]", async (accounts) => {
DynamicFeeHandlerInstance = await DynamicFeeHandlerContract.new(
BridgeInstance.address,
FeeHandlerRouterInstance.address,
0,
0
);
FeeHandlerRouterInstance.adminSetResourceHandler(
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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());
});
});
Loading