R4R: Fix runTx Gas Consumption during Tx Aborting

[alexanderbez]

For a given block, if a tx fails/aborts, we currently do not update the context and as such the defered function call in BaseApp#runTx still uses the infiniteGasMeter and subsequent txs may well exceed the limit.

closes: #3242

---

• Targeted PR against correct branch (see CONTRIBUTING.md)

• 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)

[codecov]

Codecov Report

Merging #3244 into develop will decrease coverage by <.01%.
The diff coverage is 80%.

@@             Coverage Diff             @@
##           develop    #3244      +/-   ##
===========================================
- Coverage    54.88%   54.87%   -0.01%     
===========================================
  Files          133      133              
  Lines         9555     9553       -2     
===========================================
- Hits          5244     5242       -2     
  Misses        3989     3989              
  Partials       322      322

[hendrikhofstadt]

utACK

[hendrikhofstadt]

		stdTx, ok := tx.(StdTx)
		if !ok {
			return ctx, sdk.ErrInternal("tx must be StdTx").Result(), true
		}

One could still provoke the infinite gas meter to be used by including a non stdtx

[alexanderbez]

Updated @SLAMPER

[jackzampolin]

Nice! Is there any way we can add a test for this behavior?

[alexanderbez]

It would have to bypass checkTx I think? So I'm not totally sure.

[alessio]

Despite the lack of a test case, I give my nihil obstat. I think we are excused here to go ahead and validate this anyway, having seen the difficulty of testing this in an automated fashion. Furthermore, the bug fixed by this PR is easily reproducible by hand.

[jackzampolin]

Good fix @alexanderbez

[ValarDragon]

Can a follow-up issue be written up for getting a test for this? It is very critical that we have tests for these fundamental baseapp responsibilities, we've had lots of severe bugs regarding this lately.

[alexanderbez]

Completely agree @ValarDragon -- suggestions on how to test something like this?