JPEG'd contest - FSchmoede's results

Gas optimization report

Keep revert strings below 32 bytes

Strings are stored in slots of 32 bytes, and hence the length of the revert string should be at max 32 bytes to fit inside 1 slot. If the string is just 33 bytes it will occupy 2 slots (64 bytes). Keeping the string size at 32 bytes or below will save gas on both deployment and when the revert condition is met.

Examples revert strings longer than 32 bytes:

JPEG.sol line 23:         "JPEG: must have minter role to mint"

StableCoin.sol line 41:   "StableCoin: must have minter role to mint"
StableCoin.sol line 55:   "StableCoin: must have pauser role to pause"
StableCoin.sol line 69:   "StableCoin: must have pauser role to unpause"

NFTVault.sol line 394:    "credit_rate_exceeds_or_equals_liquidation_rate"

State variable totalDebtAmount should be cached

The state variable totalDebtAmount is read 2 times from memory on line 585 and 590. By caching the variable into memory one of the expensive SLOAD operations can be saved.

The code

function _calculateAdditionalInterest() internal view returns (uint256) {
        // Number of seconds since {accrue} was called
        uint256 elapsedTime = block.timestamp - totalDebtAccruedAt;
        if (elapsedTime == 0) {
            return 0;
        }

        if (totalDebtAmount == 0) { // SLOAD1
            return 0;
        }

        // Accrue interest
        uint256 interestPerYear = (totalDebtAmount * // SLOAD2
            settings.debtInterestApr.numerator) /
            settings.debtInterestApr.denominator;
        uint256 interestPerSec = interestPerYear / 365 days;

        return elapsedTime * interestPerSec;
  }

Can be changed into

function _calculateAdditionalInterest() internal view returns (uint256) {
        // Number of seconds since {accrue} was called
        uint256 elapsedTime = block.timestamp - totalDebtAccruedAt;
        if (elapsedTime == 0) {
            return 0;
        }

				uint256 debtAmount = totalDebtAmount; // SLOAD		

        if (debtAmount == 0) { // MLOAD1
            return 0;
        }

        // Accrue interest
        uint256 interestPerYear = (debtAmount * // MLOAD2
            settings.debtInterestApr.numerator) /
            settings.debtInterestApr.denominator;
        uint256 interestPerSec = interestPerYear / 365 days;

        return elapsedTime * interestPerSec;
  }

This will instead use 1 SLOAD and 2 MLOAD operations, which is by far cheaper than 2 SLOAD operations. However, this will be a bit more expensive if the totalDebtAmount == 0 condition is met, since that will cost 1 SLOAD and 1 MLOAD compared to only 1 SLOAD. But I think the gas savings when the zero condition is not met weights up for this.

State variable position.borrowType should be cached

The state variable position.borrowType is read 3 times; 2 times in require statements (line 700 and 719) and in one if-statement (line 728). The variable should be cached to save 2 SLOAD operations.

The code

require(
    position.borrowType == BorrowType.NOT_CONFIRMED || // SLOAD1
        (position.borrowType == BorrowType.USE_INSURANCE && _useInsurance) ||
        (position.borrowType == BorrowType.NON_INSURANCE && !_useInsurance),
    "invalid_insurance_mode"
);

uint256 creditLimit = _getCreditLimit(_nftIndex);
uint256 debtAmount = _getDebtAmount(_nftIndex);
require(debtAmount + _amount <= creditLimit, "insufficient_credit");

//calculate the borrow fee
uint256 organizationFee = (_amount *
    settings.organizationFeeRate.numerator) /
    settings.organizationFeeRate.denominator;

uint256 feeAmount = organizationFee;
//if the position is insured, calculate the insurance fee
if (position.borrowType == BorrowType.USE_INSURANCE || _useInsurance) { // SLOAD2
    feeAmount +=
        (_amount * settings.insurancePurchaseRate.numerator) /
        settings.insurancePurchaseRate.denominator;
}
totalFeeCollected += feeAmount;
//subtract the fee from the amount borrowed
stablecoin.mint(msg.sender, _amount - feeAmount);

if (position.borrowType == BorrowType.NOT_CONFIRMED) { // SLOAD3
    position.borrowType = _useInsurance
        ? BorrowType.USE_INSURANCE
        : BorrowType.NON_INSURANCE;
}

Can be changed into

BorrowType borrowType = position.borrowType; // SLOAD1
require(
    borrowType == BorrowType.NOT_CONFIRMED || // MLOAD1
        (borrowType == BorrowType.USE_INSURANCE && _useInsurance) ||
        (borrowType == BorrowType.NON_INSURANCE && !_useInsurance),
    "invalid_insurance_mode"
);

uint256 creditLimit = _getCreditLimit(_nftIndex);
uint256 debtAmount = _getDebtAmount(_nftIndex);
require(debtAmount + _amount <= creditLimit, "insufficient_credit");

//calculate the borrow fee
uint256 organizationFee = (_amount *
    settings.organizationFeeRate.numerator) /
    settings.organizationFeeRate.denominator;

uint256 feeAmount = organizationFee;
//if the position is insured, calculate the insurance fee
if (borrowType == BorrowType.USE_INSURANCE || _useInsurance) { // MLOAD2
    feeAmount +=
        (_amount * settings.insurancePurchaseRate.numerator) /
        settings.insurancePurchaseRate.denominator;
}
totalFeeCollected += feeAmount;
//subtract the fee from the amount borrowed
stablecoin.mint(msg.sender, _amount - feeAmount);

if (borrowType == BorrowType.NOT_CONFIRMED) { // MLOAD3
    position.borrowType = _useInsurance
        ? BorrowType.USE_INSURANCE
        : BorrowType.NON_INSURANCE;
}

This will instead use 1 SLOAD and 3 MLOAD operations, which is by far cheaper than 3 SLOAD operations.