Cyfrin / solidity-by-example.github.io

Solidity By Example
https://solidity-by-example.org/
MIT License
614 stars 201 forks source link

A bug in NFT English Auction code example? #212

Closed tnkrxyz closed 2 years ago

tnkrxyz commented 2 years ago

In the English auction code, when a bidder calls bid(), it sends the bid amount (msg.value) to the contract address, but in the bid() function the bids array only adds value of the current highestBid (before highestBid gets set to msg.value in line 69). A bidder won't receive a full refund if it withdraws before it wins or gets outbidded by others.

A hypothetical example may help see the bug better: Assuming the auction contract gets initiated with _startingBid = 0, when the first bidder bidder1 bids 1 ether for the NFT, bids[bidder1] = 0 at line 65. If bidder1 withdraws its bid now, it will receive 0 refund from the contract address, even though it sends 1 ether to the contract when calling bid().

The fix is easy, by moving the 3 lines starting at Line 64 to currently Line 70. The winner should also have the bid amount subtract from amount withdrawable.

I created a pull request with fixes & re-generated page.

t4sk commented 2 years ago

I wrote the contract with the following restriction. Highest bidder cannot withdraw the current highest bid.

I don't think there is a bug. Am I wrong?

tnkrxyz commented 2 years ago

I realize you are right here. I will close the pull request.