Fix auction outbid refunds with pull credits

[punk6529]

Summary

• Replaces bid-path push refunds in StreamAuctions with bidder pull credits.
• Adds guarded bidder credit withdrawals, bidder owed/active escrow views, and auction-local emergency-withdrawable surplus protection.
• Adds auction payment/reentrancy tests for rejecting previous bidders, failed withdrawals, withdrawal reentrancy, bid thresholds, emergency escrow/surplus boundaries, and settlement replay.
• Updates ADRs, roadmap, Slither baseline, blockers, test docs, and durable autonomous state.

Closes #12.

Validation

• forge test --match-contract StreamAuctionPaymentsTest -vvv passed with 10 tests.
• forge test --match-contract StreamDropsIntegrationCharacterizationTest -vvv passed with 10 tests.
• make check passed with 69 tests.
• powershell -NoProfile -ExecutionPolicy Bypass -File scripts\check.ps1 passed with 69 tests.
• forge fmt --check smart-contracts\AuctionContract.sol test\StreamAuctionPayments.t.sol passed.
• git diff --check passed.
• Markdown heading scan passed for touched docs/state files.
• slither . --config-file slither.config.json --foundry-compile-all --json <temp-file> returned expected non-zero exit because unrelated baseline findings remain; JSON showed REENTRANCY_ETH_TOTAL=0, TARGET_PARTICIPATE_TO_AUCTION=0, and AUCTION_ARBITRARY_SEND_ETH=0.

Notes

• This PR is intentionally scoped to P0-AUCT-002 bid/outbid refunds.
• It does not complete ADR 0002 custody/state machine, with-bid settlement credits, fixed-price pull payments, curator reward credits, or full P0-PAY emergency surplus work.

Summary by CodeRabbit

Release Notes

• New Features

  • Outbid auction bidders now accumulate withdrawable credits instead of receiving immediate refunds, enabling flexible claim timing.
  • Added dedicated withdrawal functions for bidders to claim their accumulated credits.

• Bug Fixes

  • Fixed reentrancy vulnerability in auction participation by eliminating push-based refunds.
  • Emergency withdrawals now properly exclude active auction bids and owed bidder credits.

• Tests

  • Added comprehensive test suite covering auction payment flows, bid settlement, withdrawal behavior, and emergency withdrawal constraints.

• Documentation

  • Updated architecture and roadmap documentation to reflect new auction credit system and implementation progress.

[claude]

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, push a new commit or reopen this pull request to trigger a review.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

PR #58 converts StreamAuctions outbid refunds from immediate push payments to withdrawable bidder credits. The contract gains reentrancy protection, per-bidder credit mappings, and escrow-based state tracking. participateToAuction now creates credits instead of calling external receive, while claimAuction settles from tracked balances. New withdrawal functions let bidders claim credits, and emergencyWithdraw respects all owed amounts. A comprehensive test suite validates credit issuance, withdrawal, reentrancy resistance, bid rules, emergency bounds, and settlement idempotency. Documentation updates reflect the design, mark Slither findings as fixed, and track implementation progress.

Auction Bid-Credit Accounting

Layer / File(s)	Summary
Contract state, events, and reentrancy protection smart-contracts/AuctionContract.sol	StreamAuctions inherits ReentrancyGuard and declares OutbidCreditCreated and BidderCreditWithdrawn events. Constructor/modifier formatting is updated while preserving signatures.
Auction bidding with credit accounting smart-contracts/AuctionContract.sol	participateToAuction is marked nonReentrant and updated to track highest bidder, create credits for previous bidders, update escrow/owed totals, and emit events. claimAuction settles from tracked escrow instead of prior refunds while maintaining payout splits and NFT transfers.
Credit withdrawal and emergency withdrawal bounds smart-contracts/AuctionContract.sol	withdrawBidderCredit and withdrawBidderCreditTo functions allow pull-payment claiming. totalOwed and emergencyWithdrawable view helpers enforce emergency withdrawal limits to only surplus amounts, protecting owed credits and active escrow.
Test framework, setup helpers, and receiver contracts test/StreamAuctionPayments.t.sol	Introduces test scaffolding, AuctionSetup struct for auction deployment/configuration, and RejectingBidder/ReentrantBidder helper contracts to simulate failure and reentrancy scenarios.
Bid credit and withdrawal behavior tests test/StreamAuctionPayments.t.sol	Validates outbid credit creation, withdrawal success/failure, reentrancy resistance, minimum bid enforcement, and increment rule enforcement with assertions on highest bid/bidder, bidder credits, totals, escrow, and events.
Emergency withdrawal and settlement tests test/StreamAuctionPayments.t.sol	Confirms emergencyWithdraw preserves credits/escrow while withdrawing only surplus, and that settlement payouts are correct and idempotent while releasing escrow.
Design documentation, project tracking, and status updates docs/adr/0002-auction-custody.md, docs/adr/0003-payment-accounting.md, docs/known-blockers.md, ops/ROADMAP.md, ops/SLITHER_BASELINE.md, test/README.md, ops/AUTONOMOUS_RUN.md	ADRs document the credit model. Roadmap marks P0-AUCT-002 bid/outbid portion implemented with Slither rows marked Fixed. Test README, known blockers, and AUTONOMOUS_RUN record scope, coverage targets, and execution progress through PR #57 merge and PR #58 initiation.

Sequence Diagram

sequenceDiagram
  participant Bidder1
  participant Bidder2
  participant participateToAuction
  participant auctionBidderCredits
  participant claimAuction
  Bidder1->>participateToAuction: bid 1.0 ETH (first bid)
  participateToAuction->>participateToAuction: highest = 1.0, highestBidder = Bidder1
  Bidder2->>participateToAuction: bid 2.0 ETH (outbid)
  participateToAuction->>auctionBidderCredits: credits[Bidder1] = 1.0
  participateToAuction->>participateToAuction: highest = 2.0, highestBidder = Bidder2
  participateToAuction->>participateToAuction: emit OutbidCreditCreated(Bidder1, tokenid, 1.0)
  claimAuction->>participateToAuction: settle highest bid
  claimAuction->>participateToAuction: pay poster, protocol, curators
  claimAuction->>participateToAuction: transfer NFT to Bidder2
  Bidder1->>participateToAuction: withdrawBidderCredit()
  participateToAuction->>auctionBidderCredits: credits[Bidder1] = 0
  participateToAuction->>Bidder1: transfer 1.0 ETH
  participateToAuction->>participateToAuction: emit BidderCreditWithdrawn(Bidder1, Bidder1, 1.0)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• #28: Main issue implements the removal of push refunds from participateToAuction and introduction of bidder-credit bookkeeping with new state mappings, withdrawal functions, and events.
• #22: Main issue implements the auction custody/state-machine work called for in P0-AUCT-001 through new claim-status and settlement-tracking mappings.
• #25: Main issue implements auction-side pull-payment accounting with bidder credit mappings, totalOwed/emergencyWithdrawable helpers, and escrow bookkeeping aligned to payment-ledger and emergency-withdrawal requirements.

Possibly related PRs

• 6529-Collections/6529Stream#3: Both PRs extend the same control-plane documentation files—ops/AUTONOMOUS_RUN.md and ops/ROADMAP.md—with the main PR advancing roadmap/queue state to cover the auction outbid-refund credit work.
• 6529-Collections/6529Stream#32: Both PRs modify docs/adr/0003-payment-accounting.md, with the main PR updating ADR0003's "Current Behavior" to reflect the new pull-credit/outbid refund semantics.
• 6529-Collections/6529Stream#7: The main PR updates ops/SLITHER_BASELINE.md to mark StreamAuctions Slither findings as fixed, building on the baseline setup introduced in PR #7.

Poem

🐰 A rabbit hops through auction grounds,
where credits now securely bound—
no more refunds that rush and fail,
pull payments pass the reentrancy tale!
Fresh tests dance to every beat,
emergency bounds kept neat. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title accurately and concisely summarizes the main change: converting auction outbid refunds from push to pull-credit model. It is clear, specific, and directly reflects the primary objective.
Linked Issues check	✅ Passed	All primary coding requirements from issue #12 are met: reentrancy protection added via nonReentrant, outbid refunds converted to withdrawable credits, events emitted, and required tests provided.
Out of Scope Changes check	✅ Passed	Changes are properly scoped to P0-AUCT-002. Documentation updates (ADRs, roadmap, blockers, Slither baseline) appropriately reflect the PR scope and intentionally mark remaining work (settlement, custody completion, curator credits) as out of scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/auction-pull-credits

---

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

[punk6529]

@claude please review this PR.

Focus areas:

• Confirm StreamAuctions.participateToAuction has no ETH call in the outbid path and cannot be blocked by a reverting previous bidder.
• Check consistency of auctionBidderCredits, totalBidderOwed, totalAuctionBidEscrow, totalOwed(), and emergencyWithdrawable() across bid, outbid, withdrawal, emergency withdrawal, and settlement paths.
• Confirm bidder credit withdrawal is nonReentrant, uses effects before interaction, and failed withdrawal preserves credits via revert.
• Confirm auction emergency withdrawal only releases auction-local surplus and cannot drain bidder credits or active highest-bid escrow.
• Note that final auction settlement push payments, full custody/state-machine work, fixed-price pull payments, curator credits, and protocol-wide payment ledger work intentionally remain separate P0 items.
• Review test/StreamAuctionPayments.t.sol coverage for rejecting previous bidders, failed withdrawals, reentrant withdrawals, bid thresholds, forced surplus, and settlement replay.

[punk6529]

@coderabbitai review

The head moved from ca5ca09 to 91a92f6 only to update ops/AUTONOMOUS_RUN.md with the PR URL and Claude review comment. Please review the latest head 91a92f6461355ea108252c49dbd5f7db5ffc4a34.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find key files
fd -e sol | head -30
fd -e sol -e md | grep -i auction | head -20

Length of output: 1230

---

🏁 Script executed:

#!/bin/bash
# Find the auction contract and test file
fd "AuctionContract.sol" --type f
fd "StreamAuctionPayments" --type f
fd "StreamAuctions" --type f

Length of output: 266

---

🏁 Script executed:

#!/bin/bash
cat smart-contracts/AuctionContract.sol

Length of output: 9366

---

🏁 Script executed:

#!/bin/bash
cat test/StreamAuctionPayments.t.sol

Length of output: 15662

---

---

@punk6529 — reviewed against head 91a92f6. The change from ca5ca09 is purely the ops/AUTONOMOUS_RUN.md update; the contract and test logic are unchanged. Here is the full focused review:

---

1 — No ETH call in the outbid path ✅

participateToAuction makes no external call when displacing the previous bidder. It only writes:

auctionBidderCredits[previousBidder] += previousBid;
totalBidderOwed += previousBid;

A reverting receive() (as exercised by RejectingBidder) cannot block or grief a new bid. Confirmed.

---

2 — Accounting consistency across all paths ✅

Tracing the five paths with two bids V1 (first) and V2 (outbid):

Path	totalAuctionBidEscrow	totalBidderOwed	balance
First bid V1	+V1	0	+V1
Outbid V2	V1→V2	+V1	+V1+V2
Withdraw credit V1	V2	0	V2
Settlement V2	0	0	0
Emergency (no surplus)	unchanged	unchanged	unchanged

totalOwed() = totalBidderOwed + totalAuctionBidEscrow tracks exactly the ETH the contract must not touch. emergencyWithdrawable() = balance − totalOwed() correctly returns 0 in the normal case and only the injected surplus in testEmergencyWithdrawOnlyWithdrawsAuctionLocalSurplus. Accounting is consistent.

Minor precision note (no funds at risk): In claimAuction, highestBid/2 + highestBid/4 + highestBid/4 can be up to 3 wei less than highestBid when highestBid % 4 != 0 (Solidity integer truncation). totalAuctionBidEscrow is debited the full highestBid, so the residual falls into emergencyWithdrawable() and is recoverable by emergencyWithdraw(). Not a safety issue, just cosmetic dust.

---

3 — withdrawBidderCreditTo: guards, CEI, failure safety ✅

function withdrawBidderCreditTo(address payable _recipient) public nonReentrant {
    require(_recipient != address(0), "Zero recipient");
    uint256 credit = auctionBidderCredits[msg.sender];
    require(credit != 0, "No credit");
    // Effects first
    auctionBidderCredits[msg.sender] = 0;
    totalBidderOwed -= credit;
    // Interaction last
    (bool success,) = _recipient.call{ value: credit }("");
    require(success, "ETH failed");
    ...
}

• nonReentrant is present. withdrawBidderCredit() calls withdrawBidderCreditTo, and the shared ReentrancyGuard lock is acquired inside withdrawBidderCreditTo, so re-entry via either entry point is blocked (confirmed by testReentrantCreditWithdrawalCannotDrainMoreThanCredit: reentered == false, balance delta exactly one credit).
• CEI order is correct.
• require(success, "ETH failed") rolls back the effect writes on failure, preserving auctionBidderCredits[msg.sender] and totalBidderOwed (confirmed by testCreditWithdrawalFailurePreservesCredit). ✅

---

4 — Emergency withdrawal is bounded to auction-local surplus ✅

emergencyWithdraw() reads emergencyWithdrawable() before sending:

function emergencyWithdrawable() public view returns (uint256) {
    uint256 balance = address(this).balance;
    uint256 owed = totalOwed();          // bidder credits + active bid escrow
    if (balance <= owed) return 0;
    return balance - owed;
}

Cannot drain bidder credits or active highest-bid escrow under any scenario. testEmergencyWithdrawCannotDrainBidCreditsOrActiveBidEscrow confirms zero withdrawal when balance == owed; testEmergencyWithdrawOnlyWithdrawsAuctionLocalSurplus confirms exact surplus is withdrawn and owed balance is preserved. ✅

---

5 — Out-of-scope items (noted, not reviewed)

Final auction settlement push payments, full custody/state-machine work, fixed-price pull payments, curator reward credits, and protocol-wide payment ledger are intentionally deferred as separate P0 items. The settlement's three push calls (poster, payOutAddress, curatorsPoolAddress) in claimAuction are the expected placeholder; push-payment risk there is acknowledged out of scope.

---

6 — Test coverage ✅

Scenario	Test
Rejecting previous bidder cannot block bid	testRejectingPreviousBidderCannotBlockHigherBid
Failed withdrawal preserves credit	testCreditWithdrawalFailurePreservesCredit
Reentrant withdrawal is blocked	testReentrantCreditWithdrawalCannotDrainMoreThanCredit
1-wei underbid rejects + no credit created	testBidOneWeiBelowMinimumFailsWithoutCreditingPreviousBidder
Exact-minimum bid passes + credits previous	testBidExactlyAtMinimumPassesAndCreditsPreviousBidder
Zero and 200% increment rules	testZeroAndHighIncrementRulesAreApplied
Emergency cannot drain credits/escrow	testEmergencyWithdrawCannotDrainBidCreditsOrActiveBidEscrow
Emergency only drains surplus	testEmergencyWithdrawOnlyWithdrawsAuctionLocalSurplus
Settlement replay blocked + escrow released	testWithBidSettlementIsIdempotentAndReleasesActiveEscrow
Withdraw-to-recipient with event check	testPreviousBidderWithdrawsCreditToChosenRecipient

All 10 tests map directly to the specified focus scenarios. One minor gap: there is no test for the zero-bid auction settlement path (the if (highestBid == 0) branch in claimAuction that transfers the NFT to retrieveExecutionAddress). Not a blocker for P0-AUCT-002 since that path has no ETH movement, but worth adding in a follow-up.

---

Overall: The implementation correctly eliminates the push-refund reentrancy/blocking vector, accounting invariants hold across all five paths, withdrawal is properly guarded, and emergency withdrawal cannot access protected funds. No material issues found within the stated P0-AUCT-002 scope.

[punk6529]

Thanks. On the zero-bid settlement note: that path is already covered by StreamDropsIntegrationCharacterizationTest.testNoBidAuctionSettlementTransfersToPoster, which remains in the full make check/Windows wrapper suite. The broader no-bid custody/state-machine target-state work remains tracked under ADR 0002 / P0-AUCT-001, so I am keeping this PR scoped to the bid/outbid payment fix.

[punk6529]

Autonomous merge evidence for PR #58:

• GitHub CI passed on latest head 91a92f6461355ea108252c49dbd5f7db5ffc4a34 in run 27264085501.
• Local validation passed before push: focused auction suite 10 tests, full make check 69 tests, Windows wrapper 69 tests, formatting, whitespace, Markdown heading scan, and Slither JSON delta (REENTRANCY_ETH_TOTAL=0, TARGET_PARTICIPATE_TO_AUCTION=0, AUCTION_ARBITRARY_SEND_ETH=0).
• Claude was explicitly requested in comment 4668219408 but skipped due to the organization overage limit.
• CodeRabbit initially failed because the head moved during the state-file push; it was explicitly re-requested in comment 4668232366.
• CodeRabbit then reviewed latest head 91a92f6, found no material issues within P0-AUCT-002 scope, confirmed no ETH call in the outbid path, consistent bidder-credit/escrow accounting, guarded withdrawal, and bounded emergency surplus. It noted zero-bid settlement as a follow-up, already covered by StreamDropsIntegrationCharacterizationTest.testNoBidAuctionSettlementTransfersToPoster and tracked under ADR 0002 / P0-AUCT-001.
• No inline review threads are open. PR is mergeable.

CodeRabbit's commit status remains pending despite the completed clean review comment and release-note update, matching the stale-status behavior already observed on PR #57. Proceeding with squash merge under the documented autonomous stale-status exception.