From 0e8052a85532e82f24461e8a790dac16b41b33cc Mon Sep 17 00:00:00 2001 From: Gwydce Date: Wed, 18 Jan 2023 19:08:38 -0300 Subject: [PATCH 1/5] Checking share price on deposit/withdraw --- contracts/Controller.sol | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/contracts/Controller.sol b/contracts/Controller.sol index ac4f9ba..cf31679 100644 --- a/contracts/Controller.sol +++ b/contracts/Controller.sol @@ -35,6 +35,9 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { uint public depositLimit; uint public userDepositLimit; + uint public depositShareThreshold; + uint public withdrawShareThreshold; + event NewStrategy(address oldStrategy, address newStrategy); event NewTreasury(address oldTreasury, address newTreasury); event NewDepositLimit(uint oldLimit, uint newLimit); @@ -56,6 +59,9 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { want = _want; archimedes = _archimedes; treasury = _treasury; + + depositShareThreshold = (10 ** _want.decimals()); + withdrawShareThreshold = (10 ** _want.decimals()); } function decimals() override public view returns (uint8) { @@ -90,6 +96,18 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { return pid; } + function setDepositShareThreshold(uint _t) external onlyOwner nonReentrant { + require(depositShareThreshold == _t, "Same value"); + + depositShareThreshold = _t; + } + + function setWithdrawShareThreshold(uint _t) external onlyOwner nonReentrant { + require(withdrawShareThreshold == _t, "Same value"); + + withdrawShareThreshold = _t; + } + function setTreasury(address _treasury) external onlyOwner nonReentrant { require(_treasury != treasury, "Same address"); require(_treasury != address(0), "!ZeroAddress"); @@ -98,6 +116,7 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { treasury = _treasury; } + function setStrategy(address newStrategy) external onlyOwner nonReentrant { require(newStrategy != strategy, "Same strategy"); require(newStrategy != address(0), "!ZeroAddress"); @@ -145,6 +164,8 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { function deposit(address _senderUser, uint _amount) external onlyArchimedes nonReentrant { require(!_strategyPaused(), "Strategy paused"); require(_amount > 0, "Insufficient amount"); + // Ensure pool is healthy at deposit + require(currentSharePrice() > depositShareThreshold, "Low Share Price"); _checkDepositLimit(_senderUser, _amount); IStrategy(strategy).beforeMovement(); @@ -169,11 +190,16 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { _mint(_senderUser, shares); _strategyDeposit(); + + // Ensure the final step still is healthy + require(currentSharePrice() > depositShareThreshold, "Low Share Price"); } // Withdraw partial funds, normally used with a vault withdrawal function withdraw(address _senderUser, uint _shares) external onlyArchimedes nonReentrant returns (uint) { require(_shares > 0, "Insufficient shares"); + // Ensure pool is healthy before withdraw + require(_currentSharePrice() > withdrawShareThreshold, "Low Share Price"); IStrategy(strategy).beforeMovement(); // This line has to be calc before burn @@ -208,6 +234,9 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { if (!_strategyPaused()) { _strategyDeposit(); } + // Ensure pool still healthy + require(_currentSharePrice() > withdrawShareThreshold, "Low Share Price"); + return withdrawn; } @@ -277,4 +306,11 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { require(_amount <= availableUserDeposit(_user), "Max userDepositLimit reached"); } } + + function currentSharePrice() public view returns (uint) { + uint _totalSupply = totalSupply(); + uint _precision = 10 ** decimals(); + + return _totalSupply <= 0 ? _precision : (balance() * _precision / _totalSupply); + } } From 269df519024811efd5980c5e42871a8b01e790fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?N=C3=A9stor=20Coppi?= Date: Wed, 18 Jan 2023 20:47:50 -0300 Subject: [PATCH 2/5] ASd --- contracts/Controller.sol | 22 +++++++++------ test/contracts/Controller-test.js | 47 +++++++++++++++++++++++++++++++ utils | 2 +- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/contracts/Controller.sol b/contracts/Controller.sol index cf31679..4a68322 100644 --- a/contracts/Controller.sol +++ b/contracts/Controller.sol @@ -7,7 +7,7 @@ import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; - +import "hardhat/console.sol"; import "../interfaces/IArchimedes.sol"; import "../interfaces/IStrategy.sol"; @@ -60,8 +60,8 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { archimedes = _archimedes; treasury = _treasury; - depositShareThreshold = (10 ** _want.decimals()); - withdrawShareThreshold = (10 ** _want.decimals()); + depositShareThreshold = (10 ** _want.decimals()) * 9900 / RATIO_PRECISION; + withdrawShareThreshold = (10 ** _want.decimals()) * 9900 / RATIO_PRECISION; } function decimals() override public view returns (uint8) { @@ -97,13 +97,13 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { } function setDepositShareThreshold(uint _t) external onlyOwner nonReentrant { - require(depositShareThreshold == _t, "Same value"); + require(depositShareThreshold != _t, "Same value"); depositShareThreshold = _t; } function setWithdrawShareThreshold(uint _t) external onlyOwner nonReentrant { - require(withdrawShareThreshold == _t, "Same value"); + require(withdrawShareThreshold != _t, "Same value"); withdrawShareThreshold = _t; } @@ -165,7 +165,8 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { require(!_strategyPaused(), "Strategy paused"); require(_amount > 0, "Insufficient amount"); // Ensure pool is healthy at deposit - require(currentSharePrice() > depositShareThreshold, "Low Share Price"); + console.log("Current share:", currentSharePrice()); + require(currentSharePrice() >= depositShareThreshold, "Low Share Price"); _checkDepositLimit(_senderUser, _amount); IStrategy(strategy).beforeMovement(); @@ -192,14 +193,16 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { _strategyDeposit(); // Ensure the final step still is healthy - require(currentSharePrice() > depositShareThreshold, "Low Share Price"); + console.log("Current Luego deposito share:", currentSharePrice()); + require(currentSharePrice() >= depositShareThreshold, "Low Share Price"); } // Withdraw partial funds, normally used with a vault withdrawal function withdraw(address _senderUser, uint _shares) external onlyArchimedes nonReentrant returns (uint) { require(_shares > 0, "Insufficient shares"); // Ensure pool is healthy before withdraw - require(_currentSharePrice() > withdrawShareThreshold, "Low Share Price"); + console.log("Withdraw share price:", currentSharePrice()); + require(currentSharePrice() >= withdrawShareThreshold, "Low Share Price"); IStrategy(strategy).beforeMovement(); // This line has to be calc before burn @@ -234,8 +237,9 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { if (!_strategyPaused()) { _strategyDeposit(); } + console.log("AfterWithdraw share price:", currentSharePrice()); // Ensure pool still healthy - require(_currentSharePrice() > withdrawShareThreshold, "Low Share Price"); + require(currentSharePrice() >= withdrawShareThreshold, "Low Share Price"); return withdrawn; } diff --git a/test/contracts/Controller-test.js b/test/contracts/Controller-test.js index dac5597..93a2803 100644 --- a/test/contracts/Controller-test.js +++ b/test/contracts/Controller-test.js @@ -459,4 +459,51 @@ describe('Controller', () => { expect(await controller.availableDeposit()).to.be.equal(0) }) }) + describe('setDepositShareThreshold', async () => { + it('should be reverted for non admin', async () => { + await expect(controller.connect(bob).setDepositShareThreshold(10)).to.be.revertedWith( + 'Ownable: caller is not the owner' + ) + }) + + it('should change depositShareThreshold', async () => { + expect(await controller.depositShareThreshold()).to.be.equal(0.99e18 + '') + await controller.setDepositShareThreshold(10) + expect(await controller.depositShareThreshold()).to.be.equal(10) + }) + + it('should revert for low depositShareThreshold', async () => { + await waitFor(archimedes.addNewPool(piToken.address, controller.address, 1, false)) + await controller.setDepositShareThreshold(1.1e18 + '') // just to prove it + + await piToken.transfer(bob.address, 10001) + await piToken.connect(bob).approve(archimedes.address, 10001) + + await expect(archimedes.connect(bob).deposit(0, 10000, zeroAddress)).to.be.revertedWith('Low Share Price') + }) + }) + describe('setWithdrawShareThreshold', async () => { + it('should be reverted for non admin', async () => { + await expect(controller.connect(bob).setWithdrawShareThreshold(10)).to.be.revertedWith( + 'Ownable: caller is not the owner' + ) + }) + + it('should change withdrawShareThreshold', async () => { + expect(await controller.withdrawShareThreshold()).to.be.equal(0.99e18 + '') + await controller.setWithdrawShareThreshold(10) + expect(await controller.withdrawShareThreshold()).to.be.equal(10) + }) + + it('should revert for low withdrawShareThreshold', async () => { + await waitFor(archimedes.addNewPool(piToken.address, controller.address, 1, false)) + await controller.setWithdrawShareThreshold(1.1e18 + '') // just to prove it + + await piToken.transfer(bob.address, 10001) + await piToken.connect(bob).approve(archimedes.address, 10001) + + await waitFor(archimedes.connect(bob).deposit(0, 10000, zeroAddress)) + await expect(archimedes.connect(bob).withdrawAll(0)).to.be.revertedWith('Low Share Price') + }) + }) }) diff --git a/utils b/utils index db6562b..4dc8bb2 160000 --- a/utils +++ b/utils @@ -1 +1 @@ -Subproject commit db6562b5e0c8caabe5b0f54e65bbb85636f1b46a +Subproject commit 4dc8bb27a78d291838496d826c91a472e439487b From 7301f2c2f1671d8e685c7dc90428d81cba4631ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?N=C3=A9stor=20Coppi?= Date: Wed, 18 Jan 2023 21:03:50 -0300 Subject: [PATCH 3/5] Asd --- test/contracts/Controller-test.js | 46 ------------------- .../ControllerMStableStrat-test.js | 46 +++++++++++++++++++ 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/test/contracts/Controller-test.js b/test/contracts/Controller-test.js index 93a2803..d2ec872 100644 --- a/test/contracts/Controller-test.js +++ b/test/contracts/Controller-test.js @@ -459,51 +459,5 @@ describe('Controller', () => { expect(await controller.availableDeposit()).to.be.equal(0) }) }) - describe('setDepositShareThreshold', async () => { - it('should be reverted for non admin', async () => { - await expect(controller.connect(bob).setDepositShareThreshold(10)).to.be.revertedWith( - 'Ownable: caller is not the owner' - ) - }) - - it('should change depositShareThreshold', async () => { - expect(await controller.depositShareThreshold()).to.be.equal(0.99e18 + '') - await controller.setDepositShareThreshold(10) - expect(await controller.depositShareThreshold()).to.be.equal(10) - }) - - it('should revert for low depositShareThreshold', async () => { - await waitFor(archimedes.addNewPool(piToken.address, controller.address, 1, false)) - await controller.setDepositShareThreshold(1.1e18 + '') // just to prove it - - await piToken.transfer(bob.address, 10001) - await piToken.connect(bob).approve(archimedes.address, 10001) - - await expect(archimedes.connect(bob).deposit(0, 10000, zeroAddress)).to.be.revertedWith('Low Share Price') - }) - }) - describe('setWithdrawShareThreshold', async () => { - it('should be reverted for non admin', async () => { - await expect(controller.connect(bob).setWithdrawShareThreshold(10)).to.be.revertedWith( - 'Ownable: caller is not the owner' - ) - }) - - it('should change withdrawShareThreshold', async () => { - expect(await controller.withdrawShareThreshold()).to.be.equal(0.99e18 + '') - await controller.setWithdrawShareThreshold(10) - expect(await controller.withdrawShareThreshold()).to.be.equal(10) - }) - - it('should revert for low withdrawShareThreshold', async () => { - await waitFor(archimedes.addNewPool(piToken.address, controller.address, 1, false)) - await controller.setWithdrawShareThreshold(1.1e18 + '') // just to prove it - - await piToken.transfer(bob.address, 10001) - await piToken.connect(bob).approve(archimedes.address, 10001) - await waitFor(archimedes.connect(bob).deposit(0, 10000, zeroAddress)) - await expect(archimedes.connect(bob).withdrawAll(0)).to.be.revertedWith('Low Share Price') - }) - }) }) diff --git a/test/integration/ControllerMStableStrat-test.js b/test/integration/ControllerMStableStrat-test.js index 3b54d56..5f6cdc2 100644 --- a/test/integration/ControllerMStableStrat-test.js +++ b/test/integration/ControllerMStableStrat-test.js @@ -511,4 +511,50 @@ describe('Controller mStable Strat with DAI', () => { expect(await DAI.balanceOf(strat.address)).to.be.equal(0) expect(await REWARD_TOKEN.balanceOf(strat.address)).to.be.equal(0) }) + describe.only('setDepositShareThreshold', async () => { + it('should be reverted for non admin', async () => { + await expect(controller.connect(bob).setDepositShareThreshold(10)).to.be.revertedWith( + 'Ownable: caller is not the owner' + ) + }) + + it('should change depositShareThreshold', async () => { + expect(await controller.depositShareThreshold()).to.be.equal(0.99e18 + '') + await controller.setDepositShareThreshold(10) + expect(await controller.depositShareThreshold()).to.be.equal(10) + }) + + it('should revert for low depositShareThreshold', async () => { + const newBalance = ethers.BigNumber.from('' + 1e18).mul(100000) // 100000 DAI + await setCustomBalanceFor(DAI.address, bob.address, newBalance) + + await controller.setDepositShareThreshold(1.1e18 + '') // just to prove it + + await DAI.connect(bob).approve(archimedes.address, newBalance) + await expect(archimedes.connect(bob).depositAll(0, zeroAddress)).to.be.revertedWith('Low Share Price') + }) + }) + describe.only('setWithdrawShareThreshold', async () => { + it('should be reverted for non admin', async () => { + await expect(controller.connect(bob).setWithdrawShareThreshold(10)).to.be.revertedWith( + 'Ownable: caller is not the owner' + ) + }) + + it('should change withdrawShareThreshold', async () => { + expect(await controller.withdrawShareThreshold()).to.be.equal(0.99e18 + '') + await controller.setWithdrawShareThreshold(10) + expect(await controller.withdrawShareThreshold()).to.be.equal(10) + }) + + it('should revert for low withdrawShareThreshold', async () => { + const newBalance = ethers.BigNumber.from('' + 1e18).mul(100000) // 100000 DAI + await setCustomBalanceFor(DAI.address, bob.address, newBalance) + await controller.setWithdrawShareThreshold(1.1e18 + '') // just to prove it + + await DAI.connect(bob).approve(archimedes.address, newBalance) + await waitFor(archimedes.connect(bob).depositAll(0, zeroAddress)) + await expect(archimedes.connect(bob).withdrawAll(0)).to.be.revertedWith('Low Share Price') + }) + }) }) From a9e393bb31940bfbdec8d00157e59106b02c080a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?N=C3=A9stor=20Coppi?= Date: Wed, 18 Jan 2023 21:12:14 -0300 Subject: [PATCH 4/5] Aassas --- test/integration/ControllerMStableStrat-test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/integration/ControllerMStableStrat-test.js b/test/integration/ControllerMStableStrat-test.js index 5f6cdc2..4d4ef4c 100644 --- a/test/integration/ControllerMStableStrat-test.js +++ b/test/integration/ControllerMStableStrat-test.js @@ -533,7 +533,18 @@ describe('Controller mStable Strat with DAI', () => { await DAI.connect(bob).approve(archimedes.address, newBalance) await expect(archimedes.connect(bob).depositAll(0, zeroAddress)).to.be.revertedWith('Low Share Price') }) + + it('should revert for low depositShareThreshold after deposit in strat', async () => { + const newBalance = ethers.BigNumber.from('' + 1e18).mul(100000) // 100000 DAI + await setCustomBalanceFor(DAI.address, bob.address, newBalance) + + await controller.setDepositShareThreshold(1.0e18 + '') // just to prove it + + await DAI.connect(bob).approve(archimedes.address, newBalance) + await expect(archimedes.connect(bob).depositAll(0, zeroAddress)).to.be.revertedWith('Low Share Price') + }) }) + describe.only('setWithdrawShareThreshold', async () => { it('should be reverted for non admin', async () => { await expect(controller.connect(bob).setWithdrawShareThreshold(10)).to.be.revertedWith( From 10d89c672c0c81f67661d97ecf4a9d4ab9169673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?N=C3=A9stor=20Coppi?= Date: Fri, 20 Jan 2023 08:24:41 -0300 Subject: [PATCH 5/5] Little changes --- contracts/Controller.sol | 8 ++------ test/contracts/Controller-test.js | 1 - test/integration/ControllerMStableStrat-test.js | 5 +++-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/contracts/Controller.sol b/contracts/Controller.sol index 4a68322..39557a4 100644 --- a/contracts/Controller.sol +++ b/contracts/Controller.sol @@ -7,7 +7,7 @@ import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import "hardhat/console.sol"; + import "../interfaces/IArchimedes.sol"; import "../interfaces/IStrategy.sol"; @@ -60,6 +60,7 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { archimedes = _archimedes; treasury = _treasury; + // 99% of share/token value depositShareThreshold = (10 ** _want.decimals()) * 9900 / RATIO_PRECISION; withdrawShareThreshold = (10 ** _want.decimals()) * 9900 / RATIO_PRECISION; } @@ -116,7 +117,6 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { treasury = _treasury; } - function setStrategy(address newStrategy) external onlyOwner nonReentrant { require(newStrategy != strategy, "Same strategy"); require(newStrategy != address(0), "!ZeroAddress"); @@ -165,7 +165,6 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { require(!_strategyPaused(), "Strategy paused"); require(_amount > 0, "Insufficient amount"); // Ensure pool is healthy at deposit - console.log("Current share:", currentSharePrice()); require(currentSharePrice() >= depositShareThreshold, "Low Share Price"); _checkDepositLimit(_senderUser, _amount); @@ -193,7 +192,6 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { _strategyDeposit(); // Ensure the final step still is healthy - console.log("Current Luego deposito share:", currentSharePrice()); require(currentSharePrice() >= depositShareThreshold, "Low Share Price"); } @@ -201,7 +199,6 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { function withdraw(address _senderUser, uint _shares) external onlyArchimedes nonReentrant returns (uint) { require(_shares > 0, "Insufficient shares"); // Ensure pool is healthy before withdraw - console.log("Withdraw share price:", currentSharePrice()); require(currentSharePrice() >= withdrawShareThreshold, "Low Share Price"); IStrategy(strategy).beforeMovement(); @@ -237,7 +234,6 @@ contract Controller is ERC20, Ownable, ReentrancyGuard { if (!_strategyPaused()) { _strategyDeposit(); } - console.log("AfterWithdraw share price:", currentSharePrice()); // Ensure pool still healthy require(currentSharePrice() >= withdrawShareThreshold, "Low Share Price"); diff --git a/test/contracts/Controller-test.js b/test/contracts/Controller-test.js index d2ec872..dac5597 100644 --- a/test/contracts/Controller-test.js +++ b/test/contracts/Controller-test.js @@ -459,5 +459,4 @@ describe('Controller', () => { expect(await controller.availableDeposit()).to.be.equal(0) }) }) - }) diff --git a/test/integration/ControllerMStableStrat-test.js b/test/integration/ControllerMStableStrat-test.js index 4d4ef4c..d858474 100644 --- a/test/integration/ControllerMStableStrat-test.js +++ b/test/integration/ControllerMStableStrat-test.js @@ -511,7 +511,8 @@ describe('Controller mStable Strat with DAI', () => { expect(await DAI.balanceOf(strat.address)).to.be.equal(0) expect(await REWARD_TOKEN.balanceOf(strat.address)).to.be.equal(0) }) - describe.only('setDepositShareThreshold', async () => { + + describe('setDepositShareThreshold', async () => { it('should be reverted for non admin', async () => { await expect(controller.connect(bob).setDepositShareThreshold(10)).to.be.revertedWith( 'Ownable: caller is not the owner' @@ -545,7 +546,7 @@ describe('Controller mStable Strat with DAI', () => { }) }) - describe.only('setWithdrawShareThreshold', async () => { + describe('setWithdrawShareThreshold', async () => { it('should be reverted for non admin', async () => { await expect(controller.connect(bob).setWithdrawShareThreshold(10)).to.be.revertedWith( 'Ownable: caller is not the owner'