Replace assertInvariantsBlockly invariantsCheckFrequency

[alessio]

Replace gaiad's --assert-invariants-blockly with --inv-check-period

Closes: #4053

• Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
• Wrote tests
• Updated relevant documentation (docs/)
• Added a relevant changelog entry: sdkch add [section] [stanza] [message]
• 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]

Codecov Report

Merging #4083 into develop will increase coverage by 0.01%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           develop   #4083      +/-   ##
==========================================
+ Coverage    59.98%     60%   +0.01%     
==========================================
  Files          211     211              
  Lines        15109   15113       +4     
==========================================
+ Hits          9063    9068       +5     
+ Misses        5426    5425       -1     
  Partials       620     620

[alessio]

Attaching test evidence:

• Run gaiad with defaults:

alessio@bangalter:~/work/cosmos-sdk$ gaiad start --log_level invariants:info,state:info
E[2019-04-10|11:59:06.011] Couldn't connect to any seeds                module=p2p 
I[2019-04-10|11:59:11.088] Asserted all invariants                      module=invariants duration=762.345µs height=188
I[2019-04-10|11:59:11.088] Executed block                               module=state height=189 validTxs=0 invalidTxs=0
I[2019-04-10|11:59:11.089] Committed state                              module=state height=189 txs=0 appHash=42A2A1EF4D9D7BDBE1B0C054B73E41CBE24FF5DD5AA309051F45CC46DB5D0906
I[2019-04-10|11:59:16.107] Asserted all invariants                      module=invariants duration=696.589µs height=189
I[2019-04-10|11:59:16.107] Executed block                               module=state height=190 validTxs=0 invalidTxs=0
I[2019-04-10|11:59:16.109] Committed state                              module=state height=190 txs=0 appHash=E2E0013F540EBB6C0690802EF6EDB92D7E230CBFF94D8929DF87D3262673A9DC
I[2019-04-10|11:59:21.178] Asserted all invariants                      module=invariants duration=659.56µs height=190
I[2019-04-10|11:59:21.178] Executed block                               module=state height=191 validTxs=0 invalidTxs=0
I[2019-04-10|11:59:21.180] Committed state                              module=state height=191 txs=0 appHash=B0AF137A09C908E3F9191F6FC772D383AB5C24F601B113D5EFCCFA9B01CCF454

• Run gaiad with --inv-checks-period=0:

alessio@bangalter:~/work/cosmos-sdk$ gaiad start --log_level invariants:info,state:info --inv-checks-period=0
E[2019-04-10|12:00:32.508] Couldn't connect to any seeds                module=p2p 
I[2019-04-10|12:00:37.573] Executed block                               module=state height=192 validTxs=0 invalidTxs=0
I[2019-04-10|12:00:37.575] Committed state                              module=state height=192 txs=0 appHash=94FAD83189EF6F76E7EF61DA5B245B466484350FB9A36324F8D5614CD896F905
I[2019-04-10|12:00:42.590] Executed block                               module=state height=193 validTxs=0 invalidTxs=0
I[2019-04-10|12:00:42.592] Committed state                              module=state height=193 txs=0 appHash=5F2DB4F0530574221AB135D3D4D15BBF67808B33B925C85CAFF8DD2FA0BCA7E8
I[2019-04-10|12:00:47.662] Executed block                               module=state height=194 validTxs=0 invalidTxs=0
I[2019-04-10|12:00:47.664] Committed state                              module=state height=194 txs=0 appHash=867589001360B4FAE12CBE7215846E677E855B0CBE05ED10FC68CD7532FF8FB7
I[2019-04-10|12:00:52.681] Executed block                               module=state height=195 validTxs=0 invalidTxs=0
I[2019-04-10|12:00:52.683] Committed state                              module=state height=195 txs=0 appHash=539CAD761809F49538DE8145B79B50D19EEC0400F1A27DF5E71691A5A1701122

• Run gaiad with --inv-checks-period=2:

alessio@bangalter:~/work/cosmos-sdk$ gaiad start --log_level invariants:info,state:info --inv-checks-period=2
E[2019-04-10|12:01:30.567] Couldn't connect to any seeds                module=p2p 
I[2019-04-10|12:01:35.627] Asserted all invariants                      module=invariants duration=779.615µs height=195
I[2019-04-10|12:01:35.627] Executed block                               module=state height=196 validTxs=0 invalidTxs=0
I[2019-04-10|12:01:35.628] Committed state                              module=state height=196 txs=0 appHash=539B3CF60033451390BEAF736D4F58FD5A2B28D940DA304B8DC1D77425921AB7
I[2019-04-10|12:01:40.642] Executed block                               module=state height=197 validTxs=0 invalidTxs=0
I[2019-04-10|12:01:40.645] Committed state                              module=state height=197 txs=0 appHash=290CA795C066DD864A12CD19B78AC6906AC63A108D8191035D2A133B13B84E8C
I[2019-04-10|12:01:45.713] Asserted all invariants                      module=invariants duration=621.615µs height=197
I[2019-04-10|12:01:45.713] Executed block                               module=state height=198 validTxs=0 invalidTxs=0
I[2019-04-10|12:01:45.715] Committed state                              module=state height=198 txs=0 appHash=93CE3629E7DAE3388CD6B9146C38518E5BF7BA776BEB947C1FFEC11D548F5C60
I[2019-04-10|12:01:50.724] Executed block                               module=state height=199 validTxs=0 invalidTxs=0
I[2019-04-10|12:01:50.725] Committed state                              module=state height=199 txs=0 appHash=5BFF38500C01EBC0115FADC84D86829850D3403D3A463A97AAF00F137C3AA254
I[2019-04-10|12:01:55.796] Asserted all invariants                      module=invariants duration=666.565µs height=199
I[2019-04-10|12:01:55.796] Executed block                               module=state height=200 validTxs=0 invalidTxs=0
I[2019-04-10|12:01:55.798] Committed state                              module=state height=200 txs=0 appHash=BDD3D9D425839B6EED9C152DACCFD44F01D2D4507E22A79E3621DD27DD8A844D
E[2019-04-10|12:02:00.567] Couldn't connect to any seeds                module=p2p 

[cwgoes]

Concept ACK.

The thing you call "frequency" is a "period" between invariant checks - a "frequency" would be its inverse. Also the logic could be simplified a bit (see comment).

[fedekunze]

LGTM

[alexanderbez]

A minor nit on naming, but functionally LGTM.

Comments were incorporated

[alexanderbez]

Actually, @alessio can you please update the testnet command to also set a period of idk...maybe 10?

	gaiaConfig := srvconfig.DefaultConfig()
	gaiaConfig.MinGasPrices = viper.GetString(server.FlagMinGasPrices)
        // set the value here

[alessio]

I disagree @alexanderbez for 2 reasons:

1. this PR already meets story's ACs as it is now
2. that flag cannot be passed via config file cause config belongs to the server package, which is and should remain gaia-agnostic

[alexanderbez]

I disagree @alexanderbez for 2 reasons:

1. this PR already meets story's ACs as it is now
2. that flag cannot be passed via config file cause config belongs to the server package, which is and should remain gaia-agnostic

I respectfully disagree. I don't think we should absolutely hinder the development and refinement of PRs, especially from the context of review, due to the scope of the ACs -- this is my primary beef with forcing us to have concrete ACs. I can think of a whole host of prior PRs that would end up being half-baked in nature because they simply met what would be their ACs at the time. As long as issues have a clearly stated bug or feature and the desired actionable items, that suffices in most cases.

I'll be more than happy to add this feature myself -- it's a one liner (Dockerfile).

EDIT/TL;DR: What I'm really trying to say is that the ACs, as they're defined (regardless of format), should be "soft" criteria.

[fedekunze]

what about if we move this conv outside so we don't block this PR ?

[alessio]

Dear @alexanderbez

First and foremost, I want to thank you for raising this issue and reassure you that I am sympathetic with you. Clearly you want to deliver the best outcome possible. So do I. I am confident that you believe me.
I also believe that we should enable users to configure gaiad either via both a command line flag and a configuration option.

Having said that, I also believe that giving the user the option to customise gaiad behaviour via command line flag only is the minimum viable solution that we should adopt for now as a temporary mitigation to the disruption caused to users by not having this feature at all. I thouroughly believe that it is better to allow users to set a invariant check period via command line flag than not allowing them to do so at all.

Stories' Acceptance Criteria are there for a reason. They by no means represent the ultimate solutions to problems: you're absolutely right there, they're intrinsically meant to partially address issues. AC are though functional to our common goal of delivering the best possible product for our users.

In my very humble opinion, even before delivering this, we should sit down and write acceptance criteria for a new issue whose ultimate goal should be to address what would be left to be done once this is PR is approved and merged.
By doing so, I believe we'd deliver the users the greatest possible value as we would:

• put them in the position to try and evaluate the benefits of being able to configure the period which gaiad runs invariance checks
• give us feedback - which we allegedly are eager for

In light of the above, supposing that there are no other technical implementation-related concerns that might prevent us of approving and merging this PR, we really should let this small piece of code in and work out our next step towards a better product.