Merge pull request #5 from yieldnest/test/ynBNB-mainnet-extra-tests

add fuzz test to verify deposit vs withdrawal
yieldnest · Sep 14, 2024 · bd467b0 · bd467b0
2 parents cb57f84 + 328c907
commit bd467b0
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 6 deletions.
diff --git a/README.md b/README.md
@@ -100,3 +100,8 @@ export EXECUTOR_2="your_executor_2"
 make single-vault
 ```
 
+## Security Notes
+
+There's a loss incurred by the user that's roughly less than `Max(amount, rewards) / 1e18` when he withdraws the exact same shares he received  when compared to what he deposited.
+
+I believe this is  because of that decimal offset in OZ 46426Upgradeable to prevent donation attacks. The loss maxes out at 10000 wei for 10000 ether amounts so it's small.
diff --git a/test/single/integration/ynBNB.bsc.t.sol b/test/single/integration/ynBNB.bsc.t.sol
@@ -19,10 +19,10 @@ contract KslisBNB_Test is Test, BscActors, BscContracts, ynBNBConstants, AssetHe
     VaultFactory public factory;
     SingleVault public ynBNB;
     IERC20 public slis;
-    IERC4626 kslis = IERC4626(KARAK_KslisBNB);
-    IKarakVaultSupervisor ksup = IKarakVaultSupervisor(KARAK_VAULT_SUPERVISOR);
+    IERC4626 public kslis = IERC4626(KARAK_KslisBNB);
+    IKarakVaultSupervisor public ksup = IKarakVaultSupervisor(KARAK_VAULT_SUPERVISOR);
 
-    address USER = 0x0c099101d43e9094E4ae9bC2FC38f8b9875c23c5;
+    address public USER = 0x0c099101d43e9094E4ae9bC2FC38f8b9875c23c5;
 
     modifier onlyBsc() {
         if (block.chainid != 56) return;
@@ -31,7 +31,7 @@ contract KslisBNB_Test is Test, BscActors, BscContracts, ynBNBConstants, AssetHe
 
     function setUp() public onlyBsc {
         slis = IERC20(slisBNB);
-        get_slisBNB(address(this), 10_000 ether);
+        get_slisBNB(address(this), 100_000 ether);
         factory = VaultFactory(address(VAULT_FACTORY));
         slis.approve(address(factory), 1 ether);
         slis.transfer(address(factory), 1 ether);
@@ -90,4 +90,45 @@ contract KslisBNB_Test is Test, BscActors, BscContracts, ynBNBConstants, AssetHe
         slis.approve(address(kslis), 1 ether);
         ksup.deposit(address(kslis), 1 ether, 1 ether);
     }
+
+    function test_ynBNB_deposit_withdraw_sameAmount_fuzz(uint256 amount, uint256 rewards) public onlyBsc {
+        address user = USER;
+        vm.assume(amount > 0 && amount <= 10000 ether);
+        vm.assume(rewards >= 0 && rewards <= 10000 ether);
+
+        slis.approve(user, amount);
+        slis.transfer(user, amount);
+
+        slis.transfer(address(ynBNB), rewards);
+
+        vm.startPrank(user);
+        slis.approve(address(ynBNB), amount);
+        uint256 shares = ynBNB.deposit(amount, user);
+
+        assertEq(ynBNB.totalSupply(), shares + 1 ether, "Total supply should equal shares plus initial 1 ether");
+        assertEq(
+            ynBNB.totalAssets(), amount + rewards + 1 ether, "Total assets should equal shares plus initial 1 ether"
+        );
+        vm.stopPrank();
+
+        // Withdraw shares and assert received amount
+        vm.startPrank(user);
+        uint256 preWithdrawBalance = slis.balanceOf(user);
+        uint256 assetsReceived = ynBNB.redeem(shares, user, user);
+        uint256 postWithdrawBalance = slis.balanceOf(user);
+
+        assertEq(
+            postWithdrawBalance - preWithdrawBalance, assetsReceived, "User balance delta should equal assetsReceived"
+        );
+
+        assertGe(amount, assetsReceived, "Amount deposited should be greater than or equal to assets received");
+
+        uint256 assetLoss = amount - assetsReceived;
+        uint256 maxLoss = (amount > rewards ? amount : rewards) / 1e18;
+        assertLe(assetLoss, maxLoss + 2, "Asset loss should be less than or equal to maxLoss");
+
+        assertEq(ynBNB.totalSupply(), 1 ether, "Total supply should return to initial 1 ether");
+        assertEq(ynBNB.totalAssets(), rewards + 1 ether + assetLoss, "Total assets should return to initial 1 ether");
+        vm.stopPrank();
+    }
 }
diff --git a/test/single/unit/invariants.sol b/test/single/unit/invariants.sol
@@ -16,7 +16,7 @@ contract SingleInvariantTests is Test, LocalActors, TestConstants {
 
     SingleVault public vault;
     MockERC20 public asset;
-    address USER = address(33);
+    address public USER = address(33);
 
     function setUp() public {
         vm.startPrank(ADMIN);

diff --git a/test/single/unit/upgrade.t.sol b/test/single/unit/upgrade.t.sol
@@ -32,7 +32,12 @@ contract SingleVaultUpgradeTests is Test, LocalActors, TestConstants {
         factory = IVaultFactory(setup.factory());
     }
 
-    function testUpgrade() public {
+    modifier onlyLocal() {
+        if (block.chainid != 31337) return;
+        _;
+    }
+
+    function testUpgrade() public onlyLocal {
         vm.startPrank(ADMIN);
         SingleVault newVault = new MockSingleVault();