diff --git a/.changeset/healthy-gorillas-applaud.md b/.changeset/healthy-gorillas-applaud.md new file mode 100644 index 00000000000..1d4156ebfae --- /dev/null +++ b/.changeset/healthy-gorillas-applaud.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': major +--- + +`VestingWallet`: Use `Ownable` instead of an immutable `beneficiary`. diff --git a/contracts/finance/VestingWallet.sol b/contracts/finance/VestingWallet.sol index 2e8fdab8255..1edb0113ef0 100644 --- a/contracts/finance/VestingWallet.sol +++ b/contracts/finance/VestingWallet.sol @@ -6,24 +6,28 @@ 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 {Ownable} from "../access/Ownable.sol"; /** - * @title VestingWallet - * @dev This contract handles the vesting of Eth and ERC20 tokens for a given beneficiary. Custody of multiple tokens - * can be given to this contract, which will release the token to the beneficiary following a given vesting schedule. - * The vesting schedule is customizable through the {vestedAmount} function. + * @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. * - * 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. * + * NOTE: Since the wallet is {Ownable}, and ownership can be transferred, it is possible to sell unvested tokens. + * Preventing this in a smart contract is difficult, considering that: 1) a beneficiary address could be a + * counterfactually deployed contract, 2) there is likely to be a migration path for EOAs to become contracts in the + * near future. + * * NOTE: When using this contract with any token whose balance is adjusted automatically (i.e. a rebase token), make sure * to account the supply/balance adjustment in the vesting schedule to ensure the vested amount is as intended. */ -contract VestingWallet is Context { +contract VestingWallet is Context, Ownable { event EtherReleased(uint256 amount); event ERC20Released(address indexed token, uint256 amount); @@ -34,18 +38,18 @@ contract VestingWallet is Context { uint256 private _released; mapping(address => uint256) private _erc20Released; - address private immutable _beneficiary; uint64 private immutable _start; uint64 private immutable _duration; /** - * @dev Set the beneficiary, start timestamp and vesting duration of the vesting wallet. + * @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 { - if (beneficiaryAddress == address(0)) { + constructor(address beneficiary, uint64 startTimestamp, uint64 durationSeconds) payable Ownable(beneficiary) { + if (beneficiary == address(0)) { revert VestingWalletInvalidBeneficiary(address(0)); } - _beneficiary = beneficiaryAddress; + _start = startTimestamp; _duration = durationSeconds; } @@ -55,13 +59,6 @@ contract VestingWallet is Context { */ receive() external payable virtual {} - /** - * @dev Getter for the beneficiary address. - */ - function beneficiary() public view virtual returns (address) { - return _beneficiary; - } - /** * @dev Getter for the start timestamp. */ @@ -121,7 +118,7 @@ contract VestingWallet is Context { uint256 amount = releasable(); _released += amount; emit EtherReleased(amount); - Address.sendValue(payable(beneficiary()), amount); + Address.sendValue(payable(owner()), amount); } /** @@ -133,7 +130,7 @@ contract VestingWallet is Context { uint256 amount = releasable(token); _erc20Released[token] += amount; emit ERC20Released(token, amount); - SafeERC20.safeTransfer(IERC20(token), beneficiary(), amount); + SafeERC20.safeTransfer(IERC20(token), owner(), amount); } /** diff --git a/test/finance/VestingWallet.test.js b/test/finance/VestingWallet.test.js index 91ca04da06b..d79aea1955a 100644 --- a/test/finance/VestingWallet.test.js +++ b/test/finance/VestingWallet.test.js @@ -29,7 +29,7 @@ contract('VestingWallet', function (accounts) { }); it('check vesting contract', async function () { - expect(await this.mock.beneficiary()).to.be.equal(beneficiary); + expect(await this.mock.owner()).to.be.equal(beneficiary); expect(await this.mock.start()).to.be.bignumber.equal(this.start); expect(await this.mock.duration()).to.be.bignumber.equal(duration); expect(await this.mock.end()).to.be.bignumber.equal(this.start.add(duration)); diff --git a/test/utils/Create2.test.js b/test/utils/Create2.test.js index 4215e37c0ca..336ab1acc23 100644 --- a/test/utils/Create2.test.js +++ b/test/utils/Create2.test.js @@ -59,7 +59,9 @@ contract('Create2', function (accounts) { addr: offChainComputed, }); - expect(await VestingWallet.at(offChainComputed).then(instance => instance.beneficiary())).to.be.equal(other); + const instance = await VestingWallet.at(offChainComputed); + + expect(await instance.owner()).to.be.equal(other); }); it('deploys a contract with funds deposited in the factory', async function () {