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

Module Development Task: Vesting Escrow Wallet #269

Closed
CPSTL opened this issue Sep 19, 2018 · 33 comments
Closed

Module Development Task: Vesting Escrow Wallet #269

CPSTL opened this issue Sep 19, 2018 · 33 comments
Labels
Module development Used when new module is created open-source community Something suggest or developed by external developers

Comments

@CPSTL
Copy link
Contributor

CPSTL commented Sep 19, 2018

Module Development Task

Vesting Escrow Wallet

Module Type Bounty Payout Completion deadline
Wallet 5, 000 POLY 14 days to complete once task is confirmed by external developer(s).

Short Summary

As an Issuer, I can program an automated token vesting schedule for employees and/or affiliates so that Tokens get delivered to their wallets as contractually defined.

This would be a smart contract that the issuer can send tokens to and then select the address that would be able to withdraw them according to their specific vesting schedule.

Bounty Requirements

  1. Module specs are fulfilled
  2. Module is implemented using appropriate module interface
  3. Module is tested with >95% branch coverage
  4. Module is delivered before deadline
  5. Module does not present any bugs.
  6. 14 days to complete module from developers initial start date.

IMPORTANT: Please work from the development-1.5.0 branch instead of master.

Module Details and Specs (Wireframe)

Wireframe Link: https://www.lucidchart.com/invitations/accept/450646a3-5af0-42b5-b155-de5ec94232d4

Assumptions & User/Issuer Stories

  1. As an issuer I want to be able to add a module for vesting schedule to my STO

  2. As an issuer I want to add vesting schedules to employees / affiliates I have minted tokens so that I can incentivize them for the long term

  3. As an issuer I want to add vesting schedules to employees / affiliates I have not minted tokens so that I can incentive new employees / affiliates for the long term

  4. As an issuer I want to be able to create a template of a vesting schedule so that I can easily apply them to each of the employees / affiliates

  5. As an issuer I want to be able to bulk add vesting schedules for each of my employees / affiliates so that I don't have to manually do it for each individual one

  6. As an issuer I want to be able to void a vesting schedule so that I can stop the automated vesting if an employee / affiliate leaves

  7. As an issuer I want to be able to add multiple vesting schedules for each employee / affiliate so that I can issue out tokens as bonus and continue to incentivize them for the long term

  8. As an issuer I want to be able to view all my vesting schedules and it's progress so I can have a quick overview on how much tokens each of my employees / affiliates have

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 5000.0 POLY (749.5 USD @ $0.15/POLY) attached to it.

@gitcoinbot
Copy link

gitcoinbot commented Sep 21, 2018

Issue Status: 1. Open 2. Cancelled


Work has been started.

These users each claimed they can complete the work by 4 months, 2 weeks ago.
Please review their action plans below:

1) shanefontaine has been approved to start work.

The following are steps that will be taken to complete this task:

  • Full understand all expected roles of each participant
  • Begin to implement issuer abilities
  • Create vesting schedules/timing funcitonality
  • Create functionality to void vesting schedules
  • Create functionality to add data/schedules in bulk (saving on gas and transaction count)
  • Add any additional features required for the issuer
  • Add progress tracking abilities (in the form of variables or functions)
  • Write programmatic tests to cover 100% of the contract's functionality
  • Manually check contract for known Solidity bugs/issues/exploits
  • Use Consensys' Mythril tool as a final sanity check

The following are questions that I have related to the task:

  • Is the distribution of tokens a push or pull action (does the issuer push out the tokens each time, or are the employees / affiliates required to pull the tokens when they become available?
  • Does this contract need to be designed to be upgradable?
  • Is the vesting schedule have a consistent disbursement rate (e.g. 1% of tokens every day for 100 days) or will they all be variable?
  • Do the vesting tokens have a cliff?

Note: I have written a very similar contract before and have experience with this kind of smart contract.

Learn more on the Gitcoin Issue Details page.

@satyamakgec
Copy link
Contributor

@shanefontaine Sounds like a plan from your side.
Answers to your queries -

  • Employees/Affiliates can pull the tokens when they are available.

  • As per our current ecosystem, we don't have the upgradeable modules so I prefer you can follow the same standard as we have in our existing module ecosystem.

  • Vesting schedule is variable for some employees it will be 4 yrs some for 2 yrs and some for 1 yr or 5 yrs and according to that they have different disbursement rate quaterly, halferly, Annualy etc.

  • Yes, vesting tokens have the cliff you can check the specs. In specs, it is written as Vesting Frequency.

If you have more queries then please let me know.

@satyamakgec satyamakgec added Module development Used when new module is created open-source community Something suggest or developed by external developers labels Sep 22, 2018
@shanefontaine
Copy link

@satyamakgec Am I approved and allowed to proceed on this work?

@satyamakgec
Copy link
Contributor

@CPSTL please put some light here.

@CPSTL
Copy link
Contributor Author

CPSTL commented Sep 23, 2018

@shanefontaine Please let me know if you have any questions as you work towards this task.

@shanefontaine
Copy link

shanefontaine commented Sep 26, 2018

@CPSTL @satyamakgec A few questions:

Business Logic

  1. Will the number of tokens in a specific vesting schedule ever increase (will you add more tokens into an employee's current vesting contract)?
  2. Are all tokens going to be granted in whole units (10^18)?
  3. Will there always be an exact number of tokens to distribute at each distribution event? If not, when will the additional tokens be distributed?
    • If an employee collects tokens quarterly for a year, will the number of tokens they are granted always be a multiple of 4? If they are given 5 tokens, when will that additional token be granted? Or (if the answer to (2) above is no), will they be granted 1.25 each distribution?
  4. How will the tokens-to-be-vested be sent to the smart contract? Will the be sent prior to initiating the vesting schedule? Will they be sent after initiating the vesting schedule? Will they be send in the same transaction as the initiation of the vesting schedule?
    • Lucidchart assumptions and flow diagram seem to show two different answers to this question.
  5. What do you expect to happen in a scenario where a user has not yet collected certain tokens that they are owed, but the vesting schedule is cancelled?
    • Does the contract pays out the unclaimed tokens to the user upon cancellation?
    • Do the unclaimed tokens get collected by the issuer as well?
  6. Should a cancelled order be deleted or should a flag be raised (in the struct) that states that that particular vesting schedule is cancelled?
  7. What is the difference between (1) and (2) in the Assumptions & User/Issuer Stories post above?
  8. Can (4) in the Assumptions & User/Issuer Stories be done off chain? To me, it makes more sense to create the template off-chain and simply pass it in for each user, as opposed to saving an unknown number of states on-chain. I can go either way, so let me know.
  9. The Lucidchart assumptions number (5) states "Beneficiary may receive tokens automatically or retrieve them from the Vesting Smart Contract". How would they automatically receive them? Is an admin allowed to push tokens out to the user?
  10. The Lucidchart use cases number (3) states "Employee leaves the company, leading to the remaining tokens either being re-assigned to another employee or being moved back to the Treasury wallet." Would the reassignment be done automatically in this contract, or would the tokens be sent back to the issuer, who would then re-initialize a new vesting schedule for someone else?
    • IMO, it would be the latter.
  11. The Lucidchart use cases number (4) states "Employee gets promoted or company gets acquired, leading to an acceleration of the vesting schedule for one or more employees." Is this acceleration be done automatically in this contract, or would the tokens be sent back to the issuer, who would then send them to the employee in question?
    • IMO, it would be the latter.

Architecture

  1. This is a new Wallet module, correct?
    • If so, I am not exactly sure how you guys want me to construct IWallet.sol. I can do what I think may be best, but without a spec I am not sure how closely I can mimic what you desire.
  2. What type of Module Factory is this(example)?
    • 5 seems to be the next logical type
  3. What tag is this (example)?

The code so far is here. I'd like you to take a look and make sure nothing is out of the ordinary. A good chunk of the business logic is there, but my next step is to integrate it with your existing architecture and then test it of course.

@satyamakgec
Copy link
Contributor

satyamakgec commented Sep 26, 2018

@shanefontaine

Will the number of tokens in a specific vesting schedule ever increase (will you add more tokens into an employee's current vesting contract)?

Yes, many possible scenarios where the employee gets the more tokens as vested ex- Performance bonus.
So if Bob started a job in ABC Inc (at 4 April 2018) and gets 10000 tokens(as joining bonus) vested for 2 years with a cliff of 6 months (half yearly). so at 4 April 2019 he has 5000 tokens(2500 at 4 oct 2018 and 2500 more at 4 April 2019) now may be the same day Bob get the performance bonus of 50000 tokens vested for 1 yr with a cliff of 6 months then in the next 4 0ct 2019 he will get 27500 tokens.

Are all tokens going to be granted in whole units (10^18)?

It depends upon the divisibilty of the security token by default all securitytoken with in the polymath ecosystem have 18 decimals but issuer can choose whether he/she want their ST divisible or not. so you have to keep these things in mind at the time of creation of this module.

Will there always be an exact number of tokens to distribute at each distribution event? If not, when will the additional tokens be distributed?
If an employee collects tokens quarterly for a year, will the number of tokens they are granted always be a multiple of 4? If they are given 5 tokens, when will that additional token be granted? Or (if the answer to (2) above is no), will they be granted 1.25 each distribution?

The sum is granted at once. so whatever the tokens are assigned at first call then it gets automatically divided as per the cliffing strategy issuer or delegate (Authorised person) put into the contract.
you need to add the logic to check whether the given amount is valid as per the divisibility of the ST or not then only tokens get assigned to the vesting contract.

How will the tokens-to-be-vested be sent to the smart contract? Will the be sent prior to initiating the vesting schedule? Will they be sent after initiating the vesting schedule? Will they send in the same transaction as the initiation of the vesting schedule?
Lucidchart assumptions and flow diagram seem to show two different answers to this question.

As per the lucidchart you need to create something like that
Ex- if Jane joins the ABC Inc and get 100000 vested tokens for a period of 2 years with a cliff of 6 months then you should write the logic that will initiate a single txn and takes Janes wallet address, noOfTokens need to vest (100000), vesting period, cliff. this txn will vest 100000 tokens and vesting schedule get started. All things need to do in a single txn.

What do you expect to happen in a scenario where a user has not yet collected certain tokens that they are owed, but the vesting schedule is canceled?
Does the contract pay out the unclaimed tokens to the user upon cancellation?
Do the unclaimed tokens get collected by the issuer as well?

hmm Interesting IMO employee should get a right to take its unclaimed token. But yes there will be some strategy to pull out the unclaimed tokens by the issuer after a certain challenging period. (will give you more info after internal discussion).

Should a canceled order be deleted or should a flag be raised (in the struct) that states that that particular vesting schedule is canceled?

I prefer to be deleted no need to keep the unnecessary data we already have all the logs present in the blockchain. You will delete the vesting schedule but you need to provide two options(as described in lucidchart) to issuer/delegate related to the remaining tokens management.

What is the difference between (1) and (2) in the Assumptions & User/Issuer Stories post above?

(1) It will work as conjunction with STO module so suppose you created some buying restriction in your STO. ex- restriction 1 - If someone buys the token amount more than 50000 then his/her tokens get vested for 2 years with a cliff of 4 months so these 50000 tokens get vested into the vested contract with the template data provided from the STO. I think now you can compare 1 and 2 clearly.

Can (4) in the Assumptions & User/Issuer Stories be done off chain? To me, it makes more sense to create the template off-chain and simply pass it in for each user, as opposed to saving an unknown number of states on-chain. I can go either way, so let me know.

An issuer can create some templates not too many, only those that issuer can frequently use (ex- for every new joinee, for providing gratuity etc.)

      ex - Template 1 
      template name - Rookies
      Vested Period - 2 year
      Cliff - Quater
      Token - 20000

The Lucidchart assumptions number (5) states "Beneficiary may receive tokens automatically or retrieve them from the Vesting Smart Contract". How would they automatically receive them? Is an admin allowed to push tokens out to the user?

Yes, Admin can push the token.

The Lucidchart use cases number (3) states "Employee leaves the company, leading to the remaining tokens either being re-assigned to another employee or being moved back to the Treasury wallet." Would the reassignment be done automatically in this contract, or would the tokens be sent back to the issuer, who would then re-initialize a new vesting schedule for someone else?
IMO, it would be the latter.

You will have some variable in contract unAssginedTokens(you can choose a much better name) that will keep track of remaining or canceled tokens during the creation of new vesting schedule you will have some logic that will check this variable and allocate part of these token to the new vesting schedule.

The Lucidchart use cases number (4) states "Employee gets promoted or the company gets acquired, leading to an acceleration of the vesting schedule for one or more employees." Is this acceleration be done automatically in this contract, or would the tokens be sent back to the issuer, who would then send them to the employee in question?
IMO, it would be the latter.

No, Issuer will have the edit option to change the vesting schedule using this all the tokens get live quickly and an employee can withdraw the tokens/issuer push these tokens to them.

Architecture

  • For nomenclature you can check our existing module names according to that you can choose one. I prefer VestedEscrowWallet.sol, VestedEscrowWalletFactory.sol. and yes you can IWallet.sol will be the right name..

  • Yes 5 type works

  • Tags will be Vested Wallet, Escrow, any more you can think of.

@shanefontaine
Copy link

Thank you for the quick response. I am implementing those changes as we speak.

On issue I am trying to wrap my head around right now is a matter of architecture. I have been reading through this, as well as a number of other references on your site about how the factories and interfaces work. I believe I understand it, but am having an issue compiling my code. When I compile it, I get the following errors:

./polymath-core/contracts/modules/Wallet/VestingEscrowWalletFactory.sol:33:51: TypeError: Trying to create an instance of an abstract contract.
        VestingEscrowWallet vestingEscrowWallet = new VestingEscrowWallet(msg.sender, address(polyToken));
./polymath-core/contracts/interfaces/IModule.sol:16:5: Missing implementation:
    function getPermissions() external view returns(bytes32[]);

The first error is here in my Wallet Factory. I understand that a flag is raised when the contract is an abstract contract, but I do not think that this is the case here. Is there something I am missing during this initialization?

I thought that the IModule "interface defines some functionality which is common across all modules." Why would I be getting this error if I neither edited iModule nor seemingly interacted with it?

@satyamakgec
Copy link
Contributor

FYI more info on this

What do you expect to happen in a scenario where a user has not yet collected certain tokens that they are owed, but the vesting schedule is canceled?
Does the contract pay out the unclaimed tokens to the user upon cancellation?
Do the unclaimed tokens get collected by the issuer as well?

Yes, option one will be Implemented and Issuer can retrieve any remaining unvested portion.

For your error, you need to implement a getPermissions() function in your Module. As IModule.sol have its declaration but you are not implementing it when you are inheriting the IModule.

I hope this will help. Thanks

@shanefontaine
Copy link

You said option one and issuer, which are contracdictory. Which did you mean?

@CPSTL
Copy link
Contributor Author

CPSTL commented Sep 27, 2018

@shanefontaine The Employee / Affiliate should be able to claim any tokens vested up until the schedule is voided. With that being said, afterwards the Issuer can retrieve any remaining unvested portion of tokens. Does that help answer your question?

@shanefontaine
Copy link

@CPSTL Yes, it does, partially. What if the employee/affiliate has unclaimed, vested tokens after void of contract. I believe (and suggest) the solution is to make it in such a way that upon cancellation, the issuer receives the unvested tokens, the employee receives all vested tokens, and the account gets deleted. Does that sound reasonable?

@shanefontaine
Copy link

shanefontaine commented Sep 28, 2018

@satyamakgec

Will the number of tokens in a specific vesting schedule ever increase (will you add more tokens into an employee's current vesting contract)?

Yes, many possible scenarios where the employee gets the more tokens as vested ex- Performance bonus.
So if Bob started a job in ABC Inc (at 4 April 2018) and gets 10000 tokens(as joining bonus) vested for 2 years with a cliff of 6 months (half yearly). so at 4 April 2019 he has 5000 tokens(2500 at 4 oct 2018 and 2500 more at 4 April 2019) now may be the same day Bob get the performance bonus of 50000 tokens vested for 1 yr with a cliff of 6 months then in the next 4 0ct 2019 he will get 27500 tokens.

Can this simply be done by creating a new vesting schedule with these tokens (as opposed to adding onto an existing schedule). Creating a new vesting schedule for new tokens makes sense, and is easy to implement/understand from an issuer's perspective. Adding tokens onto an existing schedule makes implementation much more complex, causing a state change for each variable needing changing and recalculations/checks for each parameter.

I cannot think of a scenario where an issuer would need to add to an existing vesting schedule and NOT simply create a new one (even if it behaves the same way as the old one).

The Lucidchart use cases number (4) states "Employee gets promoted or the company gets acquired, leading to an acceleration of the vesting schedule for one or more employees." Is this acceleration be done automatically in this contract, or would the tokens be sent back to the issuer, who would then send them to the employee in question?
IMO, it would be the latter.

No, Issuer will have the edit option to change the vesting schedule using this all the tokens get live quickly and an employee can withdraw the tokens/issuer push these tokens to them.

In this scenario, does the employee always get all of their tokens?

@satyamakgec
Copy link
Contributor

What if the employee/affiliate has unclaimed, vested tokens after void of contract. I believe (and suggest) the solution is to make it in such a way that upon cancellation, the issuer receives the unvested tokens, the employee receives all vested tokens, and the account gets deleted. Does that sound reasonable?

Yes

Can this simply be done by creating a new vesting schedule with these tokens (as opposed to adding onto an existing schedule). Creating a new vesting schedule for new tokens makes sense, and is easy to implement/understand from an issuer's perspective. Adding tokens onto an existing schedule makes implementation much more complex, causing a state change for each variable needing changing and recalculations/checks for each parameter.
I cannot think of a scenario where an issuer would need to add to an existing vesting schedule and NOT simply create a new one (even if it behaves the same way as the old one).

Yes simply create a new, My example has only sketched a scenario and I am a favor of creating a new one not to edit the same schedule.

In this scenario, does the employee always get all of their tokens?

Yes

@gitcoinbot
Copy link

@shanefontaine Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@CPSTL
Copy link
Contributor Author

CPSTL commented Oct 1, 2018

Hey @shanefontaine how are you doing with this task? Let us know if you have any more questions :)

@shanefontaine
Copy link

@CPSTL Great. I am almost complete. I have a few questions regarding testing, so I am setting up a time to chat with @satyamakgec. After our call, I should be able to complete it w/in 24 hours and I'll submit a PR.

@shanefontaine
Copy link

@CPSTL @satyamakgec I am finishing it up now and will have a PR submitted soon. I apologize for the delay.

@satyamakgec
Copy link
Contributor

Cool no worries!. When you submit the PR then let me know so I can start the reviewing process.

@shanefontaine shanefontaine mentioned this issue Oct 5, 2018
3 tasks
@shanefontaine
Copy link

PR has been submitted.

@gitcoinbot
Copy link

@shanefontaine Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@shanefontaine
Copy link

I have submitted a PR.

@gitcoinbot
Copy link

@shanefontaine Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@shanefontaine
Copy link

I have submitted a PR.

@gitcoinbot
Copy link

gitcoinbot commented Oct 17, 2018

@shanefontaine Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

1 similar comment
@gitcoinbot
Copy link

@shanefontaine Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@samparsky
Copy link

samparsky commented Oct 21, 2018

@shanefontaine You have to submit the PR on gitcoin via submit task for the bot to stop commenting

@gitcoinbot
Copy link

@shanefontaine Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


@shanefontaine due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Cancelled


The funding of 5000.0 POLY (870.0 USD @ $0.17/POLY) attached to this issue has been cancelled by the bounty submitter

1 similar comment
@gitcoinbot
Copy link

Issue Status: 1. Open 2. Cancelled


The funding of 5000.0 POLY (870.0 USD @ $0.17/POLY) attached to this issue has been cancelled by the bounty submitter

@satyamakgec
Copy link
Contributor

closing in the favor of #422

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module development Used when new module is created open-source community Something suggest or developed by external developers
Projects
None yet
Development

No branches or pull requests

5 participants