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.

Sign up for GitHub

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

fix: max redeem rounding #83

Merged
merged 3 commits into from
Jan 31, 2024
Merged

fix: max redeem rounding #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad797d6
fix: use convert to shares
Schlagonia Jan 30, 2024
eefcd5c
fix: tests
Schlagonia Jan 30, 2024
3ae385f
test: add check
Schlagonia Jan 30, 2024
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
4 changes: 2 additions & 2 deletions .gitmodules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
[submodule "lib/openzeppelin-contracts"]
path = lib/openzeppelin-contracts
url = https://github.com/OpenZeppelin/openzeppelin-contracts
release = v4.8.2
branch = v4.8.2
release = v4.9.5
branch = v4.9.5
[submodule "lib/erc4626-tests"]
path = lib/erc4626-tests
url = https://github.com/a16z/erc4626-tests
Expand Down
2 changes: 1 addition & 1 deletion lib/openzeppelin-contracts
Open in desktop
Submodule openzeppelin-contracts updated 544 files
4 changes: 2 additions & 2 deletions src/TokenizedStrategy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,8 +935,8 @@ contract TokenizedStrategy {
maxRedeem_ = _balanceOf(S, owner);
} else {
maxRedeem_ = Math.min(
// Use preview withdraw to round up
_previewWithdraw(S, maxRedeem_),
// Can't redeem more than the balance.
_convertToShares(S, maxRedeem_),
_balanceOf(S, owner)
);
}
Expand Down
28 changes: 15 additions & 13 deletions src/test/CustomImplementation.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ contract CustomImplementationsTest is Setup {

// Make sure max withdraw and redeem return the correct amounts
assertEq(strategy.maxWithdraw(_address), idle);
assertEq(strategy.maxRedeem(_address), strategy.previewWithdraw(idle));
assertEq(strategy.maxRedeem(_address), strategy.convertToShares(idle));
assertLe(
strategy.maxRedeem(_address),
strategy.convertToAssets(strategy.maxRedeem(_address)),
strategy.availableWithdrawLimit(_address)
);

Expand All @@ -67,9 +67,9 @@ contract CustomImplementationsTest is Setup {

// Make sure max withdraw and redeem return the correct amounts
assertEq(strategy.maxWithdraw(_address), idle);
assertEq(strategy.maxRedeem(_address), strategy.previewWithdraw(idle));
assertEq(strategy.maxRedeem(_address), strategy.convertToShares(idle));
assertLe(
strategy.maxRedeem(_address),
strategy.convertToAssets(strategy.maxRedeem(_address)),
strategy.availableWithdrawLimit(_address)
);

Expand All @@ -82,18 +82,20 @@ contract CustomImplementationsTest is Setup {
strategy.withdraw(_amount, _address, _address);

uint256 before = asset.balanceOf(_address);
uint256 redeem = strategy.previewWithdraw(idle);
uint256 redeem = strategy.maxRedeem(_address);
uint256 conversion = strategy.convertToAssets(_amount);

vm.prank(_address);
strategy.redeem(redeem, _address, _address);

// We need to give a i wei rounding buffer
assertApproxEq(asset.balanceOf(_address) - before, idle, 1);
assertApproxEq(strategy.availableWithdrawLimit(_address), 0, 1);
assertApproxEq(strategy.maxWithdraw(_address), 0, 1);
assertApproxEq(strategy.maxRedeem(_address), 0, 1);
strategy.redeem(redeem, _address, _address, 0);

// We need to give 2 wei rounding buffer
assertApproxEq(strategy.convertToAssets(_amount), conversion, 2);
assertApproxEq(asset.balanceOf(_address) - before, idle, 2);
assertApproxEq(strategy.availableWithdrawLimit(_address), 0, 2);
assertApproxEq(strategy.maxWithdraw(_address), 0, 2);
assertApproxEq(strategy.maxRedeem(_address), 0, 2);
assertLe(
strategy.maxRedeem(_address),
strategy.convertToAssets(strategy.maxRedeem(_address)),
strategy.availableWithdrawLimit(_address)
);
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/e2e.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ contract e2eTest is Setup {
uint256 i;

for (i; i < toMake; ++i) {
asset = new ERC20Mock("Mock asset", "mcAsset", user, 0);
asset = new ERC20Mock();
yieldSource = new MockYieldSource(address(asset));
IMockStrategy newStrategy = IMockStrategy(setUpStrategy());

Expand Down Expand Up @@ -133,7 +133,7 @@ contract e2eTest is Setup {
uint256 i;

for (i; i < toMake; ++i) {
asset = new ERC20Mock("Mock asset", "mcAsset", user, 0);
asset = new ERC20Mock();
yieldSource = new MockYieldSource(address(asset));
IMockStrategy newStrategy = IMockStrategy(setUpStrategy());

Expand Down Expand Up @@ -247,7 +247,7 @@ contract e2eTest is Setup {
uint256 i;

for (i; i < toMake; ++i) {
asset = new ERC20Mock("Mock asset", "mcAsset", user, 0);
asset = new ERC20Mock();
yieldSource = new MockYieldSource(address(asset));
IMockStrategy newStrategy = IMockStrategy(setUpStrategy());

Expand Down
2 changes: 1 addition & 1 deletion src/test/utils/Setup.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ contract Setup is ExtendedTest, IEvents {
tokenizedStrategy = new TokenizedStrategy();

// create asset we will be using as the underlying asset
asset = new ERC20Mock("Mock asset", "mcAsset", user, 0);
asset = new ERC20Mock();

// create a mock yield source to deposit into
yieldSource = new MockYieldSource(address(asset));
Expand Down
Loading