Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add fuzz test to verify deposit vs withdrawal #5

Merged
merged 3 commits into from
Sep 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
49 changes: 45 additions & 4 deletions test/single/integration/ynBNB.bsc.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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();
}
}
2 changes: 1 addition & 1 deletion test/single/unit/invariants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion test/single/unit/upgrade.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down