Skip to content

Commit 83ca038

Browse files
committed
fix(StakeVault): make unstaking actually work
Unstaking didn't actually work because it was using `transferFrom()` on the `StakeVault` with the `from` address being the vault itself. This would result in an approval error because the vault isn't creating any approvals to spend its own funds. The solution is to use `transfer` instead, or rather `safeTransfer` using SafeERC20 utilities.
1 parent cf7a8b6 commit 83ca038

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed

contracts/StakeVault.sol

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,17 @@ pragma solidity ^0.8.18;
44

55
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
66
import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
7+
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
78
import { StakeManager } from "./StakeManager.sol";
89

910
/**
1011
* @title StakeVault
1112
* @author Ricardo Guilherme Schmidt <ricardo3@status.im>
1213
* @notice Secures user stake
1314
*/
14-
1515
contract StakeVault is Ownable {
16+
using SafeERC20 for ERC20;
17+
1618
error StakeVault__MigrationNotAvailable();
1719

1820
StakeManager private stakeManager;
@@ -28,7 +30,7 @@ contract StakeVault is Ownable {
2830
}
2931

3032
function stake(uint256 _amount, uint256 _time) external onlyOwner {
31-
STAKED_TOKEN.transferFrom(msg.sender, address(this), _amount);
33+
STAKED_TOKEN.safeTransferFrom(msg.sender, address(this), _amount);
3234
stakeManager.stake(_amount, _time);
3335

3436
emit Staked(msg.sender, address(this), _amount, _time);
@@ -40,7 +42,7 @@ contract StakeVault is Ownable {
4042

4143
function unstake(uint256 _amount) external onlyOwner {
4244
stakeManager.unstake(_amount);
43-
STAKED_TOKEN.transferFrom(address(this), msg.sender, _amount);
45+
STAKED_TOKEN.safeTransfer(msg.sender, _amount);
4446
}
4547

4648
function leave() external onlyOwner {

test/StakeManager.t.sol

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,24 @@ contract UnstakeTest is StakeManagerTest {
9797
vm.expectRevert(StakeManager.StakeManager__FundsLocked.selector);
9898
userVault.unstake(100);
9999
}
100+
101+
function test_UnstakeShouldReturnFunds() public {
102+
// ensure user has funds
103+
deal(stakeToken, testUser, 1000);
104+
StakeVault userVault = _createTestVault(testUser);
105+
106+
vm.startPrank(testUser);
107+
ERC20(stakeToken).approve(address(userVault), 100);
108+
109+
userVault.stake(100, 0);
110+
assertEq(ERC20(stakeToken).balanceOf(testUser), 900);
111+
112+
userVault.unstake(100);
113+
114+
assertEq(stakeManager.stakeSupply(), 0);
115+
assertEq(ERC20(stakeToken).balanceOf(address(userVault)), 0);
116+
assertEq(ERC20(stakeToken).balanceOf(testUser), 1000);
117+
}
100118
}
101119

102120
contract LockTest is StakeManagerTest {

0 commit comments

Comments
 (0)