feat(billboard): refund overpaid on placeBid

thematters · Nov 22, 2023 · 58ff978 · 58ff978
1 parent 4d13fcb
commit 58ff978
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 23 deletions.
diff --git a/.gas-snapshot b/.gas-snapshot
@@ -12,15 +12,15 @@ ACLManagerTest:testTransferRole() (gas: 21528)
 BillboardTest:testAddToWhitelist() (gas: 37205)
 BillboardTest:testApproveAndTransfer() (gas: 162759)
 BillboardTest:testCalculateTax() (gas: 29439)
-BillboardTest:testCannnotWithdrawTaxIfSmallAmount(uint8) (runs: 256, μ: 420946, ~: 434258)
-BillboardTest:testCannnotWithdrawTaxIfZero() (gas: 386461)
+BillboardTest:testCannnotWithdrawTaxIfSmallAmount(uint8) (runs: 256, μ: 421050, ~: 434362)
+BillboardTest:testCannnotWithdrawTaxIfZero() (gas: 386565)
 BillboardTest:testCannotAddToWhitelistByAttacker() (gas: 11794)
 BillboardTest:testCannotApproveByAttacker() (gas: 130412)
-BillboardTest:testCannotClearAuctionIfAuctionNotEnded() (gas: 604409)
+BillboardTest:testCannotClearAuctionIfAuctionNotEnded() (gas: 604617)
 BillboardTest:testCannotClearAuctionOnNewBoard() (gas: 136255)
 BillboardTest:testCannotMintBoardByAttacker() (gas: 13993)
 BillboardTest:testCannotPlaceBidByAttacker() (gas: 142428)
-BillboardTest:testCannotPlaceBidTwice(uint96) (runs: 256, μ: 650773, ~: 657004)
+BillboardTest:testCannotPlaceBidTwice(uint96) (runs: 256, μ: 650981, ~: 657212)
 BillboardTest:testCannotRemoveToWhitelistByAttacker() (gas: 11861)
 BillboardTest:testCannotSafeTransferByAttacker() (gas: 128214)
 BillboardTest:testCannotSetBoardProprtiesByAttacker() (gas: 159548)
@@ -29,34 +29,34 @@ BillboardTest:testCannotSetTaxRateByAttacker() (gas: 11763)
 BillboardTest:testCannotTransferByOperator() (gas: 132890)
 BillboardTest:testCannotTransferToZeroAddress() (gas: 128377)
 BillboardTest:testCannotUpgradeRegistryByAttacker() (gas: 11885)
-BillboardTest:testCannotWithBidTwice(uint96) (runs: 256, μ: 934384, ~: 934384)
-BillboardTest:testCannotWithdrawBidIfAuctionNotCleared(uint96) (runs: 256, μ: 770975, ~: 770975)
-BillboardTest:testCannotWithdrawBidIfAuctionNotEnded(uint96) (runs: 256, μ: 646138, ~: 646138)
-BillboardTest:testCannotWithdrawBidIfNotFound() (gas: 419189)
-BillboardTest:testCannotWithdrawBidIfWon(uint96) (runs: 256, μ: 740936, ~: 740936)
+BillboardTest:testCannotWithBidTwice(uint96) (runs: 256, μ: 934696, ~: 934696)
+BillboardTest:testCannotWithdrawBidIfAuctionNotCleared(uint96) (runs: 256, μ: 771287, ~: 771287)
+BillboardTest:testCannotWithdrawBidIfAuctionNotEnded(uint96) (runs: 256, μ: 646346, ~: 646346)
+BillboardTest:testCannotWithdrawBidIfNotFound() (gas: 419293)
+BillboardTest:testCannotWithdrawBidIfWon(uint96) (runs: 256, μ: 741144, ~: 741144)
 BillboardTest:testCannotWithdrawTaxByAttacker() (gas: 21710)
-BillboardTest:testClearAuctionIfAuctionEnded() (gas: 649435)
-BillboardTest:testClearAuctionsIfAuctionEnded() (gas: 1205928)
-BillboardTest:testGetBids(uint8,uint8,uint8) (runs: 256, μ: 2877775, ~: 1456013)
+BillboardTest:testClearAuctionIfAuctionEnded() (gas: 649643)
+BillboardTest:testClearAuctionsIfAuctionEnded() (gas: 1206344)
+BillboardTest:testGetBids(uint8,uint8,uint8) (runs: 256, μ: 3055023, ~: 1465106)
 BillboardTest:testGetTokenURI() (gas: 155305)
 BillboardTest:testMintBoard() (gas: 225991)
 BillboardTest:testMintBoardByWhitelist() (gas: 157825)
 BillboardTest:testMintBoardIfOpened() (gas: 130863)
-BillboardTest:testPlaceBidByWhitelist() (gas: 469968)
-BillboardTest:testPlaceBidIfAuctionEnded() (gas: 936799)
-BillboardTest:testPlaceBidOnNewBoard(uint96) (runs: 256, μ: 486781, ~: 497500)
-BillboardTest:testPlaceBidWithHigherPrice(uint96) (runs: 256, μ: 766505, ~: 773735)
-BillboardTest:testPlaceBidWithSamePrices(uint96) (runs: 256, μ: 764587, ~: 776263)
-BillboardTest:testPlaceBidZeroPrice() (gas: 357676)
+BillboardTest:testPlaceBidByWhitelist() (gas: 470072)
+BillboardTest:testPlaceBidIfAuctionEnded() (gas: 937111)
+BillboardTest:testPlaceBidOnNewBoard(uint96) (runs: 256, μ: 519199, ~: 529681)
+BillboardTest:testPlaceBidWithHigherPrice(uint96) (runs: 256, μ: 766817, ~: 774047)
+BillboardTest:testPlaceBidWithSamePrices(uint96) (runs: 256, μ: 764899, ~: 776575)
+BillboardTest:testPlaceBidZeroPrice() (gas: 357780)
 BillboardTest:testRemoveToWhitelist() (gas: 24939)
 BillboardTest:testSafeTransferByOperator() (gas: 141290)
 BillboardTest:testSetBoardProperties() (gas: 305548)
 BillboardTest:testSetBoardPropertiesAfterTransfer() (gas: 338157)
 BillboardTest:testSetIsOpened() (gas: 15978)
 BillboardTest:testSetTaxRate() (gas: 27263)
-BillboardTest:testUpgradeRegistry() (gas: 2637423)
-BillboardTest:testWithdrawBid(uint96) (runs: 256, μ: 935022, ~: 935022)
-BillboardTest:testWithdrawTax(uint96) (runs: 256, μ: 509481, ~: 509481)
+BillboardTest:testUpgradeRegistry() (gas: 2662891)
+BillboardTest:testWithdrawBid(uint96) (runs: 256, μ: 935334, ~: 935334)
+BillboardTest:testWithdrawTax(uint96) (runs: 256, μ: 509585, ~: 509585)
 CurationTest:testCannotCurateERC20CurateZeroAmount() (gas: 12194)
 CurationTest:testCannotCurateERC20EmptyURI() (gas: 15797)
 CurationTest:testCannotCurateERC20IfNotApproval() (gas: 21624)

diff --git a/src/Billboard/Billboard.sol b/src/Billboard/Billboard.sol
@@ -259,10 +259,20 @@ contract Billboard is IBillboard {
     }
 
     function _lockBidPriceAndTax(uint256 amount_) private {
+        // transfer bid price and tax to the registry
         (bool _success, ) = address(registry).call{value: amount_}("");
         if (!_success) {
             revert TransferFailed();
         }
+
+        // refund if overpaid
+        uint256 _overpaid = msg.value - amount_;
+        if (_overpaid > 0) {
+            (bool _refundSuccess, ) = msg.sender.call{value: _overpaid}("");
+            if (!_refundSuccess) {
+                revert TransferFailed();
+            }
+        }
     }
 
     /// @inheritdoc IBillboard

diff --git a/src/test/Billboard/BillboardTest.t.sol b/src/test/Billboard/BillboardTest.t.sol
@@ -354,8 +354,9 @@ contract BillboardTest is BillboardTestBase {
 
         uint256 _tokenId = _mintBoard();
         uint256 _tax = operator.calculateTax(_amount);
+        uint256 _overpaid = 0.1 ether;
         uint256 _total = _amount + _tax;
-        vm.deal(USER_A, _total);
+        vm.deal(USER_A, _total + _overpaid);
 
         uint256 _prevNextActionId = registry.nextBoardAuctionId(_tokenId);
         uint256 _prevCreatorBalance = ADMIN.balance;
@@ -384,7 +385,7 @@ contract BillboardTest is BillboardTestBase {
         );
 
         vm.prank(USER_A);
-        operator.placeBid{value: _total}(_tokenId, _amount);
+        operator.placeBid{value: _total + _overpaid}(_tokenId, _amount);
 
         // check balances
         assertEq(ADMIN.balance, _prevCreatorBalance + _amount);