BorrowUtils Code Style Findings

BUS-01C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		BorrowUtils.sol:L65, L77, L160, L164

Description:

The linked mathematical operation is guaranteed to be performed safely by surrounding conditionals evaluated in either require checks or if-else constructs.

Example:

Copy
159if (owed > prevOwed) {
160    uint256 change = (owed.toAssetsUp() - prevOwed.toAssetsUp()).toUint();
161    emit Borrow(account, change);
162    DToken(dTokenAddress).emitTransfer(address(0), account, change);
163} else if (prevOwed > owed) {
164    uint256 change = (prevOwed.toAssetsUp() - owed.toAssetsUp()).toUint();
165
166    emit Repay(account, change);
167    DToken(dTokenAddress).emitTransfer(account, address(0), change);
168}

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statement to be wrapped in an unchecked code block thereby optimizing its execution cost.

Alleviation (fb2dd77a6f):

The optimization has been partially applied as only the example referenced in the exhibit has been optimized. We advise all referenced instances by the exhibit (i.e. the Location lines) to be addressed, optimizing the codebase further.

Alleviation (0f2192ac81):

The optimization proposed has been applied to the remaining instances, rendering this exhibit fully addressed.

BUS-02C: Inefficient Update of User's Debt

Type	Severity	Location
Gas Optimization		BorrowUtils.sol:L36, L46, L63

Description:

The BorrowUtils::increaseBorrow function will inefficiently adjust the vaultStorage.users[account] data location twice redundantly, once when synchronizing the user's borrow position and once when incrementing it.

Example:

Copy
29function updateUserBorrow(VaultCache memory vaultCache, address account)
30    private
31    returns (Owed newOwed, Owed prevOwed)
32{
33    prevOwed = vaultStorage.users[account].getOwed();
34    newOwed = getCurrentOwed(vaultCache, account, prevOwed);
35
36    vaultStorage.users[account].setOwed(newOwed);
37    vaultStorage.users[account].interestAccumulator = vaultCache.interestAccumulator;
38}
39
40function increaseBorrow(VaultCache memory vaultCache, address account, Assets assets) internal {
41    (Owed owed, Owed prevOwed) = updateUserBorrow(vaultCache, account);
42
43    Owed amount = assets.toOwed();
44    owed = owed + amount;
45
46    vaultStorage.users[account].setOwed(owed);
47    vaultStorage.totalBorrows = vaultCache.totalBorrows = vaultCache.totalBorrows + amount;
48
49    logBorrowChange(account, prevOwed, owed);
50}

Recommendation:

We advise the BorrowUtils::updateUserBorrow function to accept an Owed / uint256 argument that indicates whether the newOwed value should be incremented and by how much, optimizing the code's gas cost by one warm SSTORE operation.

A similar optimization can be applied for the BorrowUtils::decreaseBorrow function albeit with different operations based on an Assets instead of Owed argument.

Alleviation (fb2dd77a6ff9b7f710edb48e7eb5437e0db4fc1a):

The code has been optimized per our recommendation, splitting the BorrowUtils::updateUserBorrow function into the BorrowUtils::loadUserBorrow and BorrowUtils::setUserBorrow functions and thus permitting them to be invoked separately in an optimal way.

BUS-03C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		BorrowUtils.sol:L33, L36, L37

Description:

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

Example:

Copy
29function updateUserBorrow(VaultCache memory vaultCache, address account)
30    private
31    returns (Owed newOwed, Owed prevOwed)
32{
33    prevOwed = vaultStorage.users[account].getOwed();
34    newOwed = getCurrentOwed(vaultCache, account, prevOwed);
35
36    vaultStorage.users[account].setOwed(newOwed);
37    vaultStorage.users[account].interestAccumulator = vaultCache.interestAccumulator;
38}

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.

As the compiler's optimizations may take care of these caching operations automatically at-times, we advise the optimization to be selectively applied, tested, and then fully adopted to ensure that the proposed caching model indeed leads to a reduction in gas costs.

Alleviation (fb2dd77a6ff9b7f710edb48e7eb5437e0db4fc1a):

The exhibit's recommendation has been applied to the partial version of the original function under BorrowUtils::setUserBorrow, rendering the exhibit alleviated.