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.

Sign up for GitHub

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

Jump to bottom

fix!: Charge gas for key length in gas meter (backport #10247) #10697

Merged
merged 6 commits into from
Dec 10, 2021
Merged

fix!: Charge gas for key length in gas meter (backport #10247) #10697

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fca2d5e
fix!: Charge gas for key length in gas meter (#10247)
likhita-809 Oct 4, 2021
e7aeda3
fix conflicts++
tac0turtle Dec 8, 2021
b937179
Move to state machine breaking
amaury1093 Dec 8, 2021
520893b
revert deletion
amaury1093 Dec 8, 2021
2e6cc72
Merge branch 'release/v0.45.x' of ssh://github.com/cosmos/cosmos-sdk …
amaury1093 Dec 9, 2021
2eec60d
Fix test
amaury1093 Dec 9, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### State Machine Breaking

* (store) [#10218](https://github.com/cosmos/cosmos-sdk/pull/10218) Charge gas even when there are no entries while seeking.
* (store) [#10247](https://github.com/cosmos/cosmos-sdk/pull/10247) Charge gas for the key length in gas meter.

## [v0.44.5](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.44.5) - 2021-12-02

Expand Down
5 changes: 5 additions & 0 deletions store/gaskv/store.go
View file
Open in desktop
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)
gs.gasMeter.ConsumeGas(gs.gasConfig.WriteCostPerByte*types.Gas(len(value)), types.GasWritePerByteDesc)
gs.parent.Set(key, value)
}
Expand Down Expand Up @@ -173,9 +175,12 @@ 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)
}
12 changes: 7 additions & 5 deletions store/gaskv/store_test.go
View file
Open in desktop
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,18 @@ 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 @@ -101,7 +103,7 @@ func TestGasKVStoreIterator(t *testing.T) {
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
2 changes: 1 addition & 1 deletion testutil/testdata/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,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
5 changes: 5 additions & 0 deletions x/authz/client/testutil/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
"valid txn: (undelegate half tokens)",
[]string{
execMsg.Name(),
fmt.Sprintf("--%s=%s", flags.FlagGas, "250000"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
Expand All @@ -948,6 +949,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
"valid txn: (undelegate remaining half tokens)",
[]string{
execMsg.Name(),
fmt.Sprintf("--%s=%s", flags.FlagGas, "250000"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
Expand All @@ -961,6 +963,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
"failed with error no authorization found",
[]string{
execMsg.Name(),
fmt.Sprintf("--%s=%s", flags.FlagGas, "250000"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
Expand Down Expand Up @@ -1025,6 +1028,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
"valid txn",
[]string{
execMsg.Name(),
fmt.Sprintf("--%s=%s", flags.FlagGas, "250000"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
Expand All @@ -1038,6 +1042,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
"valid txn",
[]string{
execMsg.Name(),
fmt.Sprintf("--%s=%s", flags.FlagGas, "250000"),
fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
Expand Down