feat: node gigabyte_prices and hourly_prices should only contain the allowed denoms

Author: ironman0x7b2

types/v1/price.go

@@ -302,7 +302,7 @@ func (p Prices) Copy() Prices {
 
 // String converts the Prices slice to a semicolon-separated string.
 func (p Prices) String() string {
-	var parts []string
+	parts := make([]string, 0, len(p))
 	for _, price := range p {
 		parts = append(parts, price.String())
 	}
@@ -410,6 +410,36 @@ func (p Prices) Find(denom string) (Price, bool) {
 	return Price{}, false
 }
 
+// Add adds multiple Price values to the Prices slice.
+func (p Prices) Add(items ...Price) Prices {
+	return p.add(items)
+}
+
+// Sub subtracts multiple Price values from the Prices slice.
+func (p Prices) Sub(items ...Price) Prices {
+	return p.sub(items)
+}
+
+// Map converts the Prices slice into a map[denom]Price.
+// Panics if a duplicate denom exists in the result.
+func (p Prices) Map() map[string]Price {
+	m := make(map[string]Price, len(p))
+	for _, price := range p {
+		if _, exists := m[price.Denom]; exists {
+			panic(fmt.Errorf("duplicate denom %s", price.Denom))
+		}
+
+		m[price.Denom] = price
+	}
+
+	return m
+}
+
+// add adds values of another Prices slice to this one.
+func (p Prices) add(v Prices) Prices {
+	return p.merge(v)
+}
+
 // merge merges and sums two Prices slices by denom.
 func (p Prices) merge(v Prices) Prices {
 	m := make(map[string]Price)
@@ -444,22 +474,7 @@ func (p Prices) negative() Prices {
 	return v
 }
 
-// add adds values of another Prices slice to this one.
-func (p Prices) add(v Prices) Prices {
-	return p.merge(v)
-}
-
-// Add adds multiple Price values to the Prices slice.
-func (p Prices) Add(items ...Price) Prices {
-	return p.add(items)
-}
-
 // sub subtracts values of another Prices slice from this one.
 func (p Prices) sub(v Prices) Prices {
 	return p.merge(v.negative())
 }
-
-// Sub subtracts multiple Price values from the Prices slice.
-func (p Prices) Sub(items ...Price) Prices {
-	return p.sub(items)
-}

x/node/keeper/msg_handler.go

@@ -17,13 +17,13 @@ import (
 // It validates the pricing fields, checks for duplicates, collects deposit, and stores the new node in an inactive state.
 func (k *Keeper) HandleMsgRegisterNode(ctx sdk.Context, msg *v3.MsgRegisterNodeRequest) (*v3.MsgRegisterNodeResponse, error) {
 	// Validate submitted gigabyte prices
-	if !k.IsValidGigabytePrices(ctx, msg.GigabytePrices) {
-		return nil, types.NewErrorInvalidPrices(msg.GigabytePrices)
+	if err := k.ValidateGigabytePrices(ctx, msg.GigabytePrices); err != nil {
+		return nil, types.NewErrorInvalidPrices(err)
 	}
 
 	// Validate submitted hourly prices
-	if !k.IsValidHourlyPrices(ctx, msg.HourlyPrices) {
-		return nil, types.NewErrorInvalidPrices(msg.HourlyPrices)
+	if err := k.ValidateHourlyPrices(ctx, msg.HourlyPrices); err != nil {
+		return nil, types.NewErrorInvalidPrices(err)
 	}
 
 	// Parse the account address from the sender string
@@ -78,13 +78,13 @@ func (k *Keeper) HandleMsgRegisterNode(ctx sdk.Context, msg *v3.MsgRegisterNodeR
 // It verifies node existence and applies new pricing and metadata, emitting an update event.
 func (k *Keeper) HandleMsgUpdateNodeDetails(ctx sdk.Context, msg *v3.MsgUpdateNodeDetailsRequest) (*v3.MsgUpdateNodeDetailsResponse, error) {
 	// Validate new gigabyte prices
-	if !k.IsValidGigabytePrices(ctx, msg.GigabytePrices) {
-		return nil, types.NewErrorInvalidPrices(msg.GigabytePrices)
+	if err := k.ValidateGigabytePrices(ctx, msg.GigabytePrices); err != nil {
+		return nil, types.NewErrorInvalidPrices(err)
 	}
 
 	// Validate new hourly prices
-	if !k.IsValidHourlyPrices(ctx, msg.HourlyPrices) {
-		return nil, types.NewErrorInvalidPrices(msg.HourlyPrices)
+	if err := k.ValidateHourlyPrices(ctx, msg.HourlyPrices); err != nil {
+		return nil, types.NewErrorInvalidPrices(err)
 	}
 
 	// Parse sender's node address

x/node/keeper/params.go

@@ -1,6 +1,7 @@
 package keeper
 
 import (
+	"fmt"
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -50,46 +51,18 @@ func (k *Keeper) MinHourlyPrices(ctx sdk.Context) v1base.Prices {
 	return k.GetParams(ctx).MinHourlyPrices
 }
 
-// IsValidGigabytePrices checks if the provided gigabyte prices are valid based on the minimum prices defined in the module's parameters.
-func (k *Keeper) IsValidGigabytePrices(ctx sdk.Context, prices v1base.Prices) bool {
-	if prices.Len() == 0 {
-		return true
-	}
-
+// ValidateGigabytePrices validates prices against the minimum gigabyte prices.
+func (k *Keeper) ValidateGigabytePrices(ctx sdk.Context, prices v1base.Prices) error {
 	minPrices := k.MinGigabytePrices(ctx)
-	for _, price := range minPrices {
-		baseValue, quoteValue := prices.AmountOf(price.Denom)
-		if !baseValue.IsZero() && baseValue.LT(price.BaseValue) {
-			return false
-		}
-
-		if !quoteValue.IsZero() && quoteValue.LT(price.QuoteValue) {
-			return false
-		}
-	}
 
-	return true
+	return validatePrices(prices, minPrices)
 }
 
-// IsValidHourlyPrices checks if the provided hourly prices are valid based on the minimum prices defined in the module's parameters.
-func (k *Keeper) IsValidHourlyPrices(ctx sdk.Context, prices v1base.Prices) bool {
-	if prices.Len() == 0 {
-		return true
-	}
-
+// ValidateHourlyPrices validates prices against the minimum hourly prices.
+func (k *Keeper) ValidateHourlyPrices(ctx sdk.Context, prices v1base.Prices) error {
 	minPrices := k.MinHourlyPrices(ctx)
-	for _, price := range minPrices {
-		baseValue, quoteValue := prices.AmountOf(price.Denom)
-		if !baseValue.IsZero() && baseValue.LT(price.BaseValue) {
-			return false
-		}
-
-		if !quoteValue.IsZero() && quoteValue.LT(price.QuoteValue) {
-			return false
-		}
-	}
 
-	return true
+	return validatePrices(prices, minPrices)
 }
 
 // GetInactiveAt returns the inactive time by adding ActiveDuration to the current block time.
@@ -98,3 +71,35 @@ func (k *Keeper) GetInactiveAt(ctx sdk.Context) time.Time {
 
 	return ctx.BlockTime().Add(d)
 }
+
+// validatePrices checks that all provided prices use allowed denoms and
+// meet or exceed the given minimums (zero values and empty lists are valid).
+func validatePrices(prices v1base.Prices, minPrices v1base.Prices) error {
+	// Empty set of prices is valid: treated as zero prices for all denoms.
+	if prices.Len() == 0 {
+		return nil
+	}
+
+	// Build a lookup map for minPrices
+	m := minPrices.Map()
+
+	for _, price := range prices {
+		minPrice, ok := m[price.Denom]
+		if !ok {
+			return fmt.Errorf("denom %s is not allowed", price.Denom)
+		}
+
+		// Only check non-zero values
+		if !price.BaseValue.IsZero() && price.BaseValue.LT(minPrice.BaseValue) {
+			return fmt.Errorf("base value %s for denom %s is lesser than %s",
+				price.BaseValue, price.Denom, minPrice.BaseValue)
+		}
+
+		if !price.QuoteValue.IsZero() && price.QuoteValue.LT(minPrice.QuoteValue) {
+			return fmt.Errorf("quote value %s for denom %s is lesser than %s",
+				price.QuoteValue, price.Denom, minPrice.QuoteValue)
+		}
+	}
+
+	return nil
+}

x/node/types/errors.go

@@ -69,8 +69,8 @@ func NewErrorInvalidPrice(price v1base.Price) error {
 }
 
 // NewErrorInvalidPrices returns an error indicating that the provided prices are invalid.
-func NewErrorInvalidPrices(prices v1base.Prices) error {
-	return sdkerrors.Wrapf(ErrInvalidPrices, "invalid prices %s", prices)
+func NewErrorInvalidPrices(err error) error {
+	return sdkerrors.Wrap(ErrInvalidPrices, err.Error())
 }
 
 // NewErrorInvalidSessionStatus returns an error indicating that the provided status is invalid for the session.