chore(data-proxy): additional validation and improved errors

Part-of: #316
sedaprotocol · Aug 22, 2024 · 2403bf4 · 2403bf4
1 parent c8b5a9a
commit 2403bf4
Show file tree

Hide file tree

Showing 6 changed files with 200 additions and 23 deletions.
diff --git a/x/data-proxy/keeper/msg_server.go b/x/data-proxy/keeper/msg_server.go
@@ -138,8 +138,6 @@ func (m msgServer) EditDataProxy(goCtx context.Context, msg *types.MsgEditDataPr
 		return &types.MsgEditDataProxyResponse{}, nil
 	}
 
-	// TODO check if fee is in native denom
-
 	params, err := m.Keeper.Params.Get(ctx)
 	if err != nil {
 		return nil, err
@@ -229,7 +227,7 @@ func (m msgServer) UpdateParams(goCtx context.Context, req *types.MsgUpdateParam
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
 	if _, err := sdk.AccAddressFromBech32(req.Authority); err != nil {
-		return nil, fmt.Errorf("invalid authority address: %s", err)
+		return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid authority address: %s", req.Authority)
 	}
 	if m.GetAuthority() != req.Authority {
 		return nil, sdkerrors.ErrorInvalidSigner.Wrapf("unauthorized authority; expected %s, got %s", m.GetAuthority(), req.Authority)

diff --git a/x/data-proxy/keeper/msg_server_test.go b/x/data-proxy/keeper/msg_server_test.go
@@ -5,6 +5,7 @@ import (
 
 	"cosmossdk.io/collections"
 
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
 	"github.com/sedaprotocol/seda-chain/x/data-proxy/types"
@@ -107,6 +108,35 @@ func (s *KeeperTestSuite) TestMsgServer_RegisterDataProxy() {
 			expected: nil,
 			wantErr:  hex.InvalidByteError(byte('g')),
 		},
+		{
+			name: "Empty payout address",
+			msg: &types.MsgRegisterDataProxy{
+				AdminAddress:  "seda1uea9km4nup9q7qu96ak683kc67x9jf7ste45z5",
+				PayoutAddress: "",
+				Fee:           s.NewFeeFromString("10000000000000000000"),
+				Memo:          "",
+				PubKey:        "02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4f3",
+				Signature:     "5076d9d98754505d2f6f94f5a44062b9e95c2c5cfe7f21c69270814dc947bd285f5ed64e595aa956004687a225263f2831252cb41379cab2e3505b90f3da2701",
+			},
+			expected: nil,
+			wantErr:  sdkerrors.ErrInvalidRequest,
+		},
+		{
+			name: "Invalid fee denom",
+			msg: &types.MsgRegisterDataProxy{
+				AdminAddress:  "seda1uea9km4nup9q7qu96ak683kc67x9jf7ste45z5",
+				PayoutAddress: "seda1uea9km4nup9q7qu96ak683kc67x9jf7ste45z5",
+				Fee: &sdk.Coin{
+					Denom:  "uatom",
+					Amount: s.NewIntFromString("10000"),
+				},
+				Memo:      "",
+				PubKey:    "02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4f3",
+				Signature: "5076d9d98754505d2f6f94f5a44062b9e95c2c5cfe7f21c69270814dc947bd285f5ed64e595aa956004687a225263f2831252cb41379cab2e3505b90f3da2701",
+			},
+			expected: nil,
+			wantErr:  sdkerrors.ErrInvalidRequest,
+		},
 	}
 	for _, tt := range tests {
 		s.Run(tt.name, func() {
@@ -276,6 +306,21 @@ func (s *KeeperTestSuite) TestMsgServer_EditDataProxy() {
 			expected: nil,
 			wantErr:  sdkerrors.ErrorInvalidSigner,
 		},
+		{
+			name: "Update fee with invalid fee denom",
+			msg: &types.MsgEditDataProxy{
+				Sender:           "seda1uea9km4nup9q7qu96ak683kc67x9jf7ste45z5",
+				NewPayoutAddress: types.DoNotModifyField,
+				NewMemo:          types.DoNotModifyField,
+				NewFee: &sdk.Coin{
+					Denom:  "uatom",
+					Amount: s.NewIntFromString("10000"),
+				},
+				PubKey: "02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4c3",
+			},
+			expected: nil,
+			wantErr:  sdkerrors.ErrInvalidRequest,
+		},
 	}
 	for _, tt := range tests {
 		s.Run(tt.name, func() {
@@ -390,3 +435,126 @@ func (s *KeeperTestSuite) TestMsgServer_EditDataProxy() {
 		s.Require().NotNil(successfulEdit)
 	})
 }
+
+func (s *KeeperTestSuite) TestMsgServer_UpdateParamsErrors() {
+	authority := s.keeper.GetAuthority()
+	cases := []struct {
+		name    string
+		input   types.MsgUpdateParams
+		wantErr error
+	}{
+		{
+			name: "invalid minimum update delay",
+			input: types.MsgUpdateParams{
+				Authority: authority,
+				Params: types.Params{
+					MinFeeUpdateDelay: 0,
+				},
+			},
+			wantErr: sdkerrors.ErrInvalidRequest,
+		},
+		{
+			name: "invalid authority",
+			input: types.MsgUpdateParams{
+				Authority: "seda1ucv5709wlf9jn84ynyjzyzeavwvurmdyxat26l",
+				Params: types.Params{
+					MinFeeUpdateDelay: 8000,
+				},
+			},
+			wantErr: sdkerrors.ErrorInvalidSigner,
+		},
+	}
+
+	s.SetupTest()
+	for _, tt := range cases {
+		s.Run(tt.name, func() {
+			res, err := s.msgSrvr.UpdateParams(s.ctx, &tt.input)
+			s.Require().ErrorIs(err, tt.wantErr)
+			s.Require().Nil(res)
+		})
+	}
+}
+
+func (s *KeeperTestSuite) TestMsgServer_UpdateParams() {
+	authority := s.keeper.GetAuthority()
+	pubKeyHex := "02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4c3"
+	pubKeyBytes, err := hex.DecodeString(pubKeyHex)
+	s.Require().NoError(err)
+
+	initialProxyConfig := types.ProxyConfig{
+		PayoutAddress: "seda1uea9km4nup9q7qu96ak683kc67x9jf7ste45z5",
+		Fee:           s.NewFeeFromString("9"),
+		Memo:          "test",
+		FeeUpdate:     nil,
+		AdminAddress:  "seda1uea9km4nup9q7qu96ak683kc67x9jf7ste45z5",
+	}
+
+	s.Run("Updating the minimum delay should not affect existing updates", func() {
+		s.SetupTest()
+
+		// Register data proxy
+		err = s.keeper.DataProxyConfigs.Set(s.ctx, pubKeyBytes, initialProxyConfig)
+		s.Require().NoError(err)
+
+		// Edit data proxy fee and verify it is scheduled with the default delay
+		firstEditMsg := &types.MsgEditDataProxy{
+			Sender:           "seda1uea9km4nup9q7qu96ak683kc67x9jf7ste45z5",
+			NewPayoutAddress: types.DoNotModifyField,
+			NewMemo:          types.DoNotModifyField,
+			NewFee:           s.NewFeeFromString("1337"),
+			PubKey:           pubKeyHex,
+		}
+
+		firstEditRes, err := s.msgSrvr.EditDataProxy(s.ctx, firstEditMsg)
+		s.Require().NoError(err)
+		s.Require().NotNil(firstEditRes)
+
+		firstProxyConfig, err := s.keeper.GetDataProxyConfig(s.ctx, firstEditMsg.PubKey)
+		s.Require().NoError(err)
+		s.Require().NotNil(firstProxyConfig.FeeUpdate)
+
+		firstUpdateScheduled, err := s.keeper.FeeUpdateQueue.Has(s.ctx, collections.Join(firstEditRes.FeeUpdateHeight, pubKeyBytes))
+		s.Require().NoError(err)
+		s.Require().True(firstUpdateScheduled)
+
+		// Update params, increasing the minimum delay
+		_, err = s.msgSrvr.UpdateParams(s.ctx, &types.MsgUpdateParams{
+			Authority: authority,
+			Params: types.Params{
+				MinFeeUpdateDelay: types.DefaultMinFeeUpdateDelay + 100,
+			},
+		})
+		s.Require().NoError(err)
+
+		// Verify update is still pending at the original height
+		firstUpdateStillScheduled, err := s.keeper.FeeUpdateQueue.Has(s.ctx, collections.Join(firstEditRes.FeeUpdateHeight, pubKeyBytes))
+		s.Require().NoError(err)
+		s.Require().True(firstUpdateStillScheduled)
+
+		// Schedule a new fee update
+		secondEditMsg := &types.MsgEditDataProxy{
+			Sender:           "seda1uea9km4nup9q7qu96ak683kc67x9jf7ste45z5",
+			NewPayoutAddress: types.DoNotModifyField,
+			NewMemo:          types.DoNotModifyField,
+			NewFee:           s.NewFeeFromString("1984"),
+			PubKey:           "02100efce2a783cc7a3fbf9c5d15d4cc6e263337651312f21a35d30c16cb38f4c3",
+		}
+
+		secondEditRes, err := s.msgSrvr.EditDataProxy(s.ctx, secondEditMsg)
+		s.Require().NoError(err)
+		s.Require().NotNil(secondEditRes)
+
+		secondProxyConfig, err := s.keeper.GetDataProxyConfig(s.ctx, secondEditMsg.PubKey)
+		s.Require().NoError(err)
+		s.Require().NotNil(secondProxyConfig.FeeUpdate)
+
+		// Verify the new update is scheduled and the old one is cancelled
+		secondUpdateScheduled, err := s.keeper.FeeUpdateQueue.Has(s.ctx, collections.Join(secondEditRes.FeeUpdateHeight, pubKeyBytes))
+		s.Require().NoError(err)
+		s.Require().True(secondUpdateScheduled)
+
+		firstUpdateNoLongerScheduled, err := s.keeper.FeeUpdateQueue.Has(s.ctx, collections.Join(firstEditRes.FeeUpdateHeight, pubKeyBytes))
+		s.Require().NoError(err)
+		s.Require().False(firstUpdateNoLongerScheduled)
+	})
+}
diff --git a/x/data-proxy/types/errors.go b/x/data-proxy/types/errors.go
@@ -3,10 +3,8 @@ package types
 import "cosmossdk.io/errors"
 
 var (
-	ErrEmptyValue       = errors.Register(ModuleName, 1, "empty value")
-	ErrInvalidParam     = errors.Register(ModuleName, 2, "invalid parameter")
-	ErrAlreadyExists    = errors.Register(ModuleName, 3, "data proxy already exists")
-	ErrInvalidSignature = errors.Register(ModuleName, 4, "invalid signature")
-	ErrInvalidDelay     = errors.Register(ModuleName, 5, "invalid update delay")
-	ErrEmptyUpdate      = errors.Register(ModuleName, 6, "nothing to update")
+	ErrAlreadyExists    = errors.Register(ModuleName, 2, "data proxy already exists")
+	ErrInvalidSignature = errors.Register(ModuleName, 3, "invalid signature")
+	ErrInvalidDelay     = errors.Register(ModuleName, 4, "invalid update delay")
+	ErrEmptyUpdate      = errors.Register(ModuleName, 5, "nothing to update")
 )
diff --git a/x/data-proxy/types/params.go b/x/data-proxy/types/params.go
@@ -1,11 +1,12 @@
 package types
 
 import (
-	"cosmossdk.io/errors"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 const (
 	DefaultMinFeeUpdateDelay uint32 = 86400 // Roughly 1 week with a ~7 sec block time
+	LowestFeeUpdateDelay     uint32 = 1
 )
 
 // DefaultParams returns default wasm-storage module parameters.
@@ -18,8 +19,8 @@ func DefaultParams() Params {
 // ValidateBasic performs basic validation on wasm-storage
 // module parameters.
 func (p *Params) Validate() error {
-	if p.MinFeeUpdateDelay < 1 {
-		return errors.Wrapf(ErrInvalidParam, "MinFeeUpdateDelay %d < 1", p.MinFeeUpdateDelay)
+	if p.MinFeeUpdateDelay < LowestFeeUpdateDelay {
+		return sdkerrors.ErrInvalidRequest.Wrapf("MinFeeUpdateDelay lower than %d < %d", p.MinFeeUpdateDelay, LowestFeeUpdateDelay)
 	}
 	return nil
 }
diff --git a/x/data-proxy/types/proxy_config.go b/x/data-proxy/types/proxy_config.go
@@ -1,5 +1,7 @@
 package types
 
+import sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
 const (
 	MaxMemoLength    = 3000
 	DoNotModifyField = "[do-not-modify]"
@@ -8,7 +10,7 @@ const (
 
 func (p *ProxyConfig) Validate() error {
 	if len(p.Memo) > MaxMemoLength {
-		return ErrInvalidParam.Wrapf("invalid memo length; got: %d, max < %d", len(p.Memo), MaxMemoLength)
+		return sdkerrors.ErrInvalidRequest.Wrapf("invalid memo length; got: %d, max < %d", len(p.Memo), MaxMemoLength)
 	}
 
 	return nil

diff --git a/x/data-proxy/types/tx.go b/x/data-proxy/types/tx.go
@@ -3,34 +3,39 @@ package types
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
+	appparams "github.com/sedaprotocol/seda-chain/app/params"
 )
 
 func (m *MsgRegisterDataProxy) Validate() error {
 	if m.PayoutAddress == "" {
-		return ErrEmptyValue.Wrap("empty payout address")
+		return sdkerrors.ErrInvalidRequest.Wrap("empty payout address")
 	}
 	if _, err := sdk.AccAddressFromBech32(m.PayoutAddress); err != nil {
 		return sdkerrors.ErrInvalidAddress.Wrapf("invalid payout address: %s", m.PayoutAddress)
 	}
 	if m.Fee == nil {
-		return ErrEmptyValue.Wrap("empty fee")
+		return sdkerrors.ErrInvalidRequest.Wrap("empty fee")
 	}
 	if m.PubKey == "" {
-		return ErrEmptyValue.Wrap("empty public key")
+		return sdkerrors.ErrInvalidRequest.Wrap("empty public key")
 	}
 	if m.Signature == "" {
-		return ErrEmptyValue.Wrap("empty signature")
+		return sdkerrors.ErrInvalidRequest.Wrap("empty signature")
+	}
+	if m.Fee.Denom != appparams.DefaultBondDenom {
+		return sdkerrors.ErrInvalidRequest.Wrapf("invalid coin denomination: got %s, expected %s", m.Fee.Denom, appparams.DefaultBondDenom)
 	}
 
 	return nil
 }
 
 func (m *MsgEditDataProxy) Validate() error {
 	if m.PubKey == "" {
-		return ErrEmptyValue.Wrap("empty public key")
+		return sdkerrors.ErrInvalidRequest.Wrap("empty public key")
 	}
 	if m.Sender == "" {
-		return ErrEmptyValue.Wrap("empty sender")
+		return sdkerrors.ErrInvalidRequest.Wrap("empty sender")
 	}
 
 	hasNewPayoutAddress := m.NewPayoutAddress != DoNotModifyField
@@ -47,16 +52,22 @@ func (m *MsgEditDataProxy) Validate() error {
 		}
 	}
 
+	if hasNewFee {
+		if m.NewFee.Denom != appparams.DefaultBondDenom {
+			return sdkerrors.ErrInvalidRequest.Wrapf("invalid coin denomination: got %s, expected %s", m.NewFee.Denom, appparams.DefaultBondDenom)
+		}
+	}
+
 	return nil
 }
 
 func (m *MsgTransferAdmin) Validate() error {
 	if m.Sender == "" {
-		return ErrEmptyValue.Wrap("empty sender")
+		return sdkerrors.ErrInvalidRequest.Wrap("empty sender")
 	}
 
 	if m.NewAdminAddress == "" {
-		return ErrEmptyValue.Wrap("empty admin address")
+		return sdkerrors.ErrInvalidRequest.Wrap("empty admin address")
 	}
 	if _, err := sdk.AccAddressFromBech32(m.NewAdminAddress); err != nil {
 		return sdkerrors.ErrInvalidAddress.Wrapf("invalid new admin address: %s", m.NewAdminAddress)
@@ -66,6 +77,5 @@ func (m *MsgTransferAdmin) Validate() error {
 }
 
 func (m *MsgUpdateParams) Validate() error {
-	// TODO
-	return nil
+	return m.Params.Validate()
 }