R4R: Broken-Invar Tx - aka. Crisis module #3656

rigelrozanski · 2019-02-14T22:46:35Z

This PR also introduces gaiad assert-invariants-blockly

closes #2935
offshoot-issue #3967

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)

cwgoes

Thanks for the spec, some notes. I think we should keep this as simple as possible initially.

docs/spec/crisis/README.md

x/crisis/handler.go

docs/spec/crisis/README.md

int

int int int

x/crisis/handler.go

cwgoes

Thanks @rigelrozanski - structure is mostly solid I think; see comments.

cmd/gaia/app/app.go

docs/spec/crisis/02_messages.md

x/crisis/codec.go

x/crisis/handler.go

x/crisis/msg.go

x/distribution/keeper/fee_pool.go

mossid

Just a few comments on the code 👍

x/crisis/handler.go

x/crisis/keeper.go

x/distribution/keeper/invariants.go

x/crisis/params.go

Co-Authored-By: rigelrozanski <rigel.rozanski@gmail.com>

int

rigelrozanski · 2019-03-25T18:53:57Z

@cwgoes I've added in usage of all the invariants from the crisis module instead of the previously restrictive list:

cosmos-sdk/cmd/gaia/app/invariants.go

Lines 19 to 26 in 148d21e

    
           invarRoutes := app.crisisKeeper.Routes() 
        
           for _, ir := range invarRoutes { 
        
           	if err := ir.Invar(ctx); err != nil { 
        
           		panic(fmt.Errorf("invariant broken: %s\n"+ 
        
           			"\tCRITICAL please submit the following transaction:\n"+ 
        
           			"\t\t gaiacli tx crisis invariant-broken %v %v", err, ir.ModuleName, ir.Route)) 
        
           	} 
        
           }

Now a bunch of import/export tests fail in distribution. Notably here is a fast/simple failing test:

go test ./cmd/gaia/app -run TestGaiaSimulationAfterImport -SimulationEnabled=true -SimulationNumBlocks=5 -SimulationVerbose=true -SimulationCommit=true -SimulationSeed=1 -v -timeout 24h

debug startingPeriod: 3
debug endingPeriod: 2
--- FAIL: TestGaiaSimulationAfterImport (7.93s)
panic: startingPeriod cannot be greater than endingPeriod [recovered]
	panic: startingPeriod cannot be greater than endingPeriod

goroutine 19 [running]:
testing.tRunner.func1(0xc0004e7400)
	/usr/local/Cellar/go/1.12.1/libexec/src/testing/testing.go:830 +0x392
panic(0x18e1c00, 0x1c9e9e0)
	/usr/local/Cellar/go/1.12.1/libexec/src/runtime/panic.go:522 +0x1b5
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.calculateDelegationRewardsBetween(0x1cbcb00, 0xc000966d10, 0xc00019f8f0, 0xc00019f8f0, 0x1cbcb00, 0xc000966d60, 0x1cbcb40, 0xc000966d70, 0xc0012caf80, 0x5, ...)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/delegation.go:37 +0x70c
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.calculateDelegationRewards(0x1cbcb00, 0xc000966d10, 0xc00019f8f0, 0xc00019f8f0, 0x1cbcb00, 0xc000966d60, 0x1cbcb40, 0xc000966d70, 0xc0012caf80, 0x5, ...)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/delegation.go:103 +0x3fc
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.withdrawDelegationRewards(0x1cbcb00, 0xc000966d10, 0xc00019f8f0, 0xc00019f8f0, 0x1cbcb00, 0xc000966d60, 0x1cbcb40, 0xc000966d70, 0xc0012caf80, 0x5, ...)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/delegation.go:117 +0x301
github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.WithdrawDelegationRewards(0x1cbcb00, 0xc000966d10, 0xc00019f8f0, 0xc00019f8f0, 0x1cbcb00, 0xc000966d60, 0x1cbcb40, 0xc000966d70, 0xc0012caf80, 0x5, ...)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/keeper.go:62 +0x20c
github.com/cosmos/cosmos-sdk/x/distribution/keeper.CanWithdrawInvariant.func1.1(0x0, 0x1cd7760, 0xc0038c4700, 0xc0038c4700)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/invariants.go:84 +0x62b
github.com/cosmos/cosmos-sdk/x/staking/keeper.Keeper.IterateValidators(0x1cbcb00, 0xc000966ce0, 0x1cbcb40, 0xc000966cf0, 0xc00019f8f0, 0x331a188, 0xc00000c5a0, 0x1cd4860, 0xc0000d9c20, 0xc00019f8f0, ...)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/x/staking/keeper/alias_functions.go:21 +0x1fa
github.com/cosmos/cosmos-sdk/x/distribution/keeper.CanWithdrawInvariant.func1(0x1cc89c0, 0xc0018cc930, 0xc0046b9b40, 0xc, 0x0, 0x0)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/x/distribution/keeper/invariants.go:78 +0x372
github.com/cosmos/cosmos-sdk/cmd/gaia/app.(*GaiaApp).assertRuntimeInvariantsOnContext(0xc0003c8000, 0x1cc89c0, 0xc0018cc930, 0xc0046b9b40, 0xc)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/cmd/gaia/app/invariants.go:21 +0x373
github.com/cosmos/cosmos-sdk/cmd/gaia/app.(*GaiaApp).assertRuntimeInvariants(0xc0003c8000)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/cmd/gaia/app/invariants.go:14 +0x100
github.com/cosmos/cosmos-sdk/cmd/gaia/app.(*GaiaApp).initChainer(0xc0003c8000, 0x1cc89c0, 0xc000f3a210, 0xc0009ac540, 0xd, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/cmd/gaia/app/app.go:343 +0x37c
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).InitChain(0xc0003c6200, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/baseapp/baseapp.go:354 +0x2c1
github.com/cosmos/cosmos-sdk/cmd/gaia/app.TestGaiaSimulationAfterImport(0xc0004e7400)
	/Users/rigelrozanski/go/src/github.com/cosmos/cosmos-sdk/cmd/gaia/app/sim_test.go:519 +0x728
testing.tRunner(0xc0004e7400, 0x1b89100)
	/usr/local/Cellar/go/1.12.1/libexec/src/testing/testing.go:865 +0xc0
created by testing.(*T).Run
	/usr/local/Cellar/go/1.12.1/libexec/src/testing/testing.go:916 +0x35a
FAIL	github.com/cosmos/cosmos-sdk/cmd/gaia/app	8.012s

client/utils/utils.go

cmd/gaia/cmd/gaiacli/main.go

cmd/gaia/cmd/gaiad/main.go

docs/spec/crisis/02_messages.md

docs/spec/crisis/03_tags.md

x/auth/stdtx.go

x/crisis/handler.go

alexanderbez · 2019-03-26T15:51:07Z

x/distribution/alias.go

+	ReferenceCountInvariant                   = keeper.ReferenceCountInvariant
+	CreateTestInputDefault                    = keeper.CreateTestInputDefault
+	CreateTestInputAdvanced                   = keeper.CreateTestInputAdvanced
+	TestAddrs                                 = keeper.TestAddrs


What is TestAddrs? Anything pertaining to tests?

yes, they are predefined addresses to test with.

In that case I don't think they should be defined or set here.

Could you be more specific? How better should this be organized?

I just don't think they should be set or defined in "core" code. i.e. keep them in tests files or test utilities.

so this is defined in a "test" file (aka keeper/test_common.go) I'm kinda peeved at all the code dup between the this test_common.go file across all the modules... but maybe it's worth refactoring into a new testing utilities package like x/testing_utilities - what you think of that?

Co-Authored-By: rigelrozanski <rigel.rozanski@gmail.com>

cmd/gaia/app/export.go

x/crisis/keeper.go

alexanderbez · 2019-03-27T13:19:44Z

x/distribution/alias.go

+	ReferenceCountInvariant                   = keeper.ReferenceCountInvariant
+	CreateTestInputDefault                    = keeper.CreateTestInputDefault
+	CreateTestInputAdvanced                   = keeper.CreateTestInputAdvanced
+	TestAddrs                                 = keeper.TestAddrs


I just don't think they should be set or defined in "core" code. i.e. keep them in tests files or test utilities.

cmd/gaia/app/app.go

cwgoes · 2019-03-28T18:37:50Z

ACK though see comment at https://github.com/cosmos/cosmos-sdk/pull/3999/files#r270145744

rigelrozanski added 2 commits February 14, 2019 05:28

beginning thinking on issue

f667287

...

6fa1607

rigelrozanski added this to the v0.33.0 (Enable Transfers) milestone Feb 14, 2019

rigelrozanski requested review from cwgoes and ebuchman as code owners February 14, 2019 22:46

rigelrozanski added 5 commits February 16, 2019 21:12

working

2dc3540

Merge remote-tracking branch 'origin/develop' into rigel/crisis-module

58dde28

working

9d32106

working fee pool distribution

797129e

spek outline

2765404

rigelrozanski changed the title ~~WIP: Broken-Invar Tx - aka. Crisis module~~ SPEC-R4R: Broken-Invar Tx - aka. Crisis module Feb 19, 2019

rigelrozanski added 2 commits February 19, 2019 01:27

spec update

1ec77ae

gas refund calculations

037796d

cwgoes suggested changes Feb 22, 2019

View reviewed changes

rigelrozanski added 15 commits March 6, 2019 22:13

simulation saved to ~/.gaiad/simulations/

dae544b

lean simulation output

ba44894

int

cleanup bank simulation messages

d79e4f8

operation messges

0a6ea3b

int

lint

de6f42f

move simulation to its own module

d65a9c9

move simulation log code to log.go

b57a771

logger overhaul

dea0ce1

int

distribution comments

9add266

fix compiling

81f28ba

cleanup modifications to x/distribution/keeper/allocation.go

e1d80fb

int int int

gov bug

3f8e5a3

result.IsOK() minimization

95b6b8b

importExport typo bug

bfeb0de

pending

3d94987

alexanderbez removed this from the v0.34.0 milestone Mar 11, 2019

rigelrozanski added this to the v0.34.0 milestone Mar 23, 2019

rigelrozanski requested a review from cwgoes March 23, 2019 19:15

fedekunze reviewed Mar 23, 2019

View reviewed changes

x/crisis/handler.go Show resolved Hide resolved

cwgoes suggested changes Mar 24, 2019

View reviewed changes

mossid suggested changes Mar 25, 2019

View reviewed changes

x/crisis/handler.go Show resolved Hide resolved

x/crisis/keeper.go Show resolved Hide resolved

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

x/crisis/params.go Outdated Show resolved Hide resolved

x/crisis/params.go Outdated Show resolved Hide resolved

transaction details in runtime invariance panic

2d79e24

rigelrozanski mentioned this pull request Mar 25, 2019

Runtime Invariance to automatically submit invariant-broken transaction #3969

Closed

cwgoes and others added 3 commits March 25, 2019 12:58

Apply suggestions from code review

164fe8a

Co-Authored-By: rigelrozanski <rigel.rozanski@gmail.com>

sender tags

35fd299

@mossid suggestions

a90cecc

int

rigelrozanski requested a review from mossid March 25, 2019 18:18

@cwgoes comments final

148d21e

rigelrozanski requested a review from cwgoes March 25, 2019 18:34

alexanderbez suggested changes Mar 26, 2019

View reviewed changes

Apply suggestions from code review

5db5051

Co-Authored-By: rigelrozanski <rigel.rozanski@gmail.com>

rigelrozanski requested a review from alexanderbez March 26, 2019 16:19

alexanderbez suggested changes Mar 27, 2019

View reviewed changes

alexanderbez reviewed Mar 27, 2019

View reviewed changes

cmd/gaia/app/app.go Show resolved Hide resolved

rigelrozanski mentioned this pull request Mar 27, 2019

TO ADD rigelrozanski/aes-fmt#2

Open

rigelrozanski force-pushed the rigel/crisis-module branch from af03c33 to 5db5051 Compare March 28, 2019 16:38

bug seems fixed (#3998)

3e040de

cwgoes approved these changes Mar 28, 2019

View reviewed changes

mossid approved these changes Mar 28, 2019

View reviewed changes

delete unused line in zero height export bug

addbabe

rigelrozanski mentioned this pull request Mar 28, 2019

V0.33.1 zero export bugfix #3999

Merged

5 tasks

alexanderbez approved these changes Mar 28, 2019

View reviewed changes

rigelrozanski merged commit df43941 into develop Mar 28, 2019

rigelrozanski deleted the rigel/crisis-module branch March 28, 2019 23:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Broken-Invar Tx - aka. Crisis module #3656

R4R: Broken-Invar Tx - aka. Crisis module #3656

rigelrozanski commented Feb 14, 2019 •

edited

Loading

cwgoes left a comment

cwgoes left a comment

mossid left a comment

rigelrozanski commented Mar 25, 2019 •

edited

Loading

alexanderbez Mar 26, 2019

rigelrozanski Mar 26, 2019

alexanderbez Mar 26, 2019

rigelrozanski Mar 26, 2019

alexanderbez Mar 27, 2019

rigelrozanski Mar 27, 2019

alexanderbez Mar 27, 2019

cwgoes commented Mar 28, 2019

R4R: Broken-Invar Tx - aka. Crisis module #3656

R4R: Broken-Invar Tx - aka. Crisis module #3656

Conversation

rigelrozanski commented Feb 14, 2019 • edited Loading

cwgoes left a comment

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

mossid left a comment

Choose a reason for hiding this comment

rigelrozanski commented Mar 25, 2019 • edited Loading

alexanderbez Mar 26, 2019

Choose a reason for hiding this comment

rigelrozanski Mar 26, 2019

Choose a reason for hiding this comment

alexanderbez Mar 26, 2019

Choose a reason for hiding this comment

rigelrozanski Mar 26, 2019

Choose a reason for hiding this comment

alexanderbez Mar 27, 2019

Choose a reason for hiding this comment

rigelrozanski Mar 27, 2019

Choose a reason for hiding this comment

alexanderbez Mar 27, 2019

Choose a reason for hiding this comment

cwgoes commented Mar 28, 2019

rigelrozanski commented Feb 14, 2019 •

edited

Loading

rigelrozanski commented Mar 25, 2019 •

edited

Loading