Skip to content

fix: reject zero-value bids in bid()#67

Open
SIDDHANTCOOKIE wants to merge 2 commits into
StabilityNexus:mainfrom
SIDDHANTCOOKIE:fix/zero-value-bids
Open

fix: reject zero-value bids in bid()#67
SIDDHANTCOOKIE wants to merge 2 commits into
StabilityNexus:mainfrom
SIDDHANTCOOKIE:fix/zero-value-bids

Conversation

@SIDDHANTCOOKIE

@SIDDHANTCOOKIE SIDDHANTCOOKIE commented Mar 23, 2026

Copy link
Copy Markdown
Member

Addressed Issues:

Fixes #65

  • Added an early revert in contracts/EnglishAuction.sol inside bid(uint256 auctionId, uint256 bidAmount) immediately after:
    AuctionData storage auction = auctions[auctionId];
  • New check:
    require(bidAmount > 0, "Bid amount must be greater than zero");

Screenshots/Recordings:

image

Additional Notes:

AI Usage Disclosure:

We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.

Check one of the checkboxes below:

  • This PR does not contain AI-generated code at all.
  • This PR contains AI-generated code. I have read the AI Usage Policy and this PR complies with this policy. I have tested the code locally and I am responsible for it.

I have used the following AI models and tools: TODO

Checklist

  • My PR addresses a single issue, fixes a single bug or makes a single improvement.
  • My code follows the project's code style and conventions
  • If applicable, I have made corresponding changes or additions to the documentation
  • If applicable, I have made corresponding changes or additions to tests
  • My changes generate no new warnings or errors
  • I have joined the Discord server and I will share a link to this PR with the project maintainers there
  • I have read the Contribution Guidelines
  • Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.
  • I have filled this PR template completely and carefully, and I understand that my PR may be closed without review otherwise.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced bid validation to reject zero-value bids, preventing invalid auction interactions and improving contract robustness.

@coderabbitai

coderabbitai Bot commented Mar 23, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@SIDDHANTCOOKIE has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 1 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cff90f28-cd51-49e1-be7a-a217fb865154

📥 Commits

Reviewing files that changed from the base of the PR and between 91fd8d3 and 7e94dfb.

📒 Files selected for processing (1)
  • test/EnglishAuction.test.ts

Walkthrough

Added input validation to the bid() function in EnglishAuction.sol that rejects bids with zero amounts. This prevents zero-value bids from bypassing minimum bid delta checks and exploiting the auction's winner and deadline extension logic.

Changes

Cohort / File(s) Summary
Input Validation
contracts/EnglishAuction.sol
Added require(bidAmount > 0) check in bid() function to reject zero-value bids early in the execution flow.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

Solidity Lang

Poem

🐰 A zero bid caused quite the fright,
Extending auctions through the night!
But now we check before we play—
No more spam bids to save the day! 🎯✨

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: reject zero-value bids in bid()' clearly and specifically describes the main change: adding validation to reject zero-value bids in the bid function.
Linked Issues check ✅ Passed The pull request implements the exact fix required by issue #65: adding a require statement to validate bidAmount > 0, preventing zero-value bids from bypassing minBidDelta checks and enabling infinite deadline extensions.
Out of Scope Changes check ✅ Passed The pull request contains only the single required change: adding input validation for bidAmount > 0 in the bid function, with no additional out-of-scope modifications.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@contracts/EnglishAuction.sol`:
- Around line 93-95: Add a unit test that calls the bid(uint256 auctionId,
uint256 bidAmount) function and asserts it reverts with "Bid amount must be
greater than zero" when bidAmount == 0; locate the auction via AuctionData in
auctions[auctionId] and ensure the test covers both a case where the auction's
minimumBid > 0 and a case where minimumBid == 0 so the require in bid(...)
always triggers; use the exists and beforeDeadline preconditions (set up a valid
auctionId and deadline) so the revert is caused by the zero bid check in bid().
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e52bcb52-1761-4849-9e1a-619fbebc4405

📥 Commits

Reviewing files that changed from the base of the PR and between 886ea71 and 91fd8d3.

📒 Files selected for processing (1)
  • contracts/EnglishAuction.sol

Comment thread contracts/EnglishAuction.sol
function bid(uint256 auctionId, uint256 bidAmount) external nonReentrant exists(auctionId) beforeDeadline(auctions[auctionId].deadline) {
function bid(uint256 auctionId, uint256 bidAmount) external exists(auctionId) beforeDeadline(auctions[auctionId].deadline) {
AuctionData storage auction = auctions[auctionId];
require(bidAmount > 0, "Bid amount must be greater than zero");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the next line, we are checking if bid amount is greater than min bid amount or not. So if auctioner has made starting bid to be 0, we should allow users to place bid with 0.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, but a user can keep bidding 0 for free, stay as winner, and keep extending the auction without paying anything. If we truly want to allow zero bids, we can try a different design (e.g., explicit first-bid state), not just removing the zero-bid guard. We can separate “first valid bid” from the bid amount itself, and only let meaningful bids update winner/deadline. Then 0 would be treated as a non-valid bid (no winner update, no deadline extension).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Zero-Value Bids Bypass minBidDelta and Allow Infinite Extensions

2 participants