fix(market): bound bids per order with a monotonic bids-ever-created counter

x/market/handler/handler_test.go

@@ -915,6 +915,135 @@ func TestCreateBidValid(t *testing.T) {
 	require.True(t, found)
 }
 
+// TestCreateBidExceedsOrderMaxBids is a regression test for the OrderMaxBids
+// off-by-one (akash-network/support#413). The original check used `>`, which
+// allowed OrderMaxBids+1 bids per order. With the cap set to 1 and one bid
+// already present, a further bid must be rejected.
+func TestCreateBidExceedsOrderMaxBids(t *testing.T) {
+	suite := setupTestSuite(t)
+
+	order, gspec := suite.createOrder(testutil.Resources(t, testutil.WithDenom("uact")))
+
+	params, err := suite.MarketKeeper().GetParams(suite.Context())
+	require.NoError(t, err)
+	params.OrderMaxBids = 1
+	require.NoError(t, suite.MarketKeeper().SetParams(suite.Context(), params))
+
+	// Seed the single permitted bid directly via the keeper so the order is at
+	// its cap. The OrderMaxBids check runs before any provider/escrow lookup, so
+	// no provider registration or bank mocks are required here.
+	roffer := mvbeta.ResourceOfferFromRU(gspec.Resources)
+	_, err = suite.MarketKeeper().CreateBid(suite.Context(), mv1.MakeBidID(order.ID, testutil.AccAddress(t)), order.Price(), roffer, nil)
+	require.NoError(t, err)
+	require.Equal(t, uint32(1), suite.MarketKeeper().BidCountForOrder(suite.Context(), order.ID))
+
+	// With the cap reached, a further bid must be rejected.
+	msg := &mvbeta.MsgCreateBid{
+		ID:    mv1.MakeBidID(order.ID, testutil.AccAddress(t)),
+		Price: sdk.NewDecCoin(sdkutil.DenomUact, sdkmath.NewInt(1)),
+		Deposit: deposit.Deposit{
+			Amount:  mvbeta.DefaultBidMinDepositACT,
+			Sources: deposit.Sources{deposit.SourceBalance},
+		},
+	}
+
+	res, err := suite.handler(suite.Context(), msg)
+	require.Nil(t, res)
+	require.Error(t, err)
+	require.ErrorIs(t, err, mv1.ErrInvalidBid)
+	require.Contains(t, err.Error(), "too many existing bids")
+}
+
+// TestCreateBidCapSurvivesBidChurn verifies that the OrderMaxBids cap is
+// enforced against the monotonic bids-ever-created counter rather than the
+// live bid count. Closing a bid must not free up cap capacity, otherwise a
+// provider could loop CreateBid/CloseBid to accumulate unbounded bid records.
+func TestCreateBidCapSurvivesBidChurn(t *testing.T) {
+	suite := setupTestSuite(t)
+
+	order, gspec := suite.createOrder(testutil.Resources(t, testutil.WithDenom("uact")))
+
+	params, err := suite.MarketKeeper().GetParams(suite.Context())
+	require.NoError(t, err)
+	params.OrderMaxBids = 1
+	require.NoError(t, suite.MarketKeeper().SetParams(suite.Context(), params))
+
+	roffer := mvbeta.ResourceOfferFromRU(gspec.Resources)
+	bid, err := suite.MarketKeeper().CreateBid(suite.Context(), mv1.MakeBidID(order.ID, testutil.AccAddress(t)), order.Price(), roffer, nil)
+	require.NoError(t, err)
+
+	// Closing the bid drops the live count to zero...
+	require.NoError(t, suite.MarketKeeper().OnBidClosed(suite.Context(), bid))
+	require.Equal(t, uint32(0), suite.MarketKeeper().BidCountForOrder(suite.Context(), order.ID))
+
+	// ...but the monotonic counter still holds the order at its cap.
+	require.Equal(t, uint32(1), suite.MarketKeeper().BidsEverCreatedForOrder(suite.Context(), order.ID))
+
+	msg := &mvbeta.MsgCreateBid{
+		ID:    mv1.MakeBidID(order.ID, testutil.AccAddress(t)),
+		Price: sdk.NewDecCoin(sdkutil.DenomUact, sdkmath.NewInt(1)),
+		Deposit: deposit.Deposit{
+			Amount:  mvbeta.DefaultBidMinDepositACT,
+			Sources: deposit.Sources{deposit.SourceBalance},
+		},
+	}
+
+	res, err := suite.handler(suite.Context(), msg)
+	require.Nil(t, res)
+	require.Error(t, err)
+	require.ErrorIs(t, err, mv1.ErrInvalidBid)
+	require.Contains(t, err.Error(), "too many existing bids")
+}
+
+// TestBidsEverCreatedLazySeedsFromStore covers orders that predate the
+// bids-ever-created counter: with no counter record present the count is
+// derived from all stored bids, including closed and lost ones, so the cap
+// carries over an upgrade or genesis import unchanged.
+func TestBidsEverCreatedLazySeedsFromStore(t *testing.T) {
+	suite := setupTestSuite(t)
+
+	order, gspec := suite.createOrder(testutil.Resources(t, testutil.WithDenom("uact")))
+
+	params, err := suite.MarketKeeper().GetParams(suite.Context())
+	require.NoError(t, err)
+	params.OrderMaxBids = 2
+	require.NoError(t, suite.MarketKeeper().SetParams(suite.Context(), params))
+
+	// Seed bids directly in the store, the same way genesis import and
+	// pre-counter state do, so no counter record exists for the order.
+	roffer := mvbeta.ResourceOfferFromRU(gspec.Resources)
+	for _, bstate := range []mvbeta.Bid_State{mvbeta.BidClosed, mvbeta.BidLost} {
+		err := suite.MarketKeeper().SaveBid(suite.Context(), mvbeta.Bid{
+			ID:             mv1.MakeBidID(order.ID, testutil.AccAddress(t)),
+			State:          bstate,
+			Price:          order.Price(),
+			CreatedAt:      suite.Context().BlockHeight(),
+			ResourcesOffer: roffer,
+		})
+		require.NoError(t, err)
+	}
+
+	// The live count excludes the seeded bids, but the lazily derived
+	// bids-ever-created count sees both and holds the order at its cap.
+	require.Equal(t, uint32(0), suite.MarketKeeper().BidCountForOrder(suite.Context(), order.ID))
+	require.Equal(t, uint32(2), suite.MarketKeeper().BidsEverCreatedForOrder(suite.Context(), order.ID))
+
+	msg := &mvbeta.MsgCreateBid{
+		ID:    mv1.MakeBidID(order.ID, testutil.AccAddress(t)),
+		Price: sdk.NewDecCoin(sdkutil.DenomUact, sdkmath.NewInt(1)),
+		Deposit: deposit.Deposit{
+			Amount:  mvbeta.DefaultBidMinDepositACT,
+			Sources: deposit.Sources{deposit.SourceBalance},
+		},
+	}
+
+	res, err := suite.handler(suite.Context(), msg)
+	require.Nil(t, res)
+	require.Error(t, err)
+	require.ErrorIs(t, err, mv1.ErrInvalidBid)
+	require.Contains(t, err.Error(), "too many existing bids")
+}
+
 func TestCreateBidInvalidPrice(t *testing.T) {
 	suite := setupTestSuite(t)
 	suite.PrepareMocks(func(ts *state.TestSuite) {

x/market/handler/server.go

@@ -42,7 +42,10 @@ func (ms msgServer) CreateBid(goCtx context.Context, msg *mvbeta.MsgCreateBid) (
 		return nil, fmt.Errorf("%w: minimum:%v received:%v", mv1.ErrInvalidDeposit, sdk.NewCoin(msg.Deposit.Amount.Denom, minDeposit), msg.Deposit)
 	}
 
-	if ms.keepers.Market.BidCountForOrder(ctx, msg.ID.OrderID()) > params.OrderMaxBids {
+	// Cap on bids ever created, not live bids: closing a bid must not free up
+	// capacity, otherwise a provider can loop CreateBid/CloseBid to grow state
+	// without bound and crowd out other providers.
+	if ms.keepers.Market.BidsEverCreatedForOrder(ctx, msg.ID.OrderID()) >= params.OrderMaxBids {
 		return nil, fmt.Errorf("%w: too many existing bids (%v)", mv1.ErrInvalidBid, params.OrderMaxBids)
 	}
 

x/market/keeper/keeper.go

@@ -1,6 +1,7 @@
 package keeper
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
@@ -44,6 +45,7 @@ type IKeeper interface {
 	WithLeases(ctx sdk.Context, fn func(mv1.Lease) bool)
 	WithOrdersForGroup(ctx sdk.Context, id dtypes.GroupID, state types.Order_State, fn func(types.Order) bool)
 	BidCountForOrder(ctx sdk.Context, id mv1.OrderID) uint32
+	BidsEverCreatedForOrder(ctx sdk.Context, id mv1.OrderID) uint32
 	GetParams(ctx sdk.Context) (types.Params, error)
 	SetParams(ctx sdk.Context, params types.Params) error
 	GetAuthority() string
@@ -61,11 +63,12 @@ type Keeper struct {
 	// This should be the x/gov module account.
 	authority string
 
-	schema collections.Schema
-	bids   *collections.IndexedMap[keys.BidPrimaryKey, types.Bid, BidIndexes]
-	orders *collections.IndexedMap[keys.OrderPrimaryKey, types.Order, OrderIndexes]
-	leases *collections.IndexedMap[keys.LeasePrimaryKey, mv1.Lease, LeaseIndexes]
-	Params collections.Item[types.Params]
+	schema    collections.Schema
+	bids      *collections.IndexedMap[keys.BidPrimaryKey, types.Bid, BidIndexes]
+	orders    *collections.IndexedMap[keys.OrderPrimaryKey, types.Order, OrderIndexes]
+	leases    *collections.IndexedMap[keys.LeasePrimaryKey, mv1.Lease, LeaseIndexes]
+	bidCounts collections.Map[keys.OrderPrimaryKey, uint32]
+	Params    collections.Item[types.Params]
 }
 
 // NewKeeper creates and returns an instance for Market keeper
@@ -80,6 +83,7 @@ func NewKeeper(cdc codec.BinaryCodec, skey *storetypes.KVStoreKey, ekeeper Escro
 	bids := collections.NewIndexedMap(sb, collections.NewPrefix(keys.BidPrefixNew), "bids", keys.BidPrimaryKeyCodec, codec.CollValue[types.Bid](cdc), bidIndexes)
 	orders := collections.NewIndexedMap(sb, collections.NewPrefix(keys.OrderPrefixNew), "orders", keys.OrderPrimaryKeyCodec, codec.CollValue[types.Order](cdc), orderIndexes)
 	leases := collections.NewIndexedMap(sb, collections.NewPrefix(keys.LeasePrefixNew), "leases", keys.LeasePrimaryKeyCodec, codec.CollValue[mv1.Lease](cdc), leaseIndexes)
+	bidCounts := collections.NewMap(sb, collections.NewPrefix(keys.BidCountPrefix), "bid_counts", keys.OrderPrimaryKeyCodec, collections.Uint32Value)
 	params := collections.NewItem(sb, keys.ParamsPrefix, "params", codec.CollValue[types.Params](cdc))
 
 	schema, err := sb.Build()
@@ -96,6 +100,7 @@ func NewKeeper(cdc codec.BinaryCodec, skey *storetypes.KVStoreKey, ekeeper Escro
 		bids:      bids,
 		orders:    orders,
 		leases:    leases,
+		bidCounts: bidCounts,
 		Params:    params,
 	}
 
@@ -230,6 +235,15 @@ func (k Keeper) CreateBid(ctx sdk.Context, id mv1.BidID, price sdk.DecCoin, roff
 		ReclamationWindow: reclaimWindow,
 	}
 
+	// Bump the monotonic bids-ever-created counter for the order. It never
+	// decrements when bids close, so total bid records per order stay bounded
+	// by OrderMaxBids regardless of create/close churn. Read before storing
+	// the new bid: a lazy seed scans stored bids and must not see this one.
+	count := k.BidsEverCreatedForOrder(ctx, id.OrderID())
+	if err := k.bidCounts.Set(ctx, keys.OrderIDToKey(id.OrderID()), count+1); err != nil {
+		return types.Bid{}, err
+	}
+
 	if err := k.bids.Set(ctx, pk, bid); err != nil {
 		return types.Bid{}, err
 	}
@@ -559,15 +573,40 @@ func (k Keeper) WithBidsForOrder(ctx sdk.Context, id mv1.OrderID, state types.Bi
 	}
 }
 
+// BidCountForOrder returns the number of live (open or active) bids on an
+// order. Closed and lost bids are excluded; use BidsEverCreatedForOrder for
+// the monotonic count that backs the OrderMaxBids cap.
 func (k Keeper) BidCountForOrder(ctx sdk.Context, id mv1.OrderID) uint32 {
+	return k.bidCountForStates(ctx, id, []types.Bid_State{types.BidOpen, types.BidActive})
+}
+
+// BidsEverCreatedForOrder returns the monotonic count of bids ever created for
+// an order. The counter is bumped on CreateBid and never decremented when a
+// bid closes or loses, so it bounds the total number of bid records an order
+// can accumulate. Orders that predate the counter are seeded lazily from a
+// scan of all stored bids, which preserves the cap across the upgrade and
+// after genesis import.
+func (k Keeper) BidsEverCreatedForOrder(ctx sdk.Context, id mv1.OrderID) uint32 {
+	count, err := k.bidCounts.Get(ctx, keys.OrderIDToKey(id))
+	if err == nil {
+		return count
+	}
+	if !errors.Is(err, collections.ErrNotFound) {
+		panic(fmt.Sprintf("BidsEverCreatedForOrder failed: %v", err))
+	}
+
+	return k.bidCountForStates(ctx, id, []types.Bid_State{types.BidOpen, types.BidActive, types.BidLost, types.BidClosed})
+}
+
+func (k Keeper) bidCountForStates(ctx sdk.Context, id mv1.OrderID, states []types.Bid_State) uint32 {
 	orderPart := collections.Join4(id.Owner, id.DSeq, id.GSeq, id.OSeq)
 	count := uint32(0)
 
-	for _, state := range []types.Bid_State{types.BidOpen, types.BidActive, types.BidClosed} {
+	for _, state := range states {
 		refKey := collections.Join(orderPart, int32(state))
 		iter, err := k.bids.Indexes.OrderState.MatchExact(ctx, refKey)
 		if err != nil {
-			panic(fmt.Sprintf("BidCountForOrder failed: %v", err))
+			panic(fmt.Sprintf("bidCountForStates failed: %v", err))
 		}
 		for ; iter.Valid(); iter.Next() {
 			count++

x/market/keeper/keys/key.go

@@ -40,6 +40,7 @@ var (
 	BidIndexStatePrefix               = []byte{0x12, 0x03}
 	BidIndexProviderPrefix            = []byte{0x12, 0x04}
 	BidIndexOrderStatePrefix          = []byte{0x12, 0x05}
+	BidCountPrefix                    = []byte{0x12, 0x06}
 	BidStateOpenPrefix                = []byte{BidStateOpenPrefixID}
 	BidStateActivePrefix              = []byte{BidStateActivePrefixID}
 	BidStateLostPrefix                = []byte{BidStateLostPrefixID}