-
-
Notifications
You must be signed in to change notification settings - Fork 15
Open
Description
-
In a Vickrey auction, users first commit bids secretly.
-
Later, everyone reveals their bids.
-
During reveal, the contract tries to:
-
Refund losers -
Refund extra amount to the winner -
These refunds are sent immediately.
-
If One user’s address refuses to accept tokens.
-
Because of that, everyone else’s reveal fails.
-
Honest users are now stuck.
The revealBid() function performs external token transfers during critical auction state updates. Any failure in these transfers causes the full transaction to revert.
(Before Fix)
if (highestBid < bidAmount) {
if (highestBid > 0 && auction.winner != msg.sender && auction.winner != auction.auctioneer) {
//Immediate refund can revert and block reveal
sendERC20(auction.biddingToken, auction.winner, highestBid);
}
auction.availableFunds = highestBid;
}
else if (bidAmount > auction.winningBid) {
sendERC20(auction.biddingToken, msg.sender, bidAmount); // refund caller
}
else {
sendERC20(auction.biddingToken, msg.sender, bidAmount); // refund caller
}Fix
Adopt a pull-based refund mechanism:
- Store refunds in contract state
- Allow users to withdraw refunds separately
- Avoid external calls during reveal logic
Fix Implementation
1. New State Variable
// user => token => amount
mapping(address => mapping(address => uint256)) public pendingReturns;2. Updated revealBid() Logic
if (highestBid < bidAmount) {
if (highestBid > 0 && auction.winner != msg.sender && auction.winner != auction.auctioneer) {
// Store refund instead of sending
pendingReturns[auction.winner][auction.biddingToken] += highestBid;
}
auction.availableFunds = highestBid;
}
else if (bidAmount > auction.winningBid) {
// Store refund for caller
pendingReturns[msg.sender][auction.biddingToken] += bidAmount;
}
else {
// Store refund for caller
pendingReturns[msg.sender][auction.biddingToken] += bidAmount;
}3. New withdrawRefund() Function
function withdrawRefund(address tokenAddress) external {
uint256 amount = pendingReturns[msg.sender][tokenAddress];
require(amount > 0, "No funds to withdraw");
// Clear state before transfer
pendingReturns[msg.sender][tokenAddress] = 0;
sendERC20(tokenAddress, msg.sender, amount);
}Why This Fix Works
- Reveal phase no longer depends on external token transfers
- One bidder cannot affect others
- Auction can always complete
- Refund failures are isolated to individual users
@kaneki003 What's your view on this , Please feel free to assign
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels