PunkBought event is emitted from "acceptBidForPunk" with 0x0 for "toAddress" and 0 for "value"

[serejke]

Problem

PunkBought event emitted from acceptBidForPunk always has toAddress = 0 and value = 0.

See an example transaction on Etherscan

Investigation

At line 216

Bid bid = punkBids[punkIndex];`

bid is set to reference the map's value

and at line 277

punkBids[punkIndex] = Bid(false, punkIndex, 0x0, 0);`

the map's value is overridden with "null" Bid having 0x0 for bid.bidder and 0 for value,

then at line 229

PunkBought(punkIndex, bid.value, seller, bid.bidder);

bid.bidder = 0x0 and bid.value = 0

211.   function acceptBidForPunk(uint punkIndex, uint minPrice) {
212.        if (punkIndex >= 10000) throw;
213.        if (!allPunksAssigned) throw;                
214.        if (punkIndexToAddress[punkIndex] != msg.sender) throw;
215.        address seller = msg.sender;
216.        Bid bid = punkBids[punkIndex];
217.        if (bid.value == 0) throw;
218.        if (bid.value < minPrice) throw;
219.
220.        punkIndexToAddress[punkIndex] = bid.bidder;
221.        balanceOf[seller]--;
222.        balanceOf[bid.bidder]++;
223.        Transfer(seller, bid.bidder, 1);
224.
225.        punksOfferedForSale[punkIndex] = Offer(false, punkIndex, bid.bidder, 0, 0x0);
226.        uint amount = bid.value;
227.        punkBids[punkIndex] = Bid(false, punkIndex, 0x0, 0);
228.        pendingWithdrawals[seller] += amount;
229.        PunkBought(punkIndex, bid.value, seller, bid.bidder);
230.    }

Who is affected

DApps listening for CryptoPunk's events and willing to know history of punk's transfers with prices have to implement non-trivial workarounds to extract real values for toAddress and value:

• toAddress value can be obtained from the event Transfer emitted at line 223, at which point the bid.bidder was not yet overridden
• value can not be extracted precisely, since the Transfer emits 1 as the value.
  I see 2 options how a DApp can obtain the value:
  • listen for enterBidForPunk events for the same bidder and record the latest bid's value
  • call punkBids[punkIndex] right before the transaction for acceptBidForPunk is executed

All in all, this seems to be a minor bug not affecting correctness of the CryptoPunks contract but slightly complicating implementation for DApps.

[nerder]

That's a very nice analysis indeed. What will be the approach to fix this?

[davnex]

Problem

PunkBought event emitted from acceptBidForPunk always has toAddress = 0 and value = 0.

See an example transaction on Etherscan

Investigation

At line 216

Bid bid = punkBids[punkIndex];`

bid is set to reference the map's value

and at line 277

punkBids[punkIndex] = Bid(false, punkIndex, 0x0, 0);`

the map's value is overridden with "null" Bid having 0x0 for bid.bidder and 0 for value,

then at line 229

PunkBought(punkIndex, bid.value, seller, bid.bidder);

bid.bidder = 0x0 and bid.value = 0

211.   function acceptBidForPunk(uint punkIndex, uint minPrice) {
212.        if (punkIndex >= 10000) throw;
213.        if (!allPunksAssigned) throw;                
214.        if (punkIndexToAddress[punkIndex] != msg.sender) throw;
215.        address seller = msg.sender;
216.        Bid bid = punkBids[punkIndex];
217.        if (bid.value == 0) throw;
218.        if (bid.value < minPrice) throw;
219.
220.        punkIndexToAddress[punkIndex] = bid.bidder;
221.        balanceOf[seller]--;
222.        balanceOf[bid.bidder]++;
223.        Transfer(seller, bid.bidder, 1);
224.
225.        punksOfferedForSale[punkIndex] = Offer(false, punkIndex, bid.bidder, 0, 0x0);
226.        uint amount = bid.value;
227.        punkBids[punkIndex] = Bid(false, punkIndex, 0x0, 0);
228.        pendingWithdrawals[seller] += amount;
229.        PunkBought(punkIndex, bid.value, seller, bid.bidder);
230.    }

Who is affected

DApps listening for CryptoPunk's events and willing to know history of punk's transfers with prices have to implement non-trivial workarounds to extract real values for toAddress and value:

• toAddress value can be obtained from the event Transfer emitted at line 223, at which point the bid.bidder was not yet overridden

• value can not be extracted precisely, since the Transfer emits 1 as the value.
  I see 2 options how a DApp can obtain the value:

  • listen for enterBidForPunk events for the same bidder and record the latest bid's value
  • call punkBids[punkIndex] right before the transaction for acceptBidForPunk is executed

All in all, this seems to be a minor bug not affecting correctness of the CryptoPunks contract but slightly complicating implementation for DApps.