From 370711537dd2189a3f6500e04528e6f3d697cb5f Mon Sep 17 00:00:00 2001 From: Santo Sinar Pandean Date: Mon, 4 Sep 2023 13:34:37 +0700 Subject: [PATCH 1/4] store wrbtc & iwrbtc address in feesharing storage --- .gitignore | 1 + .../FeeSharingCollector.sol | 128 +++++++++--------- .../FeeSharingCollectorStorage.sol | 10 ++ .../mockup/FeeSharingCollectorMockup.sol | 4 - tests/FeeSharingCollectorTest.js | 70 +++++++++- tests/swaps/SwapsExternal.js | 1 + 6 files changed, 146 insertions(+), 68 deletions(-) diff --git a/.gitignore b/.gitignore index 13364c526..43c7ba8f3 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,4 @@ typechain/ */deployments/*forked* venv/ !/external/artifacts/ +cache_hardhat/ diff --git a/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol b/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol index 337f5b582..1c477df58 100644 --- a/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol +++ b/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol @@ -104,6 +104,18 @@ contract FeeSharingCollector is event RBTCWithdrawn(address indexed sender, address indexed receiver, uint256 amount); + event SetWrbtcToken( + address indexed sender, + address indexed oldWrbtcToken, + address indexed newWrbtcToken + ); + + event SetLoanTokenWrbtc( + address indexed sender, + address indexed oldLoanTokenWrbtc, + address indexed newLoanTokenWrbtc + ); + /* Modifier */ modifier oneTimeExecution(bytes4 _funcSig) { require( @@ -119,6 +131,40 @@ contract FeeSharingCollector is /// @dev fallback function to support rbtc transfer when unwrap the wrbtc. function() external payable {} + /** + * @dev initialize function for fee sharing collector proxy + * @param wrbtcToken wrbtc token address + * @param loanWrbtcToken address of loan token wrbtc (IWrbtc) + */ + function initialize(address wrbtcToken, address loanWrbtcToken) external onlyOwner { + setWrbtcToken(wrbtcToken); + setLoanTokenWrbtc(loanWrbtcToken); + } + + /** + * @notice Set the wrbtc token address of fee sharing collector. + * + * only owner can perform this action. + * + * @param newWrbtcTokenAddress The new address of the wrbtc token. + * */ + function setWrbtcToken(address newWrbtcTokenAddress) public onlyOwner { + emit SetWrbtcToken(msg.sender, wrbtcTokenAddress, newWrbtcTokenAddress); + wrbtcTokenAddress = newWrbtcTokenAddress; + } + + /** + * @notice Set the loan wrbtc token address of fee sharing collector. + * + * only owner can perform this action. + * + * @param newLoanTokenWrbtcAddress The new address of the loan wrbtc token. + * */ + function setLoanTokenWrbtc(address newLoanTokenWrbtcAddress) public onlyOwner { + emit SetLoanTokenWrbtc(msg.sender, loanTokenWrbtcAddress, newLoanTokenWrbtcAddress); + loanTokenWrbtcAddress = newLoanTokenWrbtcAddress; + } + /** * @notice Withdraw fees for the given token: * lendingFee + tradingFee + borrowingFee @@ -137,11 +183,11 @@ contract FeeSharingCollector is uint256 wrbtcAmountWithdrawn = protocol.withdrawFees(_tokens, address(this)); - IWrbtcERC20 wRBTCToken = protocol.wrbtcToken(); + IWrbtcERC20 wrbtcToken = IWrbtcERC20(wrbtcTokenAddress); if (wrbtcAmountWithdrawn > 0) { // unwrap the wrbtc to rbtc, and hold the rbtc. - wRBTCToken.withdraw(wrbtcAmountWithdrawn); + wrbtcToken.withdraw(wrbtcAmountWithdrawn); /// @notice Update unprocessed amount of tokens uint96 amount96 = @@ -168,7 +214,7 @@ contract FeeSharingCollector is * @param _converters array addresses of the converters * */ function withdrawFeesAMM(address[] memory _converters) public { - IWrbtcERC20 wRBTCToken = protocol.wrbtcToken(); + IWrbtcERC20 wrbtcToken = IWrbtcERC20(wrbtcTokenAddress); // Validate _validateWhitelistedConverter(_converters); @@ -180,7 +226,7 @@ contract FeeSharingCollector is if (wrbtcAmountWithdrawn > 0) { // unwrap wrbtc to rbtc, and hold the rbtc - wRBTCToken.withdraw(wrbtcAmountWithdrawn); + wrbtcToken.withdraw(wrbtcAmountWithdrawn); /// @notice Update unprocessed amount of tokens uint96 amount96 = @@ -220,7 +266,7 @@ contract FeeSharingCollector is require(success, "Staking::transferTokens: token transfer failed"); // if _token is wrbtc, need to unwrap it to rbtc - IWrbtcERC20 wrbtcToken = protocol.wrbtcToken(); + IWrbtcERC20 wrbtcToken = IWrbtcERC20(wrbtcTokenAddress); if (_token == address(wrbtcToken)) { wrbtcToken.withdraw(_amount); _token = RBTC_DUMMY_ADDRESS_FOR_CHECKPOINT; @@ -284,9 +330,6 @@ contract FeeSharingCollector is "FeeSharingCollector::withdraw: _maxCheckpoints should be positive" ); - address wRBTCAddress = address(protocol.wrbtcToken()); - address loanPoolTokenWRBTC = _getAndValidateLoanPoolWRBTC(wRBTCAddress); - address user = msg.sender; if (_receiver == ZERO_ADDRESS) { _receiver = msg.sender; @@ -306,7 +349,7 @@ contract FeeSharingCollector is } processedCheckpoints[user][_token] = end; - if (loanPoolTokenWRBTC == _token) { + if (loanTokenWrbtcAddress == _token) { // We will change, so that feeSharingCollector will directly burn then loanToken (IWRBTC) to rbtc and send to the user --- by call burnToBTC function ILoanTokenWRBTC(_token).burnToBTC(_receiver, amount, false); } else { @@ -395,16 +438,12 @@ contract FeeSharingCollector is } function validRBTCBasedTokens(address[] memory _tokens) private view { - IWrbtcERC20 wrbtcToken = protocol.wrbtcToken(); - - address loanPoolTokenWRBTC = _getAndValidateLoanPoolWRBTC(address(wrbtcToken)); - for (uint256 i = 0; i < _tokens.length; i++) { address _token = _tokens[i]; if ( _token != RBTC_DUMMY_ADDRESS_FOR_CHECKPOINT && - _token != address(wrbtcToken) && - _token != loanPoolTokenWRBTC + _token != wrbtcTokenAddress && + _token != loanTokenWrbtcAddress ) { revert("only rbtc-based tokens are allowed"); } @@ -440,9 +479,6 @@ contract FeeSharingCollector is _receiver = msg.sender; } - IWrbtcERC20 wrbtcToken = protocol.wrbtcToken(); - address loanPoolTokenWRBTC = _getAndValidateLoanPoolWRBTC(address(wrbtcToken)); - uint256 rbtcAmountToSend; for (uint256 i = 0; i < _tokens.length; i++) { @@ -459,8 +495,8 @@ contract FeeSharingCollector is : previousProcessedUserCheckpoints; if ( - _token == address(wrbtcToken) || - _token == loanPoolTokenWRBTC || + _token == wrbtcTokenAddress || + _token == loanTokenWrbtcAddress || _token == RBTC_DUMMY_ADDRESS_FOR_CHECKPOINT ) { (totalAmount, endToken) = _withdrawRbtcTokenStartingFromCheckpoint( @@ -516,9 +552,7 @@ contract FeeSharingCollector is { address user = msg.sender; - IWrbtcERC20 wrbtcToken = protocol.wrbtcToken(); - - address loanPoolTokenWRBTC = _getAndValidateLoanPoolWRBTC(address(wrbtcToken)); + IWrbtcERC20 wrbtcToken = IWrbtcERC20(wrbtcTokenAddress); (totalAmount, endTokenCheckpoint) = _getRBTCBalance(_token, user, _maxCheckpoints); @@ -527,10 +561,10 @@ contract FeeSharingCollector is if (_token == address(wrbtcToken)) { // unwrap the wrbtc wrbtcToken.withdraw(totalAmount); - } else if (_token == loanPoolTokenWRBTC) { + } else if (_token == loanTokenWrbtcAddress) { // pull out the iWRBTC to rbtc to this feeSharingCollector contract /** @dev will use the burned result from IWRBTC to RBTC as return total amount */ - totalAmount = ILoanTokenWRBTC(loanPoolTokenWRBTC).burnToBTC( + totalAmount = ILoanTokenWRBTC(loanTokenWrbtcAddress).burnToBTC( address(this), totalAmount, false @@ -985,12 +1019,12 @@ contract FeeSharingCollector is } function withdrawWRBTC(address receiver, uint256 wrbtcAmount) external onlyOwner { - address wRBTCAddress = address(protocol.wrbtcToken()); + IERC20 wrbtcToken = IERC20(wrbtcTokenAddress); - uint256 balance = IERC20(wRBTCAddress).balanceOf(address(this)); + uint256 balance = wrbtcToken.balanceOf(address(this)); require(wrbtcAmount <= balance, "Insufficient balance"); - IERC20(wRBTCAddress).safeTransfer(receiver, wrbtcAmount); + wrbtcToken.safeTransfer(receiver, wrbtcAmount); } /** @@ -1040,10 +1074,8 @@ contract FeeSharingCollector is function getAccumulatedRBTCFeeBalances(address _user) external view returns (uint256) { (uint256 _rbtcAmount, uint256 _wrbtcAmount, uint256 _iWrbtcAmount, , , ) = _getRBTCBalances(_user, 0); - IWrbtcERC20 wrbtcToken = protocol.wrbtcToken(); - address loanPoolTokenWRBTC = _getAndValidateLoanPoolWRBTC(address(wrbtcToken)); uint256 iWRBTCAmountInRBTC = - _iWrbtcAmount.mul(ILoanTokenWRBTC(loanPoolTokenWRBTC).tokenPrice()).div(1e18); + _iWrbtcAmount.mul(ILoanTokenWRBTC(loanTokenWrbtcAddress).tokenPrice()).div(1e18); return _rbtcAmount.add(_wrbtcAmount).add(iWRBTCAmountInRBTC); } @@ -1072,10 +1104,6 @@ contract FeeSharingCollector is uint256 _endIWRBTC ) { - IWrbtcERC20 wrbtcToken = protocol.wrbtcToken(); - - address loanPoolTokenWRBTC = _getAndValidateLoanPoolWRBTC(address(wrbtcToken)); - (_rbtcAmount, _endRBTC) = _getAccumulatedFees({ _user: _user, _token: RBTC_DUMMY_ADDRESS_FOR_CHECKPOINT, @@ -1085,13 +1113,13 @@ contract FeeSharingCollector is (_wrbtcAmount, _endWRBTC) = _getAccumulatedFees({ _user: _user, - _token: address(wrbtcToken), + _token: wrbtcTokenAddress, _startFrom: 0, _maxCheckpoints: _maxCheckpoints }); (_iWrbtcAmount, _endIWRBTC) = _getAccumulatedFees({ _user: _user, - _token: loanPoolTokenWRBTC, + _token: loanTokenWrbtcAddress, _startFrom: 0, _maxCheckpoints: _maxCheckpoints }); @@ -1112,14 +1140,10 @@ contract FeeSharingCollector is address _user, uint32 _maxCheckpoints ) internal view returns (uint256 _tokenAmount, uint256 _endToken) { - IWrbtcERC20 wrbtcToken = protocol.wrbtcToken(); - - address loanPoolTokenWRBTC = _getAndValidateLoanPoolWRBTC(address(wrbtcToken)); - if ( _token == RBTC_DUMMY_ADDRESS_FOR_CHECKPOINT || - _token == address(wrbtcToken) || - _token == loanPoolTokenWRBTC + _token == wrbtcTokenAddress || + _token == loanTokenWrbtcAddress ) { (_tokenAmount, _endToken) = _getAccumulatedFees({ _user: _user, @@ -1132,24 +1156,6 @@ contract FeeSharingCollector is } } - /** - * @dev private function to get and validate the wrbtc loan pool token address based on the wrbtc token address. - * @dev will revert if wrbtc loan pool token does not exist (zero address) - * - * @param _wRBTCAddress wrbtc token address. - * - * @return wrbtc loan pool wrbtc token address - */ - function _getAndValidateLoanPoolWRBTC(address _wRBTCAddress) internal view returns (address) { - address loanPoolTokenWRBTC = protocol.underlyingToLoanPool(_wRBTCAddress); - require( - loanPoolTokenWRBTC != ZERO_ADDRESS, - "FeeSharingCollector::withdraw: loan wRBTC not found" - ); - - return loanPoolTokenWRBTC; - } - // @todo update dependency `numTokenCheckpoints` -> `totalTokenCheckpoints` and deprecate numTokenCheckpoints function /** * @dev This getter function `numTokenCheckpoints` is added for backwards compatibility diff --git a/contracts/governance/FeeSharingCollector/FeeSharingCollectorStorage.sol b/contracts/governance/FeeSharingCollector/FeeSharingCollectorStorage.sol index 6b580ee18..2d3b5bc1f 100644 --- a/contracts/governance/FeeSharingCollector/FeeSharingCollectorStorage.sol +++ b/contracts/governance/FeeSharingCollector/FeeSharingCollectorStorage.sol @@ -71,6 +71,16 @@ contract FeeSharingCollectorStorage is Ownable { mapping(bytes4 => bool) public isFunctionExecuted; + /** + * @dev Wrbtc token adress + */ + address public wrbtcTokenAddress; + + /** + * @dev IWrbtc token adress + */ + address public loanTokenWrbtcAddress; + /** * @dev Prevents a contract from calling itself, directly or indirectly. * If you mark a function `nonReentrant`, you should also diff --git a/contracts/mockup/FeeSharingCollectorMockup.sol b/contracts/mockup/FeeSharingCollectorMockup.sol index 3b2c9e11a..8ecfafc4a 100644 --- a/contracts/mockup/FeeSharingCollectorMockup.sol +++ b/contracts/mockup/FeeSharingCollectorMockup.sol @@ -61,10 +61,6 @@ contract FeeSharingCollectorMockup is FeeSharingCollector { (amount, end) = _getAccumulatedFees(_user, _token, 0, _maxCheckpoints); } - function invalidLoanPoolWRBTC() public view returns (address) { - return _getAndValidateLoanPoolWRBTC(address(0)); - } - function endOfRangeWithZeroMaxCheckpoint(address _token) public view returns (uint256) { return _getEndOfRange(0, _token, 0); } diff --git a/tests/FeeSharingCollectorTest.js b/tests/FeeSharingCollectorTest.js index e18facd1f..cb36b7797 100644 --- a/tests/FeeSharingCollectorTest.js +++ b/tests/FeeSharingCollectorTest.js @@ -227,6 +227,7 @@ contract("FeeSharingCollector:", (accounts) => { ); await feeSharingCollectorProxyObj.setImplementation(feeSharingCollectorLogic.address); feeSharingCollector = await FeeSharingCollector.at(feeSharingCollectorProxyObj.address); + await sovryn.setFeesController(feeSharingCollector.address); // Set loan pool for wRBTC -- because our fee sharing proxy required the loanPool of wRBTC @@ -293,6 +294,8 @@ contract("FeeSharingCollector:", (accounts) => { RBTC_DUMMY_ADDRESS_FOR_CHECKPOINT = await feeSharingCollector.RBTC_DUMMY_ADDRESS_FOR_CHECKPOINT(); + await feeSharingCollector.initialize(WRBTC.address, loanTokenWrbtc.address); + return sovryn; } @@ -300,6 +303,61 @@ contract("FeeSharingCollector:", (accounts) => { await loadFixture(protocolDeploymentFixture); }); + describe("initialization", async () => { + it("revert if initialize called by non-owner account", async () => { + await expectRevert( + feeSharingCollector.initialize(WRBTC.address, loanTokenWrbtc.address, { + from: account3, + }), + "unauthorized" + ); + }); + + it("revert if setWrbtcToken called by non-owner account", async () => { + await expectRevert( + feeSharingCollector.setWrbtcToken(WRBTC.address, { from: account3 }), + "unauthorized" + ); + }); + + it("revert if setLoanTokenWrbtc called by non-owner account", async () => { + await expectRevert( + feeSharingCollector.setLoanTokenWrbtc(loanTokenWrbtc.address, { from: account3 }), + "unauthorized" + ); + }); + + it("initialized successfully", async () => { + const wrbtcAddress = (await TestToken.new("WRBTC", "WRBTC", 18, 100)).address; + const loanTokenWrbtcAddress = (await TestToken.new("IWRBTC", "IWRBTC", 18, 100)) + .address; + await feeSharingCollector.initialize(wrbtcAddress, loanTokenWrbtcAddress); + expect(await feeSharingCollector.wrbtcTokenAddress()).to.equal(wrbtcAddress); + expect(await feeSharingCollector.loanTokenWrbtcAddress()).to.equal( + loanTokenWrbtcAddress + ); + }); + + it("setWrbtcToken should set the wrbtc token address properly", async () => { + expect(await feeSharingCollector.wrbtcTokenAddress()).to.equal(WRBTC.address); + const newWrbtcAddress = (await TestToken.new("WRBTC", "WRBTC", 18, 100)).address; + await feeSharingCollector.setWrbtcToken(newWrbtcAddress); + expect(await feeSharingCollector.wrbtcTokenAddress()).to.equal(newWrbtcAddress); + }); + + it("setLoanTokenWrbtc should set the wrbtc token address properly", async () => { + expect(await feeSharingCollector.loanTokenWrbtcAddress()).to.equal( + loanTokenWrbtc.address + ); + const newLoanTokenWrbtcAddress = (await TestToken.new("IWRBTC", "IWRBTC", 18, 100)) + .address; + await feeSharingCollector.setLoanTokenWrbtc(newLoanTokenWrbtcAddress); + expect(await feeSharingCollector.loanTokenWrbtcAddress()).to.equal( + newLoanTokenWrbtcAddress + ); + }); + }); + describe("withdrawStartingFromCheckpoint, withdrawRBTCStartingFromCheckpoint, withdrawRbtcTokenStartingFromCheckpoint and getNextPositiveUserCheckpoint", () => { let snapshot; before(async () => { @@ -1212,11 +1270,14 @@ contract("FeeSharingCollector:", (accounts) => { nextCheckpoint.hasFees, ]).to.eql([10, true, true]); }); + it("getNextPositiveUserCheckpoint for RBTC returns correct [checkpointNum, hasSkippedCheckpoints, hasFees]", async () => { feeSharingCollector = await FeeSharingCollectorMockup.new( sovryn.address, staking.address ); + + await feeSharingCollector.initialize(WRBTC.address, loanTokenWrbtc.address); await sovryn.setFeesController(feeSharingCollector.address); let nextCheckpoint = await feeSharingCollector.getNextPositiveUserCheckpoint( @@ -1233,7 +1294,6 @@ contract("FeeSharingCollector:", (accounts) => { await stake(900, root); const userStake = 100; - await createCheckpoints(9); nextCheckpoint = await feeSharingCollector.getNextPositiveUserCheckpoint( @@ -2427,6 +2487,7 @@ contract("FeeSharingCollector:", (accounts) => { staking.address ); + await feeSharingCollector.initialize(WRBTC.address, loanTokenWrbtc.address); await sovryn.setFeesController(feeSharingCollector.address); await WRBTC.mint(feeSharingCollector.address, wei("2", "ether")); @@ -2576,6 +2637,7 @@ contract("FeeSharingCollector:", (accounts) => { staking.address ); + await feeSharingCollector.initialize(WRBTC.address, loanTokenWrbtc.address); await sovryn.setFeesController(feeSharingCollector.address); await WRBTC.mint(feeSharingCollector.address, wei("2", "ether")); @@ -2690,6 +2752,7 @@ contract("FeeSharingCollector:", (accounts) => { staking.address ); + await feeSharingCollector.initialize(WRBTC.address, loanTokenWrbtc.address); await sovryn.setFeesController(feeSharingCollector.address); await WRBTC.mint(feeSharingCollector.address, wei("2", "ether")); @@ -2810,6 +2873,7 @@ contract("FeeSharingCollector:", (accounts) => { staking.address ); + await feeSharingCollector.initialize(WRBTC.address, loanTokenWrbtc.address); await sovryn.setFeesController(feeSharingCollector.address); await WRBTC.mint(feeSharingCollector.address, wei("2", "ether")); @@ -4346,8 +4410,8 @@ contract("FeeSharingCollector:", (accounts) => { ); await expectRevert( - feeSharingCollector.invalidLoanPoolWRBTC(), - "FeeSharingCollector::withdraw: loan wRBTC not found" + feeSharingCollector.getAccumulatedRBTCFeeBalances(root), + "Transaction reverted: function call to a non-contract account" ); }); diff --git a/tests/swaps/SwapsExternal.js b/tests/swaps/SwapsExternal.js index 1eb11fed2..fc5b7907f 100644 --- a/tests/swaps/SwapsExternal.js +++ b/tests/swaps/SwapsExternal.js @@ -145,6 +145,7 @@ contract("SwapsExternal", (accounts) => { WRBTC.address ); await loanTokenWrbtc.initialize(WRBTC.address, "iWRBTC", "iWRBTC"); + await feeSharingCollectorProxy.initialize(WRBTC.address, loanTokenWrbtc.address); loanTokenWrbtc = await LoanTokenLogicWrbtc.at(loanTokenWrbtc.address); const loanTokenAddressWrbtc = await loanTokenWrbtc.loanTokenAddress(); From d3e520f0166c70d672d645ac02824662abc1c874 Mon Sep 17 00:00:00 2001 From: Santo Sinar Pandean Date: Wed, 20 Sep 2023 09:52:45 +0700 Subject: [PATCH 2/4] fix based on review --- .../FeeSharingCollector.sol | 11 +- .../FeeSharingCollectorStorage.sol | 4 +- hardhat/tasks/feeSharingCollector.js | 109 ++++++++++++++++++ hardhat/tasks/index.js | 1 + tests/FeeSharingCollectorTest.js | 47 +++++++- 5 files changed, 164 insertions(+), 8 deletions(-) create mode 100644 hardhat/tasks/feeSharingCollector.js diff --git a/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol b/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol index 501466e04..899111092 100644 --- a/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol +++ b/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol @@ -136,7 +136,11 @@ contract FeeSharingCollector is * @param wrbtcToken wrbtc token address * @param loanWrbtcToken address of loan token wrbtc (IWrbtc) */ - function initialize(address wrbtcToken, address loanWrbtcToken) external onlyOwner { + function initialize(address wrbtcToken, address loanWrbtcToken) + external + onlyOwner + oneTimeExecution(this.initialize.selector) + { setWrbtcToken(wrbtcToken); setLoanTokenWrbtc(loanWrbtcToken); } @@ -149,6 +153,7 @@ contract FeeSharingCollector is * @param newWrbtcTokenAddress The new address of the wrbtc token. * */ function setWrbtcToken(address newWrbtcTokenAddress) public onlyOwner { + require(Address.isContract(newWrbtcTokenAddress), "newWrbtcTokenAddress not a contract"); emit SetWrbtcToken(msg.sender, wrbtcTokenAddress, newWrbtcTokenAddress); wrbtcTokenAddress = newWrbtcTokenAddress; } @@ -161,6 +166,10 @@ contract FeeSharingCollector is * @param newLoanTokenWrbtcAddress The new address of the loan wrbtc token. * */ function setLoanTokenWrbtc(address newLoanTokenWrbtcAddress) public onlyOwner { + require( + Address.isContract(newLoanTokenWrbtcAddress), + "newLoanTokenWrbtcAddress not a contract" + ); emit SetLoanTokenWrbtc(msg.sender, loanTokenWrbtcAddress, newLoanTokenWrbtcAddress); loanTokenWrbtcAddress = newLoanTokenWrbtcAddress; } diff --git a/contracts/governance/FeeSharingCollector/FeeSharingCollectorStorage.sol b/contracts/governance/FeeSharingCollector/FeeSharingCollectorStorage.sol index e4a9ad31e..47939d932 100644 --- a/contracts/governance/FeeSharingCollector/FeeSharingCollectorStorage.sol +++ b/contracts/governance/FeeSharingCollector/FeeSharingCollectorStorage.sol @@ -77,12 +77,12 @@ contract FeeSharingCollectorStorage is Ownable { mapping(bytes4 => bool) public isFunctionExecuted; /** - * @dev Wrbtc token adress + * @dev Wrbtc token address */ address public wrbtcTokenAddress; /** - * @dev IWrbtc token adress + * @dev iWrbtc loan token address */ address public loanTokenWrbtcAddress; diff --git a/hardhat/tasks/feeSharingCollector.js b/hardhat/tasks/feeSharingCollector.js new file mode 100644 index 000000000..88c998a30 --- /dev/null +++ b/hardhat/tasks/feeSharingCollector.js @@ -0,0 +1,109 @@ +const { task } = require("hardhat/config"); +const Logs = require("node-logs"); +const logger = new Logs().showInConsole(true); +const { sendWithMultisig } = require("../../deployment/helpers/helpers"); + +task("feeSharingCollector:initialize", "Initialize feeSharingCollector") + .addParam("wrbtcToken", "wrbtc token address") + .addParam("loanWrbtcToken", "iWrbtc loan token address") + .addOptionalParam("signer", "Signer name: 'signer' or 'deployer'", "deployer") + .setAction(async ({ wrbtcToken, loanWrbtcToken, signer }, hre) => { + await initializeFeeSharingCollector(hre, wrbtcToken, loanWrbtcToken, signer, true); + }); + +task("feeSharingCollector:setWrtbcTokenAddress", "Set wrbtc token address in feeSharingCollector") + .addParam("wrbtcToken", "wrbtc token address") + .addOptionalParam("signer", "Signer name: 'signer' or 'deployer'", "deployer") + .setAction(async ({ wrbtcToken, signer }, hre) => { + await setWrbtcTokenAddress(hre, wrbtcToken, signer, true); + }); + +task( + "feeSharingCollector:setLoanTokenWrtbcAddress", + "Set wrbtc token address in feeSharingCollector" +) + .addParam("loanWrbtcToken", "iWrbtc loan token address") + .addOptionalParam("signer", "Signer name: 'signer' or 'deployer'", "deployer") + .setAction(async ({ loanWrbtcToken, signer }, hre) => { + await setLoanTokenWrbtcAddress(hre, loanWrbtcToken, signer, true); + }); + +const initializeFeeSharingCollector = async (hre, wrbtcToken, loanWrbtcToken, signer) => { + const { + deployments: { get }, + ethers, + } = hre; + + if (!ethers.utils.isAddress(wrbtcToken)) { + logger.error(`wrbtcToken - ${wrbtcToken} is invalid address`); + return; + } + + if (!ethers.utils.isAddress(loanWrbtcToken)) { + logger.error(`loanWrbtcToken - ${loanWrbtcToken} is invalid address`); + return; + } + + const multisigDeployment = await get("MultiSigWallet"); + let initializeSelector = ethers.utils.id("initialize(address,address)").substring(0, 10); + const isInitialized = await ( + await ethers.getContract("FeeSharingCollector") + ).isFunctionExecuted(initializeSelector); + + if (isInitialized) { + logger.error("FeeSharingCollector has been initialized"); + return; + } + + const signerAcc = (await hre.getNamedAccounts())[signer]; + const targetDeploymentAddress = (await get("FeeSharingCollector")).address; + const iface = new ethers.utils.Interface([ + "function initialize(address wrbtcToken, address loanWrbtcToken)", + ]); + let data = await iface.encodeFunctionData("initialize", [wrbtcToken, loanWrbtcToken]); + await sendWithMultisig(multisigDeployment.address, targetDeploymentAddress, data, signerAcc); +}; + +const setWrbtcTokenAddress = async (hre, wrbtcToken, signer) => { + const { + deployments: { get }, + ethers, + } = hre; + + if (!ethers.utils.isAddress(wrbtcToken)) { + logger.error(`wrbtcToken - ${wrbtcToken} is invalid address`); + return; + } + + const multisigDeployment = await get("MultiSigWallet"); + + const signerAcc = (await hre.getNamedAccounts())[signer]; + const targetDeploymentAddress = (await get("FeeSharingCollector")).address; + const iface = new ethers.utils.Interface([ + "function setWrbtcToken(address newWrbtcTokenAddress)", + ]); + let data = await iface.encodeFunctionData("setWrbtcToken", [wrbtcToken]); + await sendWithMultisig(multisigDeployment.address, targetDeploymentAddress, data, signerAcc); +}; + +const setLoanTokenWrbtcAddress = async (hre, loanWrbtcToken, signer) => { + const { + deployments: { get }, + ethers, + } = hre; + + if (!ethers.utils.isAddress(loanWrbtcToken)) { + logger.error(`loanWrbtcToken - ${loanWrbtcToken} is invalid address`); + return; + } + + const multisigDeployment = await get("MultiSigWallet"); + + const signerAcc = (await hre.getNamedAccounts())[signer]; + const targetDeploymentAddress = (await get("FeeSharingCollector")).address; + const iface = new ethers.utils.Interface([ + "function setLoanTokenWrbtc(address newLoanTokenWrbtcAddress)", + ]); + let data = await iface.encodeFunctionData("setLoanTokenWrbtc", [loanWrbtcToken]); + await sendWithMultisig(multisigDeployment.address, targetDeploymentAddress, data, signerAcc); +}; diff --git a/hardhat/tasks/index.js b/hardhat/tasks/index.js index 4e51c18e8..cc5adefbf 100644 --- a/hardhat/tasks/index.js +++ b/hardhat/tasks/index.js @@ -5,3 +5,4 @@ require("./amm"); require("./utils"); require("./misc"); require("./governance"); +require("./feeSharingCollector"); diff --git a/tests/FeeSharingCollectorTest.js b/tests/FeeSharingCollectorTest.js index 0bb158bad..59c61eb3a 100644 --- a/tests/FeeSharingCollectorTest.js +++ b/tests/FeeSharingCollectorTest.js @@ -327,17 +327,32 @@ contract("FeeSharingCollector:", (accounts) => { ); }); - it("initialized successfully", async () => { + it("should revert if initialized more than once", async () => { const wrbtcAddress = (await TestToken.new("WRBTC", "WRBTC", 18, 100)).address; const loanTokenWrbtcAddress = (await TestToken.new("IWRBTC", "IWRBTC", 18, 100)) .address; - await feeSharingCollector.initialize(wrbtcAddress, loanTokenWrbtcAddress); - expect(await feeSharingCollector.wrbtcTokenAddress()).to.equal(wrbtcAddress); - expect(await feeSharingCollector.loanTokenWrbtcAddress()).to.equal( - loanTokenWrbtcAddress + await expectRevert( + feeSharingCollector.initialize(wrbtcAddress, loanTokenWrbtcAddress), + "function can only be called once" ); }); + it("setWrbtcToken should revert if try to set non-contract address", async () => { + expect(await feeSharingCollector.wrbtcTokenAddress()).to.equal(WRBTC.address); + let newInvalidWrbtcAddress = accounts[0]; + await expectRevert( + feeSharingCollector.setWrbtcToken(newInvalidWrbtcAddress), + "newWrbtcTokenAddress not a contract" + ); + + newInvalidWrbtcAddress = ZERO_ADDRESS; + await expectRevert( + feeSharingCollector.setWrbtcToken(newInvalidWrbtcAddress), + "newWrbtcTokenAddress not a contract" + ); + expect(await feeSharingCollector.wrbtcTokenAddress()).to.equal(WRBTC.address); + }); + it("setWrbtcToken should set the wrbtc token address properly", async () => { expect(await feeSharingCollector.wrbtcTokenAddress()).to.equal(WRBTC.address); const newWrbtcAddress = (await TestToken.new("WRBTC", "WRBTC", 18, 100)).address; @@ -345,6 +360,28 @@ contract("FeeSharingCollector:", (accounts) => { expect(await feeSharingCollector.wrbtcTokenAddress()).to.equal(newWrbtcAddress); }); + it("setLoanTokenWrbtc should revert if try to set non-contract addrerss", async () => { + expect(await feeSharingCollector.loanTokenWrbtcAddress()).to.equal( + loanTokenWrbtc.address + ); + + let newInvalidLoanTokenWrbtcAddress = accounts[0]; + await expectRevert( + feeSharingCollector.setLoanTokenWrbtc(newInvalidLoanTokenWrbtcAddress), + "newLoanTokenWrbtcAddress not a contract" + ); + + newInvalidLoanTokenWrbtcAddress = ZERO_ADDRESS; + await expectRevert( + feeSharingCollector.setLoanTokenWrbtc(newInvalidLoanTokenWrbtcAddress), + "newLoanTokenWrbtcAddress not a contract" + ); + + expect(await feeSharingCollector.loanTokenWrbtcAddress()).to.equal( + loanTokenWrbtc.address + ); + }); + it("setLoanTokenWrbtc should set the wrbtc token address properly", async () => { expect(await feeSharingCollector.loanTokenWrbtcAddress()).to.equal( loanTokenWrbtc.address From 6171b3ee29bb54a79152fbd8df1daa64c027aa4e Mon Sep 17 00:00:00 2001 From: Santo Sinar Pandean Date: Wed, 20 Sep 2023 13:34:06 +0700 Subject: [PATCH 3/4] require wrbtc and iWrbtc to be zero address in initialization --- .../governance/FeeSharingCollector/FeeSharingCollector.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol b/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol index 899111092..5be5f8f0f 100644 --- a/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol +++ b/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol @@ -141,6 +141,10 @@ contract FeeSharingCollector is onlyOwner oneTimeExecution(this.initialize.selector) { + require( + wrbtcTokenAddress == address(0) && loanTokenWrbtcAddress == address(0), + "wrbtcToken and loanWrbtcToken has been initialized" + ); setWrbtcToken(wrbtcToken); setLoanTokenWrbtc(loanWrbtcToken); } From b1cf5fc1bbd1253698c57879a40473193873fee5 Mon Sep 17 00:00:00 2001 From: Santo Sinar Pandean Date: Wed, 20 Sep 2023 14:11:47 +0700 Subject: [PATCH 4/4] add test case --- .../FeeSharingCollector.sol | 2 +- tests/FeeSharingCollectorTest.js | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol b/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol index 5be5f8f0f..99c1de6ef 100644 --- a/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol +++ b/contracts/governance/FeeSharingCollector/FeeSharingCollector.sol @@ -143,7 +143,7 @@ contract FeeSharingCollector is { require( wrbtcTokenAddress == address(0) && loanTokenWrbtcAddress == address(0), - "wrbtcToken and loanWrbtcToken has been initialized" + "wrbtcToken or loanWrbtcToken has been initialized" ); setWrbtcToken(wrbtcToken); setLoanTokenWrbtc(loanWrbtcToken); diff --git a/tests/FeeSharingCollectorTest.js b/tests/FeeSharingCollectorTest.js index 59c61eb3a..9c8b34027 100644 --- a/tests/FeeSharingCollectorTest.js +++ b/tests/FeeSharingCollectorTest.js @@ -304,6 +304,36 @@ contract("FeeSharingCollector:", (accounts) => { }); describe("initialization", async () => { + it("initialize should revert if wrbtc has been set", async () => { + const feeSharingCollectorMock = await FeeSharingCollectorMockup.new( + sovryn.address, + staking.address + ); + + await feeSharingCollectorMock.setWrbtcToken(WRBTC.address); + expect(await feeSharingCollectorMock.wrbtcTokenAddress()).to.equal(WRBTC.address); + + await expectRevert( + feeSharingCollectorMock.initialize(WRBTC.address, loanTokenWrbtc.address), + "wrbtcToken or loanWrbtcToken has been initialized" + ); + }); + + it("initialize should revert if iWrbtc has been set", async () => { + const feeSharingCollectorMock = await FeeSharingCollectorMockup.new( + sovryn.address, + staking.address + ); + + await feeSharingCollectorMock.setWrbtcToken(WRBTC.address); + expect(await feeSharingCollectorMock.wrbtcTokenAddress()).to.equal(WRBTC.address); + + await expectRevert( + feeSharingCollectorMock.initialize(WRBTC.address, loanTokenWrbtc.address), + "wrbtcToken or loanWrbtcToken has been initialized" + ); + }); + it("revert if initialize called by non-owner account", async () => { await expectRevert( feeSharingCollector.initialize(WRBTC.address, loanTokenWrbtc.address, {