Codecov Report

Merging #2360 into develop will increase coverage by 0.08%.
The diff coverage is 87.5%.

@@             Coverage Diff             @@
##           develop    #2360      +/-   ##
===========================================
+ Coverage       61%   61.09%   +0.08%     
===========================================
  Files          197      197              
  Lines        16294    16300       +6     
===========================================
+ Hits          9940     9958      +18     
+ Misses        5495     5482      -13     
- Partials       859      860       +1

We should add this check:

Each DeliverTx returns an amount GasUsed. The total GasUsed by the block must be less than the BlockSize.MaxGas consensus param.

Also, would be great to see more tests.

Agreed for adding more tests.

Per the issue, I thought we put it on the app to require that gas used is less than gas wanted. We don't run through the block until h+1 so if we decided that the gas used was actually too high, what would we do? Just drop the last txs?

Added all tests I could think of. I set GasWanted=1 within kvstore's CheckTx to enable this. This seems reasonable to me, but if unreasonable, I can make a new proxy app which does something for GasWanted. This tests all cases which I could think of as valuable to test.

Hrmm, looks like switching the gas from kv store to persistent kv store caused things to break. Not really sure why, since persistent kv store just wraps kv store. I think I'll switch it back to kv store regardless, since it feels kinda strange only the variant that writes to disk does something for gasWanted.

Oh actually this doesn't update the filter, to ensure no tx is greater than max gas. I think that should be done in a second PR though.