feat!: periodic vesting msg #9596

zmanian · 2021-06-28T13:35:10Z

Closes: #9595

Enables creating a periodic vesting account on a running chain.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

Basic test of sending periodic vesting messages Add cli command for Periodic Vesting tx

more realistic example values

account messages

…iodic-vesting-msg

x/auth/vesting/client/cli/tx.go

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

clevinson

Thanks for adding this!

A few change requests:

Test needs updating (the one test added doesn't compile)
We should remove the --delayed flag from this command as it doesn't apply.

Other than that, some comment / naming suggestions, and I wanted to raise a question abt allowing period == 0. Don't have a super strong opinion on this, but potentially worth reconsidering if there is a valid use case for that.

x/auth/vesting/client/cli/tx.go

x/auth/vesting/msg_server.go

x/auth/vesting/types/msgs.go

x/auth/vesting/handler_test.go

CHANGELOG.md

proto/cosmos/vesting/v1beta1/tx.proto

x/auth/vesting/client/cli/tx.go

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

Co-authored-by: Cory <cjlevinson@gmail.com>

orijbot · 2021-07-16T03:13:46Z

Visit https://dashboard.github.orijtech.com?pr=9596&repo=cosmos%2Fcosmos-sdk to see benchmark details.

actually committing the account state to periodic vesting was missing.

Co-authored-by: Cory <cjlevinson@gmail.com>

robert-zaremba · 2021-08-20T22:18:08Z

This PR is almost done and close to be merged. @zmanian - could you do remaining updates?

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

zmanian · 2021-08-25T01:50:25Z

lgtm, but we should really add tests.

@zmanian if you don't have bandwidth to write tests, I'm okay to create a follow-up issue and assign to someone on the sdk team

Yeah the vesting module generally doesn't have a enough tests. I would appreciate help writing them

stake

amaury1093 · 2021-08-25T10:06:25Z

follow-up: #10005

mergify · 2021-08-25T10:06:51Z

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

…#10007)  ## Description We merged #9596, but we forgot to add proto comments for one message. Then, proto lint CI fails on unrelated PRs, e.g. https://github.com/cosmos/cosmos-sdk/pull/9759/checks?check_run_id=3421076482#step:4:24. This fixes it. Also related to #9978  --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [x] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [x] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)

(cherry picked from commit 5d01af4)

* custom/vesting-acc: feat: periodic vesting msg, cherry-pick 147d798 cosmos#9596

(cherry picked from commit 5d01af4) (cherry picked from commit aefda6b)

…k-0.45.3 * custom/0.45.3/vesting-acc: feat: periodic vesting msg, cherry-pick 147d798 cosmos#9596

Closes: cosmos#9595 Enables creating a periodic vesting account on a running chain. I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [x] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) chore: Add proto comments for MsgCreatePeriodicVestingAccountResponse (cosmos#10007)  We merged cosmos#9596, but we forgot to add proto comments for one message. Then, proto lint CI fails on unrelated PRs, e.g. https://github.com/cosmos/cosmos-sdk/pull/9759/checks?check_run_id=3421076482#step:4:24. This fixes it. Also related to cosmos#9978  --- *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [x] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [x] confirmed all CI checks have passed *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)

(cherry picked from commit 5d01af4) (cherry picked from commit aefda6b) (cherry picked from commit 82c4293)

(cherry picked from commit 5d01af4) (cherry picked from commit aefda6b) (cherry picked from commit 82c4293) (cherry picked from commit 071fe25)

(cherry picked from commit 5d01af4) (cherry picked from commit aefda6b) (cherry picked from commit 82c4293) (cherry picked from commit 071fe25) (cherry picked from commit 54143fc)

zmanian added 8 commits May 8, 2021 22:14

Add a msg for create PeriodicVestingAccount

abd8677

Basic test of sending periodic vesting messages Add cli command for Periodic Vesting tx

Address code review comments

5e65a9c

validate periodic vesting messages period length is greater than 0.

48098c5

update error message on invalid vesting lenght

d0a7a0a

error on invalid lengths in json

b4b4d81

more realistic example values

update the help message

663cc47

Add support start time to periodic vesting

95b4f12

account messages

Merge branch 'zaki-periodic-vesting-msg-alpha' into zaki-upstream-per…

c58b3fd

…iodic-vesting-msg

zmanian requested review from aaronc and alexanderbez as code owners June 28, 2021 13:35

github-actions bot added the C:x/auth label Jun 28, 2021

Add changelog entry

57f3014

alexanderbez approved these changes Jun 28, 2021

View reviewed changes

x/auth/vesting/client/cli/tx.go Outdated Show resolved Hide resolved

x/auth/vesting/client/cli/tx.go Outdated Show resolved Hide resolved

x/auth/vesting/client/cli/tx.go Show resolved Hide resolved

clevinson self-assigned this Jun 28, 2021

zmanian and others added 2 commits June 28, 2021 11:00

Update x/auth/vesting/client/cli/tx.go

bfc95c2

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

Update x/auth/vesting/client/cli/tx.go

f5c45c5

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

clevinson previously requested changes Jul 1, 2021

View reviewed changes

amaury1093 self-assigned this Jul 1, 2021

zmanian added 4 commits July 2, 2021 07:04

Add starttime to the vesting input file

548f4da

Fix tests

ce81fc6

Remove the delayed flag

1830947

add json tags

48640b9

amaury1093 reviewed Jul 5, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

proto/cosmos/vesting/v1beta1/tx.proto Outdated Show resolved Hide resolved

proto/cosmos/vesting/v1beta1/tx.proto Show resolved Hide resolved

x/auth/vesting/client/cli/tx.go Show resolved Hide resolved

zmanian and others added 2 commits July 6, 2021 10:16

Update proto/cosmos/vesting/v1beta1/tx.proto

334fc1a

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

Update x/auth/vesting/types/msgs.go

190bf7e

Co-authored-by: Cory <cjlevinson@gmail.com>

zmanian and others added 2 commits July 15, 2021 20:15

Update msg_server.go

c74c346

actually committing the account state to periodic vesting was missing.

Update x/auth/vesting/types/msgs.go

e9de0e1

Co-authored-by: Cory <cjlevinson@gmail.com>

clevinson mentioned this pull request Aug 9, 2021

Permanently Locked accounts + Msg for account creation on-chain regen-network/regen-ledger#188

Closed

Update x/auth/vesting/client/cli/tx.go

e233f18

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

Merge branch 'master' into zaki-upstream-periodic-vesting-msg

ccdcbd6

amaury1093 approved these changes Aug 25, 2021

View reviewed changes

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Aug 25, 2021

mergify bot merged commit 147d798 into master Aug 25, 2021

mergify bot deleted the zaki-upstream-periodic-vesting-msg branch August 25, 2021 10:03

amaury1093 mentioned this pull request Aug 25, 2021

Write test for periodic vesting msg #10005

Closed

4 tasks

amaury1093 mentioned this pull request Aug 25, 2021

chore: Add proto comments for MsgCreatePeriodicVestingAccountResponse #10007

Merged

19 tasks

clevinson mentioned this pull request Jan 24, 2022

Add support for permanent locked vesting accounts #11016

Closed

dongsam mentioned this pull request Feb 21, 2022

bump custom sdk for MsgCreatePeriodicVestingAccount, delegate only for spendable coins cosmosquad-labs/squad#201

Closed

4 tasks

dongsam pushed a commit to cosmosquad-labs/cosmos-sdk that referenced this pull request Feb 22, 2022

feat: periodic vesting msg, cherry-pick 147d798 cosmos#9596

5d01af4

dongsam pushed a commit to cosmosquad-labs/cosmos-sdk that referenced this pull request Mar 11, 2022

feat: periodic vesting msg, cherry-pick 147d798 cosmos#9596

aefda6b

(cherry picked from commit 5d01af4)

dongsam mentioned this pull request Mar 11, 2022

custom-diff: x/vesting cosmosquad-labs/cosmos-sdk#1

Closed

dongsam added a commit to cosmosquad-labs/cosmos-sdk that referenced this pull request Mar 11, 2022

Merge branch 'custom/vesting-acc' into custom/v1.0.0-based-v0.44.5

dabc768

* custom/vesting-acc: feat: periodic vesting msg, cherry-pick 147d798 cosmos#9596

dongsam pushed a commit to cosmosquad-labs/cosmos-sdk that referenced this pull request Apr 22, 2022

feat: periodic vesting msg, cherry-pick 147d798 cosmos#9596

82c4293

(cherry picked from commit 5d01af4) (cherry picked from commit aefda6b)

dongsam added a commit to cosmosquad-labs/cosmos-sdk that referenced this pull request Apr 22, 2022

Merge branch 'custom/0.45.3/vesting-acc' into custom/0.45.3/v2.0.x-sd…

3f87822

…k-0.45.3 * custom/0.45.3/vesting-acc: feat: periodic vesting msg, cherry-pick 147d798 cosmos#9596

dongsam mentioned this pull request Apr 26, 2022

custom-diff: x/vesting [v0.45.3] cosmosquad-labs/cosmos-sdk#11

Draft

jaybxyz pushed a commit to cosmosquad-labs/cosmos-sdk that referenced this pull request Oct 17, 2022

feat: periodic vesting msg, cherry-pick 147d798 cosmos#9596

071fe25

(cherry picked from commit 5d01af4) (cherry picked from commit aefda6b) (cherry picked from commit 82c4293)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: periodic vesting msg #9596

feat!: periodic vesting msg #9596

zmanian commented Jun 28, 2021 •

edited

Loading

clevinson left a comment •

edited

Loading

orijbot commented Jul 16, 2021

robert-zaremba commented Aug 20, 2021

zmanian commented Aug 25, 2021

amaury1093 commented Aug 25, 2021

mergify bot commented Aug 25, 2021

feat!: periodic vesting msg #9596

feat!: periodic vesting msg #9596

Conversation

zmanian commented Jun 28, 2021 • edited Loading

Reviewers Checklist

clevinson left a comment • edited Loading

Choose a reason for hiding this comment

orijbot commented Jul 16, 2021

robert-zaremba commented Aug 20, 2021

zmanian commented Aug 25, 2021

amaury1093 commented Aug 25, 2021

mergify bot commented Aug 25, 2021

zmanian commented Jun 28, 2021 •

edited

Loading

clevinson left a comment •

edited

Loading