Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Non-critical/Informational Report

Rolezn

Non-critical Issues

Issue Contexts
NC‑1 Array lengths not checked 1
NC‑2 Avoid Floating Pragmas: The Version Should Be Locked 9
NC‑3 Condition will not revert when block.timestamp is == to the compared variable 1
NC‑4 Consider adding a deny-list 1
NC‑5 Consider disabling renounceOwnership() 7
NC‑6 Consider using named mappings 4
NC‑7 Constants Should Be Defined Rather Than Using Magic Numbers 2
NC‑8 Constants in comparisons should appear on the left side 2
NC‑9 Contract does not follow the Solidity style guide's suggested layout ordering 1
NC‑10 Critical Changes Should Use Two-step Procedure 4
NC‑11 Custom error has no error details 15
NC‑12 Declare interfaces on separate files 1
NC‑13 Empty bytes check is missing 8
NC‑14 File is missing NatSpec 5
NC‑15 Function names should differ to make the code more readable 2
NC‑16 Function must not be longer than 50 lines 4
NC‑17 Functions that alter state should emit events 3
NC‑18 Use delete to Clear Variables 1
NC‑19 No need to initialize uints to zero 2
NC‑20 Initial value check is missing in Set Functions 3
NC‑21 Interfaces should be indicated with an I prefix in the contract name 1
NC‑22 internal functions not called by the contract should be removed 2
NC‑23 Long functions should be refactored into multiple, smaller, functions 2
NC‑24 NatSpec comments should be increased in contracts 2
NC‑25 NatSpec @return argument is missing 1
NC‑26 Non-usage of specific imports 4
NC‑27 Use the latest solidity (prior to 0.8.20 if on L2s) for deployment 9
NC‑28 Operations such as the changing of the owner should be behind a timelock 1
NC‑29 Overly complicated arithmetic 1
NC‑30 It is standard for all external and public functions to be overridden from an interface 27
NC‑31 uint variables should have the bit size defined explicitly 2
NC‑32 Unused struct definition 1
NC‑33 Large multiples of ten should use scientific notation 6
NC‑34 Use SMTChecker 1
NC‑35 Use Underscores for Number Literals 6
NC‑36 Zero as a function argument should have a descriptive meaning 1

Total: 143 contexts over 36 issues

Non Critical Issues

[NC‑1] Array lengths not checked

If the length of the arrays are not required to be of the same length, user operations may not be fully executed

Proof Of Concept

File: Lender.sol
​
​
function giveLoan(
uint256[] calldata loanIds,
bytes32[] calldata poolIds
) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L355

[NC‑2] Avoid Floating Pragmas: The Version Should Be Locked

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

Proof Of Concept

File: Beedle.sol
​
Found usage of floating pragmas ^0.8.19 of Solidity in [Beedle.sol]

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Beedle.sol#L2

File: Fees.sol
​
Found usage of floating pragmas ^0.8.19 of Solidity in [Fees.sol]

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Fees.sol#L2

File: Lender.sol
​
Found usage of floating pragmas ^0.8.19 of Solidity in [Lender.sol]

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L2

File: Staking.sol
​
Found usage of floating pragmas ^0.8.19 of Solidity in [Staking.sol]

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L2

File: IERC20.sol
​
Found usage of floating pragmas ^0.8.19 of Solidity in [IERC20.sol]

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/interfaces/IERC20.sol#L2

File: ISwapRouter.sol
​
Found usage of floating pragmas ^0.8.19 of Solidity in [ISwapRouter.sol]

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/interfaces/ISwapRouter.sol#L2

File: Errors.sol
​
Found usage of floating pragmas ^0.8.19 of Solidity in [Errors.sol]

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Errors.sol#L2

File: Ownable.sol
​
Found usage of floating pragmas ^0.8.19 of Solidity in [Ownable.sol]

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Ownable.sol#L2

File: Structs.sol
​
Found usage of floating pragmas ^0.8.19 of Solidity in [Structs.sol]

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Structs.sol#L2

[NC‑3] Condition will not revert when block.timestamp is == to the compared variable

The condition does not revert when block.timestamp is equal to the compared > or < variable. For example, if there is a check for block.timestamp > loan.auctionStartTimestamp + loan.auctionLength then there should be a check for cases where block.timestamp is == to loan.auctionStartTimestamp + loan.auctionLength

Proof Of Concept

File: Lender.sol
​
471: if (block.timestamp > loan.auctionStartTimestamp + loan.auctionLength)

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L471

[NC‑4] Consider adding a deny-list

Doing so will significantly increase centralization, but will help to prevent hackers from using stolen tokens

Proof Of Concept

File: Beedle.sol
​
9: contract Beedle is Ownable, ERC20, ERC20Permit, ERC20Votes
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Beedle.sol#L9

[NC‑5] Consider disabling renounceOwnership()

Typically, the contract's owner is the account that deploys the contract. As a result, the owner is able to perform certain privileged activities. The OpenZeppelin's Ownable is used in this project contract implements renounceOwnership. This can represent a certain risk if the ownership is renounced for any other reason than by design. Renouncing ownership will leave the contract without an owner, thereby removing any functionality that is only available to the owner.

Proof Of Concept

File: Beedle.sol
​
9: contract Beedle is Ownable, ERC20, ERC20Permit, ERC20Votes {
11: constructor() ERC20("Beedle", "BDL") ERC20Permit("Beedle") Ownable(msg.sender) {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Beedle.sol#L9

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Beedle.sol#L11

File: Lender.sol
​
10: contract Lender is Ownable {
73: constructor() Ownable(msg.sender) {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L10

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L73

File: Staking.sol
​
11: contract Staking is Ownable {
31: constructor(address _token, address _weth) Ownable(msg.sender) {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L11

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L31

File: Ownable.sol
​
4: abstract contract Ownable {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Ownable.sol#L4

[NC‑6] Consider using named mappings

Consider moving to solidity version 0.8.18 or later, and using named mappings to make it easier to understand the purpose of each mapping

Proof Of Concept

File: Lender.sol
​
70: mapping(bytes32 => Pool) public pools;
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L70

File: Staking.sol
​
19: mapping(address => uint256) public supplyIndex;
22: mapping(address => uint256) public balances;
24: mapping(address => uint256) public claimable;
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L19

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L22

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L24

[NC‑7] Constants Should Be Defined Rather Than Using Magic Numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

Proof Of Concept

File: Lender.sol
​
85: if (_fee > 5000) revert FeeTooHigh();

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L85

File: Lender.sol
​
93: if (_fee > 500) revert FeeTooHigh();

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L93

[NC‑8] Constants in comparisons should appear on the left side

Doing so will prevent typo bugs

Proof Of Concept

File: Lender.sol
​
212: if (maxLoanRatio == 0) revert PoolConfig();

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L212

File: Lender.sol
​
244: if (collateral == 0) revert ZeroCollateral();

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L244

[NC‑9] Contract does not follow the Solidity style guide's suggested layout ordering

The style guide says that, within a contract, the ordering should be 1) Type declarations, 2) State variables, 3) Events, 4) Errors 5) Modifiers, and 6) Functions, but the contract(s) below do not follow this ordering

Proof Of Concept

File:
​
Lender.sol

[NC‑10] Critical Changes Should Use Two-step Procedure

The critical procedures should be two step process.

See similar findings in previous Code4rena contests for reference:
https://code4rena.com/reports/2022-06-illuminate/#2-critical-changes-should-use-two-step-procedure

Proof Of Concept

File: Lender.sol
​
84: function setLenderFee(uint256 _fee) external onlyOwner {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L84

File: Lender.sol
​
92: function setBorrowerFee(uint256 _fee) external onlyOwner {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L92

File: Lender.sol
​
100: function setFeeReceiver(address _feeReceiver) external onlyOwner {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L100

File: Lender.sol
​
130: function setPool(Pool calldata p) public returns (bytes32 poolId) {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L130

Recommended Mitigation Steps

Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.

[NC‑11] Custom error has no error details

Consider adding parameters to the error to indicate which user or values caused the failure

Proof Of Concept

File: Errors.sol
​
4: error PoolConfig();
5: error LoanTooSmall();
6: error LoanTooLarge();
7: error RatioTooHigh();
8: error FeeTooHigh();
9: error TokenMismatch();
10: error Unauthorized();
11: error AuctionStarted();
12: error AuctionNotStarted();
13: error AuctionEnded();
14: error AuctionNotEnded();
15: error RateTooHigh();
16: error PoolTooSmall();
17: error ZeroCollateral();
18: error AuctionTooShort();
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Errors.sol#L4-L18

[NC‑12] Declare interfaces on separate files

Interfaces should be declared on a separate file and not on the same file where the contract resides in.

Proof Of Concept

File: Staking.sol
​
7: interface FeeDistribution {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L7

[NC‑13] Empty bytes check is missing

To avoid mistakenly accepting empty bytes as a valid value, consider to check for empty bytes. This helps ensure that empty bytes are not treated as valid inputs, preventing potential issues.

Proof Of Concept

File: Lender.sol
​
182: function addToPool(bytes32 poolId, uint256 amount) external {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L182

File: Lender.sol
​
198: function removeFromPool(bytes32 poolId, uint256 amount) external {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L198

File: Lender.sol
​
210: function updateMaxLoanRatio(bytes32 poolId, uint256 maxLoanRatio) external {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L210

File: Lender.sol
​
221: function updateInterestRate(bytes32 poolId, uint256 interestRate) external {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L221

File: Lender.sol
​
355: function giveLoan(
uint256[] calldata loanIds,
bytes32[] calldata poolIds
) external {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L355

File: Lender.sol
​
356: function giveLoan(
uint256[] calldata loanIds,
bytes32[] calldata poolIds
) external {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L356

File: Lender.sol
​
465: function buyLoan(uint256 loanId, bytes32 poolId) public {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L465

File: Lender.sol
​
732: function _updatePoolBalance(bytes32 poolId, uint256 newBalance) internal {

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L732

[NC‑14] File is missing NatSpec

Proof Of Concept

File:
​
Beedle.sol
File:
​
IERC20.sol
File:
​
ISwapRouter.sol
File:
​
Errors.sol
File:
​
Ownable.sol

[NC‑15] Function names should differ to make the code more readable

In Solidity, while function overriding allows for functions with the same name to coexist, it is advisable to avoid this practice to enhance code readability and maintainability. Having multiple functions with the same name, even with different parameters or in inherited contracts, can cause confusion and increase the likelihood of errors during development, testing, and debugging. Using distinct and descriptive function names not only clarifies the purpose and behavior of each function, but also helps prevent unintended function calls or incorrect overriding. By adopting a clear and consistent naming convention, developers can create more comprehensible and maintainable smart contracts.

Proof Of Concept

File: Staking.sol
​
8: function claim

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L8

File: Staking.sol
​
53: function claim

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L53

[NC‑16] Function must not be longer than 50 lines

Proof Of Concept

File: Lender.sol
​
232: function borrow(
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L232

File: Lender.sol
​
355: function giveLoan(
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L355

File: Lender.sol
​
465: function buyLoan(
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L465

File: Lender.sol
​
591: function refinance(
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L591

[NC‑17] Functions that alter state should emit events

Proof Of Concept

File: Lender.sol
​
84: function setLenderFee(uint256 _fee) external onlyOwner {
if (_fee > 5000) revert FeeTooHigh();
lenderFee = _fee;
}

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L84

File: Lender.sol
​
92: function setBorrowerFee(uint256 _fee) external onlyOwner {
if (_fee > 500) revert FeeTooHigh();
borrowerFee = _fee;
}

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L92

File: Lender.sol
​
100: function setFeeReceiver(address _feeReceiver) external onlyOwner {
feeReceiver = _feeReceiver;
}

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L100

[NC‑18] Use delete to Clear Variables

delete a assigns the initial value for the type to a. i.e. for integers it is equivalent to a = 0, but it can also be used on arrays, where it assigns a dynamic array of length zero or a static array of the same length with all elements reset. For structs, it assigns a struct with all members reset. Similarly, it can also be used to set an address to zero address. It has no effect on whole mappings though (as the keys of mappings may be arbitrary and are generally unknown). However, individual keys and what they map to can be deleted: If a is a mapping, then delete a[x] will delete the value stored at x.

The delete key better conveys the intention and is also more idiomatic. Consider replacing assignments of zero with delete statements.

Proof Of Concept

File: Staking.sol
​
56: claimable[msg.sender] = 0;
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L56

[NC‑19] No need to initialize uints to zero

There is no need to initialize uint variables to zero as their default value is 0

Proof Of Concept

File: Staking.sol
​
14: uint256 public balance = 0;
16: uint256 public index = 0;
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L14

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L16

[NC‑20] Initial value check is missing in Set Functions

A check regarding whether the current value and the new value are the same should be added

Proof Of Concept

File: Lender.sol
​
84: function setLenderFee(uint256 _fee) external onlyOwner {
if (_fee > 5000) revert FeeTooHigh();
lenderFee = _fee;
}

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L84

File: Lender.sol
​
92: function setBorrowerFee(uint256 _fee) external onlyOwner {
if (_fee > 500) revert FeeTooHigh();
borrowerFee = _fee;
}

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L92

File: Lender.sol
​
100: function setFeeReceiver(address _feeReceiver) external onlyOwner {
feeReceiver = _feeReceiver;
}

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L100

[NC‑21] Interfaces should be indicated with an I prefix in the contract name

Proof Of Concept

File: Staking.sol
​
7: interface FeeDistribution {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L7

[NC‑22] internal functions not called by the contract should be removed

Proof Of Concept

File: Beedle.sol
​
function _afterTokenTransfer(address from, address to, uint256 amount)
internal
override(ERC20, ERC20Votes)
{

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Beedle.sol#L15

File: Beedle.sol
​
function _burn(address account, uint256 amount)
internal
override(ERC20, ERC20Votes)
{

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Beedle.sol#L29

[NC‑23] Long functions should be refactored into multiple, smaller, functions

Proof Of Concept

File: Lender.sol
​
355: function giveLoan(
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L355

File: Lender.sol
​
591: function refinance(
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L591

[NC‑24] NatSpec comments should be increased in contracts

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability. https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

Proof Of Concept

File:
​
Beedle.sol
File:
​
Lender.sol

Recommended Mitigation Steps

NatSpec comments should be increased in contracts

[NC‑25] NatSpec @return argument is missing

Proof Of Concept

File: Lender.sol
​
127: /// @notice set the info for a pool
/// updates pool info for msg.sender
/// @param p the new pool info
function setPool(Pool calldata p) public returns (bytes32 poolId) {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L127

[NC‑26] Non-usage of specific imports

The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace.
Instead, the Solidity docs recommend specifying imported symbols explicitly.
https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files

A good example:

import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol";
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";
import {SafeCastLib} from "solmate/utils/SafeCastLib.sol";
import {ERC20} from "solmate/tokens/ERC20.sol";
import {IProducer} from "src/interfaces/IProducer.sol";
import {GlobalState, UserState} from "src/Common.sol";

Proof Of Concept

File: Fees.sol
​
4: import "./utils/Errors.sol";
5: import "./utils/Structs.sol";
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Fees.sol#L4-L5

File: Lender.sol
​
4: import "./utils/Errors.sol";
5: import "./utils/Structs.sol";
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L4-L5

Recommended Mitigation Steps

Use specific imports syntax per solidity docs recommendation.

[NC‑27] Use the latest solidity (prior to 0.8.20 if on L2s) for deployment

When deploying contracts, you should use the latest released version of Solidity. Apart from exceptional cases, only the latest version receives security fixes.

0.8.20:

  • Assembler: Use push0 for placing 0 on the stack for EVM versions starting from “Shanghai”. This decreases the deployment and runtime costs.

  • EVM: Set default EVM version to “Shanghai”.

  • EVM: Support for the EVM Version “Shanghai”.

  • Optimizer: Re-implement simplified version of UnusedAssignEliminator and UnusedStoreEliminator. It can correctly remove some unused assignments in deeply nested loops that were ignored by the old version.

  • Parser: Unary plus is no longer recognized as a unary operator in the AST and triggers an error at the parsing stage (rather than later during the analysis).

  • SMTChecker: Group all messages about unsupported language features in a single warning. The CLI option --model-checker-show-unsupported and the JSON option settings.modelChecker.showUnsupported can be enabled to show the full list.

  • SMTChecker: Properties that are proved safe are now reported explicitly at the end of analysis. By default, only the number of safe properties is shown. The CLI option --model-checker-show-proved-safe and the JSON option settings.modelChecker.showProvedSafe can be enabled to show the full list of safe properties.

  • Standard JSON Interface: Add experimental support for importing ASTs via Standard JSON.

  • Yul EVM Code Transform: If available, use push0 instead of codesize to produce an arbitrary value on stack in order to create equal stack heights between branches.

Proof Of Concept

File: Beedle.sol
​
pragma solidity ^0.8.19;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Beedle.sol#L2

File: Fees.sol
​
pragma solidity ^0.8.19;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Fees.sol#L2

File: Lender.sol
​
pragma solidity ^0.8.19;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L2

File: Staking.sol
​
pragma solidity ^0.8.19;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L2

File: IERC20.sol
​
pragma solidity ^0.8.19;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/interfaces/IERC20.sol#L2

File: ISwapRouter.sol
​
pragma solidity ^0.8.19;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/interfaces/ISwapRouter.sol#L2

File: Errors.sol
​
pragma solidity ^0.8.19;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Errors.sol#L2

File: Ownable.sol
​
pragma solidity ^0.8.19;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Ownable.sol#L2

File: Structs.sol
​
pragma solidity ^0.8.19;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Structs.sol#L2

Recommended Mitigation Steps

Consider updating to a more recent solidity version.

[NC‑28] Operations such as the changing of the owner should be behind a timelock

From the point of view of a user, the changing of the owner of a contract is a high risk operation that may have outcomes ranging from an attacker gaining control over the protocol, to the function no longer functioning due to a typo in the destination address. To give users plenty of warning so that they can validate any ownership changes, changes of ownership should be behind a timelock.

Proof Of Concept

File: Ownable.sol
​
19: function transferOwnership(
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Ownable.sol#L19

[NC‑29] Overly complicated arithmetic

To maintain readability in code, particularly in Solidity which can involve complex mathematical operations, it is often recommended to limit the number of arithmetic operations to a maximum of 2-3 per line. Too many operations in a single line can make the code difficult to read and understand, increase the likelihood of mistakes, and complicate the process of debugging and reviewing the code. Consider splitting such operations over more than one line, take special care when dealing with division however. Try to limit the number of arithmetic operations to a maximum of 3 per line.

Proof Of Concept

File: Lender.sol
​
724: interest = (l.interestRate * l.debt * timeElapsed) / 10000 / 365 days;
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L724

[NC‑30] It is standard for all external and public functions to be overridden from an interface

Check that all public or external functions are override. This is iseful to make sure that the whole API is extracted in an interface.

Proof Of Concept

File: Beedle.sol
​
36: function mint(address to, uint256 amount) external onlyOwner {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Beedle.sol#L36

File: Fees.sol
​
26: function sellProfits(address _profits) public {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Fees.sol#L26

File: Lender.sol
​
84: function setLenderFee(uint256 _fee) external onlyOwner {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L84

File: Lender.sol
​
92: function setBorrowerFee(uint256 _fee) external onlyOwner {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L92

File: Lender.sol
​
100: function setFeeReceiver(address _feeReceiver) external onlyOwner {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L100

File: Lender.sol
​
108: function getPoolId(
address lender,
address loanToken,
address collateralToken
) public pure returns (bytes32 poolId) {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L108

File: Lender.sol
​
116: function getLoanDebt(uint256 loanId) external view returns (uint256 debt) {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L116

File: Lender.sol
​
130: function setPool(Pool calldata p) public returns (bytes32 poolId) {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L130

File: Lender.sol
​
182: function addToPool(bytes32 poolId, uint256 amount) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L182

File: Lender.sol
​
198: function removeFromPool(bytes32 poolId, uint256 amount) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L198

File: Lender.sol
​
210: function updateMaxLoanRatio(bytes32 poolId, uint256 maxLoanRatio) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L210

File: Lender.sol
​
221: function updateInterestRate(bytes32 poolId, uint256 interestRate) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L221

File: Lender.sol
​
232: function borrow(Borrow[] calldata borrows) public {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L232

File: Lender.sol
​
292: function repay(uint256[] calldata loanIds) public {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L292

File: Lender.sol
​
355: function giveLoan(
uint256[] calldata loanIds,
bytes32[] calldata poolIds
) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L355

File: Lender.sol
​
437: function startAuction(uint256[] calldata loanIds) public {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L437

File: Lender.sol
​
465: function buyLoan(uint256 loanId, bytes32 poolId) public {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L465

File: Lender.sol
​
540: function zapBuyLoan(Pool calldata p, uint256 loanId) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L540

File: Lender.sol
​
548: function seizeLoan(uint256[] calldata loanIds) public {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L548

File: Lender.sol
​
591: function refinance(Refinance[] calldata refinances) public {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L591

File: Staking.sol
​
8: function claim(address) external;
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L8

File: Staking.sol
​
38: function deposit(uint _amount) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L38

File: Staking.sol
​
46: function withdraw(uint _amount) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L46

File: Staking.sol
​
53: function claim() external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L53

File: Staking.sol
​
61: function update() public {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L61

File: Staking.sol
​
80: function updateFor(address recipient) public {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L80

File: Ownable.sol
​
19: function transferOwnership(address _owner) public virtual onlyOwner {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Ownable.sol#L19

[NC‑31] uint variables should have the bit size defined explicitly

Instead of using uint to declare uint256, explicitly define uint256 to ensure there is no confusion

Proof Of Concept

File: Staking.sol
​
38: function deposit(uint _amount) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L38

File: Staking.sol
​
46: function withdraw(uint _amount) external {
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Staking.sol#L46

[NC‑32] Unused struct definition

Note that there may be cases where a struct superficially appears to be used, but this is only because there are multiple definitions of the struct in different files. In such cases, the struct definition should be moved into a separate file. The instances below are the unused definitions.

Proof Of Concept

File: Structs.sol
​
68: struct Staked {
/// @notice the amount of tokens staked
uint256 amount;
/// @notice the timestamp the stake unlocks
uint256 unlock;
/// @notice the multiplier of the stake based on lock length
uint256 multiplier;
/// @notice the virtual balance based on the multipier
uint256 virtualBalance;
/// @notice the amount of points currently accumulated
uint256 points;
}
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/utils/Structs.sol#L68

[NC‑33] Large multiples of ten should use scientific notation

Use (e.g. 1e6) rather than decimal literals (e.g. 100000), for better code readability.

Proof Of Concept

File: Lender.sol
​
59: uint256 public constant MAX_INTEREST_RATE = 100000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L59

File: Lender.sol
​
265: uint256 fees = (debt * borrowerFee) / 10000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L265

File: Lender.sol
​
561: uint256 govFee = (borrowerFee * loan.collateral) / 10000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L561

File: Lender.sol
​
650: uint256 fee = (borrowerFee * (debt - debtToPay)) / 10000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L650

File: Lender.sol
​
724: interest = (l.interestRate * l.debt * timeElapsed) / 10000 / 365 days;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L724

File: Lender.sol
​
725: fees = (lenderFee * interest) / 10000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L725

[NC‑34] Use SMTChecker

The highest tier of smart contract behavior assurance is formal mathematical verification. All assertions that are made are guaranteed to be true across all inputs → The quality of your asserts is the quality of your verification

https://twitter.com/0xOwenThurm/status/1614359896350425088?t=dbG9gHFigBX85Rv29lOjIQ&s=19

[NC‑35] Use Underscores for Number Literals

Proof Of Concept

File: Lender.sol
​
59: uint256 public constant MAX_INTEREST_RATE = 100000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L59

File: Lender.sol
​
265: uint256 fees = (debt * borrowerFee) / 10000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L265

File: Lender.sol
​
561: uint256 govFee = (borrowerFee * loan.collateral) / 10000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L561

File: Lender.sol
​
650: uint256 fee = (borrowerFee * (debt - debtToPay)) / 10000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L650

File: Lender.sol
​
724: interest = (l.interestRate * l.debt * timeElapsed) / 10000 / 365 days;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L724

File: Lender.sol
​
725: fees = (lenderFee * interest) / 10000;

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Lender.sol#L725

[NC‑36] Zero as a function argument should have a descriptive meaning

Consider using descriptive constants or an enum instead of passing zero directly on function calls, as that might be error-prone, to fully describe the caller's intention.

Proof Of Concept

File: Fees.sol
​
38: amountOutMinimum: 0,
​

https://github.com/Cyfrin/2023-07-beedle/tree/main/src/Fees.sol#L38

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.