From 09c8d8cce40162723575443b0eddc846c7a768e4 Mon Sep 17 00:00:00 2001 From: 3xHarry Date: Mon, 10 Apr 2023 19:16:59 +0200 Subject: [PATCH] fix https://github.com/sherlock-audit/2023-03-Y2K-judging/issues/208 --- src/v2/Carousel/Carousel.sol | 8 +++--- src/v2/VaultFactoryV2.sol | 27 +------------------ src/v2/VaultV2.sol | 28 +++++++------------- src/v2/interfaces/IVaultFactoryV2.sol | 2 ++ src/v2/interfaces/IVaultV2.sol | 1 - src/v2/libraries/CarouselCreator.sol | 1 - src/v2/libraries/VaultV2Creator.sol | 3 +-- test/V2/Carousel/CarouselTest.t.sol | 5 +++- test/V2/FactoryV2Test.t.sol | 21 --------------- test/V2/VaultV2Test.t.sol | 37 ++++++++------------------- 10 files changed, 31 insertions(+), 102 deletions(-) diff --git a/src/v2/Carousel/Carousel.sol b/src/v2/Carousel/Carousel.sol index e0ee9c8f..0099f825 100644 --- a/src/v2/Carousel/Carousel.sol +++ b/src/v2/Carousel/Carousel.sol @@ -46,8 +46,7 @@ contract Carousel is VaultV2 { _data.tokenURI, _data.token, _data.strike, - _data.controller, - _data.treasury + _data.controller ) { if (_data.relayerFee < 10000) revert RelayerFeeToLow(); @@ -334,7 +333,7 @@ contract Carousel is VaultV2 { if (depositFee > 0) { (uint256 feeAmount, uint256 assetsAfterFee) = getEpochDepositFee(_epochId, assetsToDeposit); assetsToDeposit = assetsAfterFee; - _asset().safeTransfer(treasury, feeAmount); + _asset().safeTransfer(treasury(), feeAmount); } _mintShares( @@ -496,7 +495,7 @@ contract Carousel is VaultV2 { if (depositFee > 0) { (uint256 feeAmount, uint256 assetsAfterFee) = getEpochDepositFee(_id, _assets); assetsToDeposit = assetsAfterFee; - _asset().safeTransfer(treasury, feeAmount); + _asset().safeTransfer(treasury(), feeAmount); } _mintShares(_receiver, _id, assetsToDeposit); @@ -834,7 +833,6 @@ contract Carousel is VaultV2 { address token; uint256 strike; address controller; - address treasury; address emissionsToken; uint256 relayerFee; uint256 depositFee; diff --git a/src/v2/VaultFactoryV2.sol b/src/v2/VaultFactoryV2.sol index 57983050..6327060b 100644 --- a/src/v2/VaultFactoryV2.sol +++ b/src/v2/VaultFactoryV2.sol @@ -236,7 +236,7 @@ contract VaultFactoryV2 is Ownable { /** @notice Function to whitelist controller smart contract, only owner or timelocker can add more controllers. - owner can set controller once, all future controllers must be set by timelocker. + @dev owner can set controller once, all future controllers must be set by timelocker. @param _controller Address of the controller smart contract */ function whitelistController(address _controller) public { @@ -254,31 +254,6 @@ contract VaultFactoryV2 is Ownable { } } - /** - @notice Admin function, whitelists an address on vault for sendTokens function - @param _treasury Treasury address - @param _marketId Target market index - */ - function changeTreasury(uint256 _marketId, address _treasury) - public - onlyTimeLocker - { - if (_treasury == address(0)) revert AddressZero(); - - address[2] memory vaults = marketIdToVaults[_marketId]; - - if (vaults[0] == address(0) || vaults[1] == address(0)) { - revert MarketDoesNotExist(_marketId); - } - - IVaultV2(vaults[0]).whiteListAddress(_treasury); - IVaultV2(vaults[1]).whiteListAddress(_treasury); - IVaultV2(vaults[0]).setTreasury(_treasury); - IVaultV2(vaults[1]).setTreasury(_treasury); - - emit AddressWhitelisted(_treasury, _marketId); - } - /** @notice Admin function, whitelists an address on vault for sendTokens function @param _marketId Target market index diff --git a/src/v2/VaultV2.sol b/src/v2/VaultV2.sol index d481a293..b9979db4 100644 --- a/src/v2/VaultV2.sol +++ b/src/v2/VaultV2.sol @@ -6,6 +6,7 @@ import { } from "@openzeppelin/contracts/security/ReentrancyGuard.sol"; import "./SemiFungibleVault.sol"; import {IVaultV2} from "./interfaces/IVaultV2.sol"; +import {IVaultFactoryV2} from "./interfaces/IVaultFactoryV2.sol"; import {IWETH} from "./interfaces/IWETH.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { @@ -28,7 +29,6 @@ contract VaultV2 is IVaultV2, SemiFungibleVault, ReentrancyGuard { uint256 public immutable strike; // Earthquake bussiness logic bool public immutable isWETH; - address public treasury; address public counterPartyVault; address public factory; address public controller; @@ -55,7 +55,6 @@ contract VaultV2 is IVaultV2, SemiFungibleVault, ReentrancyGuard { @param _token address of the token that will be used as collateral; @param _strike uint256 representing the strike price of the vault; @param _controller address of the controller of the vault; - @param _treasury address of the treasury of the vault; */ constructor( bool _isWETH, @@ -65,19 +64,15 @@ contract VaultV2 is IVaultV2, SemiFungibleVault, ReentrancyGuard { string memory _tokenURI, address _token, uint256 _strike, - address _controller, - address _treasury + address _controller ) SemiFungibleVault(IERC20(_assetAddress), _name, _symbol, _tokenURI) { if (_controller == address(0)) revert AddressZero(); if (_token == address(0)) revert AddressZero(); if (_assetAddress == address(0)) revert AddressZero(); - if (_treasury == address(0)) revert AddressZero(); token = _token; strike = _strike; factory = msg.sender; controller = _controller; - treasury = _treasury; - whitelistedAddresses[_treasury] = true; isWETH = _isWETH; } @@ -251,22 +246,13 @@ contract VaultV2 is IVaultV2, SemiFungibleVault, ReentrancyGuard { /** @notice Factory function, whitelist address - @param _wAddress New treasury address - */ + @param _wAddress whitelist destination address + */ function whiteListAddress(address _wAddress) public onlyFactory { if (_wAddress == address(0)) revert AddressZero(); whitelistedAddresses[_wAddress] = !whitelistedAddresses[_wAddress]; } - /** - @notice Factory function, changes treasury address - @param _treasury New treasury address - */ - function setTreasury(address _treasury) public onlyFactory { - if (_treasury == address(0)) revert AddressZero(); - treasury = _treasury; - } - /** @notice Factory function, changes _counterPartyVault address @param _counterPartyVault New _counterPartyVault address @@ -313,7 +299,7 @@ contract VaultV2 is IVaultV2, SemiFungibleVault, ReentrancyGuard { if (_amount > finalTVL[_id]) revert AmountExceedsTVL(); if (epochAccounting[_id] + _amount > finalTVL[_id]) revert AmountExceedsTVL(); - if (!whitelistedAddresses[_receiver] && _receiver != counterPartyVault) + if (!whitelistedAddresses[_receiver] && _receiver != counterPartyVault && _receiver != treasury()) revert DestinationNotAuthorized(_receiver); epochAccounting[_id] += _amount; SemiFungibleVault.asset.safeTransfer(_receiver, _amount); @@ -395,6 +381,10 @@ contract VaultV2 is IVaultV2, SemiFungibleVault, ReentrancyGuard { epochCreation = epochConfig[_id].epochCreation; } + function treasury() public returns (address) { + return IVaultFactoryV2(factory).treasury(); + } + function _asset() internal view returns (IERC20) { return asset; } diff --git a/src/v2/interfaces/IVaultFactoryV2.sol b/src/v2/interfaces/IVaultFactoryV2.sol index f30e9370..51879747 100644 --- a/src/v2/interfaces/IVaultFactoryV2.sol +++ b/src/v2/interfaces/IVaultFactoryV2.sol @@ -11,6 +11,8 @@ interface IVaultFactoryV2 { string memory name ) external returns (address); + function treasury() external view returns (address); + function getVaults(uint256) external view returns (address[2] memory); function getEpochFee(uint256) external view returns (uint16); diff --git a/src/v2/interfaces/IVaultV2.sol b/src/v2/interfaces/IVaultV2.sol index 7a5a3f62..531394c5 100644 --- a/src/v2/interfaces/IVaultV2.sol +++ b/src/v2/interfaces/IVaultV2.sol @@ -62,5 +62,4 @@ interface IVaultV2 { view returns (bool); - function setTreasury(address _treasury) external; } diff --git a/src/v2/libraries/CarouselCreator.sol b/src/v2/libraries/CarouselCreator.sol index ead966a3..d88ee259 100644 --- a/src/v2/libraries/CarouselCreator.sol +++ b/src/v2/libraries/CarouselCreator.sol @@ -35,7 +35,6 @@ library CarouselCreator { _marketConfig.token, _marketConfig.strike, _marketConfig.controller, - _marketConfig.treasury, _marketConfig.emissionsToken, _marketConfig.relayerFee, _marketConfig.depositFee, diff --git a/src/v2/libraries/VaultV2Creator.sol b/src/v2/libraries/VaultV2Creator.sol index 0314292c..8aa63346 100644 --- a/src/v2/libraries/VaultV2Creator.sol +++ b/src/v2/libraries/VaultV2Creator.sol @@ -29,8 +29,7 @@ library VaultV2Creator { _marketConfig.tokenURI, _marketConfig.token, _marketConfig.strike, - _marketConfig.controller, - _marketConfig.treasury + _marketConfig.controller ) ); } diff --git a/test/V2/Carousel/CarouselTest.t.sol b/test/V2/Carousel/CarouselTest.t.sol index e6a6427d..2aaaf322 100644 --- a/test/V2/Carousel/CarouselTest.t.sol +++ b/test/V2/Carousel/CarouselTest.t.sol @@ -41,7 +41,6 @@ contract CarouselTest is Helper { TOKEN, STRIKE, controller, - TREASURY, emissionsToken, relayerFee, depositFee, @@ -391,5 +390,9 @@ contract CarouselTest is Helper { vm.stopPrank(); } + // deployer contract acts as factory and must emulate VaultFactoryV2.treasury() + function treasury() public view returns (address) { + return TREASURY; + } } \ No newline at end of file diff --git a/test/V2/FactoryV2Test.t.sol b/test/V2/FactoryV2Test.t.sol index c7939a99..96f4b59d 100644 --- a/test/V2/FactoryV2Test.t.sol +++ b/test/V2/FactoryV2Test.t.sol @@ -264,27 +264,6 @@ contract FactoryV2Test is Helper { assertEq(epochs[1], epochId2); } - - function testChangeTreasuryOnVault() public { - // test revert cases - uint256 marketId = createMarketHelper(); - - vm.expectRevert(VaultFactoryV2.NotTimeLocker.selector); - factory.changeTreasury(uint256(0x2), address(0x20)); - - vm.startPrank(address(factory.timelocker())); - vm.expectRevert(abi.encodeWithSelector(VaultFactoryV2.MarketDoesNotExist.selector, uint256(0x2))); - factory.changeTreasury(uint256(0x2), address(0x20)); - vm.expectRevert(VaultFactoryV2.AddressZero.selector); - factory.changeTreasury(marketId, address(0)); - - // test success case - factory.changeTreasury(marketId, address(0x20)); - address[2] memory vaults = factory.getVaults(marketId); - assertTrue(IVaultV2(vaults[0]).whitelistedAddresses(address(0x20))); - assertTrue(IVaultV2(vaults[1]).whitelistedAddresses(address(0x20))); - vm.stopPrank(); - } function testSetTreasury() public { // test revert cases diff --git a/test/V2/VaultV2Test.t.sol b/test/V2/VaultV2Test.t.sol index 1e5b7d41..e7265f06 100644 --- a/test/V2/VaultV2Test.t.sol +++ b/test/V2/VaultV2Test.t.sol @@ -23,8 +23,7 @@ contract VaultV2Test is Helper { "randomURI", TOKEN, STRIKE, - controller, - TREASURY + controller ); vm.warp(120000); MintableToken(UNDERLYING).mint(address(this)); @@ -37,8 +36,7 @@ contract VaultV2Test is Helper { "randomURI", TOKEN, STRIKE, - controller, - TREASURY + controller ); vault.setCounterPartyVault(address(counterpartyVault)); @@ -58,8 +56,7 @@ contract VaultV2Test is Helper { "randomURI", TOKEN, STRIKE, - controller, - TREASURY + controller ); vm.expectRevert(VaultV2.AddressZero.selector); @@ -71,8 +68,7 @@ contract VaultV2Test is Helper { "randomURI", address(0), // wrong token STRIKE, - controller, - TREASURY + controller ); vm.expectRevert(VaultV2.AddressZero.selector); @@ -84,21 +80,7 @@ contract VaultV2Test is Helper { "randomURI", TOKEN, STRIKE, - address(0), // wrong controller - TREASURY - ); - - vm.expectRevert(VaultV2.AddressZero.selector); - new VaultV2( - false, - UNDERLYING, - "Vault", - "v", - "randomURI", - TOKEN, - STRIKE, - controller, - address(0) // wrong treasury + address(0) // wrong controller ); // test success case @@ -110,8 +92,7 @@ contract VaultV2Test is Helper { "randomURI", TOKEN, STRIKE, - controller, - TREASURY + controller ); assertEq(address(vault2.asset()), UNDERLYING); @@ -120,7 +101,6 @@ contract VaultV2Test is Helper { assertEq(vault2.token(), TOKEN); assertEq(vault2.strike(), STRIKE); assertEq(vault2.controller(), controller); - assertTrue(vault2.whitelistedAddresses(TREASURY)); assertEq(vault2.factory(), address(this)); } @@ -540,4 +520,9 @@ contract VaultV2Test is Helper { vault.setEpoch(_epochBegin, _epochEnd, _epochId); counterpartyVault.setEpoch(_epochBegin, _epochEnd, _epochId); } + + // deployer contract acts as factory and must emulate VaultFactoryV2.treasury() + function treasury() public view returns (address) { + return TREASURY; + } }