Skip to content

Commit

Permalink
Merge pull request #475 from open-dollar/fix/sah-initial-bid-reverts-…
Browse files Browse the repository at this point in the history 
…if-zero

Fix: SAH `_initialBid` no longer gets refunded
  • Loading branch information
@daopunk
daopunk authored Feb 13, 2024
2 parents b28c382 + e7389fb commit df9f810
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 32 deletions.
22 changes: 18 additions & 4 deletions src/contracts/SurplusAuctionHouse.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,30 @@ contract SurplusAuctionHouse is Authorizable, Modifiable, Disableable, ISurplusA
if (_bid <= _auction.bidAmount) revert SAH_BidNotHigher();
if (_bid * WAD < _params.bidIncrease * _auction.bidAmount) revert SAH_InsufficientIncrease();

if (msg.sender != _auction.highBidder) {
// If there was no previous bid then no transfer is needed
if (_auction.bidAmount != 0) protocolToken.safeTransferFrom(msg.sender, _auction.highBidder, _auction.bidAmount);
// The amount that will be transferred to the auction house
uint256 _deltaBidAmount = _bid;

// Check if this is the first bid or not
if (_auction.bidExpiry != 0) {
// Since this is not the first bid, it might be that we need to repay the previous bidder
if (msg.sender != _auction.highBidder) {
protocolToken.safeTransferFrom(msg.sender, _auction.highBidder, _auction.bidAmount);

_auction.highBidder = msg.sender;
}
// Either we just repaid the previous bidder,
// or this user is also the previous bidder and is incrementing his bid
_deltaBidAmount -= _auction.bidAmount;
} else {
// This is the first bid
_auction.highBidder = msg.sender;
}
protocolToken.safeTransferFrom(msg.sender, address(this), _bid - _auction.bidAmount);

_auction.bidAmount = _bid;
_auction.bidExpiry = block.timestamp + _params.bidDuration;

protocolToken.safeTransferFrom(msg.sender, address(this), _deltaBidAmount);

emit IncreaseBidSize({
_id: _id,
_bidder: msg.sender,
Expand Down
30 changes: 26 additions & 4 deletions src/contracts/settlement/PostSettlementSurplusAuctionHouse.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,38 @@ contract PostSettlementSurplusAuctionHouse is Authorizable, Modifiable, IPostSet
if (_bid <= _auction.bidAmount) revert SAH_BidNotHigher();
if (_bid * WAD < _params.bidIncrease * _auction.bidAmount) revert SAH_InsufficientIncrease();

if (msg.sender != _auction.highBidder) {
protocolToken.safeTransferFrom(msg.sender, _auction.highBidder, _auction.bidAmount);
// The amount that will be transferred to the auction house
uint256 _deltaBidAmount = _bid;

// Check if this is the first bid or not
if (_auction.bidExpiry != 0) {
// Since this is not the first bid, it might be that we need to repay the previous bidder
if (msg.sender != _auction.highBidder) {
protocolToken.safeTransferFrom(msg.sender, _auction.highBidder, _auction.bidAmount);

_auction.highBidder = msg.sender;
}
// Either we just repaid the previous bidder,
// or this user is also the previous bidder and is incrementing his bid
_deltaBidAmount -= _auction.bidAmount;
} else {
// This is the first bid
_auction.highBidder = msg.sender;
}
protocolToken.safeTransferFrom(msg.sender, address(this), _bid - _auction.bidAmount);

_auction.bidAmount = _bid;
_auction.bidExpiry = block.timestamp + _params.bidDuration;

emit IncreaseBidSize(_id, msg.sender, block.timestamp, _bid, _auction.amountToSell, _auction.bidExpiry);
protocolToken.safeTransferFrom(msg.sender, address(this), _deltaBidAmount);

emit IncreaseBidSize({
_id: _id,
_bidder: msg.sender,
_blockTimestamp: block.timestamp,
_raisedAmount: _bid,
_soldAmount: _auction.amountToSell,
_bidExpiry: _auction.bidExpiry
});
}

/// @inheritdoc ICommonSurplusAuctionHouse
Expand Down
31 changes: 31 additions & 0 deletions test/testnet/single/SurplusAuctionHouse.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,37 @@ contract SingleBurningSurplusAuctionHouseTest is DSTest {
assertEq(protocolToken.balanceOf(ali), 0);
}

function test_increase_bid_with_initial_bid() public {
uint256 _initialBid = 0.1 ether;
protocolToken.approve(address(surplusAuctionHouse), _initialBid);

uint256 id = surplusAuctionHouse.startAuction({_amountToSell: 100 ether, _initialBid: _initialBid});
// amount to buy taken from creator
assertEq(safeEngine.coinBalance(address(this)), 900 ether);

GuyBurningSurplusAuction(ali).increaseBidSize(id, 1 ether);
// bid taken from bidder
assertEq(protocolToken.balanceOf(ali), 199 ether);
// payment remains in auction
assertEq(protocolToken.balanceOf(address(surplusAuctionHouse)), 1 ether);

GuyBurningSurplusAuction(bob).increaseBidSize(id, 2 ether);
// bid taken from bidder
assertEq(protocolToken.balanceOf(bob), 198 ether);
// prev bidder refunded
assertEq(protocolToken.balanceOf(ali), 200 ether);
// excess remains in auction
assertEq(protocolToken.balanceOf(address(surplusAuctionHouse)), 2 ether);

hevm.warp(block.timestamp + 5 weeks);
GuyBurningSurplusAuction(bob).settleAuction(id);
// high bidder gets the amount sold
assertEq(safeEngine.coinBalance(address(surplusAuctionHouse)), 0 ether);
assertEq(safeEngine.coinBalance(bob), 100 ether);
// income is burned
assertEq(protocolToken.balanceOf(address(surplusAuctionHouse)), 0 ether);
}

function test_increaseBidSize() public {
uint256 id = surplusAuctionHouse.startAuction({_amountToSell: 100 ether, _initialBid: 0});
// amount to buy taken from creator
Expand Down
46 changes: 31 additions & 15 deletions test/testnet/unit/PostSettlementSurplusAuctionHouse.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ contract Unit_PostSettlementSurplusAuctionHouse_IncreaseBidSize is Base {
_mockBidDuration(_bidDuration);
}

function test_Revert_HighBidderNotSet(SurplusAuction memory _auction, uint256 _amountToBuy, uint256 _bid) public {
function test_Revert_HighBidderNotSet(SurplusAuction memory _auction, uint256 _bid) public {
_auction.highBidder = address(0);

_mockValues(_auction, 0, 0);
Expand All @@ -407,7 +407,7 @@ contract Unit_PostSettlementSurplusAuctionHouse_IncreaseBidSize is Base {
postSettlementSurplusAuctionHouse.increaseBidSize(_auction.id, _bid);
}

function test_Revert_BidAlreadyExpired(SurplusAuction memory _auction, uint256 _amountToBuy, uint256 _bid) public {
function test_Revert_BidAlreadyExpired(SurplusAuction memory _auction, uint256 _bid) public {
vm.assume(_auction.highBidder != address(0));
vm.assume(_auction.bidExpiry != 0 && _auction.bidExpiry <= block.timestamp);

Expand All @@ -418,7 +418,7 @@ contract Unit_PostSettlementSurplusAuctionHouse_IncreaseBidSize is Base {
postSettlementSurplusAuctionHouse.increaseBidSize(_auction.id, _bid);
}

function test_Revert_AuctionAlreadyExpired(SurplusAuction memory _auction, uint256 _amountToBuy, uint256 _bid) public {
function test_Revert_AuctionAlreadyExpired(SurplusAuction memory _auction, uint256 _bid) public {
vm.assume(_auction.highBidder != address(0));
vm.assume(_auction.bidExpiry == 0 || _auction.bidExpiry > block.timestamp);
vm.assume(_auction.auctionDeadline <= block.timestamp);
Expand Down Expand Up @@ -459,12 +459,19 @@ contract Unit_PostSettlementSurplusAuctionHouse_IncreaseBidSize is Base {
postSettlementSurplusAuctionHouse.increaseBidSize(_auction.id, _bid);
}

function test_Call_ProtocolToken_Move_0(
function test_Call_ProtocolToken_Move_HighBidder(
SurplusAuction memory _auction,
uint256 _bid,
uint256 _bidIncrease,
uint256 _bidDuration
) public happyPath(_auction, _bid, _bidIncrease, _bidDuration) {
uint256 _payment = _bid;

// if the user was the previous high bidder they only need to pay the increment
if (_auction.bidExpiry != 0) {
_payment = _bid - _auction.bidAmount;
}

vm.expectCall(
address(mockProtocolToken),
abi.encodeCall(mockProtocolToken.transferFrom, (_auction.highBidder, _auction.highBidder, _auction.bidAmount)),
Expand All @@ -473,8 +480,7 @@ contract Unit_PostSettlementSurplusAuctionHouse_IncreaseBidSize is Base {
vm.expectCall(
address(mockProtocolToken),
abi.encodeCall(
mockProtocolToken.transferFrom,
(_auction.highBidder, address(postSettlementSurplusAuctionHouse), _bid - _auction.bidAmount)
mockProtocolToken.transferFrom, (_auction.highBidder, address(postSettlementSurplusAuctionHouse), _payment)
),
1
);
Expand All @@ -483,22 +489,32 @@ contract Unit_PostSettlementSurplusAuctionHouse_IncreaseBidSize is Base {
postSettlementSurplusAuctionHouse.increaseBidSize(_auction.id, _bid);
}

function test_Call_ProtocolToken_Move_1(
function test_Call_ProtocolToken_Move_NotHighBidder(
SurplusAuction memory _auction,
uint256 _bid,
uint256 _bidIncrease,
uint256 _bidDuration
) public happyPath(_auction, _bid, _bidIncrease, _bidDuration) {
uint256 _payment = _bid;
uint256 _refund;

// if the user was not the previous high bidder we refund the previous high bidder
if (_auction.bidExpiry != 0) {
_refund = _auction.bidAmount;
_payment = _bid - _auction.bidAmount;
}

// If there was no initial bidAmount then this would transfer 0 tokens, so it's not called
if (_refund != 0) {
vm.expectCall(
address(mockProtocolToken),
abi.encodeCall(mockProtocolToken.transferFrom, (user, _auction.highBidder, _refund)),
1
);
}
vm.expectCall(
address(mockProtocolToken),
abi.encodeCall(mockProtocolToken.transferFrom, (user, _auction.highBidder, _auction.bidAmount)),
1
);
vm.expectCall(
address(mockProtocolToken),
abi.encodeCall(
mockProtocolToken.transferFrom, (user, address(postSettlementSurplusAuctionHouse), _bid - _auction.bidAmount)
),
abi.encodeCall(mockProtocolToken.transferFrom, (user, address(postSettlementSurplusAuctionHouse), _payment)),
1
);

Expand Down
32 changes: 23 additions & 9 deletions test/testnet/unit/SurplusAuctionHouse.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,46 +558,60 @@ contract Unit_SurplusAuctionHouse_IncreaseBidSize is Base {
surplusAuctionHouse.increaseBidSize(_auction.id, _bid);
}

function test_Call_ProtocolToken_Move_0(
function test_Call_ProtocolToken_Move_HighBidder(
SurplusAuction memory _auction,
uint256 _bid,
uint256 _bidIncrease,
uint256 _bidDuration
) public happyPath(_auction, _bid, _bidIncrease, _bidDuration) {
uint256 _payment = _bid;

// if the user was the previous high bidder they only need to pay the increment
if (_auction.bidExpiry != 0) {
_payment = _bid - _auction.bidAmount;
}

vm.expectCall(
address(mockProtocolToken),
abi.encodeCall(mockProtocolToken.transferFrom, (_auction.highBidder, _auction.highBidder, _auction.bidAmount)),
0
);
vm.expectCall(
address(mockProtocolToken),
abi.encodeCall(
mockProtocolToken.transferFrom, (_auction.highBidder, address(surplusAuctionHouse), _bid - _auction.bidAmount)
),
abi.encodeCall(mockProtocolToken.transferFrom, (_auction.highBidder, address(surplusAuctionHouse), _payment)),
1
);

changePrank(_auction.highBidder);
surplusAuctionHouse.increaseBidSize(_auction.id, _bid);
}

function test_Call_ProtocolToken_Move_1(
function test_Call_ProtocolToken_Move_NotHighBidder(
SurplusAuction memory _auction,
uint256 _bid,
uint256 _bidIncrease,
uint256 _bidDuration
) public happyPath(_auction, _bid, _bidIncrease, _bidDuration) {
// If there was no initial bidAmount then this would transfer 0 tokens, so its not called
if (_auction.bidAmount != 0) {
uint256 _payment = _bid;
uint256 _refund;

// if the user was not the previous high bidder we refund the previous high bidder
if (_auction.bidExpiry != 0) {
_refund = _auction.bidAmount;
_payment = _bid - _auction.bidAmount;
}

// If there was no initial bidAmount then this would transfer 0 tokens, so it's not called
if (_refund != 0) {
vm.expectCall(
address(mockProtocolToken),
abi.encodeCall(mockProtocolToken.transferFrom, (user, _auction.highBidder, _auction.bidAmount)),
abi.encodeCall(mockProtocolToken.transferFrom, (user, _auction.highBidder, _refund)),
1
);
}
vm.expectCall(
address(mockProtocolToken),
abi.encodeCall(mockProtocolToken.transferFrom, (user, address(surplusAuctionHouse), _bid - _auction.bidAmount)),
abi.encodeCall(mockProtocolToken.transferFrom, (user, address(surplusAuctionHouse), _payment)),
1
);

Expand Down

0 comments on commit df9f810

Please sign in to comment.