fix(rollappparams): verify denom for gov proposal gas price change (#648

)

server/proposal/proposal_handler.go

@@ -0,0 +1,98 @@
+package proposal
+
+import (
+	"encoding/json"
+	"fmt"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
+	paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
+	"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
+	"github.com/tendermint/tendermint/libs/log"
+
+	rollappparamstypes "github.com/dymensionxyz/dymension-rdk/x/rollappparams/types"
+)
+
+type (
+	paramsKeeper interface {
+		GetSubspace(string) (paramstypes.Subspace, bool)
+		Logger(ctx sdk.Context) log.Logger
+	}
+	paramsSubspace interface {
+		Update(ctx sdk.Context, key, value []byte) error
+	}
+	getSubspaceFn      func(subspace string) (paramsSubspace, bool)
+	logInfoFn          func(msg string, keyvals ...interface{})
+	hasDenomMetaDataFn func(ctx sdk.Context, denom string) bool
+)
+
+// NewCustomParamChangeProposalHandler creates a new governance Handler for a ParamChangeProposal
+// that includes additional validation logic for the minGasPrices parameter using bankKeeper.
+func NewCustomParamChangeProposalHandler(paramKeeper paramsKeeper, bankKeeper bankkeeper.Keeper) govtypes.Handler {
+	return func(ctx sdk.Context, content govtypes.Content) error {
+		switch c := content.(type) {
+		case *proposal.ParameterChangeProposal:
+			getSubspace := func(subspace string) (paramsSubspace, bool) {
+				return paramKeeper.GetSubspace(subspace)
+			}
+			return handleCustomParameterChangeProposal(
+				ctx,
+				getSubspace,
+				paramKeeper.Logger(ctx).Info,
+				bankKeeper.HasDenomMetaData,
+				c.Changes,
+			)
+		default:
+			return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized param proposal content type: %T", c)
+		}
+	}
+}
+
+func handleCustomParameterChangeProposal(
+	ctx sdk.Context,
+	getSubspace getSubspaceFn,
+	logInfo logInfoFn,
+	hasDenom hasDenomMetaDataFn,
+	changes []proposal.ParamChange,
+) error {
+	for _, c := range changes {
+		ss, ok := getSubspace(c.Subspace)
+		if !ok {
+			return sdkerrors.Wrap(proposal.ErrUnknownSubspace, c.Subspace)
+		}
+
+		logInfo(
+			fmt.Sprintf("attempting to set new parameter value; subspace: %s, key: %s, value: %s", c.Subspace, c.Key, c.Value),
+		)
+
+		// additional validation for minGasPrices in rollappparams
+		if err := validateMinGasPriceParamChange(ctx, c, hasDenom); err != nil {
+			return err
+		}
+
+		if err := ss.Update(ctx, []byte(c.Key), []byte(c.Value)); err != nil {
+			return sdkerrors.Wrapf(proposal.ErrSettingParameter, "key: %s, value: %s, err: %s", c.Key, c.Value, err.Error())
+		}
+	}
+
+	return nil
+}
+
+func validateMinGasPriceParamChange(ctx sdk.Context, c proposal.ParamChange, hasDenom hasDenomMetaDataFn) error {
+	if c.Subspace == rollappparamstypes.ModuleName && c.Key == string(rollappparamstypes.KeyMinGasPrices) {
+		var decCoins sdk.DecCoins
+		if err := json.Unmarshal([]byte(c.Value), &decCoins); err != nil {
+			return sdkerrors.Wrapf(sdkerrors.ErrJSONUnmarshal, "failed to unmarshal minGasPrices: %v", err)
+		}
+
+		// validate each denom exists before allowing the param change
+		for _, decCoin := range decCoins {
+			if !hasDenom(ctx, decCoin.Denom) {
+				return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "denom %s does not exist or has no metadata", decCoin.Denom)
+			}
+		}
+	}
+	return nil
+}

server/proposal/proposal_handler_test.go

@@ -0,0 +1,164 @@
+package proposal
+
+import (
+	"errors"
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
+	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/libs/log"
+	tmtypes "github.com/tendermint/tendermint/proto/tendermint/types"
+
+	rollappparamstypes "github.com/dymensionxyz/dymension-rdk/x/rollappparams/types"
+)
+
+func TestCustomParameterChangeProposalHandler(t *testing.T) {
+	logger := log.NewNopLogger()
+	ctx := sdk.NewContext(nil, tmtypes.Header{}, false, logger)
+	hasDenom := func(ctx sdk.Context, denom string) bool { return true }
+	hasNotDenom := func(ctx sdk.Context, denom string) bool { return false }
+
+	tests := []struct {
+		name        string
+		changes     []proposal.ParamChange
+		getSubspace getSubspaceFn
+		hasDenom    hasDenomMetaDataFn
+		expectErr   bool
+		errContains string
+	}{
+		{
+			name: "non rollappparams subspace (no special checks)",
+			changes: []proposal.ParamChange{
+				{
+					Subspace: "othersubspace",
+					Key:      "someKey",
+					Value:    "someValue",
+				},
+			},
+			getSubspace: func(subspace string) (paramsSubspace, bool) {
+				return mockSubspace{}, true
+			},
+			hasDenom:  nil,
+			expectErr: false,
+		},
+		{
+			name: "rollappparams but different key than minGasPrices",
+			changes: []proposal.ParamChange{
+				{
+					Subspace: rollappparamstypes.ModuleName,
+					Key:      "someOtherKey",
+					Value:    "something",
+				},
+			},
+			getSubspace: func(subspace string) (paramsSubspace, bool) {
+				return mockSubspace{}, true
+			},
+			hasDenom:  nil,
+			expectErr: false,
+		},
+		{
+			name: "rollappparams minGasPrices - valid denoms",
+			changes: []proposal.ParamChange{
+				{
+					Subspace: rollappparamstypes.ModuleName,
+					Key:      string(rollappparamstypes.KeyMinGasPrices),
+					Value:    `[{"denom":"adenom","amount":"20000000000.0"}]`,
+				},
+			},
+			getSubspace: func(subspace string) (paramsSubspace, bool) {
+				return mockSubspace{}, true
+			},
+			hasDenom:  hasDenom,
+			expectErr: false,
+		},
+		{
+			name: "rollappparams minGasPrices - unknown denom",
+			changes: []proposal.ParamChange{
+				{
+					Subspace: rollappparamstypes.ModuleName,
+					Key:      string(rollappparamstypes.KeyMinGasPrices),
+					Value:    `[{"denom":"baddenom","amount":"20000000000.0"}]`,
+				},
+			},
+			getSubspace: func(subspace string) (paramsSubspace, bool) {
+				return mockSubspace{}, true
+			},
+			hasDenom:    hasNotDenom,
+			expectErr:   true,
+			errContains: "denom baddenom does not exist",
+		},
+		{
+			name: "rollappparams minGasPrices - invalid JSON",
+			changes: []proposal.ParamChange{
+				{
+					Subspace: rollappparamstypes.ModuleName,
+					Key:      string(rollappparamstypes.KeyMinGasPrices),
+					Value:    `not-json`,
+				},
+			},
+			getSubspace: func(subspace string) (paramsSubspace, bool) {
+				return mockSubspace{}, true
+			},
+			hasDenom:    nil,
+			expectErr:   true,
+			errContains: "failed to unmarshal minGasPrices",
+		},
+		{
+			name: "unknown subspace",
+			changes: []proposal.ParamChange{
+				{
+					Subspace: "unknownsubspace",
+					Key:      "someKey",
+					Value:    "someValue",
+				},
+			},
+			getSubspace: func(subspace string) (paramsSubspace, bool) {
+				return mockSubspace{}, false
+			},
+			hasDenom:    hasDenom,
+			expectErr:   true,
+			errContains: "unknown subspace",
+		},
+		{
+			name: "error on update",
+			changes: []proposal.ParamChange{
+				{
+					Subspace: rollappparamstypes.ModuleName,
+					Key:      "someKey",
+					Value:    "someValue",
+				},
+			},
+			getSubspace: func(subspace string) (paramsSubspace, bool) {
+				return mockSubspace{
+					updateErr: errors.New("update failed"),
+				}, true
+			},
+			hasDenom:    hasDenom,
+			expectErr:   true,
+			errContains: "update failed",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := handleCustomParameterChangeProposal(ctx, tc.getSubspace, logger.Info, tc.hasDenom, tc.changes)
+			if tc.expectErr {
+				require.Error(t, err)
+				if tc.errContains != "" {
+					require.Contains(t, err.Error(), tc.errContains)
+				}
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+type mockSubspace struct {
+	updateErr error
+}
+
+func (m mockSubspace) Update(sdk.Context, []byte, []byte) error {
+	return m.updateErr
+}