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

Random Fuzzer panic: calculated final stake greater than current stake (on f0b690ba6e2) #4383

Closed
4 tasks
npinto opened this issue May 20, 2019 · 13 comments · Fixed by #4389
Closed
4 tasks

Comments

@npinto
Copy link
Contributor

npinto commented May 20, 2019

Summary of Bug

Simulating... block 313/500, operation 50/371. panic with err: calculated final stake for delegator cosmos13370knmrqvt3zh795apth8ujykcgmrsmj64hdp greater than current stake
        final stake:    81974944596.112730277011189511
        current stake:  81974944596.112730277011189510

Version

$ git log -1
commit f0b690ba6e22d06e13c5366195ab8e6c8c99b367 (HEAD -> master, origin/master, origin/HEAD)
Author: Nicolas Pinto <[email protected]>
Date:   Fri May 17 16:46:43 2019 -0700

    Merge PR #4366: Random Fuzzer fix: avoids VotingPeriod=0

Steps to Reproduce

$ go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=500 -SimulationGenesis= -SimulationVerbose=true -SimulationCommit=true -SimulationSeed=8090485 -SimulationPeriod=50 -v -timeout 24h

Fails with:

Simulating... block 313/500, operation 50/371. panic with err: calculated final stake for delegator cosmos13370knmrqvt3zh795apth8ujykcgmrsmj64hdp greater than current stake
        final stake:    81974944596.112730277011189511
        current stake:  81974944596.112730277011189510
goroutine 34 [running]:
runtime/debug.Stack(0x18629e0, 0xc0000c2008, 0x1549b84)
        /snap/go/3739/src/runtime/debug/stack.go:24 +0xab
github.com/cosmos/cosmos-sdk/x/simulation.SimulateFromSeed.func2(0x18629e0, 0xc0000c2008, 0x186e7e0, 0xc001078020, 0xc000f9dc20, 0xc000135660)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/simulation/simulate.go:128 +0x12b
panic(0x13ad880, 0xc002b440c0)
        /snap/go/3739/src/runtime/panic.go:522 +0x1b5
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.calculateDelegationRewards(0x186e720, 0xc0001351c0, 0xc000b3c0e0, 0xc000b3c0e0, 0x186e720, 0xc000135210, 0x186e760, 0xc000135220, 0xc000b30f40, 0x5, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/delegation.go:106 +0x81a
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.withdrawDelegationRewards(0x186e720, 0xc0001351c0, 0xc000b3c0e0, 0xc000b3c0e0, 0x186e720, 0xc000135210, 0x186e760, 0xc000135220, 0xc000b30f40, 0x5, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/delegation.go:126 +0x2cc
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Hooks.BeforeDelegationSharesModified(0x186e720, 0xc0001351c0, 0xc000b3c0e0, 0xc000b3c0e0, 0x186e720, 0xc000135210, 0x186e760, 0xc000135220, 0xc000b30f40, 0x5, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/hooks.go:86 +0x223
github.com/cosmos/cosmos-sdk/x/staking/types.MultiStakingHooks.BeforeDelegationSharesModified(0xc00011ede0, 0x2, 0x2, 0x187d480, 0xc0033502d0, 0xc002fddb40, 0xd, 0xc0000f41e0, 0x14, 0x20, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/staking/types/hooks.go:47 +0x12a
github.com/cosmos/cosmos-sdk/x/staking/keeper.Keeper.BeforeDelegationSharesModified(0x186e720, 0xc000135190, 0x186e760, 0xc0001351a0, 0xc000b3c0e0, 0x7f4e380a7318, 0xc000f9c1e0, 0x188d140, 0xc00011ee00, 0xc000b3c0e0, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/staking/keeper/hooks.go:52 +0x115
github.com/cosmos/cosmos-sdk/x/staking/keeper.Keeper.Delegate(0x186e720, 0xc000135190, 0x186e760, 0xc0001351a0, 0xc000b3c0e0, 0x7f4e380a7318, 0xc000f9c1e0, 0x188d140, 0xc00011ee00, 0xc000b3c0e0, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/staking/keeper/delegation.go:465 +0x2cf
github.com/cosmos/cosmos-sdk/x/staking.handleMsgDelegate(0x187d480, 0xc0033502d0, 0xc002fddb40, 0xd, 0xc0000f41e0, 0x14, 0x20, 0xc002d15a20, 0x14, 0x14, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/staking/handler.go:223 +0x46b
github.com/cosmos/cosmos-sdk/x/staking.NewHandler.func1(0x187d480, 0xc0033502d0, 0xc002fddb40, 0xd, 0x1881380, 0xc005d9ea50, 0x0, 0x0, 0x0, 0x0, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/staking/handler.go:28 +0x692
github.com/cosmos/cosmos-sdk/x/staking/simulation.SimulateMsgDelegate.func1(0xc0031c3f20, 0xc000014480, 0x187d480, 0xc0031c34a0, 0xc002fddb40, 0xc, 0xc000fbc000, 0x1a, 0x1a, 0x0, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/staking/simulation/msgs.go:128 +0xb34
github.com/cosmos/cosmos-sdk/x/simulation.createBlockSimulator.func1(0xc000fb67e0, 0xc000014480, 0x187d480, 0xc0031c34a0, 0xc002fddb40, 0xc, 0xc000fbc000, 0x1a, 0x1a, 0x0, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/simulation/simulate.go:263 +0x74b
github.com/cosmos/cosmos-sdk/x/simulation.SimulateFromSeed(0x18931c0, 0xc0000f3b00, 0x18629e0, 0xc0000c2008, 0xc000014480, 0x16b0e70, 0x7b7375, 0xc000f9f0e0, 0xf, 0xf, ...)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/x/simulation/simulate.go:167 +0x1a6c
github.com/cosmos/cosmos-sdk/cmd/gaia/app.TestFullGaiaSimulation(0xc0000f3b00)
        /home/cosmos/go/src/github.com/cosmos/cosmos-sdk/cmd/gaia/app/sim_test.go:376 +0x418
testing.tRunner(0xc0000f3b00, 0x16b0e30)
        /snap/go/3739/src/testing/testing.go:865 +0x164
created by testing.(*T).Run
        /snap/go/3739/src/testing/testing.go:916 +0x65b

Logs to writing to /home/cosmos/.gaiad/simulations/2019-05-20_15:36:39.log
GoLevelDB Stats
Compactions
 Level |   Tables   |    Size(MB)   |    Time(sec)  |    Read(MB)   |   Write(MB)
-------+------------+---------------+---------------+---------------+---------------
   0   |          0 |       0.00000 |       1.93235 |       0.00000 |       8.13954
   1   |          1 |       0.25174 |       0.81421 |       8.78090 |       0.89310

GoLevelDB cached block size 529442
--- FAIL: TestFullGaiaSimulation (784.02s)
    require.go:765:
                Error Trace:    sim_test.go:384
                Error:          Expected nil, but got: &errors.errorString{s:"Simulation halted due to panic on block 313"}
                Test:           TestFullGaiaSimulation
FAIL

Full log:
8090485.log

HTH!
Best,
Nico


For Admin Use

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

Thanks @npinto! So this was reproduced with a modified parameter? I'm not sure if this is benign or note. This is an off-by-one rounding error again as we've seen many times and I'm sure sure that we've fixed.

/cc @rigelrozanski

@rigelrozanski
Copy link
Contributor

rigelrozanski commented May 21, 2019

Cool I'm looking into it right now


Some updates:

Alternative command for post-gaia remove go test ./simapp/... -run TestFullAppSimulation -SimulationEnabled=true -SimulationNumBlocks=500 -SimulationGenesis= -SimulationVerbose=true -SimulationCommit=true -SimulationSeed=8090485 -SimulationPeriod=50 -v -timeout 24h

This bug starts at commit f0b690ba6e22d06e13c5366195ab8e6c8c99b367 ( Merge PR #4366: Random Fuzzer fix: avoids VotingPeriod=0)
And is resolved at commit dd89c329516e32c309a96bb455761904de0d0cee (Community pool spend proposal (#4329))
which makes sense considering that this commit simply adds more simulation options, meaning that the simulation is just totally different - meaning the bug probably exists in another seed though.

@npinto
Copy link
Contributor Author

npinto commented May 21, 2019

which makes sense considering that this commit simply adds more simulation options, meaning that the simulation is just totally different - meaning the bug probably exists in another seed though.

thanks, exactly my point!

also see:
#4367

where "This is an off-by-one rounding error" isn't true.

HTH

@alexanderbez
Copy link
Contributor

The invariance is off by one, is it not?

@npinto
Copy link
Contributor Author

npinto commented May 21, 2019

No, not here: #4367 (comment)

Simulating... block 161/500, operation 3050/3428. Panic with err
 calculated final stake for delegator cosmos1z0rq9vej70rqr9a9wt2pdf8jfwddhaalwd9ynw greater than current stake
	final stake:	2506467226.583679254317432190
	current stake:	2448222367.754094039806751512

@rigelrozanski
Copy link
Contributor

rigelrozanski commented May 21, 2019

@npinto seed 99516184 seems to pass now - Is there any reproducible "more-than-one" seeds on this branch? (the branch which fails with seed 8090485)

@npinto
Copy link
Contributor Author

npinto commented May 21, 2019

@rigelrozanski you can reproduce this bug with seed 99516184 using tag v0.34.4 and the following patch (back-ported from #4363) :

$ git diff
diff --git a/cmd/gaia/app/sim_test.go b/cmd/gaia/app/sim_test.go
index 80a268ee..798b7be3 100644
--- a/cmd/gaia/app/sim_test.go
+++ b/cmd/gaia/app/sim_test.go
@@ -165,7 +165,7 @@ func appStateRandomizedFn(r *rand.Rand, accs []simulation.Account, genesisTimest
        govGenesis := gov.GenesisState{
                StartingProposalID: uint64(r.Intn(100)),
                DepositParams: gov.DepositParams{
-                       MinDeposit:       sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, int64(r.Intn(1e3)))},
+                       MinDeposit:       sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, int64(randIntBetween(r, 1, 1e3)))},
                        MaxDepositPeriod: vp,
                },
                VotingParams: gov.VotingParams{

@npinto
Copy link
Contributor Author

npinto commented May 21, 2019

is there any reproducible "more-than-one" seeds on this branch? (the branch which fails with seed 8090485)

Do you think we could find a way to pass an exact (or partial) parameter set to the sim (e.g. via json file from the cli)? It would then be easier to reproduce (and explore) bugs like these.

@rigelrozanski
Copy link
Contributor

So we already have a way to simulate from a genesis file (where a custom parameter set could be defined) checkout this make command:

cosmos-sdk/Makefile

Lines 88 to 92 in 7521446

test_sim_app_custom_genesis_fast:
@echo "Running custom genesis simulation..."
@echo "By default, ${HOME}/.gaiad/config/genesis.json will be used."
@go test -mod=readonly $(SIMAPP) -run TestFullAppSimulation -SimulationGenesis=${HOME}/.gaiad/config/genesis.json \
-SimulationEnabled=true -SimulationNumBlocks=100 -SimulationBlockSize=200 -SimulationCommit=true -SimulationSeed=99 -SimulationPeriod=5 -v -timeout 24h

Is there a 99516184 parameter set which fails then?

@npinto
Copy link
Contributor Author

npinto commented May 21, 2019

Great! I'll try that route. What's your recommended way to export a genesis state as a json file during a given TestFullAppSimulation run ?

@rigelrozanski
Copy link
Contributor

rigelrozanski commented May 21, 2019

We should probably add a flag to do exactly this on an existing simulation, checkout how TestAppSimulationAfterImport creates the json file

@npinto
Copy link
Contributor Author

npinto commented May 21, 2019

TestAppSimulationAfterImport seems to be exporting a LevelDB representation, not a json:
https://github.com/cosmos/cosmos-sdk/blob/master/simapp/sim_test.go#L405

Am I missing something?

@rigelrozanski
Copy link
Contributor

it creates the genesis app state here #4389 but it doesn't export it to a file

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.

3 participants