salvorio / salvor-contracts

0 stars 0 forks source link

[SLG-03C] Inefficient `mapping` Lookups #3

Open HKskn opened 4 months ago

HKskn commented 4 months ago

SLG-03C: Inefficient mapping Lookups

Type Severity Location
Gas Optimization SalvorLending.sol:L153-L156, L329, L341, L345, L347-L351, L365, L379-L384, L393, L394, L397, L405, L414, L416, L422

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

/**
* @notice Extends an existing loan offer. This function is internal and checks that the loan has not been cancelled.
* @param _loanOffer The loan offer to be extended.
* @param signature The signature of the lender for validation.
* @param token The token associated with the NFT for the loan.
* @param tokenSignature The signature for the token, ensuring its authenticity.
*/
function extend(LibLending.LoanOffer memory _loanOffer, bytes memory signature, LibLending.Token memory token, bytes memory tokenSignature)
internal
isNotCancelled(LibLending.hashKey(_loanOffer))
{
    require(items[_loanOffer.nftContractAddress][token.tokenId].borrower == msg.sender, "there is no collateralized item belongs to msg.sender");

    validateLoanOffer(_loanOffer, token, tokenSignature);

    address lender = _validate(_loanOffer, signature);
    require(msg.sender != lender, "signer cannot borrow from own loan offer");

    uint256 fee = IAssetManager(assetManager).payLandingFee(lender, _loanOffer.amount);

    IAssetManager(assetManager).transferFrom(lender, msg.sender, _loanOffer.amount - fee);

    uint256 payment = _calculateRepayment(items[_loanOffer.nftContractAddress][token.tokenId].amount, items[_loanOffer.nftContractAddress][token.tokenId].rate);
    emit Extend(_loanOffer.nftContractAddress, token.tokenId, _loanOffer.salt, _loanOffer.amount, fee, payment);

    // payment sent to previous lender
    IAssetManager(assetManager).transferFrom(msg.sender, items[_loanOffer.nftContractAddress][token.tokenId].lender, payment);

    items[_loanOffer.nftContractAddress][token.tokenId].lender = lender;
    items[_loanOffer.nftContractAddress][token.tokenId].amount = _loanOffer.amount;
    items[_loanOffer.nftContractAddress][token.tokenId].duration = lendingPools[_loanOffer.nftContractAddress].duration;
    items[_loanOffer.nftContractAddress][token.tokenId].rate = lendingPools[_loanOffer.nftContractAddress].rate;
    items[_loanOffer.nftContractAddress][token.tokenId].startedAt = block.timestamp;
}

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

HKskn commented 4 months ago

Fixed