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: withdraw #647

Merged
merged 3 commits into from
Aug 22, 2024
Merged

fix: withdraw #647

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
1c3932c
fix: withdraw
0xOneTony Aug 20, 2024
e621cb1
fix: improvement
0xOneTony Aug 20, 2024
ea2d1b9
fix: linting
0xOneTony Aug 22, 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
3 changes: 3 additions & 0 deletions contracts/core/interfaces/IBaseStrategy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ interface IBaseStrategy {
/// @notice Error when the pool ID is invalid
error BaseStrategy_INVALID_POOL_ID();

/// @notice Error when the withdraw amount leaves the pool with insufficient funds
error BaseStrategy_WITHDRAW_MORE_THAN_POOL_AMOUNT();

/// ======================
/// ======= Events =======
/// ======================
Expand Down
5 changes: 5 additions & 0 deletions contracts/strategies/CoreBaseStrategy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "../core/interfaces/IBaseStrategy.sol";

/// Libraries
import {Transfer} from "./../core/libraries/Transfer.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

/// @title BaseStrategy Contract
/// @notice This contract is the base contract for all strategies
Expand Down Expand Up @@ -107,6 +108,10 @@ abstract contract CoreBaseStrategy is IBaseStrategy, Transfer {
onlyPoolManager(msg.sender)
{
_beforeWithdraw(_token, _amount, _recipient);
// If the token is the pool token, revert if the amount is greater than the pool amount
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case the strategy is keeping donation tokens in the strategy then the pool managers will be able to withdraw those donation tokens, this is important to note.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this makes sense for being the default behaviour

if (_getBalance(_token, address(this)) - _amount < poolAmount) {
revert BaseStrategy_WITHDRAW_MORE_THAN_POOL_AMOUNT();
}
_transferAmount(_token, _recipient, _amount);
_afterWithdraw(_token, _amount, _recipient);

Expand Down
30 changes: 30 additions & 0 deletions test/foundry/strategies/CoreBaseStrategy.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pragma solidity 0.8.19;

import "forge-std/Test.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

// Test libraries
import {AlloSetup} from "../shared/AlloSetup.sol";
Expand Down Expand Up @@ -43,4 +44,33 @@ contract CoreBaseStrategyTest is Test, AlloSetup {
strategy.increasePoolAmount(100);
assertEq(strategy.getPoolAmount(), 100);
}

function test_withdraw() public {
vm.mockCall(address(allo()), abi.encodeWithSelector(IAllo.isPoolManager.selector), abi.encode(true));

/// Increase pool amount
vm.prank(address(allo()));
strategy.increasePoolAmount(100);

address _token = allo().getPool(0).token;

vm.mockCall(_token, abi.encodeWithSelector(IERC20.balanceOf.selector, address(strategy)), abi.encode(150));
strategy.withdraw(_token, 50, address(this));

assertEq(strategy.getPoolAmount(), 100);
}

function testRevert_withdrawMoreThanPoolAmount() public {
vm.mockCall(address(allo()), abi.encodeWithSelector(IAllo.isPoolManager.selector), abi.encode(true));

/// Increase pool amount
vm.prank(address(allo()));
strategy.increasePoolAmount(100);

address _token = allo().getPool(0).token;

vm.mockCall(_token, abi.encodeWithSelector(IERC20.balanceOf.selector, address(strategy)), abi.encode(100));
vm.expectRevert(IBaseStrategy.BaseStrategy_WITHDRAW_MORE_THAN_POOL_AMOUNT.selector);
strategy.withdraw(_token, 50, address(this));
}
}
Loading