Beedle - Oracle free perpetual lending

Summary

In the reviewed contract, several numerical constants, such as 5000, 500, 10 ** 18, and 10000, are used repeatedly without clear definition or context. This practice can reduce code clarity and maintainability, potentially leading to errors in future modifications. While this issue doesn't pose a direct security threat, it's recommended to replace these constants with descriptively named variables or constants to enhance code readability and reduce the potential for future inconsistencies.

Vulnerability Details

The contract frequently employs unnamed numerical constants. These constants can reduce the clarity of the code, making it harder for developers to understand the purpose or significance of these numbers, and potentially leading to errors during future modifications.

Repeated Use of Unnamed Numerical Constants

The value 10000 is used as a denominator in multiple calculations, such as:

• In the borrow() function

• In the seizeLoan() function

• In the refinance() function

• In the _calculateInterest() function

The value 10 ** 18 is employed to adjust ratios, as seen in:

• In the borrow() function

• In the giveLoan() function

• In the refinance() function

The following numerical values, while not repeated, would benefit from being defined as constants for clarity and maintainability:

• Value 5000 found in setLenderFee() function:
  

• Value 500 observed in setBorrowerFee() function:
  

Impact

The absence of constants in the contract hampers readability, making code maintenance and reviews more challenging. It increases the risk of inconsistencies during updates, as hardcoded values can lead to overlooked errors. Changing a value without constants necessitates multiple codebase modifications, elevating the chance of mistakes. Although minimal, there can be slight gas inefficiencies due to repeated literals. Deviating from Ethereum's best practices, which advocate for constants, can raise standardization concerns. Lastly, the lack of named constants reduces transparency for users trying to understand the contract's behavior.

Tools Used

VSCode, Slither

Recommendations

1. Value: 10000

Recommendation: Define a constant named PERCENTAGE_DENOMINATOR or INTEREST_DENOMINATOR. This indicates that the value is used for percentage-based calculations, especially in the context of interest and fee rates.

2. Value: 5000

Recommendation: Define a constant named HALF_BASIS_POINTS or HALF_BP_DENOMINATOR to indicate that this value represents half of the total basis points.

3. Value: 500

Recommendation: Define a constant named DEFAULT_FEE_RATE or BASE_FEE_RATE. This signifies its primary use in fee calculations, especially if it's a standard or base fee rate in the system.

4. Value: 10 ** 18

Recommendation: Define a constant named TOKEN_DECIMAL_MULTIPLIER or LOAN_DECIMALS. Given that the code deals with ERC20 tokens and loans, this name indicates that the value is used to account for the decimal places in token amounts.

For each of these constants, the recommendation is to place them at the beginning of the contract, preferably in a section dedicated to constants. This will ensure that they are easily accessible and can be modified without having to search through the entire contract.