seen-haus / seen-contracts

Seen Haus contract suite
GNU General Public License v3.0
8 stars 2 forks source link

ARF-01C: Assignment Simplification #21

Closed JayWelsh closed 2 years ago

JayWelsh commented 2 years ago

ARF-01C: Assignment Simplification

Type Severity Location
Gas Optimization Informational AuctionRunnerFacet.sol:L161

Description:

The linked assignment essentially calculates the value of block.timestamp + extensionWindow.

Example:

// Should not apply to first bid
// For bids placed within the extension window
// Extend the duration so that auction still lasts for the length of the extension window
if ((block.timestamp + extensionWindow) >= endTime) {
    auction.duration += (extensionWindow - (endTime - block.timestamp));
    emit AuctionExtended(_consignmentId);
}

Recommendation:

We advise that value to be directly assigned to auction.duration instead to increase the legibility of the codebase and reduce its gas cost.

JayWelsh commented 2 years ago

Appeal:

As per the following:

"The linked assignment essentially calculates the value of block.timestamp + extensionWindow."

This seems to not be the case, while the "end time" of the Auction would indeed be block.timestamp + extensionWindow, the duration of the auction is not the same as the "end time", in this case, we are calculating the new duration (from start to finish) of the auction. If there is a simpler way to calculate this then we can make any required change but setting auction.duration to block.timestamp + extensionWindow would cause an issue.

My apology if perhaps I am making a silly error here.

JayWelsh commented 2 years ago

To put the current logic into words:

Knowing that we need to extent the auction duration because of the if ((block.timestamp + extensionWindow) >= endTime) {, and knowing that we can't straight up add the extension window to the duration as we want the remaining duration to become the duration of extensionWindow (e.g. assuming a 15 minute extension window, a bid coming in with 10 minutes remaining should set the remaining time to 15 minutes, not to 10 + 15 = 25 minutes):

  1. We take the original duration of the auction
  2. Calculate how much of the extension window needs to be added to the duration in order to get the remaining time to be equal to the extension window (e.g. with an extension window of 15 minutes and 5 minutes remaining, we need to add 10 minutes to the total duration).
  3. The way this is done is by figuring out the remaining time left (endTime - block.timestamp) and then subtracting this from the extensionWindow, this should be safe to do because the if statement wrapping the relevant logic should ensure that endTime - block.timestamp is never larger than extensionWindow (which could cause an underflow if that assumption is incorrect). We should also be able to assume that block.timestamp will never be larger than endTime because there is a require(block.timestamp <= endTime, "Auction timer has elapsed"); check above this logic, higher up in the bid function.