[Shared] Refresh shared module params logic

x/shared/keeper/msg_server_update_param.go

@@ -2,105 +2,85 @@ package keeper
 
 import (
 	"context"
+	"fmt"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 
 	"github.com/pokt-network/poktroll/x/shared/types"
 )
 
 func (k msgServer) UpdateParam(ctx context.Context, msg *types.MsgUpdateParam) (*types.MsgUpdateParamResponse, error) {
+	logger := k.logger.With(
+		"method", "UpdateParam",
+		"param_name", msg.Name,
+	)
+
 	if err := msg.ValidateBasic(); err != nil {
-		return nil, err
+		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	if k.GetAuthority() != msg.Authority {
-		return nil, types.ErrSharedInvalidSigner.Wrapf("invalid authority; expected %s, got %s", k.GetAuthority(), msg.Authority)
+		return nil, status.Error(
+			codes.InvalidArgument,
+			types.ErrSharedInvalidSigner.Wrapf(
+				"invalid authority; expected %s, got %s",
+				k.GetAuthority(), msg.Authority,
+			).Error(),
+		)
 	}
 
 	params := k.GetParams(ctx)
 
 	switch msg.Name {
 	case types.ParamNumBlocksPerSession:
-		value, ok := msg.AsType.(*types.MsgUpdateParam_AsInt64)
-		if !ok {
-			return nil, types.ErrSharedParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
-		}
-
-		params.NumBlocksPerSession = uint64(value.AsInt64)
+		logger = logger.With("param_value", msg.GetAsInt64())
+		params.NumBlocksPerSession = uint64(msg.GetAsInt64())
 	case types.ParamGracePeriodEndOffsetBlocks:
-		value, ok := msg.AsType.(*types.MsgUpdateParam_AsInt64)
-		if !ok {
-			return nil, types.ErrSharedParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
-		}
-
-		params.GracePeriodEndOffsetBlocks = uint64(value.AsInt64)
+		logger = logger.With("param_value", msg.GetAsInt64())
+		params.GracePeriodEndOffsetBlocks = uint64(msg.GetAsInt64())
 	case types.ParamClaimWindowOpenOffsetBlocks:
-		value, ok := msg.AsType.(*types.MsgUpdateParam_AsInt64)
-		if !ok {
-			return nil, types.ErrSharedParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
-		}
-
-		params.ClaimWindowOpenOffsetBlocks = uint64(value.AsInt64)
+		logger = logger.With("param_value", msg.GetAsInt64())
+		params.ClaimWindowOpenOffsetBlocks = uint64(msg.GetAsInt64())
 	case types.ParamClaimWindowCloseOffsetBlocks:
-		value, ok := msg.AsType.(*types.MsgUpdateParam_AsInt64)
-		if !ok {
-			return nil, types.ErrSharedParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
-		}
-
-		params.ClaimWindowCloseOffsetBlocks = uint64(value.AsInt64)
+		logger = logger.With("param_value", msg.GetAsInt64())
+		params.ClaimWindowCloseOffsetBlocks = uint64(msg.GetAsInt64())
 	case types.ParamProofWindowOpenOffsetBlocks:
-		value, ok := msg.AsType.(*types.MsgUpdateParam_AsInt64)
-		if !ok {
-			return nil, types.ErrSharedParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
-		}
-
-		params.ProofWindowOpenOffsetBlocks = uint64(value.AsInt64)
+		logger = logger.With("param_value", msg.GetAsInt64())
+		params.ProofWindowOpenOffsetBlocks = uint64(msg.GetAsInt64())
 	case types.ParamProofWindowCloseOffsetBlocks:
-		value, ok := msg.AsType.(*types.MsgUpdateParam_AsInt64)
-		if !ok {
-			return nil, types.ErrSharedParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
-		}
-
-		params.ProofWindowCloseOffsetBlocks = uint64(value.AsInt64)
+		logger = logger.With("param_value", msg.GetAsInt64())
+		params.ProofWindowCloseOffsetBlocks = uint64(msg.GetAsInt64())
 	case types.ParamSupplierUnbondingPeriodSessions:
-		value, ok := msg.AsType.(*types.MsgUpdateParam_AsInt64)
-		if !ok {
-			return nil, types.ErrSharedParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
-		}
-
-		params.SupplierUnbondingPeriodSessions = uint64(value.AsInt64)
+		logger = logger.With("param_value", msg.GetAsInt64())
+		params.SupplierUnbondingPeriodSessions = uint64(msg.GetAsInt64())
 	case types.ParamApplicationUnbondingPeriodSessions:
-		value, ok := msg.AsType.(*types.MsgUpdateParam_AsInt64)
-		if !ok {
-			return nil, types.ErrSharedParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
-		}
-
-		params.ApplicationUnbondingPeriodSessions = uint64(value.AsInt64)
+		logger = logger.With("param_value", msg.GetAsInt64())
+		params.ApplicationUnbondingPeriodSessions = uint64(msg.GetAsInt64())
 	case types.ParamComputeUnitsToTokensMultiplier:
-		value, ok := msg.AsType.(*types.MsgUpdateParam_AsInt64)
-		if !ok {
-			return nil, types.ErrSharedParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
-		}
-		computeUnitsToTokensMultiplier := uint64(value.AsInt64)
-
-		if err := types.ValidateComputeUnitsToTokensMultiplier(computeUnitsToTokensMultiplier); err != nil {
-			return nil, err
-		}
-
-		params.ComputeUnitsToTokensMultiplier = computeUnitsToTokensMultiplier
+		logger = logger.With("param_value", msg.GetAsInt64())
+		params.ComputeUnitsToTokensMultiplier = uint64(msg.GetAsInt64())
 	default:
-		return nil, types.ErrSharedParamInvalid.Wrapf("unsupported param %q", msg.Name)
+		return nil, status.Error(
+			codes.InvalidArgument,
+			types.ErrSharedParamInvalid.Wrapf("unsupported param %q", msg.Name).Error(),
+		)
 	}
 
 	// Perform a global validation on all params, which includes the updated param.
 	// This is needed to ensure that the updated param is valid in the context of all other params.
 	if err := params.ValidateBasic(); err != nil {
-		return nil, err
+		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	if err := k.SetParams(ctx, params); err != nil {
-		return nil, err
+		err = fmt.Errorf("unable to set params: %w", err)
+		logger.Error(fmt.Sprintf("ERROR: %s", err))
+		return nil, status.Error(codes.Internal, err.Error())
 	}
 
 	updatedParams := k.GetParams(ctx)
+
 	return &types.MsgUpdateParamResponse{
 		Params: &updatedParams,
 	}, nil

x/shared/keeper/msg_server_update_param_test.go

@@ -6,6 +6,8 @@ import (
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 
 	"github.com/pokt-network/poktroll/x/shared/keeper"
 	"github.com/pokt-network/poktroll/x/shared/types"
@@ -50,7 +52,7 @@ func TestMsgUpdateParam_UpdateNumBlocksPerSession(t *testing.T) {
 	require.Equal(t, uint64(expectedNumBlocksPerSession), res.Params.NumBlocksPerSession)
 
 	// Ensure the other parameters are unchanged
-	testkeeper.AssertDefaultParamsEqualExceptFields(t, &testSharedParams, res.Params, "NumBlocksPerSession")
+	testkeeper.AssertDefaultParamsEqualExceptFields(t, &testSharedParams, res.Params, string(sharedtypes.KeyNumBlocksPerSession))
 }
 
 func TestMsgUpdateParam_UpdateClaimWindowOpenOffsetBlocks(t *testing.T) {
@@ -95,7 +97,7 @@ func TestMsgUpdateParam_UpdateClaimWindowOpenOffsetBlocks(t *testing.T) {
 	require.Equal(t, uint64(expectedClaimWindowOpenOffestBlocks), res.Params.ClaimWindowOpenOffsetBlocks)
 
 	// Ensure the other parameters are unchanged
-	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, "ClaimWindowOpenOffsetBlocks")
+	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, string(sharedtypes.KeyClaimWindowOpenOffsetBlocks))
 }
 
 func TestMsgUpdateParam_UpdateClaimWindowCloseOffsetBlocks(t *testing.T) {
@@ -140,7 +142,7 @@ func TestMsgUpdateParam_UpdateClaimWindowCloseOffsetBlocks(t *testing.T) {
 	require.Equal(t, uint64(expectedClaimWindowCloseOffestBlocks), res.Params.ClaimWindowCloseOffsetBlocks)
 
 	// Ensure the other parameters are unchanged
-	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, "ClaimWindowCloseOffsetBlocks")
+	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, string(sharedtypes.KeyClaimWindowCloseOffsetBlocks))
 }
 
 func TestMsgUpdateParam_UpdateProofWindowOpenOffsetBlocks(t *testing.T) {
@@ -185,7 +187,7 @@ func TestMsgUpdateParam_UpdateProofWindowOpenOffsetBlocks(t *testing.T) {
 	require.Equal(t, uint64(expectedProofWindowOpenOffestBlocks), res.Params.ProofWindowOpenOffsetBlocks)
 
 	// Ensure the other parameters are unchanged
-	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, "ProofWindowOpenOffsetBlocks")
+	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, string(sharedtypes.KeyProofWindowOpenOffsetBlocks))
 }
 
 func TestMsgUpdateParam_UpdateProofWindowCloseOffsetBlocks(t *testing.T) {
@@ -230,7 +232,7 @@ func TestMsgUpdateParam_UpdateProofWindowCloseOffsetBlocks(t *testing.T) {
 	require.Equal(t, uint64(expectedProofWindowCloseOffestBlocks), res.Params.ProofWindowCloseOffsetBlocks)
 
 	// Ensure the other parameters are unchanged
-	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, "ProofWindowCloseOffsetBlocks")
+	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, string(sharedtypes.KeyProofWindowCloseOffsetBlocks))
 }
 
 func TestMsgUpdateParam_UpdateGracePeriodEndOffsetBlocks(t *testing.T) {
@@ -263,11 +265,11 @@ func TestMsgUpdateParam_UpdateGracePeriodEndOffsetBlocks(t *testing.T) {
 	require.Equal(t, uint64(expectedGracePeriodEndOffestBlocks), res.Params.GetGracePeriodEndOffsetBlocks())
 
 	// Ensure the other parameters are unchanged
-	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, "GracePeriodEndOffsetBlocks")
+	testkeeper.AssertDefaultParamsEqualExceptFields(t, &sharedParams, res.Params, string(sharedtypes.KeyGracePeriodEndOffsetBlocks))
 }
 
 func TestMsgUpdateParam_UpdateSupplierUnbondingPeriodSessions(t *testing.T) {
-	var expectedSupplierUnbondingPerid int64 = 5
+	var expectedSupplierUnbondingPeriod int64 = 5
 
 	k, ctx := testkeeper.SharedKeeper(t)
 	msgSrv := keeper.NewMsgServerImpl(k)
@@ -276,21 +278,21 @@ func TestMsgUpdateParam_UpdateSupplierUnbondingPeriodSessions(t *testing.T) {
 	require.NoError(t, k.SetParams(ctx, testSharedParams))
 
 	// Ensure the default values are different from the new values we want to set
-	require.NotEqual(t, uint64(expectedSupplierUnbondingPerid), testSharedParams.GetSupplierUnbondingPeriodSessions())
+	require.NotEqual(t, uint64(expectedSupplierUnbondingPeriod), testSharedParams.GetSupplierUnbondingPeriodSessions())
 
 	// Update the supplier unbonding period param
 	updateParamMsg := &sharedtypes.MsgUpdateParam{
 		Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 		Name:      sharedtypes.ParamSupplierUnbondingPeriodSessions,
-		AsType:    &sharedtypes.MsgUpdateParam_AsInt64{AsInt64: expectedSupplierUnbondingPerid},
+		AsType:    &sharedtypes.MsgUpdateParam_AsInt64{AsInt64: expectedSupplierUnbondingPeriod},
 	}
 	res, err := msgSrv.UpdateParam(ctx, updateParamMsg)
 	require.NoError(t, err)
 
-	require.Equal(t, uint64(expectedSupplierUnbondingPerid), res.Params.GetSupplierUnbondingPeriodSessions())
+	require.Equal(t, uint64(expectedSupplierUnbondingPeriod), res.Params.GetSupplierUnbondingPeriodSessions())
 
 	// Ensure the other parameters are unchanged
-	testkeeper.AssertDefaultParamsEqualExceptFields(t, &testSharedParams, res.Params, "SupplierUnbondingPeriodSessions")
+	testkeeper.AssertDefaultParamsEqualExceptFields(t, &testSharedParams, res.Params, string(sharedtypes.KeySupplierUnbondingPeriodSessions))
 
 	// Ensure that a supplier unbonding period that is less than the cumulative
 	// proof window close blocks is not allowed.
@@ -300,7 +302,13 @@ func TestMsgUpdateParam_UpdateSupplierUnbondingPeriodSessions(t *testing.T) {
 		AsType:    &sharedtypes.MsgUpdateParam_AsInt64{AsInt64: 1},
 	}
 	_, err = msgSrv.UpdateParam(ctx, updateParamMsg)
-	require.ErrorIs(t, err, sharedtypes.ErrSharedParamInvalid)
+	require.EqualError(t, err, status.Error(
+		codes.InvalidArgument,
+		sharedtypes.ErrSharedParamInvalid.Wrapf(
+			"SupplierUnbondingPeriodSessions (%v session) (%v blocks) must be greater than the cumulative ProofWindowCloseOffsetBlocks (%v)",
+			1, 4, 10,
+		).Error(),
+	).Error())
 }
 
 func TestMsgUpdateParam_UpdateApplicationUnbondingPeriodSessions(t *testing.T) {
@@ -327,7 +335,7 @@ func TestMsgUpdateParam_UpdateApplicationUnbondingPeriodSessions(t *testing.T) {
 	require.Equal(t, uint64(expectedApplicationUnbondingPerid), res.Params.GetApplicationUnbondingPeriodSessions())
 
 	// Ensure the other parameters are unchanged
-	testkeeper.AssertDefaultParamsEqualExceptFields(t, &testSharedParams, res.Params, "ApplicationUnbondingPeriodSessions")
+	testkeeper.AssertDefaultParamsEqualExceptFields(t, &testSharedParams, res.Params, string(sharedtypes.KeyApplicationUnbondingPeriodSessions))
 
 	// Ensure that a application unbonding period that is less than the cumulative
 	// proof window close blocks is not allowed.
@@ -337,7 +345,13 @@ func TestMsgUpdateParam_UpdateApplicationUnbondingPeriodSessions(t *testing.T) {
 		AsType:    &sharedtypes.MsgUpdateParam_AsInt64{AsInt64: 1},
 	}
 	_, err = msgSrv.UpdateParam(ctx, updateParamMsg)
-	require.ErrorIs(t, err, sharedtypes.ErrSharedParamInvalid)
+	require.EqualError(t, err, status.Error(
+		codes.InvalidArgument,
+		sharedtypes.ErrSharedParamInvalid.Wrapf(
+			"ApplicationUnbondingPeriodSessions (%v session) (%v blocks) must be greater than the cumulative ProofWindowCloseOffsetBlocks (%v)",
+			1, 4, 10,
+		).Error(),
+	).Error())
 }
 
 func TestMsgUpdateParam_ComputeUnitsToTokenMultiplier(t *testing.T) {
@@ -364,7 +378,7 @@ func TestMsgUpdateParam_ComputeUnitsToTokenMultiplier(t *testing.T) {
 	require.Equal(t, uint64(expectedComputeUnitsToTokenMultiplier), res.Params.GetComputeUnitsToTokensMultiplier())
 
 	// Ensure the other parameters are unchanged
-	testkeeper.AssertDefaultParamsEqualExceptFields(t, &testSharedParams, res.Params, "ComputeUnitsToTokensMultiplier")
+	testkeeper.AssertDefaultParamsEqualExceptFields(t, &testSharedParams, res.Params, string(sharedtypes.KeyComputeUnitsToTokensMultiplier))
 
 	// Ensure that compute units to token multiplier that is less than 1 is not allowed.
 	updateParamMsg = &sharedtypes.MsgUpdateParam{
@@ -373,7 +387,12 @@ func TestMsgUpdateParam_ComputeUnitsToTokenMultiplier(t *testing.T) {
 		AsType:    &sharedtypes.MsgUpdateParam_AsInt64{AsInt64: 0},
 	}
 	_, err = msgSrv.UpdateParam(ctx, updateParamMsg)
-	require.ErrorIs(t, err, sharedtypes.ErrSharedParamInvalid)
+	require.EqualError(t, err, status.Error(
+		codes.InvalidArgument,
+		sharedtypes.ErrSharedParamInvalid.Wrapf(
+			"invalid ComputeUnitsToTokensMultiplier: (%d)", 0,
+		).Error(),
+	).Error())
 }
 
 // getMinActorUnbondingPeriodSessions returns the actors unbonding period
@@ -386,6 +405,5 @@ func getMinActorUnbondingPeriodSessions(
 ) uint64 {
 	deltaBlocks := newParamBlocksValue - oldParamBlocksValue
 	newProofWindowCloseBlocks := types.GetSessionEndToProofWindowCloseBlocks(params) + deltaBlocks
-
 	return (newProofWindowCloseBlocks / params.NumBlocksPerSession) + 1
 }

x/shared/types/message_update_param.go

@@ -31,9 +31,10 @@ func NewMsgUpdateParam(authority string, name string, value any) (*MsgUpdatePara
 	}, nil
 }
 
-// ValidateBasic performs a basic validation of the MsgUpdateParam fields. It ensures
-// the parameter name is supported and the parameter type matches the expected type for
-// a given parameter name.
+// ValidateBasic performs a basic validation of the MsgUpdateParam fields. It ensures:
+// 1. The parameter name is supported.
+// 2. The parameter type matches the expected type for a given parameter name.
+// 3. The parameter value is valid (according to its respective validation function).
 func (msg *MsgUpdateParam) ValidateBasic() error {
 	// Validate the address
 	if _, err := sdk.AccAddressFromBech32(msg.Authority); err != nil {
@@ -48,16 +49,51 @@ func (msg *MsgUpdateParam) ValidateBasic() error {
 	// Parameter name must be supported by this module.
 	switch msg.Name {
 	// TODO_IMPROVE: Add a Uint64 asType instead of using int64 for uint64 params.
-	case ParamNumBlocksPerSession,
-		ParamGracePeriodEndOffsetBlocks,
-		ParamClaimWindowOpenOffsetBlocks,
-		ParamClaimWindowCloseOffsetBlocks,
-		ParamProofWindowOpenOffsetBlocks,
-		ParamProofWindowCloseOffsetBlocks,
-		ParamSupplierUnbondingPeriodSessions,
-		ParamApplicationUnbondingPeriodSessions,
-		ParamComputeUnitsToTokensMultiplier:
-		return msg.paramTypeIsInt64()
+	case ParamNumBlocksPerSession:
+		if err := msg.paramTypeIsInt64(); err != nil {
+			return err
+		}
+		return ValidateNumBlocksPerSession(uint64(msg.GetAsInt64()))
+	case ParamGracePeriodEndOffsetBlocks:
+		if err := msg.paramTypeIsInt64(); err != nil {
+			return err
+		}
+		return ValidateGracePeriodEndOffsetBlocks(uint64(msg.GetAsInt64()))
+	case ParamClaimWindowOpenOffsetBlocks:
+		if err := msg.paramTypeIsInt64(); err != nil {
+			return err
+		}
+		return ValidateClaimWindowOpenOffsetBlocks(uint64(msg.GetAsInt64()))
+	case ParamClaimWindowCloseOffsetBlocks:
+		if err := msg.paramTypeIsInt64(); err != nil {
+			return err
+		}
+		return ValidateClaimWindowCloseOffsetBlocks(uint64(msg.GetAsInt64()))
+	case ParamProofWindowOpenOffsetBlocks:
+		if err := msg.paramTypeIsInt64(); err != nil {
+			return err
+		}
+		return ValidateProofWindowOpenOffsetBlocks(uint64(msg.GetAsInt64()))
+	case ParamProofWindowCloseOffsetBlocks:
+		if err := msg.paramTypeIsInt64(); err != nil {
+			return err
+		}
+		return ValidateProofWindowCloseOffsetBlocks(uint64(msg.GetAsInt64()))
+	case ParamSupplierUnbondingPeriodSessions:
+		if err := msg.paramTypeIsInt64(); err != nil {
+			return err
+		}
+		return ValidateSupplierUnbondingPeriodSessions(uint64(msg.GetAsInt64()))
+	case ParamApplicationUnbondingPeriodSessions:
+		if err := msg.paramTypeIsInt64(); err != nil {
+			return err
+		}
+		return ValidateApplicationUnbondingPeriodSessions(uint64(msg.GetAsInt64()))
+	case ParamComputeUnitsToTokensMultiplier:
+		if err := msg.paramTypeIsInt64(); err != nil {
+			return err
+		}
+		return ValidateComputeUnitsToTokensMultiplier(uint64(msg.GetAsInt64()))
 	default:
 		return ErrSharedParamNameInvalid.Wrapf("unsupported param %q", msg.Name)
 	}