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

Create Vesting Account Message #4287

Closed
4 tasks
alexanderbez opened this issue May 7, 2019 · 12 comments · Fixed by #7209
Closed
4 tasks

Create Vesting Account Message #4287

alexanderbez opened this issue May 7, 2019 · 12 comments · Fixed by #7209
Assignees

Comments

@alexanderbez
Copy link
Contributor

alexanderbez commented May 7, 2019

Summary

Currently, the only way to define and utilize vesting accounts is through genesis. Now that the hub has launched, there is essentially no longer a way to create new vesting accounts.

Proposal

  • Create a new message and corresponding handler:
type MsgCreateVestingAccount struct {
	FromAddress sdk.AccAddress `json:"from_address"`
	ToAddress   sdk.AccAddress `json:"to_address"`
	Amount      sdk.Coins      `json:"amount"`
	EndTime     int64          `json:"end_time"`
        Delayed     bool           `json:"delayed"`
}

The handler for this would not differ much from a traditional MsgSend except that it would modify the newly created account to now include the new vesting fields.

Also, I decided to omit start_time and allow that to be inferred from the block time (ie. it starts vesting immediately).

/cc @cwgoes @zmanian


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@zmanian
Copy link
Member

zmanian commented May 7, 2019

I don't think we should omit start time.

@zmanian
Copy link
Member

zmanian commented May 7, 2019

I think this is mostly going to be useful in contexts other than the Hub

@fedekunze
Copy link
Collaborator

Nice proposal, it might also make sense to move vesting accs to a new module as per our discussion last week with @rigelrozanski

@rigelrozanski
Copy link
Contributor

Yeah I'd say this makes sense to happen as a part of the refactor which removes vesting from auth. But also, like it seems like there is no use for this? Why would anybody want to create a vesting account from unlocked tokens? It seems like what we actually want is more generalizable send capabilities aka smart contracts.

So I'd actually propose we hold off on this one in favour of some kind of more generalizable scheme.
Thoughts?

@alexanderbez
Copy link
Contributor Author

@zmanian I think we can remove Delayed in favor of StartTime, but my main concern is what if accounts spend coins that are meant to be vesting prior to StartTime. Will this account behave safely once StartTime has passed? I need to further analyze this.


@rigelrozanski, @fedekunze

I'm not necessarily against moving the vesting account implementation out of x/auth, but I want to understand the primary rationale behind such a move. I know IRIS forked the SDK and modified x/auth by removing vesting, but I don't necessarily understand why? You're not forced to use vesting accounts -- ie. you can use accounts as normal.

@cwgoes
Copy link
Contributor

cwgoes commented May 8, 2019

Generally my question here is around whether vesting "accounts" are really the right model or whether we ought to switch to vesting "coins" and deprecate the current vesting account system (although retain the same logic for determining which coins are spent in what order). Right now, each account can only have one kind of vesting (and one start time / end time), which is rather restrictive.

@alexanderbez
Copy link
Contributor Author

That would be a full paradigm shift @cwgoes but this approach would allow greater flexibility. Namely, as the number of tokens expands in the hub, this would be the ideal approach.

@alexanderbez
Copy link
Contributor Author

We're going to get a primitive implementation of this into Stargate. We can explore better and more flexible alternatives in the future.

@hxrts
Copy link
Contributor

hxrts commented Aug 27, 2020

hell ya

@Lockwarr
Copy link

Hey, revisiting this just now, I'm working on a module that has a single functionality - a cli command for adding a vesting account after the chain has started with an option to specify --start-time. @alexanderbez Regarding the concern mentioned above, I've tested creating an account with start-time in the future and tried spending it's coins and it was working as expected(failed - the same way as accounts with start-time in the future declared in the genesis), I was only able to use small part of the coins after my time passed the unix timestamp set for --start-time.

Just wondering if there are any blockers for this --start-time flag to be added in the create-vesting-account command to the cosmos-sdk's native vesting module

@alexanderbez
Copy link
Contributor Author

It's been a while since I've worked on this feature. But I can't think of a reason not to allow a user-supplied start time, s.t. it is > current block time.

@Lockwarr
Copy link

Here's the module in development , I've got some parts of the code from stargaze's alloc module. I also tested creating a vesting account with start time in the past and also worked as expected ( haven't done actual calculations, but noticed how just a small amount of the tokens were unlocked), but yeah most use cases will probably have s.t. > current block time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants