Skip to content
Prev Previous commit
Next Next commit
use Ownable instead of Ownable2Step and remove onlyOwner
  • Loading branch information
frangio committed Aug 4, 2023
commit 42a454edd9eb6c0506d74bbe64ab3567993b3df2
25 changes: 9 additions & 16 deletions contracts/finance/VestingWallet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,20 @@ import {IERC20} from "../token/ERC20/IERC20.sol";
import {SafeERC20} from "../token/ERC20/utils/SafeERC20.sol";
import {Address} from "../utils/Address.sol";
import {Context} from "../utils/Context.sol";
import {Ownable2Step, Ownable} from "../access/Ownable2Step.sol";
import {Ownable} from "../access/Ownable.sol";

/**
* @title VestingWallet
* @dev Handles the vesting of native currency and ERC20 tokens for a given beneficiary who gets the ownership of the
* contract by calling {Ownable2Step-acceptOwnership} to accept a 2-step ownership transfer setup at deployment with
* {Ownable2Step-transferOwnership}. The initial owner of this contract is the benefactor (`msg.sender`), enabling them
* to recover any unclaimed tokens.
* @dev A vesting wallet is an ownable contract that can receive native currency and ERC20 tokens, and release these
* assets to the wallet owner, also referred to as "beneficiary", according to a vesting schedule.
*
* Custody of multiple tokens can be given to this contract, which will release the tokens to the beneficiary following
* a given vesting schedule that is customizable through the {vestedAmount} function.
*
* Any token transferred to this contract will follow the vesting schedule as if they were locked from the beginning.
* Any assets transferred to this contract will follow the vesting schedule as if they were locked from the beginning.
* Consequently, if the vesting has already started, any amount of tokens sent to this contract will (at least partly)
* be immediately releasable.
*
* By setting the duration to 0, one can configure this contract to behave like an asset timelock that hold tokens for
* a beneficiary until a specified time.
*/
contract VestingWallet is Context, Ownable2Step {
contract VestingWallet is Context, Ownable {
event EtherReleased(uint256 amount);
event ERC20Released(address indexed token, uint256 amount);

Expand All @@ -43,11 +37,10 @@ contract VestingWallet is Context, Ownable2Step {
* @dev Sets the sender as the initial owner, the beneficiary as the pending owner, the start timestamp and the
* vesting duration of the vesting wallet.
*/
constructor(address beneficiaryAddress, uint64 startTimestamp, uint64 durationSeconds) payable Ownable(msg.sender) {
if (beneficiaryAddress == address(0)) {
constructor(address beneficiary, uint64 startTimestamp, uint64 durationSeconds) payable Ownable(beneficiary) {
if (beneficiary == address(0)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe not in the PR, but we should remove that check here and move it to Ownable's constructor

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree we should reconsider this. I opened an issue
#4511

revert VestingWalletInvalidBeneficiary(address(0));
}
transferOwnership(beneficiaryAddress);

_start = startTimestamp;
_duration = durationSeconds;
Expand Down Expand Up @@ -113,7 +106,7 @@ contract VestingWallet is Context, Ownable2Step {
*
* Emits a {EtherReleased} event.
*/
function release() public virtual onlyOwner {
function release() public virtual {
uint256 amount = releasable();
_released += amount;
emit EtherReleased(amount);
Expand All @@ -125,7 +118,7 @@ contract VestingWallet is Context, Ownable2Step {
*
* Emits a {ERC20Released} event.
*/
function release(address token) public virtual onlyOwner {
function release(address token) public virtual {
uint256 amount = releasable(token);
_erc20Released[token] += amount;
emit ERC20Released(token, amount);
Expand Down