R4R: Outstanding per-validator rewards; correctly handle same-BeginBlock redelegation-double-slash by cwgoes · Pull Request #3750 · cosmos/cosmos-sdk

cwgoes · 2019-02-26T21:03:21Z

Fixes: #3735

Track outstanding rewards per-validator instead of globally, and assert non-negativity in the associated invariant.

I think we can do this for awhile, it's not that expensive state-wise, just one entry per validator.

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)

…y checks

This reverts commit 2f9a0da.

…-to-truncation

cwgoes · 2019-02-26T21:15:12Z

Ah duh, I branched off develop.

…validator

cwgoes · 2019-02-26T21:16:27Z

Progress - fails much earlier now, at block 37:

 Unbonding Completion Time:  1970-01-01 00:00:00 +0000 UTC
  Minimum Self Delegation:    1
  Commission:                 rate: 0.000000000000000000, maxRate: 0.000000000000000000, maxChangeRate: 0.000000000000000000, updateTime: 3130-05-13 20:58:16 +0000 UTC, tokens 211768520.255285192222002237stake
remaining after: 161727115.100000009315481833stake
initialize delegation for period: 148
startingPeriod: 88
endingPeriod: 143
withdraw from Validator
  Operator Address:           cosmosvaloper1qu379fd7lzvl9pfwclw2984n99dfqdgxjypypy
  Validator Consensus Pubkey: cosmosvalconspub1addwnpepqwrkn00e0242vchkv3tyeus47j4zagtfeynlkdptrx8fqmkgy7n75km6mn6
  Jailed:                     true
  Status:                     Unbonding
  Tokens:                     957361378468
  Delegator Shares:           1057203680154.766439798905981990
  Description:                {dQGFozcavd ThVgiJfpna lNFXGmdzRs PKGqkVfQVg}
  Unbonding Height:           37
  Unbonding Completion Time:  3130-05-16 01:49:40 +0000 UTC
  Minimum Self Delegation:    1
  Commission:                 rate: 0.000000000000000000, maxRate: 0.000000000000000000, maxChangeRate: 0.000000000000000000, updateTime: 3130-05-12 17:34:24 +0000 UTC to Delegation:
  Delegator: cosmos1l67uvpuauv6wd90rvuln8ywp3trwfcc6ayj6mq
  Validator: cosmosvaloper1qu379fd7lzvl9pfwclw2984n99dfqdgxjypypy
  Shares:    223288090.033588056444346604
rewards: 8390361.801602437488152614stake, outstanding: 1949212.694301383458260220stake
Panic with err
 negative coin amount
goroutine 11 [running]:
runtime/debug.Stack(0xc002f5fdf8, 0x2, 0x2)
	/usr/lib/go/src/runtime/debug/stack.go:24 +0xa7
github.com/cosmos/cosmos-sdk/x/mock/simulation.SimulateFromSeed.func2(0xc00094c020, 0xc00017be00, 0xc0001aacd0)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/mock/simulation/simulate.go:131 +0xb6
panic(0xc76900, 0xfdc780)
	/usr/lib/go/src/runtime/panic.go:513 +0x1b9
github.com/cosmos/cosmos-sdk/types.DecCoins.Sub(0xc004392700, 0x1, 0x1, 0xc0043923c0, 0x1, 0x1, 0x0, 0x0, 0xc000a8e5c0)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/types/dec_coin.go:268 +0xbe
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.withdrawDelegationRewards(0xfe3f40, 0xc0001aa940, 0xc000afe230, 0xc000afe230, 0xfe3f40, 0xc0001aa990, 0xfe3f80, 0xc0001aa9a0, 0xc000a8e5c0, 0x5, ...)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/delegation.go:130 +0x698
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.WithdrawDelegationRewards(0xfe3f40, 0xc0001aa940, 0xc000afe230, 0xc000afe230, 0xfe3f40, 0xc0001aa990, 0xfe3f80, 0xc0001aa9a0, 0xc000a8e5c0, 0x5, ...)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/keeper.go:62 +0x20c
github.com/cosmos/cosmos-sdk/x/distribution/simulation.CanWithdrawInvariant.func1.1(0x1, 0xff1f20, 0xc001346d20, 0xc001346d20)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/distribution/simulation/invariants.go:70 +0x69b
github.com/cosmos/cosmos-sdk/x/staking/keeper.Keeper.IterateValidators(0xfe3f40, 0xc0001aa910, 0xfe3f80, 0xc0001aa920, 0xc000afe230, 0x7f2e04b9a178, 0xc00017a1e0, 0xff09a0, 0xc0000cc480, 0xc000afe230, ...)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/staking/keeper/alias_functions.go:21 +0x22c
github.com/cosmos/cosmos-sdk/x/distribution/simulation.CanWithdrawInvariant.func1(0xfead20, 0xc003bbfa10, 0xc00287efc0, 0xc, 0xfe3f40, 0xc0001aa990)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/distribution/simulation/invariants.go:63 +0x1a7
github.com/cosmos/cosmos-sdk/x/distribution/simulation.AllInvariants.func1(0xfead20, 0xc003bbfa10, 0xc00287efc0, 0xc, 0x26, 0x0)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/distribution/simulation/invariants.go:14 +0xbf
github.com/cosmos/cosmos-sdk/x/mock/simulation.PeriodicInvariant.func1(0xfead20, 0xc003bbfa10, 0xc00287efc0, 0xc, 0x0, 0x0)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/mock/simulation/util.go:135 +0xc8
github.com/cosmos/cosmos-sdk/x/mock/simulation.assertAllInvariants(0xc0001e5400, 0xc0001e5200, 0xc000f62ff0, 0x5, 0x5, 0xdba534, 0xa, 0xc00094c020)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/mock/simulation/util.go:23 +0x15c
github.com/cosmos/cosmos-sdk/x/mock/simulation.SimulateFromSeed(0xff2be0, 0xc0001e5400, 0xc0001e5200, 0xf16108, 0x1e, 0xc0000e62a0, 0xe, 0xe, 0xc000f62ff0, 0x5, ...)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/mock/simulation/simulate.go:156 +0x1962
github.com/cosmos/cosmos-sdk/cmd/gaia/app.TestFullGaiaSimulation(0xc0001e5400)
	/home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/cmd/gaia/app/sim_test.go:359 +0x339
testing.tRunner(0xc0001e5400, 0xf16098)
	/usr/lib/go/src/testing/testing.go:827 +0xbf
created by testing.(*T).Run
	/usr/lib/go/src/testing/testing.go:878 +0x35c

Now the failing validator is cosmosvaloper1qu379fd7lzvl9pfwclw2984n99dfqdgxjypypy.

The pattern of a large allocation after a slash persists.

cwgoes · 2019-02-28T23:13:42Z

Still remaining to debug: rare slight discrepancy in final calculated stake in distribution vs. expected based on DelegatorShareExRate - e.g.

go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=50 -SimulationVerbose=true -SimulationCommit=true -SimulationSeed=9 -v -timeout 24h | tee sim.out

resulting in

I[2019-03-01|00:13:11.262] Confirmed double sign from B15D03290D638D30989E58526872BCA9B118D463 at height 9, age of 0 module=x/slashing 
I[2019-03-01|00:13:11.263] validator cosmosvaloper1k9wsx2gdvwxnpxy7tpfxsu4u4xc334rrlkum3z slashed by slash factor of 0.026315789473684211; burned 6210578947 tokens module=x/staking 
I[2019-03-01|00:13:11.263] validator cosmosvalcons1k9wsx2gdvwxnpxy7tpfxsu4u4xc334rrt908ar jailed module=x/staking 
I[2019-03-01|00:13:11.263] Confirmed double sign from 946F187E3B96DF833422E90494D00E3A8A4C8AE7 at height 9, age of 0 module=x/slashing 
I[2019-03-01|00:13:11.263] validator cosmosvaloper1j3h3sl3mjm0cxdpzayzff5qw829yezh8yw08at slashed by slash factor of 0.026315789473684211; burned 1227342105 tokens module=x/staking 
I[2019-03-01|00:13:11.263] validator cosmosvalcons1j3h3sl3mjm0cxdpzayzff5qw829yezh8saum32 jailed module=x/staking 
Panic with err
 calculated final stake for delegator cosmos16w5hcamws7rnkmeprce8dnltlgxn2lp539mcau greater than current stake: 111565352687.684143828670710874, 111565352687.684143798380454842
goroutine 10 [running]:
runtime/debug.Stack(0xc004191cd8, 0x2, 0x2) 
  /usr/lib/go/src/runtime/debug/stack.go:24 +0xa7
github.com/cosmos/cosmos-sdk/x/mock/simulation.SimulateFromSeed.func2(0xc000f36620, 0xc00016c5a0, 0xc0001d6e70)
  /home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/mock/simulation/simulate.go:131 +0xb6
panic(0xc76a00, 0xc001063dd0)
  /usr/lib/go/src/runtime/panic.go:513 +0x1b9
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.calculateDelegationRewards(0xfe39a0, 0xc0001d6ae0, 0xc0000d61c0, 0xc0000d61c0, 0xfe39a0, 0xc0001d6b30, 0xfe39e0, 0xc0001d6b40, 0xc000eb2c20, 0x5, ...) 
  /home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/delegation.go:89 +0x766
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.withdrawDelegationRewards(0xfe39a0, 0xc0001d6ae0, 0xc0000d61c0, 0xc0000d61c0, 0xfe39a0, 0xc0001d6b30, 0xfe39e0, 0xc0001d6b40, 0xc000eb2c20, 0x5, ...) 
  /home/cwgoes/working/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/delegation.go:107 +0x28e
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.WithdrawDelegationRewards(0xfe39a0, 0xc0001d6ae0, 0xc0000d61c0, 0xc0000d61c0, 0xfe39a0, 0xc0001d6b30, 0xfe39e0, 0xc0001d6b40, 0xc000eb2c20, 0x5, ...)

jaekwon · 2019-03-01T01:30:27Z

types/decimal.go

+	mul.Mul(mul, precisionReuse)
+
+	quo := new(big.Int).Quo(mul, d2.Int)
+	chopped := chopPrecisionAndRoundUp(quo)


instead of doing this, since multiplication/division is expensive, i think we can do:

((d.Int + (precisionReuse-1)) * precisionReuse) / d2.Int

then we wouldn't need chopPrecisionAndRoundUp.

let's do that, but in a separate PR - #3812

x/distribution/keeper/allocation.go

x/distribution/keeper/delegation.go

jaekwon

Left some questions/comments to consider, but looks good.

…cosmos-sdk into cwgoes/outstanding-per-validator

rigelrozanski · 2019-03-05T00:00:34Z

#3788 ready for merge into here.. then we should merge dis!

PENDING.md

alexanderbez · 2019-03-06T14:55:27Z

Looks good to me.

cwgoes and others added 24 commits February 22, 2019 13:02

Add altered random number generation

da12165

WIP Allow undelegation of more coins than delegated; Add more validit…

b7d1d69

…y checks

Fix tests

2f9a0da

Revert "Fix tests"

c938039

This reverts commit 2f9a0da.

fix tests

066e11f

Add comment on new behavior

f7c3cc6

Call AddCoins conditionally

637902d

Do not panic with unknown evidence

71774b8

update comments from bez's branch

d9b5071

fixing more sim issues

0c0b142

Ignore unknown proposer

aca59ac

Add bias to return 0 to RandomAmount

002e49f

Negative coins amount during fees distribution

89e84f5

Truncate multiplication & division; add back old testcase

d25f793

PENDING.md

c388d00

Address @alexanderbez comments

02b076f

Merge branch 'develop' into cwgoes/alternative-negative-coins-fix

1861c97

Merge branch 'cwgoes/alternative-negative-coins-fix' into cwgoes/back…

7972df8

…-to-truncation

Debugging...

8ff6302

More debugging...

4e15d33

Moar debugging

f29bbae

...

72a850e

...

1c57f32

Outstanding per-validator

ed989af

cwgoes added the C:x/distribution distribution module related label Feb 26, 2019

Naming; genesis fix, ...

4dd5f14

Merge branch 'cwgoes/back-to-truncation' into cwgoes/outstanding-per-…

e8b83e2

…validator

cwgoes mentioned this pull request Feb 26, 2019

Insufficient Outstanding Rewards Simulation Edge Case #3735

Closed

rigelrozanski and others added 4 commits February 28, 2019 17:22

exapand decimal tests

1d47060

Testcase fixes

9360b08

Fix stake calculation ordering

96bf7e9

Address @rigelrozanski comments

37311ca

comment

4c1fd17

jaekwon reviewed Mar 1, 2019

View reviewed changes

x/distribution/keeper/allocation.go Show resolved Hide resolved

jaekwon reviewed Mar 1, 2019

View reviewed changes

x/distribution/keeper/delegation.go Outdated Show resolved Hide resolved

jaekwon reviewed Mar 1, 2019

View reviewed changes

x/distribution/keeper/delegation.go Outdated Show resolved Hide resolved

jaekwon reviewed Mar 1, 2019

View reviewed changes

x/distribution/keeper/delegation.go Outdated Show resolved Hide resolved

jaekwon approved these changes Mar 1, 2019

View reviewed changes

cwgoes added 2 commits March 1, 2019 14:58

Address @jaekwon comments

86763f8

Merge branch 'cwgoes/outstanding-per-validator' of github.com:cosmos/…

56b2b04

…cosmos-sdk into cwgoes/outstanding-per-validator

rigelrozanski mentioned this pull request Mar 2, 2019

R4R F1 mechanism rounding fix #3788

Merged

fedekunze reviewed Mar 5, 2019

View reviewed changes

PENDING.md Outdated Show resolved Hide resolved

rigelrozanski and others added 3 commits March 6, 2019 12:19

Merge PR #3788: F1 mechanism rounding fix

afdac45

Merge branch 'develop' into cwgoes/outstanding-per-validator

31cd44d

Update PENDING.md

1217373

cwgoes mentioned this pull request Mar 6, 2019

WIP: Rounding debugging #3798

Closed

cwgoes requested review from alessio, alexanderbez, jackzampolin, jaekwon and rigelrozanski March 6, 2019 14:56

jackzampolin approved these changes Mar 6, 2019

View reviewed changes

cwgoes merged commit 4c50380 into develop Mar 6, 2019

cwgoes deleted the cwgoes/outstanding-per-validator branch March 6, 2019 18:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Outstanding per-validator rewards; correctly handle same-BeginBlock redelegation-double-slash#3750

R4R: Outstanding per-validator rewards; correctly handle same-BeginBlock redelegation-double-slash#3750
cwgoes merged 58 commits intodevelopfrom
cwgoes/outstanding-per-validator

cwgoes commented Feb 26, 2019 •

edited by jackzampolin

Loading

Uh oh!

cwgoes commented Feb 26, 2019

Uh oh!

cwgoes commented Feb 26, 2019 •

edited

Loading

Uh oh!

cwgoes commented Feb 28, 2019

Uh oh!

jaekwon Mar 1, 2019

Uh oh!

cwgoes Mar 6, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaekwon left a comment

Uh oh!

rigelrozanski commented Mar 5, 2019

Uh oh!

Uh oh!

alexanderbez commented Mar 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

cwgoes commented Feb 26, 2019 • edited by jackzampolin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cwgoes commented Feb 26, 2019

Uh oh!

cwgoes commented Feb 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cwgoes commented Feb 28, 2019

Uh oh!

jaekwon Mar 1, 2019

Choose a reason for hiding this comment

Uh oh!

cwgoes Mar 6, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaekwon left a comment

Choose a reason for hiding this comment

Uh oh!

rigelrozanski commented Mar 5, 2019

Uh oh!

Uh oh!

alexanderbez commented Mar 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cwgoes commented Feb 26, 2019 •

edited by jackzampolin

Loading

cwgoes commented Feb 26, 2019 •

edited

Loading