Skip to content
New issue

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

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

Already on GitHub? # to your account

Restrict withdrawal destination #285

Merged
merged 1 commit into from
Jun 6, 2021
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
9 changes: 4 additions & 5 deletions packages/pool/contracts/StakeUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,15 @@ abstract contract StakeUtils is TransferUtils, IStakeUtils {
return unstakeAmount;
}

/// @notice Convenience method to execute an unstake and withdraw in a
/// single transaction
/// @notice Convenience method to execute an unstake and withdraw to the
/// user's wallet in a single transaction
/// @dev Note that withdraw may revert because the user may have less than
/// `unstaked` tokens that are withdrawable
/// @param destination Token transfer destination
function unstakeAndWithdraw(address destination)
function unstakeAndWithdraw()
external
override
{
uint256 unstaked = unstake(msg.sender);
withdrawRegular(destination, unstaked);
withdrawRegular(unstaked);
}
}
23 changes: 6 additions & 17 deletions packages/pool/contracts/TransferUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,18 @@ abstract contract TransferUtils is DelegationUtils, ITransferUtils {
);
}

/// @notice Called by the user to withdraw tokens
/// @notice Called by the user to withdraw tokens to their wallet
/// @dev The user should call `userLocked()` beforehand to ensure that
/// they have at least `amount` unlocked tokens to withdraw.
/// The method is named `withdrawRegular()` to be consistent with the name
/// `depositRegular()`. See `depositRegular()` for more context.
/// @param destination Token transfer destination
/// @param amount Amount to be withdrawn
function withdrawRegular(
address destination,
uint256 amount
)
function withdrawRegular(uint256 amount)
public
override
{
mintReward();
withdraw(destination, amount, userLocked(msg.sender));
withdraw(amount, userLocked(msg.sender));
}

/// @notice Called to calculate the locked tokens of a user by making
Expand Down Expand Up @@ -119,12 +115,8 @@ abstract contract TransferUtils is DelegationUtils, ITransferUtils {
/// is calculated with repeated calls to `precalculateUserLocked()`
/// @dev Only use `precalculateUserLocked()` and this method if
/// `withdrawRegular()` hits the block gas limit
/// @param destination Token transfer destination
/// @param amount Amount to be withdrawn
function withdrawPrecalculated(
address destination,
uint256 amount
)
function withdrawPrecalculated(uint256 amount)
external
override
{
Expand All @@ -135,16 +127,14 @@ abstract contract TransferUtils is DelegationUtils, ITransferUtils {
state.initialIndEpoch == currentEpoch,
"Pool: Locked not precalculated"
);
withdraw(destination, amount, state.locked);
withdraw(amount, state.locked);
}

/// @notice Called internally after the amount of locked tokens of the user
/// is determined
/// @param destination Token transfer destination
/// @param amount Amount to be withdrawn
/// @param userLocked Amount of locked tokens of the user
function withdraw(
address destination,
uint256 amount,
uint256 userLocked
)
Expand All @@ -166,10 +156,9 @@ abstract contract TransferUtils is DelegationUtils, ITransferUtils {
user.unstaked = user.unstaked - amount;
// Should never return false because the API3 token uses the
// OpenZeppelin implementation
assert(api3Token.transfer(destination, amount));
assert(api3Token.transfer(msg.sender, amount));
emit Withdrawn(
msg.sender,
destination,
amount
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/pool/contracts/interfaces/IStakeUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ interface IStakeUtils is ITransferUtils{
external
returns (uint256);

function unstakeAndWithdraw(address destination)
function unstakeAndWithdraw()
external;
}
11 changes: 2 additions & 9 deletions packages/pool/contracts/interfaces/ITransferUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ interface ITransferUtils is IDelegationUtils{

event Withdrawn(
address indexed user,
address destination,
uint256 amount
);

Expand All @@ -29,10 +28,7 @@ interface ITransferUtils is IDelegationUtils{
function depositRegular(uint256 amount)
external;

function withdrawRegular(
address destination,
uint256 amount
)
function withdrawRegular(uint256 amount)
external;

function precalculateUserLocked(
Expand All @@ -42,9 +38,6 @@ interface ITransferUtils is IDelegationUtils{
external
returns (bool finished);

function withdrawPrecalculated(
address destination,
uint256 amount
)
function withdrawPrecalculated(uint256 amount)
external;
}
2 changes: 1 addition & 1 deletion packages/pool/test/StakeUtils.sol.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ describe("unstakeAndWithdraw", function () {
genesisEpochPlusTwo.mul(EPOCH_LENGTH).toNumber(),
]);
// Unstake and withdraw
await api3Pool.connect(roles.user1).unstakeAndWithdraw(roles.user1.address);
await api3Pool.connect(roles.user1).unstakeAndWithdraw();
const user = await api3Pool.users(roles.user1.address);
expect(user.unstaked).to.equal(ethers.BigNumber.from(0));
expect(await api3Token.balanceOf(roles.user1.address)).to.equal(
Expand Down
14 changes: 7 additions & 7 deletions packages/pool/test/TransferUtils.sol.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ describe("withdrawRegular", function () {
await expect(
api3Pool
.connect(roles.user1)
.withdrawRegular(roles.user1.address, unlocked)
.withdrawRegular(unlocked)
)
.to.emit(api3Pool, "Withdrawn")
.withArgs(roles.user1.address, roles.user1.address, unlocked);
.withArgs(roles.user1.address, unlocked);
const userAfter = await api3Pool.users(roles.user1.address);
expect(await api3Pool.userLocked(roles.user1.address)).to.equal(
userAfter.unstaked
Expand All @@ -118,7 +118,7 @@ describe("withdrawRegular", function () {
await expect(
api3Pool
.connect(roles.user1)
.withdrawRegular(roles.user1.address, ethers.BigNumber.from(1))
.withdrawRegular(ethers.BigNumber.from(1))
).to.be.revertedWith("Pool: Not enough unstaked funds");
});
});
Expand All @@ -127,7 +127,7 @@ describe("withdrawRegular", function () {
await expect(
api3Pool
.connect(roles.user1)
.withdrawRegular(roles.user1.address, ethers.BigNumber.from(1))
.withdrawRegular(ethers.BigNumber.from(1))
).to.be.revertedWith("Pool: Not enough unlocked funds");
});
});
Expand Down Expand Up @@ -272,18 +272,18 @@ describe("withdrawPrecalculated", function () {
await expect(
api3Pool
.connect(roles.user1)
.withdrawPrecalculated(roles.user1.address, user1Stake)
.withdrawPrecalculated(user1Stake)
)
.to.emit(api3Pool, "Withdrawn")
.withArgs(roles.user1.address, roles.user1.address, user1Stake);
.withArgs(roles.user1.address, user1Stake);
});
});
context("Locked amount is not precalculated", function () {
it("reverts", async function () {
await expect(
api3Pool
.connect(roles.user1)
.withdrawPrecalculated(roles.user1.address, ethers.BigNumber.from(1))
.withdrawPrecalculated(ethers.BigNumber.from(1))
).to.be.revertedWith("Pool: Locked not precalculated");
});
});
Expand Down