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(server): properly parse multiple gas config #18537

Merged
merged 2 commits into from
Nov 22, 2023
Merged

fix(server): properly parse multiple gas config #18537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d6962b
fix(server): properly parse multiple gas config
julienrbrt Nov 22, 2023
28da976
changelog
julienrbrt Nov 22, 2023
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 @@ -67,6 +67,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

* (server) [#18537](https://github.com/cosmos/cosmos-sdk/pull/18537) Fix panic when defining minimum gas config as `100stake;100uatom`. Use a `,` delimiter instead of `;`. Fixes the server config getter to use the correct delimiter.
* (client/tx) [#18472](https://github.com/cosmos/cosmos-sdk/pull/18472) Utilizes the correct Pubkey when simulating a transaction.
* (baseapp) [#18486](https://github.com/cosmos/cosmos-sdk/pull/18486) Fixed FinalizeBlock calls not being passed to ABCIListeners
* (baseapp) [#18383](https://github.com/cosmos/cosmos-sdk/pull/18383) Fixed a data race inside BaseApp.getContext, found by end-to-end (e2e) tests.
Expand Down
22 changes: 4 additions & 18 deletions server/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,26 +74,12 @@ the viper literal and passed to the application construction.
Example:

```go
func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts server.AppOptions) server.Application {
var cache sdk.MultiStorePersistentCache

if cast.ToBool(appOpts.Get(server.FlagInterBlockCache)) {
cache = store.NewCommitKVStoreCacheManager()
}

pruningOpts, err := server.GetPruningOptionsFromFlags(appOpts)
if err != nil {
panic(err)
}

func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts servertypes.AppOptions) servertypes.Application {
baseappOptions := server.DefaultBaseappOptions(appOpts)
return simapp.NewSimApp(
logger, db, traceStore, true,
baseapp.SetPruning(pruningOpts),
baseapp.SetMinGasPrices(cast.ToString(appOpts.Get(server.FlagMinGasPrices))),
baseapp.SetHaltHeight(cast.ToUint64(appOpts.Get(server.FlagHaltHeight))),
baseapp.SetHaltTime(cast.ToUint64(appOpts.Get(server.FlagHaltTime))),
baseapp.SetInterBlockCache(cache),
baseapp.SetTrace(cast.ToBool(appOpts.Get(server.FlagTrace))),
appOpts,
baseappOptions...,
)
}
```
Expand Down
17 changes: 4 additions & 13 deletions server/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package config
import (
"fmt"
"math"
"strings"

"github.com/spf13/viper"

Expand Down Expand Up @@ -202,23 +201,15 @@ func (c *Config) SetMinGasPrices(gasPrices sdk.DecCoins) {
c.MinGasPrices = gasPrices.String()
}

// GetMinGasPrices returns the validator's minimum gas prices based on the set
// configuration.
// GetMinGasPrices returns the validator's minimum gas prices based on the set configuration.
func (c *Config) GetMinGasPrices() sdk.DecCoins {
if c.MinGasPrices == "" {
return sdk.DecCoins{}
}

gasPricesStr := strings.Split(c.MinGasPrices, ";")
gasPrices := make(sdk.DecCoins, len(gasPricesStr))

for i, s := range gasPricesStr {
gasPrice, err := sdk.ParseDecCoin(s)
if err != nil {
panic(fmt.Errorf("failed to parse minimum gas price coin (%s): %s", s, err))
}

gasPrices[i] = gasPrice
gasPrices, err := sdk.ParseDecCoins(c.MinGasPrices)
if err != nil {
panic(fmt.Sprintf("invalid minimum gas prices: %v", err))
}

return gasPrices
Expand Down
12 changes: 10 additions & 2 deletions server/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,18 @@ func TestDefaultConfig(t *testing.T) {
require.True(t, cfg.GetMinGasPrices().IsZero())
}

func TestSetMinimumFees(t *testing.T) {
func TestGetAndSetMinimumGas(t *testing.T) {
cfg := DefaultConfig()
cfg.SetMinGasPrices(sdk.DecCoins{sdk.NewInt64DecCoin("foo", 5)})

input := sdk.DecCoins{sdk.NewInt64DecCoin("foo", 5)}
cfg.SetMinGasPrices(input)
require.Equal(t, "5.000000000000000000foo", cfg.MinGasPrices)
require.EqualValues(t, cfg.GetMinGasPrices(), input)

input = sdk.DecCoins{sdk.NewInt64DecCoin("bar", 1), sdk.NewInt64DecCoin("foo", 5)}
cfg.SetMinGasPrices(input)
require.Equal(t, "1.000000000000000000bar,5.000000000000000000foo", cfg.MinGasPrices)
require.EqualValues(t, cfg.GetMinGasPrices(), input)
}
Comment on lines +21 to 33
Copy link
Contributor

Choose a reason for hiding this comment

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

The test cases in TestGetAndSetMinimumGas have been updated to include scenarios with multiple minimum gas prices. However, the assertions for cfg.MinGasPrices are directly comparing the string representation of DecCoins which might not be robust if the string representation changes in the future. It would be better to compare DecCoins directly.

- require.Equal(t, "5.000000000000000000foo", cfg.MinGasPrices)
+ require.EqualValues(t, input.String(), cfg.MinGasPrices)
- require.Equal(t, "1.000000000000000000bar,5.000000000000000000foo", cfg.MinGasPrices)
+ require.EqualValues(t, input.String(), cfg.MinGasPrices)
Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
func TestGetAndSetMinimumGas(t *testing.T) {
cfg := DefaultConfig()
cfg.SetMinGasPrices(sdk.DecCoins{sdk.NewInt64DecCoin("foo", 5)})
input := sdk.DecCoins{sdk.NewInt64DecCoin("foo", 5)}
cfg.SetMinGasPrices(input)
require.Equal(t, "5.000000000000000000foo", cfg.MinGasPrices)
require.EqualValues(t, cfg.GetMinGasPrices(), input)
input = sdk.DecCoins{sdk.NewInt64DecCoin("bar", 1), sdk.NewInt64DecCoin("foo", 5)}
cfg.SetMinGasPrices(input)
require.Equal(t, "1.000000000000000000bar,5.000000000000000000foo", cfg.MinGasPrices)
require.EqualValues(t, cfg.GetMinGasPrices(), input)
}
func TestGetAndSetMinimumGas(t *testing.T) {
cfg := DefaultConfig()
input := sdk.DecCoins{sdk.NewInt64DecCoin("foo", 5)}
cfg.SetMinGasPrices(input)
require.EqualValues(t, input.String(), cfg.MinGasPrices)
require.EqualValues(t, cfg.GetMinGasPrices(), input)
input = sdk.DecCoins{sdk.NewInt64DecCoin("bar", 1), sdk.NewInt64DecCoin("foo", 5)}
cfg.SetMinGasPrices(input)
require.EqualValues(t, input.String(), cfg.MinGasPrices)
require.EqualValues(t, cfg.GetMinGasPrices(), input)
}


func TestIndexEventsMarshalling(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion server/config/toml.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const DefaultConfigTemplate = `# This is a TOML config file.

# The minimum gas prices a validator is willing to accept for processing a
# transaction. A transaction's fees must meet the minimum of any denomination
# specified in this config (e.g. 0.25token1;0.0001token2).
# specified in this config (e.g. 0.25token1,0.0001token2).
minimum-gas-prices = "{{ .BaseConfig.MinGasPrices }}"

# The maximum gas a query coming over rest/grpc may consume.
Expand Down
1 change: 0 additions & 1 deletion simapp/simd/cmd/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ func newApp(
appOpts servertypes.AppOptions,
) servertypes.Application {
baseappOptions := server.DefaultBaseappOptions(appOpts)

return simapp.NewSimApp(
logger, db, traceStore, true,
appOpts,
Expand Down
2 changes: 1 addition & 1 deletion tools/confix/data/v0.47-app.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

# The minimum gas prices a validator is willing to accept for processing a
# transaction. A transaction's fees must meet the minimum of any denomination
# specified in this config (e.g. 0.25token1;0.0001token2).
# specified in this config (e.g. 0.25token1,0.0001token2).
minimum-gas-prices = "0stake"

# default: the last 362880 states are kept, pruning at 10 block intervals
Expand Down
2 changes: 1 addition & 1 deletion tools/confix/data/v0.50-app.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

# The minimum gas prices a validator is willing to accept for processing a
# transaction. A transaction's fees must meet the minimum of any denomination
# specified in this config (e.g. 0.25token1;0.0001token2).
# specified in this config (e.g. 0.25token1,0.0001token2).
minimum-gas-prices = "0stake"

# The maximum gas a query coming over rest/grpc may consume.
Expand Down
Loading