Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add a VestingWallet #2748

Merged
merged 26 commits into from
Oct 18, 2021
Merged

Add a VestingWallet #2748

merged 26 commits into from
Oct 18, 2021

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Jul 1, 2021

Fixes #1214

Add a vesting wallet with a default linear schedule. The vesting schedule can be customized by overriding a single function.

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

contracts/finance/VestingWallet.sol Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
test/finance/VestingWallet.test.js Outdated Show resolved Hide resolved
@frangio frangio changed the title Add a VestingWallet Add a ERC20VestingWallet Jul 5, 2021
@frangio
Copy link
Contributor

frangio commented Jul 5, 2021

We should check if the contract we're proposing supports the use cases listed in the comments in #1214.

@Amxx
Copy link
Collaborator Author

Amxx commented Jul 6, 2021

We should check if the contract we're proposing supports the use cases listed in the comments in #1214.

  • "Graded vesting" can be implemented by overriding the release function with any shape/curve
  • Monthly” & non-linear basis can also be implemented by overriding the release function. I have code for that, which is not part of the "small" PR but that we used to test.

@Amxx Amxx dismissed a stale review via bb8393f July 26, 2021 15:56
@frangio
Copy link
Contributor

frangio commented Sep 2, 2021

We've decided to offer a single VestingWallet for both ERC20 and native token. The release function should operate the native token if the token address is a specific value to represent it, possibly 0x00...00 or 0xee...ee. Personally I'm leaning towards 0x0 because the "e" doesn't represent native token in other networks, and because the zero address is easily available as a constant in Ethers.js. Additionally there should be a separate release function for the native token, ideally with a different name so it doesn't cause function overloading.

@Amxx Amxx changed the title Add a ERC20VestingWallet Add a VestingWallet Sep 2, 2021
@Amxx Amxx mentioned this pull request Sep 10, 2021
1 task
@NeoXtreem
Copy link

@frangio What's the planned release date for this VestingWallet?

contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
contracts/finance/README.adoc Outdated Show resolved Hide resolved
contracts/finance/README.adoc Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
@Amxx Amxx requested a review from frangio October 13, 2021 20:00
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
contracts/finance/VestingWallet.sol Outdated Show resolved Hide resolved
test/finance/VestingWallet.behavior.js Outdated Show resolved Hide resolved
@frangio frangio merged commit 88e4b69 into OpenZeppelin:master Oct 18, 2021
@Amxx Amxx deleted the feature/vesting branch October 18, 2021 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make TokenVesting more aligned with usual requirements
3 participants