R4R: Simulation framework, including staking simulation #1620

cwgoes · 2018-07-10T18:48:42Z

Closes #1647

How to run the simulation: make test_sim. Added to CI as test_sim.

Not yet implemented (for a future PR):

Simulate Tendermint behavior (validator set updates, block signing in ABCI, evidence)
Simulated slashing (messages / tick function)
Simulated time and configuration parameters (e.g. unbonding period) to match

Updated all relevant documentation in docs
Updated all code comments where relevant
Wrote tests
Updated CHANGELOG.md
Updated Gaia/Examples
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
Added appropriate labels to PR (ex. wip, ready-for-review, docs)

rigelrozanski · 2018-07-10T19:12:26Z

excited

codecov · 2018-07-10T23:03:00Z

Codecov Report

Merging #1620 into develop will decrease coverage by 1.38%.
The diff coverage is 10.71%.

@@             Coverage Diff             @@
##           develop    #1620      +/-   ##
===========================================
- Coverage    64.16%   62.77%   -1.39%     
===========================================
  Files          115      122       +7     
  Lines         6837     7125     +288     
===========================================
+ Hits          4387     4473      +86     
- Misses        2196     2390     +194     
- Partials       254      262       +8

cwgoes · 2018-07-11T02:54:39Z

Caught a bug already! #1630

rigelrozanski · 2018-07-13T08:17:41Z

p.s. merging rigel/remove-global-shares seems to fix tests.

rigelrozanski · 2018-07-13T08:18:06Z

cmd/gaia/simulation/sim_test.go

@@ -0,0 +1,21 @@
+package simulation


++ on this as a new package

unfortunately, turns out we need to access keepers, which are unexported

hmm - clarify the problem?

rigelrozanski · 2018-07-13T08:25:10Z

x/stake/simulation/sim_test.go

+}
+
+// SupplyInvariants checks that the total supply reflects all held loose tokens, bonded tokens, and unbonding delegations
+func SupplyInvariants(ck bank.Keeper, k stake.Keeper) mock.Invariant {


Maybe this file could be broken out a bit from sim_test.go into Invariants.go, msgs.go, simulation_test.go?

cwgoes · 2018-07-18T08:02:47Z

Not the finalized version of randomized simulation (see wishlist in first comment), but this can be reviewed & merged now to avoid blocking any other x/mock refactors.

cwgoes · 2018-07-18T08:16:26Z

This is technically breaking as it includes a quick fix for #1402.

rigelrozanski · 2018-07-18T17:41:58Z

@cwgoes can you highlight the changes made for #1402? didn't see any changes to the keys.go

cwgoes · 2018-07-18T18:23:04Z

@cwgoes can you highlight the changes made for #1402? didn't see any changes to the keys.go

I didn't change keys.go, just blocked having more than one unbonding delegation to the same validator.

https://github.com/cosmos/cosmos-sdk/pull/1620/files#diff-b905817409b4c0b71e9adffb102682b4R317

…lk-down-proof-of-stake

rigelrozanski · 2018-07-19T06:01:54Z

cmd/gaia/app/sim_test.go

+	simulationEnvSeed      = "GAIA_SIMULATION_SEED"
+	simulationEnvKeys      = "GAIA_SIMULATION_KEYS"
+	simulationEnvBlocks    = "GAIA_SIMULATION_BLOCKS"
+	simulationEnvBlockSize = "GAIA_SIMULATION_BLOCK_SIZE"


What was the thinking of using environment variables to pass information to the tests?

I'm probably more into using test flags (https://stackoverflow.com/questions/47045445/idiomatic-way-to-pass-variables-to-test-cases-in-golang)

OK, flags seem slightly cleaner - switched.

rigelrozanski · 2018-07-19T06:05:47Z

cmd/gaia/app/sim_test.go

+		[]simulation.RandSetup{},
+		[]simulation.Invariant{
+			banksim.NonnegativeBalanceInvariant(app.accountMapper),
+			stakesim.AllInvariants(app.coinKeeper, app.stakeKeeper, app.accountMapper),


rigelrozanski · 2018-07-19T06:18:01Z

x/stake/simulation/msgs.go

+		description := stake.Description{
+			Moniker: simulation.RandStringOfLength(r, 10),
+		}
+		key := keys[r.Intn(len(keys))]


Maybe I'm bonkers, but I think due to the nature of this type of compact statement, our code would be more clear if we expanded this to a function to separate out concerns a bit.

key := RandomKey(r, keys) func RandomKey(r *rand.Rand, keys []crypto.PrivKey) key { return keys[r.Intn( len(keys) )] }

I've always found it a bit irritating reading tests with the rand function because of lines like this, to separate out would make my mind and heart at ease

(obv. applies to all statements like this in this PR)

Agreed, that's cleaner. Updated.

rigelrozanski · 2018-07-19T06:20:23Z

x/stake/simulation/msgs.go

+		address := sdk.AccAddress(pubkey.Address())
+		amount := m.GetAccount(ctx, address).GetCoins().AmountOf(denom)
+		if amount.GT(sdk.ZeroInt()) {
+			amount = sdk.NewInt(int64(r.Intn(int(amount.Int64()))))


Same here how about:

key := RandomAmount(r, amount)

(obv. applies to all statements like this in this PR)

Agreed, updated.

rigelrozanski · 2018-07-19T06:21:49Z

x/stake/simulation/msgs.go

+		denom := params.BondDenom
+		loose := sdk.ZeroInt()
+		mapp.AccountMapper.IterateAccounts(ctx, func(acc auth.Account) bool {
+			balance := sdk.NewInt(int64(r.Intn(1000000)))


Would use RandomAmount as outlined above

Agreed, updated.

rigelrozanski

Old comments addressed

Lookin' good, just a few final points to address before I think we should merge. See comments

Comments addressed.

rigelrozanski

Woot!

Work-in-progress staking invariants

6a119f6

cwgoes requested review from ebuchman and rigelrozanski as code owners July 10, 2018 18:48

cwgoes added the wip label Jul 10, 2018

Merge branch 'develop' into cwgoes/a-random-walk-down-proof-of-stake

a332293

ebuchman changed the base branch from develop to master July 11, 2018 00:29

cwgoes added 2 commits July 11, 2018 02:36

Invariants & random Msgs in progress

940cfa9

Add several simulated Msgs

a3d8b38

cwgoes mentioned this pull request Jul 11, 2018

Redelegation subtracts coins from redelegator #1630

Closed

cwgoes added 5 commits July 11, 2018 04:55

CacheContext()

27f157a

Merge branch 'master' into cwgoes/a-random-walk-down-proof-of-stake

5e3cd77

Changes from merge

0572a27

'make test_sim', simulation folder

53138fb

TestMsgBeginUnbonding

88364c8

cwgoes changed the title ~~WIP: Randomized testing for staking/slashing~~ WIP: Simulation framework, including staking/slashing simulation Jul 11, 2018

cwgoes added 2 commits July 11, 2018 22:44

Add 'test_sim' to CircleCI

4623923

Move files

f9f326c

cwgoes mentioned this pull request Jul 12, 2018

staking random operation upgrade #1647

Closed

cwgoes added 6 commits July 12, 2018 21:30

Merge branch 'master' into cwgoes/a-random-walk-down-proof-of-stake

50d384a

Update Gopkg.lock & Makefile

601251d

Actual Gaia app

bb217b9

Merge branch 'master' into cwgoes/a-random-walk-down-proof-of-stake

d8ec44e

Merge branch 'master' into cwgoes/a-random-walk-down-proof-of-stake

9bd09e6

Delegation bug fixed!

c272db0

rigelrozanski reviewed Jul 13, 2018

View reviewed changes

cwgoes force-pushed the cwgoes/a-random-walk-down-proof-of-stake branch from d833c2d to 47b805d Compare July 18, 2018 08:00

Remove old randomized pool testing, redundant

2c0cd73

cwgoes force-pushed the cwgoes/a-random-walk-down-proof-of-stake branch from 47b805d to 2c0cd73 Compare July 18, 2018 08:00

cwgoes changed the title ~~WIP: Simulation framework, including staking/slashing simulation~~ Simulation framework, including staking/slashing simulation Jul 18, 2018

cwgoes added ready-for-review and removed wip labels Jul 18, 2018

Update changelog

16d4e07

cwgoes changed the title ~~Simulation framework, including staking/slashing simulation~~ Simulation framework, including staking simulation Jul 18, 2018

cwgoes added the T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). label Jul 18, 2018

cwgoes changed the title ~~Simulation framework, including staking simulation~~ R4R: Simulation framework, including staking simulation Jul 18, 2018

Merge remote-tracking branch 'origin/develop' into cwgoes/a-random-wa…

423d0c4

…lk-down-proof-of-stake

rigelrozanski reviewed Jul 19, 2018

View reviewed changes

rigelrozanski previously requested changes Jul 19, 2018

View reviewed changes

cwgoes added 2 commits July 19, 2018 08:40

Environment variables => flags

1e5a799

RandomKey, RandomAmount

ee29e10

cwgoes force-pushed the cwgoes/a-random-walk-down-proof-of-stake branch from ddd8d84 to ee29e10 Compare July 19, 2018 06:48

Merge branch 'develop' into cwgoes/a-random-walk-down-proof-of-stake

7e88a50

rigelrozanski approved these changes Jul 19, 2018

View reviewed changes

rigelrozanski merged commit a054532 into develop Jul 19, 2018

rigelrozanski deleted the cwgoes/a-random-walk-down-proof-of-stake branch July 19, 2018 22:47

cwgoes mentioned this pull request Jul 20, 2018

R4R: Slashing, validator set, and governance simulation #1783

Merged

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Simulation framework, including staking simulation #1620

R4R: Simulation framework, including staking simulation #1620

cwgoes commented Jul 10, 2018 •

edited

Loading

rigelrozanski commented Jul 10, 2018

codecov bot commented Jul 10, 2018 •

edited

Loading

cwgoes commented Jul 11, 2018

rigelrozanski commented Jul 13, 2018

rigelrozanski Jul 13, 2018

cwgoes Jul 18, 2018

rigelrozanski Jul 18, 2018

rigelrozanski Jul 13, 2018

cwgoes Jul 18, 2018

cwgoes commented Jul 18, 2018 •

edited

Loading

cwgoes commented Jul 18, 2018

rigelrozanski commented Jul 18, 2018

cwgoes commented Jul 18, 2018 •

edited

Loading

rigelrozanski Jul 19, 2018

rigelrozanski Jul 19, 2018

cwgoes Jul 19, 2018

rigelrozanski Jul 19, 2018

rigelrozanski Jul 19, 2018

rigelrozanski Jul 19, 2018

cwgoes Jul 19, 2018

rigelrozanski Jul 19, 2018

rigelrozanski Jul 19, 2018

cwgoes Jul 19, 2018

rigelrozanski Jul 19, 2018

cwgoes Jul 19, 2018

rigelrozanski left a comment

rigelrozanski left a comment

R4R: Simulation framework, including staking simulation #1620

R4R: Simulation framework, including staking simulation #1620

Conversation

cwgoes commented Jul 10, 2018 • edited Loading

rigelrozanski commented Jul 10, 2018

codecov bot commented Jul 10, 2018 • edited Loading

Codecov Report

cwgoes commented Jul 11, 2018

rigelrozanski commented Jul 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwgoes commented Jul 18, 2018 • edited Loading

cwgoes commented Jul 18, 2018

rigelrozanski commented Jul 18, 2018

cwgoes commented Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rigelrozanski left a comment

Choose a reason for hiding this comment

rigelrozanski left a comment

Choose a reason for hiding this comment

cwgoes commented Jul 10, 2018 •

edited

Loading

codecov bot commented Jul 10, 2018 •

edited

Loading

cwgoes commented Jul 18, 2018 •

edited

Loading

cwgoes commented Jul 18, 2018 •

edited

Loading