fix(billboard): remove unused Board.auctionId

thematters · Nov 16, 2023 · cf25116 · cf25116
1 parent 2c1660e
commit cf25116
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 27 deletions.
diff --git a/src/Billboard/Billboard.sol b/src/Billboard/Billboard.sol
@@ -46,7 +46,7 @@ contract Billboard is IBillboard {
     }
 
     modifier isFromBoardCreator(uint256 tokenId_) {
-        (address _boardCreator, , , , , , ) = registry.boards(tokenId_);
+        (address _boardCreator, , , , , ) = registry.boards(tokenId_);
         if (_boardCreator != msg.sender) {
             revert Unauthorized("creator");
         }
@@ -145,38 +145,42 @@ contract Billboard is IBillboard {
 
     /// @inheritdoc IBillboard
     function clearAuction(uint256 tokenId_) external {
-        (address _boardCreator, , , , , , ) = registry.boards(tokenId_);
+        // revert if board not found
+        (address _boardCreator, , , , , ) = registry.boards(tokenId_);
         if (_boardCreator == address(0)) revert BoardNotFound();
 
+        // revert if it's a new board
         uint256 _nextAuctionId = registry.nextBoardAuctionId(tokenId_);
         if (_nextAuctionId == 0) revert AuctionNotFound();
 
         IBillboardRegistry.Auction memory _nextAuction = registry.getAuction(tokenId_, _nextAuctionId);
 
+        // revert if auction is still running
+        if (block.timestamp < _nextAuction.endAt) revert AuctionNotEnded();
+
         // reclaim ownership to board creator if no auction
-        if (_nextAuction.tokenId == 0) {
-            registry.safeTransferByOperator(msg.sender, _boardCreator, tokenId_);
+        address _prevOwner = registry.ownerOf(tokenId_);
+        if (_nextAuction.tokenId == 0 && _prevOwner != _boardCreator) {
+            registry.safeTransferByOperator(_prevOwner, _boardCreator, tokenId_);
             return;
         }
 
-        if (block.timestamp < _nextAuction.endAt) revert AuctionNotEnded();
-
         IBillboardRegistry.Bid memory _highestBid = registry.getBid(
             tokenId_,
             _nextAuctionId,
             _nextAuction.highestBidder
         );
         if (_highestBid.price > 0) {
             // transfer bid price to board owner (previous tenant or creator)
-            registry.transferAmount(registry.ownerOf(tokenId_), _highestBid.price);
+            registry.transferAmount(_prevOwner, _highestBid.price);
 
             // transfer bid tax to board creator's tax treasury
             (, uint256 _taxAccumulated, uint256 _taxWithdrawn) = registry.taxTreasury(_boardCreator);
             registry.setTaxTreasury(_boardCreator, _taxAccumulated + _highestBid.tax, _taxWithdrawn);
         }
 
         // transfer ownership
-        registry.safeTransferByOperator(registry.ownerOf(tokenId_), _nextAuction.highestBidder, tokenId_);
+        registry.safeTransferByOperator(_prevOwner, _nextAuction.highestBidder, tokenId_);
 
         // mark highest bid as won
         registry.setBidWon(tokenId_, _nextAuctionId, _nextAuction.highestBidder, true);
@@ -185,14 +189,11 @@ contract Billboard is IBillboard {
         uint256 leaseStartAt = block.timestamp;
         uint256 leaseEndAt = block.timestamp + 14 days;
         registry.setAuctionLease(tokenId_, _nextAuctionId, leaseStartAt, leaseEndAt);
-
-        // update Board.auctionId
-        registry.setBoardAuctionId(tokenId_, _nextAuctionId);
     }
 
     /// @inheritdoc IBillboard
     function placeBid(uint256 tokenId_, uint256 amount_) external payable isFromWhitelist {
-        (address _boardCreator, , , , , , ) = registry.boards(tokenId_);
+        (address _boardCreator, , , , , ) = registry.boards(tokenId_);
         if (_boardCreator == address(0)) revert BoardNotFound();
 
         uint256 _nextAuctionId = registry.nextBoardAuctionId(tokenId_);
@@ -304,6 +305,7 @@ contract Billboard is IBillboard {
         uint256 amount = _bid.price + _bid.tax;
 
         if (_bid.isWithdrawn) revert WithdrawFailed();
+        if (_bid.isWon) revert WithdrawFailed();
         if (amount <= 0) revert WithdrawFailed();
 
         // transfer bid price and tax back to the bidder

diff --git a/src/Billboard/BillboardRegistry.sol b/src/Billboard/BillboardRegistry.sol
@@ -73,7 +73,6 @@ contract BillboardRegistry is IBillboardRegistry, ERC721 {
 
         Board memory _newBoard = Board({
             creator: to_,
-            auctionId: 0,
             name: "",
             description: "",
             location: "",
@@ -96,11 +95,6 @@ contract BillboardRegistry is IBillboardRegistry, ERC721 {
         board = boards[tokenId_];
     }
 
-    /// @inheritdoc IBillboardRegistry
-    function setBoardAuctionId(uint256 tokenId_, uint256 auctionId_) external isFromOperator {
-        boards[tokenId_].auctionId = auctionId_;
-    }
-
     /// @inheritdoc IBillboardRegistry
     function setBoardName(uint256 tokenId_, string calldata name_) external isFromOperator {
         boards[tokenId_].name = name_;

diff --git a/src/Billboard/IBillboardRegistry.sol b/src/Billboard/IBillboardRegistry.sol
@@ -32,7 +32,6 @@ interface IBillboardRegistry is IERC721 {
 
     struct Board {
         address creator;
-        uint256 auctionId; // last lease auction ID
         string name;
         string description;
         string location;
@@ -101,14 +100,6 @@ interface IBillboardRegistry is IERC721 {
      */
     function getBoard(uint256 tokenId_) external view returns (Board memory board);
 
-    /**
-     * @notice Set the auctionId of a board.
-     *
-     * @param tokenId_ Token ID of a board.
-     * @param auctionId_ Auction ID of an auction.
-     */
-    function setBoardAuctionId(uint256 tokenId_, uint256 auctionId_) external;
-
     /**
      * @notice Set the name of a board by board creator.
      *

diff --git a/src/test/Billboard/BillboardTest.t.sol b/src/test/Billboard/BillboardTest.t.sol
@@ -260,6 +260,14 @@ contract BillboardTest is BillboardTestBase {
         registry.transferFrom(USER_B, USER_C, _tokenId);
     }
 
+    function testTransferByOperator() public {
+        uint256 _tokenId = _mintBoard();
+
+        vm.startPrank(address(operator));
+        registry.safeTransferByOperator(ADMIN, USER_A, _tokenId);
+        assertEq(registry.ownerOf(_tokenId), USER_A);
+    }
+
     function testCannotTransferByAttacker() public {
         uint256 _tokenId = _mintBoard();