fix: missing checks and coverage improvements (#6)

simapp/app.go

@@ -154,7 +154,7 @@ func NewSimApp(
 	)
 
 	app.HaloKeeper = halokeeper.NewKeeper(
-		appCodec, keys[halotypes.ModuleName], "uusyc", "uusdc", app.AccountKeeper, nil,
+		appCodec, keys[halotypes.ModuleName], "uusyc", "uusdc", app.AccountKeeper, nil, interfaceRegistry,
 	)
 	app.BankKeeper = bankkeeper.NewBaseKeeper(
 		appCodec, keys[banktypes.StoreKey], app.AccountKeeper, app.GetSubspace(banktypes.ModuleName), app.ModuleAccountAddrs(),

utils/mocks/bank.go

@@ -1,6 +1,8 @@
 package mocks
 
 import (
+	"fmt"
+
 	sdkerrors "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/errors"
@@ -15,7 +17,7 @@ type BankKeeper struct {
 	Restriction SendRestrictionFn
 }
 
-func (k BankKeeper) BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error {
+func (k BankKeeper) BurnCoins(_ sdk.Context, moduleName string, amt sdk.Coins) error {
 	address := authtypes.NewModuleAddress(moduleName).String()
 
 	balance := k.Balances[address]
@@ -29,7 +31,7 @@ func (k BankKeeper) BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins)
 	return nil
 }
 
-func (k BankKeeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error {
+func (k BankKeeper) MintCoins(_ sdk.Context, moduleName string, amt sdk.Coins) error {
 	address := authtypes.NewModuleAddress(moduleName).String()
 	k.Balances[address] = k.Balances[address].Add(amt...)
 
@@ -56,6 +58,10 @@ func NoOpSendRestrictionFn(_ sdk.Context, _, toAddr sdk.AccAddress, _ sdk.Coins)
 	return toAddr, nil
 }
 
+func FailingSendRestrictionFn(_ sdk.Context, _, toAddr sdk.AccAddress, _ sdk.Coins) (sdk.AccAddress, error) {
+	return nil, fmt.Errorf("%s is blocked from sending/receiving", toAddr.String())
+}
+
 func (k BankKeeper) WithSendCoinsRestriction(check SendRestrictionFn) BankKeeper {
 	oldRestriction := k.Restriction
 	k.Restriction = func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (newToAddr sdk.AccAddress, err error) {

utils/mocks/halo.go

@@ -39,6 +39,7 @@ func HaloKeeperWithKeepers(_ testing.TB, account AccountKeeper, bank BankKeeper)
 		"uusdc",
 		account,
 		nil,
+		registry,
 	)
 
 	bank = bank.WithSendCoinsRestriction(k.SendRestrictionFn)

x/halo/keeper/keeper.go

@@ -5,8 +5,8 @@ import (
 	"fmt"
 
 	sdkerrors "cosmossdk.io/errors"
-
 	"github.com/cosmos/cosmos-sdk/codec"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/noble-assets/halo/x/halo/types"
@@ -19,8 +19,9 @@ type Keeper struct {
 	Denom      string
 	Underlying string
 
-	accountKeeper types.AccountKeeper
-	bankKeeper    types.BankKeeper
+	accountKeeper     types.AccountKeeper
+	bankKeeper        types.BankKeeper
+	interfaceRegistry codectypes.InterfaceRegistry
 }
 
 func NewKeeper(
@@ -30,6 +31,7 @@ func NewKeeper(
 	underlying string,
 	accountKeeper types.AccountKeeper,
 	bankKeeper types.BankKeeper,
+	interfaceRegistry codectypes.InterfaceRegistry,
 ) *Keeper {
 	return &Keeper{
 		cdc:      cdc,
@@ -38,8 +40,9 @@ func NewKeeper(
 		Denom:      denom,
 		Underlying: underlying,
 
-		accountKeeper: accountKeeper,
-		bankKeeper:    bankKeeper,
+		accountKeeper:     accountKeeper,
+		bankKeeper:        bankKeeper,
+		interfaceRegistry: interfaceRegistry,
 	}
 }
 

x/halo/keeper/msg_server.go

@@ -32,6 +32,9 @@ func (k msgServer) Deposit(goCtx context.Context, msg *types.MsgDeposit) (*types
 	if !k.CanCall(ctx, signer, method) {
 		return nil, fmt.Errorf("%s cannot execute %s", msg.Signer, method)
 	}
+	if !msg.Amount.IsPositive() {
+		return nil, fmt.Errorf("invalid amount %s", msg.Amount.String())
+	}
 
 	amount, err := k.depositFor(ctx, signer, signer, msg.Amount)
 	return &types.MsgDepositResponse{Amount: amount}, err
@@ -57,6 +60,9 @@ func (k msgServer) DepositFor(goCtx context.Context, msg *types.MsgDepositFor) (
 			return nil, fmt.Errorf("%s cannot receive %s", msg.Recipient, k.Denom)
 		}
 	}
+	if !msg.Amount.IsPositive() {
+		return nil, fmt.Errorf("invalid amount %s", msg.Amount.String())
+	}
 
 	amount, err := k.depositFor(ctx, signer, recipient, msg.Amount)
 	return &types.MsgDepositResponse{Amount: amount}, err
@@ -73,6 +79,9 @@ func (k msgServer) Withdraw(goCtx context.Context, msg *types.MsgWithdraw) (*typ
 	if !k.CanCall(ctx, signer, method) {
 		return nil, fmt.Errorf("%s cannot execute %s", msg.Signer, method)
 	}
+	if !msg.Amount.IsPositive() {
+		return nil, fmt.Errorf("invalid amount %s", msg.Amount.String())
+	}
 
 	if !k.VerifyWithdrawSignature(ctx, signer, msg.Amount, msg.Signature) {
 		return nil, types.ErrInvalidSignature
@@ -102,6 +111,9 @@ func (k msgServer) WithdrawTo(goCtx context.Context, msg *types.MsgWithdrawTo) (
 			return nil, fmt.Errorf("%s cannot receive %s", msg.Recipient, k.Denom)
 		}
 	}
+	if !msg.Amount.IsPositive() {
+		return nil, fmt.Errorf("invalid amount %s", msg.Amount.String())
+	}
 
 	if !k.VerifyWithdrawSignature(ctx, recipient, msg.Amount, msg.Signature) {
 		return nil, types.ErrInvalidSignature
@@ -145,7 +157,9 @@ func (k msgServer) Burn(goCtx context.Context, msg *types.MsgBurn) (*types.MsgBu
 	if !k.CanCall(ctx, signer, method) {
 		return nil, fmt.Errorf("%s cannot execute %s", msg.Signer, method)
 	}
-
+	if !msg.Amount.IsPositive() {
+		return nil, fmt.Errorf("invalid amount %s", msg.Amount.String())
+	}
 	coins := sdk.NewCoins(sdk.NewCoin(k.Denom, msg.Amount))
 	err = k.burnCoins(ctx, signer, coins)
 
@@ -166,6 +180,9 @@ func (k msgServer) BurnFor(goCtx context.Context, msg *types.MsgBurnFor) (*types
 	if err != nil {
 		return nil, errors.Wrapf(err, "unable to decode from address %s", msg.From)
 	}
+	if !msg.Amount.IsPositive() {
+		return nil, fmt.Errorf("invalid amount %s", msg.Amount.String())
+	}
 
 	coins := sdk.NewCoins(sdk.NewCoin(k.Denom, msg.Amount))
 	err = k.burnCoins(ctx, from, coins)
@@ -190,6 +207,9 @@ func (k msgServer) Mint(goCtx context.Context, msg *types.MsgMint) (*types.MsgMi
 	if !k.CanCall(ctx, to, "transfer") {
 		return nil, fmt.Errorf("%s cannot transfer %s", msg.To, k.Denom)
 	}
+	if !msg.Amount.IsPositive() {
+		return nil, fmt.Errorf("invalid amount %s", msg.Amount.String())
+	}
 
 	coins := sdk.NewCoins(sdk.NewCoin(k.Denom, msg.Amount))
 	err = k.mintCoins(ctx, to, coins)
@@ -214,6 +234,9 @@ func (k msgServer) TradeToFiat(goCtx context.Context, msg *types.MsgTradeToFiat)
 	if !k.HasRole(ctx, signer, entitlements.ROLE_LIQUIDITY_PROVIDER) {
 		return nil, types.ErrInvalidLiquidityProvider
 	}
+	if !msg.Amount.IsPositive() {
+		return nil, fmt.Errorf("invalid amount %s", msg.Amount.String())
+	}
 
 	err = k.bankKeeper.SendCoinsFromModuleToAccount(
 		ctx, types.ModuleName, recipient,

x/halo/keeper/msg_server_aggregator.go

@@ -74,7 +74,7 @@ func (k aggregatorMsgServer) SetNextPrice(goCtx context.Context, msg *aggregator
 		return nil, err
 	}
 
-	if msg.NextPrice.IsZero() {
+	if !msg.NextPrice.IsPositive() {
 		return nil, aggregator.ErrInvalidNextPrice
 	}
 

x/halo/keeper/msg_server_aggregator_test.go

@@ -26,6 +26,16 @@ func TestReportBalance(t *testing.T) {
 	owner := utils.TestAccount()
 	k.SetAggregatorOwner(ctx, owner.Address)
 
+	// ARRANGE: Save the original LastRoundIDKey and reset it to an empty byte slice.
+	tmpLastRoundIDKey := aggregator.LastRoundIDKey
+	aggregator.LastRoundIDKey = []byte("")
+
+	// ACT: Verify the LastRoundIDKey is set to zero when the key is reset to an empty slice.
+	require.Equal(t, k.GetLastRoundId(ctx), uint64(0))
+
+	// ARRANGE: Restore the original LastRoundIDKey to its previous value.
+	aggregator.LastRoundIDKey = tmpLastRoundIDKey
+
 	// ACT: Attempt to report balance with invalid signer.
 	_, err = server.ReportBalance(goCtx, &aggregator.MsgReportBalance{
 		Signer: utils.TestAccount().Address,

x/halo/keeper/msg_server_entitlements.go

@@ -2,6 +2,7 @@ package keeper
 
 import (
 	"context"
+	"strings"
 
 	"cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -25,6 +26,11 @@ func (k entitlementsMsgServer) SetPublicCapability(goCtx context.Context, msg *e
 		return nil, err
 	}
 
+	resolved, _ := k.interfaceRegistry.Resolve(msg.Method)
+	if !(msg.Method == "transfer" || (resolved != nil && strings.HasPrefix(msg.Method, "/halo"))) {
+		return nil, errors.Wrapf(entitlements.ErrInvalidMethod, "method %s does not exist or is not allowed", msg.Method)
+	}
+
 	k.Keeper.SetPublicCapability(ctx, msg.Method, msg.Enabled)
 
 	return &entitlements.MsgSetPublicCapabilityResponse{}, ctx.EventManager().EmitTypedEvent(&entitlements.PublicCapabilityUpdated{
@@ -40,6 +46,16 @@ func (k entitlementsMsgServer) SetRoleCapability(goCtx context.Context, msg *ent
 		return nil, err
 	}
 
+	_, roleExists := entitlements.Role_value[msg.Role.String()]
+	if !roleExists {
+		return nil, errors.Wrapf(entitlements.ErrInvalidRole, "role %s does not exist", msg.Role)
+	}
+
+	resolved, _ := k.interfaceRegistry.Resolve(msg.Method)
+	if !(msg.Method == "transfer" || (resolved != nil && strings.HasPrefix(msg.Method, "/halo"))) {
+		return nil, errors.Wrapf(entitlements.ErrInvalidMethod, "method %s does not exist or is not allowed", msg.Method)
+	}
+
 	k.Keeper.SetRoleCapability(ctx, msg.Method, msg.Role, msg.Enabled)
 
 	return &entitlements.MsgSetRoleCapabilityResponse{}, ctx.EventManager().EmitTypedEvent(&entitlements.RoleCapabilityUpdated{
@@ -61,6 +77,11 @@ func (k entitlementsMsgServer) SetUserRole(goCtx context.Context, msg *entitleme
 		return nil, errors.Wrapf(err, "unable to decode user address %s", msg.User)
 	}
 
+	_, roleExists := entitlements.Role_value[msg.Role.String()]
+	if !roleExists {
+		return nil, errors.Wrapf(entitlements.ErrInvalidRole, "role %s does not exist", msg.Role)
+	}
+
 	k.Keeper.SetUserRole(ctx, user, msg.Role, msg.Enabled)
 
 	return &entitlements.MsgSetUserRoleResponse{}, ctx.EventManager().EmitTypedEvent(&entitlements.UserRoleUpdated{