Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Charge gas even when there are no entries in gaskv #10218

Merged
merged 9 commits into from
Sep 27, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

* (store) [#10218](https://github.com/cosmos/cosmos-sdk/pull/10218) Charge gas even when there are no entries while seeking.
* (x/genutil) [#10104](https://github.com/cosmos/cosmos-sdk/pull/10104) Ensure the `init` command reads the `--home` flag value correctly.
* [\#9651](https://github.com/cosmos/cosmos-sdk/pull/9651) Change inconsistent limit of `0` to `MaxUint64` on InfiniteGasMeter and add GasRemaining func to GasMeter.
* [\#9639](https://github.com/cosmos/cosmos-sdk/pull/9639) Check store keys length before accessing them by making sure that `key` is of length `m+1` (for `key[n:m]`)
Expand Down
15 changes: 6 additions & 9 deletions store/gaskv/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,7 @@ func (gs *Store) iterator(start, end []byte, ascending bool) types.Iterator {
}

gi := newGasIterator(gs.gasMeter, gs.gasConfig, parent)
if gi.Valid() {
gi.(*gasIterator).consumeSeekGas()
}
gi.(*gasIterator).consumeSeekGas()

return gi
}
Expand Down Expand Up @@ -143,10 +141,7 @@ func (gi *gasIterator) Valid() bool {
// in the iterator. It incurs a flat gas cost for seeking and a variable gas
// cost based on the current value's length if the iterator is valid.
func (gi *gasIterator) Next() {
if gi.Valid() {
gi.consumeSeekGas()
}

gi.consumeSeekGas()
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
gi.parent.Next()
}

Expand Down Expand Up @@ -177,8 +172,10 @@ func (gi *gasIterator) Error() error {
// consumeSeekGas consumes on each iteration step a flat gas cost and a variable gas cost
// based on the current value's length.
func (gi *gasIterator) consumeSeekGas() {
value := gi.Value()
if gi.Valid() {
value := gi.Value()

gi.gasMeter.ConsumeGas(gi.gasConfig.ReadCostPerByte*types.Gas(len(value)), types.GasValuePerByteDesc)
gi.gasMeter.ConsumeGas(gi.gasConfig.ReadCostPerByte*types.Gas(len(value)), types.GasValuePerByteDesc)
}
gi.gasMeter.ConsumeGas(gi.gasConfig.IterNextCostFlat, types.GasIterNextCostFlatDesc)
}
21 changes: 17 additions & 4 deletions store/gaskv/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@ import (
"fmt"
"testing"

"github.com/stretchr/testify/require"
dbm "github.com/tendermint/tm-db"

"github.com/cosmos/cosmos-sdk/store/dbadapter"
"github.com/cosmos/cosmos-sdk/store/gaskv"
"github.com/cosmos/cosmos-sdk/store/types"

"github.com/stretchr/testify/require"
)

func bz(s string) []byte { return []byte(s) }
Expand Down Expand Up @@ -41,14 +40,18 @@ func TestGasKVStoreBasic(t *testing.T) {

func TestGasKVStoreIterator(t *testing.T) {
mem := dbadapter.Store{DB: dbm.NewMemDB()}
meter := types.NewGasMeter(10000)
meter := types.NewGasMeter(100000)
st := gaskv.NewStore(mem, meter, types.KVGasConfig())
require.False(t, st.Has(keyFmt(1)))
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
require.Empty(t, st.Get(keyFmt(2)), "Expected `key2` to be empty")
require.Empty(t, st.Get(keyFmt(3)), "Expected `key3` to be empty")

st.Set(keyFmt(1), valFmt(1))
require.True(t, st.Has(keyFmt(1)))
st.Set(keyFmt(2), valFmt(2))
require.True(t, st.Has(keyFmt(2)))
st.Set(keyFmt(3), valFmt(0))

iterator := st.Iterator(nil, nil)
start, end := iterator.Domain()
Expand All @@ -71,8 +74,16 @@ func TestGasKVStoreIterator(t *testing.T) {
vb := iterator.Value()
require.Equal(t, vb, valFmt(2))
iterator.Next()
require.Equal(t, types.Gas(13377), meter.GasConsumed())
kc := iterator.Key()
require.Equal(t, kc, keyFmt(3))
vc := iterator.Value()
require.Equal(t, vc, valFmt(0))
iterator.Next()
require.Equal(t, types.Gas(13446), meter.GasConsumed())
require.False(t, iterator.Valid())
require.Panics(t, iterator.Next)
require.Equal(t, types.Gas(13476), meter.GasConsumed())
require.NoError(t, iterator.Error())

reverseIterator := st.ReverseIterator(nil, nil)
Expand All @@ -81,14 +92,16 @@ func TestGasKVStoreIterator(t *testing.T) {
t.Fatal(err)
}
})
require.Equal(t, reverseIterator.Key(), keyFmt(3))
reverseIterator.Next()
require.Equal(t, reverseIterator.Key(), keyFmt(2))
reverseIterator.Next()
require.Equal(t, reverseIterator.Key(), keyFmt(1))
reverseIterator.Next()
require.False(t, reverseIterator.Valid())
require.Panics(t, reverseIterator.Next)

require.Equal(t, types.Gas(9194), meter.GasConsumed())
require.Equal(t, types.Gas(13782), meter.GasConsumed())
}

func TestGasKVStoreOutOfGasSet(t *testing.T) {
Expand Down