R4R: implement validator queue #2451

sunnya97 · 2018-10-06T21:01:35Z

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (docs/)
Added entries in PENDING.md with issue #
rereviewed Files changed in the github PR explorer

For Admin Use:

Added appropriate labels to PR (ex. wip, ready-for-review, docs)
Reviewers Assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

codecov · 2018-10-06T21:08:11Z

Codecov Report

❗ No coverage uploaded for pull request base (develop@dd8b574). Click here to learn what that means.
The diff coverage is 53.84%.

@@            Coverage Diff             @@
##             develop    #2451   +/-   ##
==========================================
  Coverage           ?   61.93%           
==========================================
  Files              ?      150           
  Lines              ?     9476           
  Branches           ?        0           
==========================================
  Hits               ?     5869           
  Misses             ?     3199           
  Partials           ?      408

docs/spec/staking/end_block.md

fedekunze · 2018-10-06T22:48:33Z

x/stake/handler.go

+		}
+		k.UnbondingToUnbonded(ctx, val)
+
+		// endBlockerTags.AppendTags(sdk.NewTags(


Should this be deleted/uncommented ?

rigelrozanski

Dopeness! Thanks for this! - few comments

x/stake/handler.go

x/stake/keeper/val_state_change.go

x/stake/types/validator.go

rigelrozanski · 2018-10-06T23:28:58Z

x/stake/handler_test.go

+	got = handleMsgBeginUnbonding(ctx, msgBeginUnbondingValidator, keeper)
+	require.True(t, got.IsOK(), "expected no error: %v", got)
+	var finishTime time.Time
+	types.MsgCdc.MustUnmarshalBinary(got.Data, &finishTime)


I guess this is okay - I think it would be a bit nicer if we queried for the validator, and used the finish time from that object

jackzampolin · 2018-10-08T17:13:19Z

@sunnya97 Any chance you can address these comments today so we can get this merged?

jackzampolin · 2018-10-10T17:07:54Z

Lets try to get this in today cc @cwgoes @rigelrozanski @jaekwon

cwgoes · 2018-10-10T17:47:51Z

It's failing test_cover and also test_lint, those need to be fixed first.

jackzampolin · 2018-10-11T20:42:36Z

Does this have to do with the non-determinism @cwgoes

cwgoes · 2018-10-11T21:29:34Z

Not the lint, maybe test_cover.

cwgoes

A few concerns, see comments.

types/context.go

x/stake/keeper/validator.go

cwgoes · 2018-10-12T20:42:44Z

x/stake/keeper/validator.go

+}
+
+// Returns a concatenated list of all the timeslices before currTime, and deletes the timeslices from the queue
+func (k Keeper) DequeueAllMatureValidatorQueue(ctx sdk.Context, currTime time.Time) (matureValsAddrs []sdk.ValAddress) {


Why do we copy these into memory instead of executing the validator state transition during iteration?

Left over from the Unbonding/Redelegations queue where the actual unbonding was done in the Handler, so it could add tags (and not double iterate)

Maybe keeping a separate GetAllMatureValidatorQueue function could be useful for a future querier endpoint.

sunnya97 · 2018-10-13T03:13:57Z

I think test_cover failing has to do with the CI non determinism. I can't reproduce it on my local machine, and at current CI run has it passing 🤷‍♂️

rigelrozanski · 2018-10-13T04:11:58Z

yeah there's some non-determinism lingering - I noticed it in some other PRs forsure

cwgoes · 2018-10-13T06:11:38Z

LGTM - giving @rigelrozanski a chance to final-review.

jackzampolin · 2018-10-15T16:25:37Z

Nice job @sunnya97 🎉 !!!

sunnya97 added 10 commits October 5, 2018 17:09

unbonding redelegation queue

12fbe44

address some of bez and chris review

d0f49f1

delete old timeslices from queue

f690712

added Rigel's test case

49f1670

added end-time to tags

89af16e

fixed bug in staking

96b4c4d

removed prints

b0f29a1

Get -> Queue

92770ee

called Endblocker in test

668cfef

implement validator queue

32bda25

sunnya97 requested review from cwgoes, ebuchman and rigelrozanski as code owners October 6, 2018 21:01

sunnya97 changed the title ~~implement validator queue~~ WIP: implement validator queue Oct 6, 2018

Docs and PENDING

7e54ebd

sunnya97 requested a review from zramsay as a code owner October 6, 2018 21:08

sunnya97 changed the title ~~WIP: implement validator queue~~ R4R: implement validator queue Oct 6, 2018

sunnya97 added the ready-for-review label Oct 6, 2018

sunnya97 assigned rigelrozanski Oct 6, 2018

sunnya97 added the C:x/staking label Oct 6, 2018

sunnya97 added this to the 0.25 milestone Oct 6, 2018

fedekunze reviewed Oct 6, 2018

View reviewed changes

docs/spec/staking/end_block.md Show resolved Hide resolved

fedekunze reviewed Oct 6, 2018

View reviewed changes

address federicos comments

9c457cd

rigelrozanski suggested changes Oct 6, 2018

View reviewed changes

unexposed UnbondingToUnbonded

f211e19

sunnya97 changed the base branch from sunny/staking-unbonding-queue to develop October 10, 2018 08:54

Merge branch 'develop' into sunny/validator-queue

c9639cc

Merge branch 'develop' into sunny/validator-queue

6c6880e

cwgoes suggested changes Oct 12, 2018

View reviewed changes

cwgoes and others added 2 commits October 13, 2018 01:37

Merge branch 'develop' into sunny/validator-queue

903fd87

no copying unbonded val array to memory

293a2a2

cwgoes approved these changes Oct 13, 2018

View reviewed changes

Merge branch 'develop' into sunny/validator-queue

3c85908

rigelrozanski approved these changes Oct 15, 2018

View reviewed changes

rigelrozanski merged commit 6c9e71b into develop Oct 15, 2018

cwgoes deleted the sunny/validator-queue branch January 17, 2019 13:09

snyk-bot mentioned this pull request Jan 15, 2020

[Snyk] Upgrade axios from 0.19.0 to 0.19.1 Itchibon777/cosmos-sdk#1

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: implement validator queue #2451

R4R: implement validator queue #2451

sunnya97 commented Oct 6, 2018 •

edited

Loading

codecov bot commented Oct 6, 2018 •

edited

Loading

fedekunze Oct 6, 2018

rigelrozanski left a comment

rigelrozanski Oct 6, 2018

jackzampolin commented Oct 8, 2018

jackzampolin commented Oct 10, 2018

cwgoes commented Oct 10, 2018 •

edited

Loading

jackzampolin commented Oct 11, 2018

cwgoes commented Oct 11, 2018

cwgoes left a comment

cwgoes Oct 12, 2018

sunnya97 Oct 13, 2018 •

edited

Loading

sunnya97 Oct 13, 2018 •

edited

Loading

sunnya97 commented Oct 13, 2018

rigelrozanski commented Oct 13, 2018

cwgoes commented Oct 13, 2018

jackzampolin commented Oct 15, 2018

R4R: implement validator queue #2451

R4R: implement validator queue #2451

Conversation

sunnya97 commented Oct 6, 2018 • edited Loading

codecov bot commented Oct 6, 2018 • edited Loading

Codecov Report

fedekunze Oct 6, 2018

Choose a reason for hiding this comment

rigelrozanski left a comment

Choose a reason for hiding this comment

rigelrozanski Oct 6, 2018

Choose a reason for hiding this comment

jackzampolin commented Oct 8, 2018

jackzampolin commented Oct 10, 2018

cwgoes commented Oct 10, 2018 • edited Loading

jackzampolin commented Oct 11, 2018

cwgoes commented Oct 11, 2018

cwgoes left a comment

Choose a reason for hiding this comment

cwgoes Oct 12, 2018

Choose a reason for hiding this comment

sunnya97 Oct 13, 2018 • edited Loading

Choose a reason for hiding this comment

sunnya97 Oct 13, 2018 • edited Loading

Choose a reason for hiding this comment

sunnya97 commented Oct 13, 2018

rigelrozanski commented Oct 13, 2018

cwgoes commented Oct 13, 2018

jackzampolin commented Oct 15, 2018

sunnya97 commented Oct 6, 2018 •

edited

Loading

codecov bot commented Oct 6, 2018 •

edited

Loading

cwgoes commented Oct 10, 2018 •

edited

Loading

sunnya97 Oct 13, 2018 •

edited

Loading

sunnya97 Oct 13, 2018 •

edited

Loading