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 Vest program #5987

Merged
merged 17 commits into from
Oct 4, 2019
Merged

Add Vest program #5987

merged 17 commits into from
Oct 4, 2019

Conversation

garious
Copy link
Contributor

@garious garious commented Sep 19, 2019

Problem

Using Budget to manage vesting schedules is a pain. We need to create 25 Budget contracts per vesting schedule (which there may be multiple per person) and then have the maintainer to send ApplyTimestamp to each of those contracts to get them to distribute tokens.

Summary of Changes

Add a new program Vest that manages a full vesting schedule. After creation, the creator should only interact with a specific contract in order to terminate it. Otherwise, it should simply publish dates to a single trusted account, an oracle account. To redeem tokens, the payee should send a RedeemTokens instruction to the vesting account, which would include the address of the oracle account.

  • Finish Vest program
  • Debate adding a DateTime program or tossing a DateTime account type into Vest
  • More branch coverage
  • Upload fun image

image

Fixes #5978

@garious
Copy link
Contributor Author

garious commented Sep 19, 2019

@rob-solana, something like this (note: very WIP!) is what I'm thinking should be in the genesis block. As a separate requirement, we can pair Vest accounts with Budget accounts that are frozen until a given date.

@rob-solana
Copy link
Contributor

this won't support staking unvested tokens?

@garious
Copy link
Contributor Author

garious commented Sep 19, 2019

@rob-solana, I'm open to adding that support. Have you already thought through how to do it?

let mut dt = None;
let mut days_back = 0;
while dt.is_none() {
dt = Utc
Copy link
Contributor

Choose a reason for hiding this comment

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

you can't use DTC in programs, wallclocks differ box-to-box

in other places where time passes, we use sysvar::clock... specify times in Slots

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The date comes from a trusted account, not from validators.

Copy link
Contributor

Choose a reason for hiding this comment

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

you're supposed to compare the witness with the oracle? why not vest in terms of slots?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm worried someone will speed up the clock

Copy link
Contributor

@rob-solana rob-solana Sep 25, 2019

Choose a reason for hiding this comment

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

yeah, but would that be that bad? I guess there's also the problem of setting the vesting schedule based on some target mainnet launch instant...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aeyakovenko, thoughts on a date oracle versus cluster time?

@rob-solana
Copy link
Contributor

@rob-solana, I'm open to adding that support. Have you already thought through how to do it?

it's kinda there in the stake program. in reading your description here, I realized there are some missing bits

stakes are initialized with lockup and custodian, a slot at which the tokens would be available for withdrawal, unless to the custodian

the purpose of the custodian is to allow the stake owner to return the tokens to someone who does KYC

I don't have a way to withdraw back to a company, but this could maybe be handled by solving this: #5988

@garious
Copy link
Contributor Author

garious commented Sep 19, 2019

@rob-solana, we can setup the Vest accounts to pay lamports into Stake accounts.

@garious garious force-pushed the see-my-vest branch 4 times, most recently from 3ae62ff to b797575 Compare September 28, 2019 03:10
@codecov
Copy link

codecov bot commented Sep 28, 2019

Codecov Report

Merging #5987 into master will increase coverage by 0.2%.
The diff coverage is 96.1%.

@@           Coverage Diff            @@
##           master   #5987     +/-   ##
========================================
+ Coverage    77.4%   77.7%   +0.2%     
========================================
  Files         209     214      +5     
  Lines       39982   40476    +494     
========================================
+ Hits        30953   31450    +497     
+ Misses       9029    9026      -3

@rob-solana
Copy link
Contributor

@rob-solana, we can setup the Vest accounts to pay lamports into Stake accounts.

one thing about putting vesting in stakes via Stake::lockup is that the tokens can be active on the network, I'm not sure if that's the right thing, though

@garious garious marked this pull request as ready for review October 3, 2019 21:05
CriesofCarrots
CriesofCarrots previously approved these changes Oct 4, 2019
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Functionality lgtm
r+ a couple comment nits
Also made a couple variable suggestions, take 'em or leave 'em

programs/vest_api/src/vest_instruction.rs Outdated Show resolved Hide resolved
programs/vest_api/src/vest_processor.rs Outdated Show resolved Hide resolved
programs/vest_api/src/vest_state.rs Outdated Show resolved Hide resolved
programs/vest_api/src/vest_processor.rs Outdated Show resolved Hide resolved
programs/vest_api/src/vest_schedule.rs Outdated Show resolved Hide resolved
@mergify mergify bot dismissed CriesofCarrots’s stale review October 4, 2019 20:21

Pull request has been modified.

@garious garious added the automerge Merge this Pull Request automatically once CI passes label Oct 4, 2019
@solana-grimes solana-grimes merged commit 5617162 into solana-labs:master Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Budget cumbersome for vesting schedules
5 participants