Skip to content

Conversation

@pr0n00gler
Copy link
Collaborator

@pr0n00gler pr0n00gler commented Sep 30, 2025

panic(storetypes.ErrorOutOfGas{Descriptor: descriptor})
}

pgm.GasMeter.ConsumeGas(amount, descriptor)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there is a small bug here.
If consumed exceeds the new limit, then:

We will get a panic at line L73.

The consumed gas will not be recorded in the gas meter (L76).

As a result, both methods - IsPastLimit and IsOutOfGas - will return false after recovering from the panic, which doesn't seem correct.
You can verify this with a test.

func TestProxyGasMeter2(t *testing.T) {
	baseGas := uint64(1000)
	limit := uint64(100)

	bgm := storetypes.NewGasMeter(baseGas)
	bgm.ConsumeGas(500, "ff")
	pgm := NewProxyGasMeter(bgm, limit)

	require.Panics(t, func() {
		pgm.ConsumeGas(101, "")
	})
	require.True(t, pgm.IsOutOfGas())
}

Suggestion: change the execution order and remove addUint64Overflow, since overflow checking is already performed in the parent gas meter.

Suggested change
pgm.GasMeter.ConsumeGas(amount, descriptor)
pgm.GasMeter.ConsumeGas(amount, descriptor)
if pgm.GasConsumed() > pgm.limit {
panic(storetypes.ErrorOutOfGas{Descriptor: descriptor})
}

As in the parent method, we first record the consumption and then check the limits.

@swelf19 swelf19 mentioned this pull request Jan 21, 2026
@pr0n00gler pr0n00gler closed this Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants