Skip to content

Commit

Permalink
Use Ownable in VestingWallet instead of an immutable beneficiary (#…
Browse files Browse the repository at this point in the history
…4508)

Co-authored-by: Francisco Giordano <fg@frang.io>
  • Loading branch information
ernestognw and frangio authored Aug 4, 2023
1 parent f715365 commit b81bec4
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 22 deletions.
5 changes: 5 additions & 0 deletions .changeset/healthy-gorillas-applaud.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': major
---

`VestingWallet`: Use `Ownable` instead of an immutable `beneficiary`.
37 changes: 17 additions & 20 deletions contracts/finance/VestingWallet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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;
}
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion test/finance/VestingWallet.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 3 additions & 1 deletion test/utils/Create2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down

0 comments on commit b81bec4

Please sign in to comment.