feat: post tx hooks

[vladjdk]

Reopening the PR #2 from @Reecepbcups

Closes #18

[Copilot]

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

x/vm/keeper/state_transition.go:212

• The variable 'txConfig' is referenced but not defined in the provided context. Ensure that 'txConfig' is properly declared or passed in.

TxHash:            txConfig.TxHash,

x/vm/keeper/state_transition.go:231

• The variable 'commit' is used without being defined in the diff. Please ensure that 'commit' is correctly declared or initialized.

else if commit != nil {

[Eric-Warehime]

Looks good, but I think we should add tests around some of the success and failure conditions here, especially since this is modifying the state transition function.

[Eric-Warehime]

Not too familiar with this code path at this point, but is this the expected behavior? Possibly we should be stopping execution if we've gone over the gas limit and returning a out of gas error?

[vladjdk]

I viewed some historical Ethermint updates, and it seems like this value from the receipt is only used with external tooling. I still haven't found a concrete example of when something like this could happen, but if you look at

evm/x/vm/keeper/state_transition.go

Lines 410 to 431 in 6124709

	// calculate a minimum amount of gas to be charged to sender if GasLimit
	// is considerably higher than GasUsed to stay more aligned with Tendermint gas mechanics
	// for more info https://github.com/evmos/ethermint/issues/1085
	gasLimit := math.LegacyNewDec(int64(msg.Gas())) //#nosec G115 -- int overflow is not a concern here -- msg gas is not exceeding int64 max value
	minGasMultiplier := k.GetMinGasMultiplier(ctx)
	minimumGasUsed := gasLimit.Mul(minGasMultiplier)
	if !minimumGasUsed.TruncateInt().IsUint64() {
	return nil, errorsmod.Wrapf(types.ErrGasOverflow, "minimumGasUsed(%s) is not a uint64", minimumGasUsed.TruncateInt().String())
	}
	if msg.Gas() < leftoverGas {
	return nil, errorsmod.Wrapf(types.ErrGasOverflow, "message gas limit < leftover gas (%d < %d)", msg.Gas(), leftoverGas)
	}
	gasUsed := math.LegacyMaxDec(minimumGasUsed, math.LegacyNewDec(int64(temporaryGasUsed))).TruncateInt().Uint64() //#nosec G115 -- int overflow is not a concern here
	// reset leftoverGas, to be used by the tracer
	leftoverGas = msg.Gas() - gasUsed
	return &types.MsgEthereumTxResponse{
	GasUsed: gasUsed,
	VmError: vmError,

, the minimumGasUsed could be higher than the actual gas used, which may cause the cumulativeGasUsed + BlockGasMeter to exceed the limit.

[vladjdk]

I added a test where this case would be hit. This could probably happen in the real world where someone's transaction is at the end of the FIFO list and the gas limit exceeds the rest of the Cosmos block gas limit, but the consumed gas still remains under it. In this case, the EVM would refund the gas.

[Eric-Warehime]

Maybe a separate question, but since we're refunding any gas not consumed via res.GasUsed, does that mean the post tx hooks are essentially free to execute?

[vladjdk]

Yeah, added a comment above them to clarify this

[Eric-Warehime]

nit: can we use errorsmod.Wrap here?

[Eric-Warehime]

Maybe a separate question, but since we're refunding any gas not consumed via res.GasUsed, does that mean the post tx hooks are essentially free to execute?

[Eric-Warehime]

LGTM. Let's add some comment around gas usage not being accounted in the post hooks and use errorsmod if we can.

[vladjdk]

Thanks. Added a comment that the hooks won't charge for any gas and can be thought of like EndBlockers for individual EVM transactions in that sense. Fixed the error as well.