Unexpectedly high gas usage in delegate transaction on game_of_stakes_3

[zmanian]

We have seen a number of delegate transactions in Game of Stakes fail due to unexpected gas usage.

https://hubble.figment.network/chains/gos3/blocks/19486

Contains an example.

No pattern has been identified except that only delegate transactions appear to demonstrate unexpectedly high gas usage.

This is has also only been observed when adding coins to existing delegation and not when creating a new delegation.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[eon0001]

I experienced the same issue even in case of withdraw rewards transaction.

https://stargazer.certus.one/blocks/31071

[alexanderbez]

Something related to hooks most likely.

/cc @cwgoes @rigelrozanski

[hendrikhofstadt]

@alexanderbez It is actually not.

What happens is that the first transaction in the block has an invalid signature which causes the tx processing to abort.

cosmos-sdk/baseapp/baseapp.go

Lines 761 to 770 in e855780

	if abort {
	return result
	}
	if !newCtx.IsZero() {
	// At this point, newCtx.MultiStore() is cache wrapped,
	// or something else replaced by anteHandler.
	// We want the original ms, not one which was cache-wrapped
	// for the ante handler.
	ctx = newCtx.WithMultiStore(ms)
	}

However that also means that the context is not overwritten.
The problem is that the newCtx contains the actual gas meter of the transaction.
So now the deferred function is called:

cosmos-sdk/baseapp/baseapp.go

Lines 732 to 740 in e855780

	defer func() {
	if mode == runTxModeDeliver {
	ctx.BlockGasMeter().ConsumeGas(
	ctx.GasMeter().GasConsumedToLimit(), "block gas meter")
	if ctx.BlockGasMeter().GasConsumed() < startingGas {
	panic(sdk.ErrorGasOverflow{"tx gas summation"})
	}
	}
	}()

That function consumes the gas from the ctx's gas meter. However since the ctx has not been overwritten this is still the infinite gas meter from BeginBlocker.

Subsequently the whole gas from BeginBlocker is then consumed in the first TX which is too much for the block as normally that gas should have never been consumed.

[alexanderbez]

Hmmm so the infiniteGasMeter will simply return/consume the total consumed which is past the limit.

Is something along the following occurring for a given block:

1. Sig verification fails on 1st tx, gas meter is still infinite and already has some consumed gas.
2. Subsequent txs pass sig verification thus getting a non-infinite gas meter and eventually exceeding the limit.

[hendrikhofstadt]

@alexanderbez I don't have a PR open. Feel free to open one.

It should be as simple as not consuming gas if the tx is aborted (if that's the intended behaviour)

Edit: Thinking about possible DoS vectors it would make sense to just consume the gas from the newCtx that has been consumed until the execution was aborted.

[alexanderbez]

A bit confused actually, as I cannot see how during deliverTx, an infinite gas meter would ever be used (assuming there is a consensusParams.BlockSize set)?

Edit: As @SLAMPER pointed out, ctx.GasMeter().GasConsumedToLimit(), "block gas meter") is the culprit here where the meter is an infinite one when aborted.