From 83b0e9f49afa9ad9bb35eb71da2c02de01862aa2 Mon Sep 17 00:00:00 2001 From: bbenligiray Date: Sun, 6 Jun 2021 18:52:53 +0300 Subject: [PATCH] Remove the destination argument from the withdrawal methods --- packages/pool/contracts/StakeUtils.sol | 9 ++++---- packages/pool/contracts/TransferUtils.sol | 23 +++++-------------- .../pool/contracts/interfaces/IStakeUtils.sol | 2 +- .../contracts/interfaces/ITransferUtils.sol | 11 ++------- packages/pool/test/StakeUtils.sol.js | 2 +- packages/pool/test/TransferUtils.sol.js | 14 +++++------ 6 files changed, 21 insertions(+), 40 deletions(-) diff --git a/packages/pool/contracts/StakeUtils.sol b/packages/pool/contracts/StakeUtils.sol index 358ee8c4..b36c26e4 100644 --- a/packages/pool/contracts/StakeUtils.sol +++ b/packages/pool/contracts/StakeUtils.sol @@ -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); } } diff --git a/packages/pool/contracts/TransferUtils.sol b/packages/pool/contracts/TransferUtils.sol index f277425c..a8408173 100644 --- a/packages/pool/contracts/TransferUtils.sol +++ b/packages/pool/contracts/TransferUtils.sol @@ -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 @@ -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 { @@ -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 ) @@ -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 ); } diff --git a/packages/pool/contracts/interfaces/IStakeUtils.sol b/packages/pool/contracts/interfaces/IStakeUtils.sol index 3fd6e5cd..03534af3 100644 --- a/packages/pool/contracts/interfaces/IStakeUtils.sol +++ b/packages/pool/contracts/interfaces/IStakeUtils.sol @@ -34,6 +34,6 @@ interface IStakeUtils is ITransferUtils{ external returns (uint256); - function unstakeAndWithdraw(address destination) + function unstakeAndWithdraw() external; } diff --git a/packages/pool/contracts/interfaces/ITransferUtils.sol b/packages/pool/contracts/interfaces/ITransferUtils.sol index 0b2d9646..27f95766 100644 --- a/packages/pool/contracts/interfaces/ITransferUtils.sol +++ b/packages/pool/contracts/interfaces/ITransferUtils.sol @@ -11,7 +11,6 @@ interface ITransferUtils is IDelegationUtils{ event Withdrawn( address indexed user, - address destination, uint256 amount ); @@ -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( @@ -42,9 +38,6 @@ interface ITransferUtils is IDelegationUtils{ external returns (bool finished); - function withdrawPrecalculated( - address destination, - uint256 amount - ) + function withdrawPrecalculated(uint256 amount) external; } diff --git a/packages/pool/test/StakeUtils.sol.js b/packages/pool/test/StakeUtils.sol.js index 12a2e525..1fad83e1 100644 --- a/packages/pool/test/StakeUtils.sol.js +++ b/packages/pool/test/StakeUtils.sol.js @@ -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( diff --git a/packages/pool/test/TransferUtils.sol.js b/packages/pool/test/TransferUtils.sol.js index c6322bc6..abd6c98f 100644 --- a/packages/pool/test/TransferUtils.sol.js +++ b/packages/pool/test/TransferUtils.sol.js @@ -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 @@ -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"); }); }); @@ -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"); }); }); @@ -272,10 +272,10 @@ 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 () { @@ -283,7 +283,7 @@ describe("withdrawPrecalculated", 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"); }); });