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 for key length in gas meter #10247

Merged
merged 22 commits into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
deccdfa
charge gas for key length
likhita-809 Sep 28, 2021
d2e1e13
fix tests
likhita-809 Sep 28, 2021
aafdd93
add changelog
likhita-809 Sep 28, 2021
eb38cd8
fix failing tests
likhita-809 Sep 28, 2021
7842701
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Sep 28, 2021
7e36ff3
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Sep 29, 2021
f7d4d9a
fix failing tests
likhita-809 Sep 29, 2021
32b294d
charge gas for key length in Get func
likhita-809 Sep 29, 2021
bb8ce7b
fix tests
likhita-809 Sep 29, 2021
71225ee
fix failed tests
likhita-809 Sep 29, 2021
4306095
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Sep 29, 2021
33c7863
try fix failing tests
likhita-809 Sep 29, 2021
63ae5c1
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Sep 29, 2021
b7302a3
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Sep 30, 2021
6858672
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Sep 30, 2021
7caa51f
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Sep 30, 2021
f7ac177
fix changelog
likhita-809 Sep 30, 2021
435a71c
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Sep 30, 2021
bda5834
add gas-limit flag manually
likhita-809 Oct 1, 2021
6ed2c46
Merge branch 'master' into likhita/charge-gas-for-key-length
amaury1093 Oct 4, 2021
9b00df7
Merge branch 'master' into likhita/charge-gas-for-key-length
mergify[bot] Oct 4, 2021
a5ae858
Merge branch 'master' into likhita/charge-gas-for-key-length
mergify[bot] Oct 4, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#9699](https://github.com/cosmos/cosmos-sdk/pull/9699) Add `:`, `.`, `-`, and `_` as allowed characters in the default denom regular expression.

### Bug Fixes

* (store) [#10247](https://github.com/cosmos/cosmos-sdk/pull/10247) Charge gas for the key length in gas meter.
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
* [#10180](https://github.com/cosmos/cosmos-sdk/issues/10180) Documentation: make references to Cosmos SDK consistent
* (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.
Expand Down
2 changes: 1 addition & 1 deletion client/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const (
// failures due to state changes that might occur between the tx simulation
// and the actual run.
DefaultGasAdjustment = 1.0
DefaultGasLimit = 200000
DefaultGasLimit = 250000
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
GasFlagAuto = "auto"

// DefaultKeyringBackend
Expand Down
4 changes: 4 additions & 0 deletions store/gaskv/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func (gs *Store) Get(key []byte) (value []byte) {
value = gs.parent.Get(key)

// TODO overflow-safe math?
gs.gasMeter.ConsumeGas(gs.gasConfig.ReadCostPerByte*types.Gas(len(key)), types.GasReadPerByteDesc)
gs.gasMeter.ConsumeGas(gs.gasConfig.ReadCostPerByte*types.Gas(len(value)), types.GasReadPerByteDesc)

return value
Expand All @@ -50,6 +51,7 @@ func (gs *Store) Set(key []byte, value []byte) {
types.AssertValidValue(value)
gs.gasMeter.ConsumeGas(gs.gasConfig.WriteCostFlat, types.GasWriteCostFlatDesc)
// TODO overflow-safe math?
gs.gasMeter.ConsumeGas(gs.gasConfig.WriteCostPerByte*types.Gas(len(key)), types.GasWritePerByteDesc)
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
gs.gasMeter.ConsumeGas(gs.gasConfig.WriteCostPerByte*types.Gas(len(value)), types.GasWritePerByteDesc)
gs.parent.Set(key, value)
}
Expand Down Expand Up @@ -173,8 +175,10 @@ func (gi *gasIterator) Error() error {
// based on the current value's length.
func (gi *gasIterator) consumeSeekGas() {
if gi.Valid() {
key := gi.Key()
value := gi.Value()

gi.gasMeter.ConsumeGas(gi.gasConfig.ReadCostPerByte*types.Gas(len(key)), types.GasValuePerByteDesc)
gi.gasMeter.ConsumeGas(gi.gasConfig.ReadCostPerByte*types.Gas(len(value)), types.GasValuePerByteDesc)
}
gi.gasMeter.ConsumeGas(gi.gasConfig.IterNextCostFlat, types.GasIterNextCostFlatDesc)
Expand Down
11 changes: 5 additions & 6 deletions store/gaskv/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestGasKVStoreBasic(t *testing.T) {
require.Equal(t, valFmt(1), st.Get(keyFmt(1)))
st.Delete(keyFmt(1))
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
require.Equal(t, meter.GasConsumed(), types.Gas(6429))
require.Equal(t, meter.GasConsumed(), types.Gas(6858))
}

func TestGasKVStoreIterator(t *testing.T) {
Expand Down Expand Up @@ -74,16 +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())
require.Equal(t, types.Gas(14565), 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.Equal(t, types.Gas(14667), meter.GasConsumed())
require.False(t, iterator.Valid())
require.Panics(t, iterator.Next)
require.Equal(t, types.Gas(13476), meter.GasConsumed())
require.Equal(t, types.Gas(14697), meter.GasConsumed())
require.NoError(t, iterator.Error())

reverseIterator := st.ReverseIterator(nil, nil)
Expand All @@ -100,8 +100,7 @@ func TestGasKVStoreIterator(t *testing.T) {
reverseIterator.Next()
require.False(t, reverseIterator.Valid())
require.Panics(t, reverseIterator.Next)

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

func TestGasKVStoreOutOfGasSet(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions testutil/testdata/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ package testdata

import (
"encoding/json"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256r1"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

// KeyTestPubAddr generates a new secp256k1 keypair.
Expand All @@ -36,7 +36,7 @@ func NewTestFeeAmount() sdk.Coins {

// NewTestGasLimit is a test fee gas limit.
func NewTestGasLimit() uint64 {
return 100000
return 200000
}

// NewTestMsg creates a message for testing with the given signers.
Expand Down