Emitting AuctionSettled event during placeBid misrepresents active auctions as settled, confusing users and off-chain systems

Description

The BidBeastsNFTMarketPlace::AuctionSettled event is intended to signal the final settlement of an auction, indicating the winner, seller, and final price. However, it is incorrectly emitted in the BidBeastsNFTMarketPlace::placeBid function during regular bidding (not buy-now scenarios), despite the auction remaining active. This misrepresents the auction’s status, potentially confusing users and disrupting off-chain applications that rely on event logs.

Risk

Likelihood: High

• Occurs for every regular bid placed in placeBid, excluding buy-now scenarios.

Impact: Low

• User Confusion: Users monitoring events think the auction ended when it didn't

• Integration Issues: Off-chain applications (e.g., frontend, indexers) may incorrectly process auctions as settled, disrupting status displays or bidding interfaces.

Proof of Concept

• To capture event emissions, import the Vm module in BidBeastsMarketPlaceTest.t.sol:

  import {Test, console, Vm} from "forge-std/Test.sol";
  
  

• Next, add the following test to BidBeastsMarketPlaceTest.t.sol:

  function test_PrematureAuctionSettledEmission() public {

  _mintNFT();

  _listNFT();

  // Place bid - this will incorrectly emit AuctionSettled

  vm.recordLogs();

  vm.prank(BIDDER_1);

  market.placeBid{value: BID_AMOUNT}(TOKEN_ID);

  Vm.Log[] memory logs = vm.getRecordedLogs();

  // Count AuctionSettled events (should be 0, but will be 1)

  uint256 settledEventCount = 0;

  for(uint i = 0; i < logs.length; i++) {

  if(logs[i].topics[0] == keccak256("AuctionSettled(uint256,address,address,uint256)")) {

  settledEventCount++;

  }

  }

  ​

  console.log("AuctionSettled emitted during bidding:", settledEventCount);

  console.log("Auction still active?", market.getListing(TOKEN_ID).listed);

  // Now actually settle the auction

  vm.warp(block.timestamp + 16 minutes);

  vm.recordLogs();

  vm.prank(BIDDER_1);

  market.settleAuction(TOKEN_ID); // Emits `AuctionSettled` in `_executeSale()`

  logs = vm.getRecordedLogs();

  settledEventCount = 0;

  for(uint i = 0; i < logs.length; i++) {

  if(logs[i].topics[0] == keccak256("AuctionSettled(uint256,address,address,uint256)")) {

  settledEventCount++;

  }

  }

  console.log("AuctionSettled emitted after settlement:", settledEventCount);

  console.log("Number of times 'AuctionSettled' got emitted: 2 (should be 1)");

  }
  
  

• Finally, run it using the command:

  forge test --mt test_PrematureAuctionSettledEmission -vv
  
  

• The output we get:

  Ran 1 test for test/BidBeastsMarketPlaceTest.t.sol:BidBeastsNFTMarketTest

  [PASS] test_PrematureAuctionSettledEmission() (gas: 330618)

  Logs:

  AuctionSettled event emitted during bidding: 1

  Auction still active? true

  AuctionSettled event emitted after settlement: 1

  Number of times 'AuctionSettled' event got emitted: 2 (should be 1)

  ​

  Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.18ms (335.80µs CPU time)

Recommended Mitigation

Simple, and straightforward. Just remove the AuctionSettled event from placeBid function, as it doesn't belong there.