[BBE-01C] Inefficient Creation of Buyer

[zajck]

BBE-01C: Inefficient Creation of Buyer

Type	Severity	Location
Gas Optimization		BuyerBase.sol:L90

Description:

The BuyerBase::getValidBuyer function will automatically create a buyer account if one does not exist for the _buyer, however, this is done so inefficiently as the active entry of the Buyer struct as well as the buyerIdByWallet mapping will be inefficiently evaluated.

Example:

/**
 * @notice Creates a Buyer.
 *
 * Emits a BuyerCreated event if successful.
 *
 * Reverts if:
 * - Wallet address is zero address
 * - Active is not true
 * - Wallet address is not unique to this buyer
 *
 * @param _buyer - the fully populated struct with buyer id set to 0x0
 */
function createBuyerInternal(Buyer memory _buyer) internal {
    //Check for zero address
    if (_buyer.wallet == address(0)) revert InvalidAddress();

    //Check active is not set to false
    if (!_buyer.active) revert MustBeActive();

    // Get the next account id and increment the counter
    uint256 buyerId = protocolCounters().nextAccountId++;

    //check that the wallet address is unique to one buyer id
    if (protocolLookups().buyerIdByWallet[_buyer.wallet] != 0) revert BuyerAddressMustBeUnique();

    _buyer.id = buyerId;
    storeBuyer(_buyer);

    //Notify watchers of state change
    emit BuyerCreated(_buyer.id, _buyer, msgSender());
}

/**
 * @notice Stores buyer struct in storage.
 *
 * @param _buyer - the fully populated struct with buyer id set
 */
function storeBuyer(Buyer memory _buyer) internal {
    // Get storage location for buyer
    (, Buyer storage buyer) = fetchBuyer(_buyer.id);

    // Set buyer props individually since memory structs can't be copied to storage
    buyer.id = _buyer.id;
    buyer.wallet = _buyer.wallet;
    buyer.active = _buyer.active;

    //Map the buyer's wallet address to the buyerId.
    protocolLookups().buyerIdByWallet[_buyer.wallet] = _buyer.id;
}

/**
 * @notice Checks if buyer exists for buyer address. If not, account is created for buyer address.
 *
 * Reverts if buyer exists but is inactive.
 *
 * @param _buyer - the buyer address to check
 * @return buyerId - the buyer id
 */
function getValidBuyer(address payable _buyer) internal returns (uint256 buyerId) {
    // Find or create the account associated with the specified buyer address
    bool exists;
    (exists, buyerId) = getBuyerIdByWallet(_buyer);

    if (exists) {
        // Fetch the existing buyer account
        (, Buyer storage buyer) = fetchBuyer(buyerId);

        // Make sure buyer account is active
        if (!buyer.active) revert MustBeActive();
    } else {
        // Create the buyer account
        Buyer memory newBuyer;
        newBuyer.wallet = _buyer;
        newBuyer.active = true;

        createBuyerInternal(newBuyer);
        buyerId = newBuyer.id;
    }
}

Recommendation:

We advise the code to mimic the statements of BuyerBase::createBuyerInternal, simply validating that the _buyer is a non-zero address. The optimization can also be achieved by relocating the "shared" statements between the two implementations to a common internal function, easing code maintenance while achieving the optimization described.