fix(market): enforce OrderMaxBids with >= instead of >

[renezander030]

Hi, and thanks for all the work on the node.

This fixes the OrderMaxBids off-by-one reported in akash-network/support#413.

CreateBid in x/market/handler/server.go compared the existing bid count against OrderMaxBids with >:

if ms.keepers.Market.BidCountForOrder(ctx, msg.ID.OrderID()) > params.OrderMaxBids {

BidCountForOrder counts Open, Active and Closed bids, so the count reaches OrderMaxBids exactly when the cap is hit. With >, the check only fired at OrderMaxBids + 1, so one extra bid was always allowed. This switches the comparison to >=.

Test

Added TestCreateBidExceedsOrderMaxBids: it sets the cap to 1, seeds one bid via the keeper, then asserts a second bid through the handler is rejected with "too many existing bids". It fails on the old > (the second bid falls through to an "unknown provider" error) and passes with >=.

go test ./x/market/handler/ and go vet ./x/market/handler/ pass, and gofmt is clean.

Note

This changes consensus behaviour (a bid that previously succeeded now fails), so it belongs in a coordinated upgrade. I saw the code freeze for the upcoming network upgrade, so feel free to queue it for whenever that lands. Happy to adjust if you would rather have BidCountForOrder exclude Closed bids, which the issue flags as a possible follow-up.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5bd026fb-e380-4c98-802f-ed2a82fdabbc

📥 Commits

Reviewing files that changed from the base of the PR and between 7246b6c and 2558cdb.

📒 Files selected for processing (2)

• x/market/handler/handler_test.go
• x/market/handler/server.go

🚧 Files skipped from review as they are similar to previous changes (2)

• x/market/handler/server.go
• x/market/handler/handler_test.go

---

Walkthrough

CreateBid's bid-count check was changed from > to >= to reject bids when the maximum is reached. A new regression test seeds an order at the cap and verifies an additional bid is rejected with mv1.ErrInvalidBid and "too many existing bids".

Changes

Bid Maximum Boundary Condition Fix

Layer / File(s)	Summary
Bid count boundary condition fix and regression test x/market/handler/server.go, x/market/handler/handler_test.go	The CreateBid bid-count limit check changed from > params.OrderMaxBids to >= params.OrderMaxBids, rejecting bids when the count reaches the configured maximum. A new regression test TestCreateBidExceedsOrderMaxBids configures OrderMaxBids to 1, seeds one bid via the keeper, and confirms the handler rejects an extra MsgCreateBid with mv1.ErrInvalidBid and a "too many existing bids" message.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

I hopped through the code with bright-eyed cheer,
Found a cap reached and held it near,
I nudged the check to stop at the line,
Seeded a bid, then ran the sign,
Now extra bids fail — the logs are clear! 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: fixing an off-by-one bug in OrderMaxBids enforcement by changing the comparison operator from > to >=.
Description check	✅ Passed	The description thoroughly explains the bug, its cause, the fix, testing approach, and implications for consensus behavior, all directly related to the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chalabi2]

LGTM

Interested to hear your take on excluding closed bids from the bid count without causing DOS issues or allowing close records to inflate state.

[renezander030]

Thanks! I left that out of scope here since this PR is just the off-by-one, but it's worth digging into.

Closed bids count today because BidCountForOrder is the only backpressure on an order, so that count doubles as a lifetime cap. If you drop BidClosed from it, the DOS and state inflation turn into the same bug: a provider loops CreateBid > CloseBid > CreateBid forever, never hits the cap, and every cycle leaves a BidClosed record until the order settles.

So closed bids can't just be excluded, you'd need to replace the cap that count was providing. One clean option is a monotonic per-order counter (bids-ever-created), bumped on CreateBid and never decremented on close. CreateBid caps on that, so total bids stay bounded regardless of churn, which kills both the DOS and the record buildup, while a separate live count can still exclude closed bids for anything that cares about current competition. It's also O(1) instead of the current three-state scan.

I can spin that into a follow-up PR if the direction sounds right.

[chalabi2]

I can spin that into a follow-up PR if the direction sounds right.

Yeah that would be great, your implementation idea sounds similar to what I had in mind so go for it.

[chalabi2]

@renezander030 can you sign your commits?

[renezander030]

Commits are signed now. Follow-up PR with the monotonic counter is up: #2071.