Codebase Optimization Report

Auditor's Disclaimer

While we try our best to maintain readability in the provided code snippets, some functions have been truncated to highlight the affected portions.

It's important to note that during the implementation of these suggested changes, developers must exercise caution to avoid introducing vulnerabilities. Although the optimizations have been tested prior to these recommendations, it is the responsibility of the developers to conduct thorough testing again.

Code reviews and additional testing are strongly advised to minimize any potential risks associated with the refactoring process.

Table Of Contents

• Codebase Optimization Report

  • Auditor's Disclaimer

  • Table Of Contents

  • Codebase impressions

  • Note on Gas estimates.

  • Tighly pack storage variables/optimize the order of variable declaration(Save 2 SLOTS: 4200 Gas)

  • Pack structs by putting data types that can fit together next to each other

    • auctionLength and interestRate can be packed with an address(We can save 2 SLOTS: 4000 Gas)

    • Due to how values are being constrained when assigning, we can save 2 SLOTS here

  • Expensive operations inside a for loop

    • Reading state variables in a loop might be too expensive if the array length is big

    • Cache feeReceiver outside the loop

    • Cache feeReceiver outside the loop

  • Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

  • Using storage instead of memory for structs/arrays saves gas

  • Multiple accesses of a mapping/array should use a local variable cache

    • Lender.sol.setPool(): Cache pools[poolId] in local storage

  • Lender.sol.buyLoan(): We can optimize this function by caching and using storage for some variables

  • x += y costs more gas than x = x + y for state variable

  • Using unchecked blocks to save gas

  • Optimizing check order for cost efficient function execution

    • Validate function parameters before making any state reads

    • No need to validate state variables if we might end up reverting on a cheaper function parameter

    • Reorder the checks to validate cheaper variables first

    • Validate function parameters before reading from state

  • Conclusion

Codebase impressions

The developers have done an excellent job of identifying and implementing some of the most evident optimizations like use of Immutables. The code is also well commented out which makes it easy for one to follow along.
However, we have identified additional areas where optimization is possible, some of which may not be immediately apparent.

Note on Gas estimates.

There seems to be an issue with how the command forge test --gas-report --optimize gives results. It seems to be giving varying results for the same tests cases. eg For some functions one run might give an average gas of 2000 while running it for the second time gives 1500 . For this reason giving the exact gas savings from the tests included wasn't feasible.
However for the slot packings the gas can be calculated based on how many slots are being saved.

Tighly pack storage variables/optimize the order of variable declaration(Save 2 SLOTS: 4200 Gas)

The EVM works with 32 byte words. Variables less than 32 bytes can be declared next to each other in storage and this will pack the values together into a single 32 byte storage slot (if the values combined are <= 32 bytes). If the variables packed together are retrieved together in functions we will effectively save ~2000 gas with every subsequent SLOAD for that storage slot. This is due to us incurring a Gwarmaccess (100 gas) versus a Gcoldsload (2100 gas).

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L63

Reducing the size of lenderFee should be safe given how values are assigned to it. see below
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L84-L87

When setting lenderFee we have a conditional check that ensures the value assigned to our state variable is always less than 5000. This means any size from uint16 should be big enough to hold our value

Similar thing with borrowerFee where constraint the value to less than 500

As the three state variables are being accessed together, packing would save us a lot of gas

Pack structs by putting data types that can fit together next to each other

As the solidity EVM works with 32 bytes, variables less than 32 bytes should be packed inside a struct so that they can be stored in the same slot, this saves gas when writing to storage ~20000 gas

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/utils/Structs.sol#L4-L23

auctionLength and interestRate can be packed with an address(We can save 2 SLOTS: 4000 Gas)

When setting up the pool, the values of auctionLength and interestRate are being constrained as shown below

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L132-L139

Note, p.auctionLength and p.interestRate cannot be greater than MAX_AUCTION_LENGTH and MAX_INTEREST_RATE respectively as this would trigger a revert

For MAX_AUCTION_LENGTH , it's a constant with a value of 3 days while MAX_INTEREST_RATE has a value of 100000 . See https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L59C49-L61

The biggest value here is 100000 , uint64 should be big enough

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/utils/Structs.sol#L34-L55

Due to how values are being constrained when assigning, we can save 2 SLOTS here

1 SLOT = 2k gas
Total Gas Saved: 4K Gas
We pack interestRate,startTimestamp and auctionLength in one slot We can actually pack more though. See explanation below.

Before we assign our struct values , we have some validations being done for the values that can be assigned
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L373-L375

The above means two things, loan.interestRate and loan.auctionLength would always be less or equal to pool.interestRate and pool.auctionLength respectively.
This constraint is being enforced whenever we write new values to this struct.

The struct above is being set when we give out a loan https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L355-L432
Of interest to us is the following piece of code

Note the value being assigned to loans[loanId].interestRate is the value pool.interestRate which comes from a previous struct POOL

The values pool.interestRate and pool.auctionLength have some max enforced. See previous struct packing finding for more explanation
'Note, p.auctionLength and p.interestRate cannot be greater than MAX_AUCTION_LENGTH and MAX_INTEREST_RATE respectively as this would trigger a revert'

This means the variables from the struct Loan need not to be bigger than those in struct Pool.
We can reduce the size to uint64 and still be safe.

For Timestamps uint64 should also be big enough:
For auctionStartTimestamp the value seems to be hard-coded as type(uint256).max so I avoided changing it. If not really required we can switch to uint64 and save 1 more SLOT

Refactoring the above means we need to do some other changes on the other parts of code that were referencing this variables.

Expensive operations inside a for loop

Reading state variables in a loop might be too expensive if the array length is big

**We also emit an event on every loop
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L233-L286

The above loop reads borrowerFee and feeReceiver repeatedly which is too expensive.

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L325

Cache feeReceiver outside the loop

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L403

Cache feeReceiver outside the loop

Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Staking.sol#L18-L24

To help the optimizer further, we declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array.

Using storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L116-L121

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L467

Multiple accesses of a mapping/array should use a local variable cache

Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times saves ~42 gas per access due to not having to perform the same offset calculation every time.
Help the Optimizer by saving a storage variable's reference instead of repeatedly fetching it

To help the optimizer,declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array.
As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex] and use it.

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L130-L176

Lender.sol.setPool(): Cache pools[poolId] in local storage

The gas benchmarks cannot be relied on as the included tests seems to give different values on every run

	Min	Average	Median	Max
Before	201208	219563	225608	225608
After	9324	212134	225478	225478

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L182-L192

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L198-L204

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L210-L215

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L221-L226

Lender.sol.buyLoan(): We can optimize this function by caching and using storage for some variables

To avoid stack too deep error, we introduce an internal function
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L465-L534
The gas benchmarks cannot be relied on as the included tests seems to give different values on every run

	Min	Average	Median	Max
Before	2107	10665	2582	35392
After	997	9665	1481	34712

x += y costs more gas than x = x + y for state variable

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Staking.sol#L41

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Staking.sol#L48

Using unchecked blocks to save gas

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block
see resource

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Staking.sol#L66

The operation _balance - balance cannot underflow due to the check on Line 65 that ensures that _balance is greater than balance for this operation to be performed

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L155

The above operation cannot underflow as it would only be performed if p.poolBalance is greater than currentBalance due to the condition check if (p.poolBalance > currentBalance) { on Line 150
It should be safe to wrap the whole external call

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L161

This is the else block of the previous check, The code would only be evaluated if p.poolBalance is less than currentBalance due to the check on Line 157

Optimizing check order for cost efficient function execution

Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas) in a function that may ultimately revert in the unhappy case.

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L182-L192

Validate function parameters before making any state reads

The first check in the above, reads a state variable pools[poolId] and compares it to msg.sender The second check only reads a function parameter comparing it against 0.
If the first check passes but we revert on the second case, the gas spent doing the state read on the first check would be wasted. As it's cheaper to read function parameter, the check should be done first.

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L198-L200

No need to validate state variables if we might end up reverting on a cheaper function parameter

Similar explanation to the previous case, we reorder the checks here to validate function parameters first

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L210-L212

Reorder the checks to validate cheaper variables first

maxLoanRatio is a function parameter therefore cheaper to read compared to the state read pools[poolId].lender. In case of a revert on the cheaper check, we want to minimize the gas spent, thus should validate the function parameter first

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L221-L223

Validate function parameters before reading from state

As the first check reads from storage,pools[poolId].lender , The second check is way cheaper as it only involves reading a function parameter and a constant value. If we end up reverting on the second check, the gas spent making the state read in the first check would be wasted. Reorder the checks to have the cheaper check first

Conclusion

It is important to emphasize that the provided recommendations aim to enhance the efficiency of the code without compromising its readability. We understand the value of maintainable and easily understandable code to both developers and auditors.

As you proceed with implementing the suggested optimizations, please exercise caution and be diligent in conducting thorough testing. It is crucial to ensure that the changes are not introducing any new vulnerabilities and that the desired performance improvements are achieved. Review code changes, and perform thorough testing to validate the effectiveness and security of the refactored code.

Should you have any questions or need further assistance, please don't hesitate to reach out.