From d09d1f753d45674c94f8211419283f1602cf82d3 Mon Sep 17 00:00:00 2001 From: mpetrun5 Date: Wed, 27 Sep 2023 17:25:16 +0200 Subject: [PATCH 1/4] Implement fee whitelist --- contracts/handlers/FeeHandlerRouter.sol | 22 ++++ test/handlers/fee/handlerRouter.js | 154 ++++++++++++++++++++++-- 2 files changed, 167 insertions(+), 9 deletions(-) diff --git a/contracts/handlers/FeeHandlerRouter.sol b/contracts/handlers/FeeHandlerRouter.sol index e0e41eef..7d3fe74e 100644 --- a/contracts/handlers/FeeHandlerRouter.sol +++ b/contracts/handlers/FeeHandlerRouter.sol @@ -15,11 +15,15 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl { // destination domainID => resourceID => feeHandlerAddress mapping (uint8 => mapping(bytes32 => IFeeHandler)) public _domainResourceIDToFeeHandlerAddress; + // whitelisted address => is whitelisted + mapping(address => bool) public _whitelist; event FeeChanged( uint256 newFee ); + error IncorrectFeeSupplied(uint256); + modifier onlyBridge() { _onlyBridge(); _; @@ -55,6 +59,15 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl { _domainResourceIDToFeeHandlerAddress[destinationDomainID][resourceID] = handlerAddress; } + /** + @notice Sets or revokes fee whitelist from an address. + @param whitelistAddress Address to be whitelisted. + @param isWhitelisted Set to true to exempt an address from paying fees. + */ + function adminSetWhitelist(address whitelistAddress, bool isWhitelisted) external onlyAdmin { + _whitelist[whitelistAddress] = isWhitelisted; + } + /** @notice Initiates collecting fee with corresponding fee handler contract using IFeeHandler interface. @@ -66,6 +79,11 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl { @param feeData Additional data to be passed to the fee handler. */ function collectFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) payable external onlyBridge { + if (_whitelist[sender]) { + if (msg.value != 0) revert IncorrectFeeSupplied(msg.value); + return; + } + IFeeHandler feeHandler = _domainResourceIDToFeeHandlerAddress[destinationDomainID][resourceID]; feeHandler.collectFee{value: msg.value}(sender, fromDomainID, destinationDomainID, resourceID, depositData, feeData); } @@ -82,6 +100,10 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl { @return tokenAddress Returns the address of the token to be used for fee. */ function calculateFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) external view returns(uint256 fee, address tokenAddress) { + if (_whitelist[sender]) { + return (0, address(0)); + } + IFeeHandler feeHandler = _domainResourceIDToFeeHandlerAddress[destinationDomainID][resourceID]; return feeHandler.calculateFee(sender, fromDomainID, destinationDomainID, resourceID, depositData, feeData); } diff --git a/test/handlers/fee/handlerRouter.js b/test/handlers/fee/handlerRouter.js index 8c26a410..97012cc4 100644 --- a/test/handlers/fee/handlerRouter.js +++ b/test/handlers/fee/handlerRouter.js @@ -1,18 +1,24 @@ // The Licensed Work is (c) 2022 Sygma // SPDX-License-Identifier: LGPL-3.0-only +const Ethers = require("ethers"); + const TruffleAssert = require("truffle-assertions"); const Helpers = require("../../helpers"); -const DynamicFeeHandlerContract = artifacts.require("DynamicERC20FeeHandlerEVM"); +const BasicFeeHandlerContract = artifacts.require("BasicFeeHandler"); const FeeHandlerRouterContract = artifacts.require("FeeHandlerRouter"); const ERC20MintableContract = artifacts.require("ERC20PresetMinterPauser"); contract("FeeHandlerRouter", async (accounts) => { const originDomainID = 1; const destinationDomainID = 2; + const feeData = "0x0"; const nonAdmin = accounts[1]; + const whitelistAddress = accounts[2]; + const nonWhitelistAddress = accounts[3]; + const bridgeAddress = accounts[4]; const assertOnlyAdmin = (method, ...params) => { return TruffleAssert.reverts( @@ -21,27 +27,23 @@ contract("FeeHandlerRouter", async (accounts) => { ); }; - let BridgeInstance; let FeeHandlerRouterInstance; + let BasicFeeHandlerInstance; let ERC20MintableInstance; let resourceID; beforeEach(async () => { await Promise.all([ - (BridgeInstance = await Helpers.deployBridge( - destinationDomainID, - accounts[0] - )), ERC20MintableContract.new("token", "TOK").then( (instance) => (ERC20MintableInstance = instance) ), ]); FeeHandlerRouterInstance = await FeeHandlerRouterContract.new( - BridgeInstance.address + bridgeAddress ); - DynamicFeeHandlerInstance = await DynamicFeeHandlerContract.new( - BridgeInstance.address, + BasicFeeHandlerInstance = await BasicFeeHandlerContract.new( + bridgeAddress, FeeHandlerRouterInstance.address ); @@ -82,4 +84,138 @@ contract("FeeHandlerRouter", async (accounts) => { feeHandlerAddress ); }); + + it("should successfully set whitelist on an address", async () => { + assert.equal( + await FeeHandlerRouterInstance._whitelist.call( + whitelistAddress + ), + false + ); + + await FeeHandlerRouterInstance.adminSetWhitelist( + whitelistAddress, + true + ); + assert.equal( + await FeeHandlerRouterInstance._whitelist.call( + whitelistAddress + ), + true + ); + }); + + it("should require admin role to set whitelist address", async () => { + await assertOnlyAdmin( + FeeHandlerRouterInstance.adminSetWhitelist, + whitelistAddress, + true + ); + }); + + it("should return fee 0 if address whitelisted", async () => { + await FeeHandlerRouterInstance.adminSetWhitelist( + whitelistAddress, + true + ); + await FeeHandlerRouterInstance.adminSetResourceHandler( + destinationDomainID, + resourceID, + BasicFeeHandlerInstance.address + ); + await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5")); + + const depositData = Helpers.createERCDepositData(100, 20, accounts[4]); + let res = await FeeHandlerRouterInstance.calculateFee.call( + whitelistAddress, + originDomainID, + destinationDomainID, + resourceID, + depositData, + feeData + ); + assert.equal(web3.utils.fromWei(res[0], "ether"), "0") + res = await FeeHandlerRouterInstance.calculateFee.call( + nonWhitelistAddress, + originDomainID, + destinationDomainID, + resourceID, + depositData, + feeData + ); + assert.equal(web3.utils.fromWei(res[0], "ether"), "0.5") + }); + + it("should revert if whitelisted address provides fee", async () => { + await FeeHandlerRouterInstance.adminSetWhitelist( + whitelistAddress, + true + ); + await FeeHandlerRouterInstance.adminSetResourceHandler( + destinationDomainID, + resourceID, + BasicFeeHandlerInstance.address + ); + await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5")); + + const depositData = Helpers.createERCDepositData(100, 20, accounts[4]); + await Helpers.expectToRevertWithCustomError( + FeeHandlerRouterInstance.collectFee( + whitelistAddress, + originDomainID, + destinationDomainID, + resourceID, + depositData, + feeData, + { + from: bridgeAddress, + value: Ethers.utils.parseEther("0.5").toString() + } + ), + "IncorrectFeeSupplied(uint256)" + ); + await TruffleAssert.passes( + FeeHandlerRouterInstance.collectFee( + nonWhitelistAddress, + originDomainID, + destinationDomainID, + resourceID, + depositData, + feeData, + { + from: bridgeAddress, + value: Ethers.utils.parseEther("0.5").toString() + } + ), + ); + }); + + it("should not collect fee from whitelisted address", async () => { + await FeeHandlerRouterInstance.adminSetWhitelist( + whitelistAddress, + true + ); + await FeeHandlerRouterInstance.adminSetResourceHandler( + destinationDomainID, + resourceID, + BasicFeeHandlerInstance.address + ); + await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5")); + + const depositData = Helpers.createERCDepositData(100, 20, accounts[4]); + await TruffleAssert.passes( + FeeHandlerRouterInstance.collectFee( + whitelistAddress, + originDomainID, + destinationDomainID, + resourceID, + depositData, + feeData, + { + from: bridgeAddress, + value: "0" + } + ), + ); + }); }); From d84b163e57693b2f459ced0686e662d26aaf9d9b Mon Sep 17 00:00:00 2001 From: mpetrun5 Date: Wed, 27 Sep 2023 17:35:31 +0200 Subject: [PATCH 2/4] Reuse recipient address --- test/handlers/fee/handlerRouter.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/handlers/fee/handlerRouter.js b/test/handlers/fee/handlerRouter.js index 97012cc4..1e325975 100644 --- a/test/handlers/fee/handlerRouter.js +++ b/test/handlers/fee/handlerRouter.js @@ -19,6 +19,7 @@ contract("FeeHandlerRouter", async (accounts) => { const whitelistAddress = accounts[2]; const nonWhitelistAddress = accounts[3]; const bridgeAddress = accounts[4]; + const recipient = accounts[5]; const assertOnlyAdmin = (method, ...params) => { return TruffleAssert.reverts( @@ -125,7 +126,7 @@ contract("FeeHandlerRouter", async (accounts) => { ); await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5")); - const depositData = Helpers.createERCDepositData(100, 20, accounts[4]); + const depositData = Helpers.createERCDepositData(100, 20, recipient); let res = await FeeHandlerRouterInstance.calculateFee.call( whitelistAddress, originDomainID, @@ -158,7 +159,7 @@ contract("FeeHandlerRouter", async (accounts) => { ); await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5")); - const depositData = Helpers.createERCDepositData(100, 20, accounts[4]); + const depositData = Helpers.createERCDepositData(100, 20, recipient); await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.collectFee( whitelistAddress, @@ -202,7 +203,7 @@ contract("FeeHandlerRouter", async (accounts) => { ); await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5")); - const depositData = Helpers.createERCDepositData(100, 20, accounts[4]); + const depositData = Helpers.createERCDepositData(100, 20, recipient); await TruffleAssert.passes( FeeHandlerRouterInstance.collectFee( whitelistAddress, From d2a1ef929c9743f34e93c37515a68fbe0d6dd7ef Mon Sep 17 00:00:00 2001 From: mpetrun5 Date: Wed, 27 Sep 2023 18:04:08 +0200 Subject: [PATCH 3/4] Fix tests --- test/handlers/fee/handlerRouter.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/handlers/fee/handlerRouter.js b/test/handlers/fee/handlerRouter.js index 1e325975..e05d045a 100644 --- a/test/handlers/fee/handlerRouter.js +++ b/test/handlers/fee/handlerRouter.js @@ -18,8 +18,8 @@ contract("FeeHandlerRouter", async (accounts) => { const nonAdmin = accounts[1]; const whitelistAddress = accounts[2]; const nonWhitelistAddress = accounts[3]; + const recipientAddress = accounts[3]; const bridgeAddress = accounts[4]; - const recipient = accounts[5]; const assertOnlyAdmin = (method, ...params) => { return TruffleAssert.reverts( @@ -126,7 +126,7 @@ contract("FeeHandlerRouter", async (accounts) => { ); await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5")); - const depositData = Helpers.createERCDepositData(100, 20, recipient); + const depositData = Helpers.createERCDepositData(100, 20, recipientAddress); let res = await FeeHandlerRouterInstance.calculateFee.call( whitelistAddress, originDomainID, @@ -159,7 +159,7 @@ contract("FeeHandlerRouter", async (accounts) => { ); await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5")); - const depositData = Helpers.createERCDepositData(100, 20, recipient); + const depositData = Helpers.createERCDepositData(100, 20, recipientAddress); await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.collectFee( whitelistAddress, @@ -203,7 +203,7 @@ contract("FeeHandlerRouter", async (accounts) => { ); await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5")); - const depositData = Helpers.createERCDepositData(100, 20, recipient); + const depositData = Helpers.createERCDepositData(100, 20, recipientAddress); await TruffleAssert.passes( FeeHandlerRouterInstance.collectFee( whitelistAddress, From 996dc89e52db4efe2ee0c84aa13a20e91b70a90e Mon Sep 17 00:00:00 2001 From: mpetrun5 Date: Thu, 28 Sep 2023 16:40:22 +0200 Subject: [PATCH 4/4] Emit event when whitelist changed --- contracts/handlers/FeeHandlerRouter.sol | 7 +++++-- test/feeRouter/feeRouter.js | 0 test/handlers/fee/handlerRouter.js | 8 +++++++- 3 files changed, 12 insertions(+), 3 deletions(-) delete mode 100644 test/feeRouter/feeRouter.js diff --git a/contracts/handlers/FeeHandlerRouter.sol b/contracts/handlers/FeeHandlerRouter.sol index 7d3fe74e..e87451c9 100644 --- a/contracts/handlers/FeeHandlerRouter.sol +++ b/contracts/handlers/FeeHandlerRouter.sol @@ -18,8 +18,9 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl { // whitelisted address => is whitelisted mapping(address => bool) public _whitelist; - event FeeChanged( - uint256 newFee + event WhitelistChanged( + address whitelistAddress, + bool isWhitelisted ); error IncorrectFeeSupplied(uint256); @@ -66,6 +67,8 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl { */ function adminSetWhitelist(address whitelistAddress, bool isWhitelisted) external onlyAdmin { _whitelist[whitelistAddress] = isWhitelisted; + + emit WhitelistChanged(whitelistAddress, isWhitelisted); } diff --git a/test/feeRouter/feeRouter.js b/test/feeRouter/feeRouter.js deleted file mode 100644 index e69de29b..00000000 diff --git a/test/handlers/fee/handlerRouter.js b/test/handlers/fee/handlerRouter.js index e05d045a..0b8f0f45 100644 --- a/test/handlers/fee/handlerRouter.js +++ b/test/handlers/fee/handlerRouter.js @@ -94,7 +94,7 @@ contract("FeeHandlerRouter", async (accounts) => { false ); - await FeeHandlerRouterInstance.adminSetWhitelist( + const whitelistTx = await FeeHandlerRouterInstance.adminSetWhitelist( whitelistAddress, true ); @@ -104,6 +104,12 @@ contract("FeeHandlerRouter", async (accounts) => { ), true ); + TruffleAssert.eventEmitted(whitelistTx, "WhitelistChanged", (event) => { + return ( + event.whitelistAddress === whitelistAddress && + event.isWhitelisted === true + ); + }); }); it("should require admin role to set whitelist address", async () => {