[OZ#2: M-03] Incorrect recalculation of shares during deposit (#118)

* fix: reorder shares calculation and idleAssets increase * fix: added test to deposit Event amount of shares * fix: reordered refund logic and added testcase * fix: removed console.logs
pods-finance · Nov 28, 2022 · 15b62fc · 15b62fc
1 parent f283227
commit 15b62fc
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 3 deletions.
diff --git a/contracts/vaults/BaseVault.sol b/contracts/vaults/BaseVault.sol
@@ -330,8 +330,8 @@ abstract contract BaseVault is IVault, ERC20Permit, ERC4626, Capped {
         if (assets == 0) revert IVault__ZeroAssets();
 
         if (depositQueue.remove(msg.sender)) {
-            vaultState.totalIdleAssets -= assets;
             _restoreCap(convertToShares(assets));
+            vaultState.totalIdleAssets -= assets;
         }
 
         emit DepositRefunded(msg.sender, vaultState.currentRoundId, assets);

diff --git a/contracts/vaults/STETHVault.sol b/contracts/vaults/STETHVault.sol
@@ -171,10 +171,10 @@ contract STETHVault is BaseVault {
         uint256 shares
     ) internal virtual override {
         assets = _stETHTransferFrom(caller, address(this), assets);
-        shares = previewDeposit(assets);
+        _addToDepositQueue(receiver, assets);
 
+        shares = previewDeposit(assets);
         _spendCap(shares);
-        _addToDepositQueue(receiver, assets);
         emit Deposit(caller, receiver, assets, shares);
     }
 

diff --git a/test/vaults/STETHVault.ts b/test/vaults/STETHVault.ts
@@ -445,6 +445,58 @@ describe('STETHVault', () => {
     })
   })
 
+  describe('Refund', () => {
+    it('should match the variation in Cap and the refund shares', async () => {
+      const cap = ethers.utils.parseEther('30')
+
+      // This test will only work if InvestRatio = 50%
+      const user0Amount = ethers.utils.parseEther('10')
+      const user1Amount = ethers.utils.parseEther('3')
+      const user2Amount = ethers.utils.parseEther('0.2')
+      const user3Amount = ethers.utils.parseEther('1')
+
+      // Round 0
+      await configuration.setCap(vault.address, cap)
+      await vault.connect(user0).deposit(user0Amount, user0.address)
+
+      await vault.connect(vaultController).endRound()
+      await vault.connect(vaultController).processQueuedDeposits([user0.address])
+
+      // Round 1
+      await vault.connect(vaultController).startRound()
+      await asset.connect(yieldGenerator).transfer(vault.address, ethers.utils.parseEther('20'))
+      await vault.connect(vaultController).endRound()
+
+      // Round 2
+      await vault.connect(vaultController).startRound()
+      await vault.connect(user1).deposit(user1Amount, user1.address)
+      await vault.connect(user2).deposit(user2Amount, user2.address)
+      await asset.connect(yieldGenerator).transfer(vault.address, ethers.utils.parseEther('20'))
+      await asset.connect(yieldGenerator).transfer(investor.address, ethers.utils.parseEther('1300'))
+      await vault.connect(vaultController).endRound()
+      await vault.connect(vaultController).processQueuedDeposits([user1.address, user2.address])
+
+      // Round 3
+      await vault.connect(vaultController).startRound()
+      await vault.connect(user3).deposit(user3Amount, user3.address)
+
+      const avaiableCapBeforeRefund = await vault.availableCap()
+      await vault.connect(user3).refund()
+
+      const avaiableCapAfterRefund = await vault.availableCap()
+      const diffRefund = avaiableCapAfterRefund.sub(avaiableCapBeforeRefund)
+
+      await vault.connect(user3).deposit(user3Amount, user3.address)
+      await investor.buyOptionsWithYield()
+      await vault.connect(vaultController).endRound()
+      await vault.connect(vaultController).processQueuedDeposits([user3.address])
+      await vault.connect(vaultController).startRound()
+
+      const user3Shares = await vault.balanceOf(user3.address)
+      expect(user3Shares).to.be.eq(diffRefund)
+    })
+  })
+
   it('should be able to end a round even in a negative rebasing event', async () => {
     const user0DepositAmount = ethers.utils.parseEther('1').add(1)
     const user1DepositAmount = ethers.utils.parseEther('50')
@@ -664,6 +716,67 @@ describe('STETHVault', () => {
     expect(await vault.totalIdleAssets()).to.be.equal(0)
   })
 
+  it('should emit shares amount in Deposit Event accordingly', async () => {
+    // This test will only work if InvestRatio = 50%
+    const user0Amount = ethers.utils.parseEther('10')
+    const user1Amount = ethers.utils.parseEther('3')
+    const user2Amount = ethers.utils.parseEther('0.2')
+    const user3Amount = ethers.utils.parseEther('1')
+
+    // Round 0
+    const tx0 = await vault.connect(user0).deposit(user0Amount, user0.address)
+
+    // Get Emitted Shares
+    const filter0 = await vault.filters.Deposit(user0.address)
+    const events0 = await vault.queryFilter(filter0, tx0.blockNumber, tx0.blockNumber)
+    const emittedSharesBN1 = events0[0].args.shares
+
+    await vault.connect(vaultController).endRound()
+    await vault.connect(vaultController).processQueuedDeposits([user0.address])
+
+    // Round 1
+    await vault.connect(vaultController).startRound()
+    const amountOfSharesUser1After = await vault.balanceOf(user0.address)
+
+    expect(emittedSharesBN1).to.be.eq(amountOfSharesUser1After)
+
+    await asset.connect(yieldGenerator).transfer(vault.address, ethers.utils.parseEther('20'))
+    await vault.connect(vaultController).endRound()
+
+    // Round 2
+    await vault.connect(vaultController).startRound()
+    await vault.connect(user1).deposit(user1Amount, user1.address)
+    await vault.connect(user2).deposit(user2Amount, user2.address)
+    await asset.connect(yieldGenerator).transfer(vault.address, ethers.utils.parseEther('20'))
+    await asset.connect(yieldGenerator).transfer(investor.address, ethers.utils.parseEther('1300'))
+    await vault.connect(vaultController).endRound()
+    await vault.connect(vaultController).processQueuedDeposits([user1.address, user2.address])
+
+    // Round 3
+    await vault.connect(vaultController).startRound()
+    const tx1 = await vault.connect(user3).deposit(user3Amount, user3.address)
+
+    // Get Emitted Shares
+    const filter1 = await vault.filters.Deposit(user3.address)
+    const events1 = await vault.queryFilter(filter1, tx1.blockNumber, tx1.blockNumber)
+    const emittedSharesBN = events1[0].args.shares
+
+    const amountOfSharesUser3Before = await vault.balanceOf(user3.address)
+    expect(amountOfSharesUser3Before).to.be.eq(0)
+
+    await investor.buyOptionsWithYield()
+    const investorBalanceBeforeEndRound = await asset.balanceOf(await vault.investor())
+    expect(investorBalanceBeforeEndRound).to.be.closeTo(0, 1)
+
+    await vault.connect(vaultController).endRound()
+    await vault.connect(vaultController).processQueuedDeposits([user3.address])
+    await vault.connect(vaultController).startRound()
+
+    const amountOfSharesUser3After = await vault.balanceOf(user3.address)
+
+    expect(emittedSharesBN).to.be.eq(amountOfSharesUser3After)
+  })
+
   it('sanity check + startRound forgetting the process the queue case', async () => {
     // This test will only work if InvestRatio = 50%
     const user0Deposit = ethers.utils.parseEther('10')