Gas: Merge callbacks to Unlock on purchase

[code423n4]

Handle

HardlyDifficult

Vulnerability details

Impact

Save 1.7k gas on a key purchase when there's no discount, and 3.2k gas when discounts are added. A minor improvement, but this also seems easier to follow in trace logs or with a tool like Tenderly.

Proof of Concept

In the PublicLock purchase remove the calls to _purchasePriceFor and recordKeyPurchase and replace it with uint inMemoryKeyPrice = _purchase(_recipient, _referrer, _data); which was implemented as:

 function _purchase(
    address _recipient,
    address _referrer,
    bytes memory _data
  ) internal
    returns (uint minKeyPrice)
  {
    if(address(onKeyPurchaseHook) != address(0))
    {
      minKeyPrice = onKeyPurchaseHook.keyPurchasePrice(msg.sender, _recipient, _referrer, _data);
    }
    else
    {
      minKeyPrice = keyPrice;
    }

    return unlockProtocol.recordKeyPurchaseAndGetDiscount(_recipient, minKeyPrice, _referrer);
  }

Then in Unlock, add a new function:

  function recordKeyPurchaseAndGetDiscount(
    address recipient,
    uint minKeyPrice,
    address referrer
  ) public returns (uint inMemoryKeyPrice) {
    if(minKeyPrice > 0)
    {
      (uint unlockDiscount, uint unlockTokens) = computeAvailableDiscountFor(recipient, minKeyPrice);
      inMemoryKeyPrice = minKeyPrice;
      if (unlockDiscount > 0)
      {
        // INVALID_DISCOUNT_FROM_UNLOCK error assumed from safe math
        inMemoryKeyPrice -= unlockDiscount;
        recordConsumedDiscount(unlockDiscount, unlockTokens);
      }
    }

    // Record price without any tips
    recordKeyPurchase(inMemoryKeyPrice, referrer);
  }

This approach clones the current behavior, but moves logic into Unlock instead of going back and forth between PublicLock and Unlock.

Tools Used

yarn test and diff the gas reporter table.

Recommended Mitigation Steps

[0xleastwood]

I'll let @julien51 comment on this one.

[julien51]

Makes sense!