diff --git a/README.md b/README.md index 6ded122..d6d09aa 100644 --- 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. \ No newline at end of file diff --git a/test/single/integration/ynBNB.bsc.t.sol b/test/single/integration/ynBNB.bsc.t.sol index 27f2565..1167887 100644 --- 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 index 030b989..3d06fd9 100644 --- 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 index f123f8d..413b9ed 100644 --- 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();