Platform: Code4rena
Start Date: 03/11/2022
Pot Size: $115,500 USDC
Total HM: 17
Participants: 120
Period: 7 days
Judge: LSDan
Total Solo HM: 1
Id: 174
League: ETH
Rank: 28/120
Findings: 3
Award: $672.02
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: __141345__
Also found by: 0xdeadbeef0x, 8olidity, Amithuddar, Bnke0x0, Ch_301, Deivitto, IllIllI, KingNFT, Nyx, RaymondFam, RedOneN, Satyam_Sharma, SmartSek, Tomo, adriro, bananasboys, carlitox477, cccz, cloudjunky, codexploder, corerouter, cryptonue, d3e4, datapunk, joestakey, martin, merlin, minhquanym, pashov, peanuts, rvierdiiev
2.6694 USDC - $2.67
transfer() instead of call() to send ethUse of transfer() might render ETH impossible to withdraw because after istanbul hardfork, there is increases in the gas cost of the SLOAD operation and therefore breaks some existing smart contracts. Those contracts will break because their fallback functions used to consume less than 2300 gas, and they’ll now consume more, since 2300 the amount of gas a contract’s fallback function receives if it’s called via Solidity’s transfer() or send() methods.
Any smart contract that uses transfer() or send() is taking a hard dependency on gas costs by forwarding a fixed amount of gas: 2300.
https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/ https://blog.openzeppelin.com/opyn-gamma-protocol-audit/
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/LineLib.sol#L48 payable(receiver).transfer(amount);
Use call() to send eth
#0 - c4-judge
2022-11-17T16:30:41Z
dmvt marked the issue as duplicate of #14
#1 - c4-judge
2022-11-17T19:17:24Z
dmvt marked the issue as partial-50
#2 - c4-judge
2022-12-06T14:39:50Z
dmvt marked the issue as satisfactory
#3 - C4-Staff
2022-12-20T05:56:43Z
liveactionllama marked the issue as duplicate of #369
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0xNazgul, 0xRoxas, 0xSmartContract, Awesome, Aymen0909, B2, BClabs, Bnke0x0, Deekshith99, Deivitto, Diana, Dinesh11G, Funen, HE1M, HardlyCodeMan, Josiah, Nyx, Rahoz, RaymondFam, RedOneN, ReyAdmirado, Rolezn, Saintcode_, TomJ, Trust, __141345__, a12jmx, adriro, ajtra, aphak5010, apostle0x01, brgltd, btk, bulej93, c3phas, carlitox477, catwhiskeys, ch0bu, chaduke, chrisdior4, cryptonue, cryptostellar5, csanuragjain, ctf_sec, delfin454000, djxploit, durianSausage, erictee, fatherOfBlocks, gogo, i_got_hacked, immeas, joestakey, jumpdest7d, lukris02, martin, mcwildy, merlin, minhquanym, oyc_109, pashov, peanuts, pedr02b2, rbserver, rotcivegaf, rvierdiiev, sakman, saneryee, seyni, shark, slowmoses, tnevler, trustindistrust, w0Lfrum, yurahod, zaskoh
620.1198 USDC - $620.12
On several locations in the code precautions are not being taken to not divide by 0, this would revert the code.
Navigate to the following contracts,
decimals can be 0 and therefore would revert for tokens with 0 decimals
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditLib.sol#L117
return price <= 0 ? 0 : (amount * uint(price)) / (1 * 10 ** decimals);Recommend making sure division by 0 won’t occur by checking the variables beforehand and handling this edge case.
receive() function will lock ether in contractIf the intention is for the Ether to be used, the function should call another function, otherwise it should revert
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L272 receive() external payable {}
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/spigot/Spigot.sol#L227 receive() external payable {
If the intention is for the Ether to be used, the function should call another function, otherwise it should revert.
address state or immutable variablesZero address should be checked for state variables, immutable variables. A zero address can lead into problems.
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L54-L58 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/factories/LineFactory.sol#L26 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/oracle/Oracle.sol#L16 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/escrow/Escrow.sol#L49-L51 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SecuredLine.sol#L14-L19 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L50-L52 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/EscrowedLine.sol#L16-L18 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/spigot/Spigot.sol#L36-L38
Check zero address before assigning or using it
Risk of using block.timestamp for time should be considered.
block.timestamp is not an ideal proxy for time because of issues with synchronization, miner manipulation and changing block times.
This kind of issue may affect the code allowing or reverting the code before the expected deadline, modifying the normal functioning or reverting sometimes.
SWC ID: 116
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L132 if (block.timestamp >= deadline && count > 0) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L58 deadline = block.timestamp + ttl_; //the deadline is the term/maturity/expiry date of the Line of Credit facility
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/interest-rate/InterestRateCredit.sol#L48 uint256 timespan = block.timestamp - rate.lastAccrued;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/interest-rate/InterestRateCredit.sol#L50 rates[id].lastAccrued = block.timestamp;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/interest-rate/InterestRateCredit.sol#L82 lastAccrued: block.timestamp
block.timestamp as time proxy and evaluate if block numbers can be used as an approximation for the application logic. Both have risks that need to be factored in.The initialize function that initializes important contract state can be called by anyone.
The attacker can initialize the contract before the legitimate deployer, hoping that the victim continues to use the same contract.
In the best case for the victim, they notice it and have to redeploy their contract costing gas.
Use the constructor to initialize non-proxied contracts.
Return values not being checked may lead into unexpected behaviors with functions. Not events/Error are being emitted if that fails, so functions would be called even of not being working as expect.
slitherNotChecked
Check values and revert/emit events if needed
Magic numbers are hardcoded numbers used in the code which are ambiguous to their intended purpose. These should be replaced with constants to make code more readable and maintainable.
Values are hardcoded and would be more readable and maintainable if declared as a constant
10
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditLib.sol#L11710 and 5
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L42-L43100
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L90Replace magic hardcoded numbers with declared constants.
Constant naming convention is all upper case.
Some constants are not using proper style.
Constant should be in UPPER_CASE_WITH_UNDERSCORES as per Solidity Style Guide.
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/factories/LineFactory.sol#L12 uint8 constant defaultRevenueSplit = 90; // 90% to debt repayment
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/factories/LineFactory.sol#L14 uint32 constant defaultMinCRatio = 3000; // 30.00% minimum collateral ratio
Rename the constant to uppercase style: CONSTANTS_WITH_UNDERSCORES.
Some of the contracts include an unlocked pragma, e.g., pragma solidity >=0.8.9.
Locking the pragma helps ensure that contracts are not accidentally deployed using an old compiler version with unfixed bugs.
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L1 pragma solidity ^0.8.9;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L1 pragma solidity ^0.8.9;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SecuredLine.sol#L1 pragma solidity ^0.8.9;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/oracle/Oracle.sol#L2 pragma solidity ^0.8.9;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/interest-rate/InterestRateCredit.sol#L1 pragma solidity ^0.8.9;
Lock pragmas to a specific Solidity version. Consider converting ^ 0.8.9 into 0.8.9]
SPDX license should be included for avoiding compiler warnings.
Compilation warnings/errors on [ ]: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SecuredLine.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/EscrowedLine.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/spigot/Spigot.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/escrow/Escrow.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/interest-rate/InterestRateCredit.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/factories/LineFactory.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditLib.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/LineLib.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditListLib.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotedLineLib.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/MutualConsent.sol https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/LineFactoryLib.sol
Add a SPDX License to each file
Clearness of the code is important for the readability and maintainability. As Solidity guidelines says about declaration order: 1.Type declarations 2.State variables 3.Events 4.Modifiers 5.Functions Also, state variables order affects to gas in the same way as ordering structs for saving storage slots
modifiers after functions https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L74-L103
modifier after constructor https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/interest-rate/InterestRateCredit.sol#L23-L31
events and errors after functions https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L239-L289 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L221-L235
Follow solidity style guidelines https://docs.soliditylang.org/en/v0.8.15/style-guide.html
Missing Natspec and regular comments affect readability and maintainability of a codebase.
Contracts has partial or full lack of comments
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L86-L219 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L16-L427 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L1-L290 https://github.com/debtdao/Line-of-Credit/blob/c1037d7f873cfb2af48fdaf00e44cd55c7737fc1/contracts/utils/SpigotedLineLib.sol#L141-L235 https://github.com/debtdao/Line-of-Credit/blob/e7fa299645f7ae857876577aa7eda924d52f8516/contracts/utils/MutualConsent.sol#L1-L68 https://github.com/debtdao/Line-of-Credit/blob/ef130955057d2db4e73a4d8330bf65282a13aa02/contracts/utils/LineFactoryLib.sol#L56-L88 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L72-L273
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/EscrowedLine.sol#L20-L40 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/spigot/Spigot.sol#L41-L229 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/oracle/Oracle.sol#L1-L35 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/interest-rate/InterestRateCredit.sol#L1-L87 https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/factories/LineFactory.sol#L1-L194
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditLib.sol#L1-L263 https://github.com/debtdao/Line-of-Credit/blob/d7ef66035ddf873b0c96804a1c9deeebb1f798ea/contracts/utils/LineLib.sol#L1-L87
OpenZeppelin recommends that the initializer modifier be applied to constructors in order to avoid potential griefs, social engineering, or exploits.
Ensure that the modifier is applied to the implementation contract. If the default constructor is currently being used, it should be changed to be an explicit one with the modifier applied.
Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment
The code includes a TODO that affects readability and focus on the readers/auditors of the contracts
Remove already done TODO
Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L58 deadline = block.timestamp + ttl_; //the deadline is the term/maturity/expiry date of the Line of Credit facility
Long lines should be wrapped to conform with Solidity Style guidelines.
Lines that exceed the 79 (or 99) character length suggested by the Solidity Style guidelines. Reference: https://docs.soliditylang.org/en/v0.8.10/style-guide.html#maximum-line-length
Reduce line length to less than 99 at least to improve maintainability and readability of the code
Multiples of 10 can be declared as constants with scientific notation so it's easier to read them and less prone to miss/exceed a 0 of the expected value.
Value 10000 can be used in scientific notation
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/interest-rate/InterestRateCredit.sol#L9 uint256 constant BASE_DENOMINATOR = 10000;
Replace hardcoded numbers with constants that represent the scientific corresponding notation
Multiples of 10 can be declared as constants with scientific notation
Value 10**5 can be used in scientific notation
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L42 uint256 _numerator = collateralValue * 10**5; // scale to 4 decimals
Consider replacing long 0 numbers and 10**value notation to scientific notation
constant keyword helps with readability of the code and to make sure that they do not change.
Code contains state variables that do not change and so they can be declared constant
0xNotConstants
Add constant and change [[[MaxPeriod to MAX_PERIOD]]]
Require/revert statements should include error messages in order to help at monitoring the system.
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L112 require(uint(status) >= uint( LineLib.STATUS.ACTIVE));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L241 require(interestRate.setRate(id, drate, frate));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L259 require(interestRate.setRate(id, drate, frate));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L326 require(amount <= credit.principal + credit.interestAccrued);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L62 require(defaultRevenueSplit_ <= SpigotedLineLib.MAX_SPLIT);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L143 require(amount <= unusedTokens[credit.token]);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L160 require(msg.sender == borrower);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L239 require(msg.sender == arbiter);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/EscrowedLine.sol#L64 require(escrow_.liquidate(amount, targetToken, to));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/EscrowedLine.sol#L90 require(escrow.updateLine(newLine));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L91 require(amount > 0);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L105 require(msg.sender == ILineOfCredit(self.line).arbiter());
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L161 require(amount > 0);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L198 require(amount > 0);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L216 require(msg.sender == self.line);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L96 require(LineLib.sendOutTokenOrETH(token, self.treasury, claimed - escrowedAmount));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L128 require(revenueContract != address(this));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L130 require(self.settings[revenueContract].transferOwnerFunction == bytes4(0));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L155 require(success);
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L180 require(newOwner != address(0));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L189 require(newOperator != address(0));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L201 require(newTreasury != address(0));
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotedLineLib.sol#L147 require(ISpigot(spigot).updateOwner(newLine));
Add error messages
For loops are recommended to be declared such as for (, however, sometimes are declared as for(
https://docs.soliditylang.org/en/v0.8.16/style-guide.html#control-structures
for( this are missing a space
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditListLib.sol#L23
for(uint256 i; i < len; ++i) {https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditListLib.sol#L51 for(uint i = 1; i < len; ++i) {
for (
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L179
for (uint256 i; i < len; ++i) {https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L203 for (uint256 i; i < len; ++i) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L520 for (uint256 i; i <= lastSpot; ++i) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L57 for (uint256 i; i < length; ++i) {
Add a space between the for and the (
#0 - c4-judge
2022-12-07T17:29:44Z
dmvt marked the issue as grade-a
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0xRajkumar, Awesome, Aymen0909, B2, Bnke0x0, Deivitto, Diana, JC, Metatron, Rahoz, RaymondFam, RedOneN, ReyAdmirado, Rolezn, Saintcode_, TomJ, __141345__, ajtra, aphak5010, brgltd, c3phas, ch0bu, chrisdior4, cryptonue, durianSausage, emrekocak, erictee, exolorkistis, gogo, karanctf, lukris02, martin, me_na0mi, oyc_109, peanuts, rotcivegaf, saneryee, seyni, tnevler, zaskoh
49.2315 USDC - $49.23
Custom errors reduce 38 gas if the condition is met and 22 gas otherwise. Also reduces contract size and deployment costs.
Since version 0.8.4 the use of custom errors rather than revert() / require() saves gas as noticed in https://blog.soliditylang.org/2021/04/21/custom-errors/ https://github.com/code-423n4/2022-04-pooltogether-findings/issues/13
replace each error message in a require by a custom error
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met. Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.
Consider shortening the revert strings to fit in 32 bytes
Unchecked operations as the ++i on for loops are cheaper than checked one.
In Solidity 0.8+, there’s a default overflow check on unsigned integers. It’s possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline..
The code would go from:
for (uint256 i; i < numIterations; i++) { // ... }
to
for (uint256 i; i < numIterations;) { // ... unchecked { ++i; } } The risk of overflow is inexistent for a uint256 here.
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditListLib.sol#L23 for(uint256 i; i < len; ++i) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditListLib.sol#L51 for(uint i = 1; i < len; ++i) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L179 for (uint256 i; i < len; ++i) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L203 for (uint256 i; i < len; ++i) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L520 for (uint256 i; i <= lastSpot; ++i) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L57 for (uint256 i; i < length; ++i) {
Add unchecked ++i at the end of all the for loop where it's not expected to overflow and remove them from the for header
When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size than downcast where needed
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L32 uint8 public immutable defaultRevenueSplit;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L32 uint8 public immutable defaultRevenueSplit;
Consider using some data type that uses 32 bytes, for example uint256
All these variables could be combine in a Struct in order to reduce the gas cost.
As noticed in: https://gist.github.com/alexon1234/b101e3ac51bea3cbd9cf06f80eaa5bc2 When multiple mappings that access the same addresses, uints, etc, all of them can be mixed into an struct and then that data accessed like: mapping(datatype => newStructCreated) newStructMap; Also, you have this post where it explains the benefits of using Structs over mappings https://medium.com/@novablitz/storing-structs-is-costing-you-gas-774da988895e
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L16 mapping(address => bool) enabled;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L18 mapping(address => IEscrow.Deposit) deposited;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L12 mapping(address => uint256) escrowed; // token -> amount escrowed
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/SpigotLib.sol#L16 mapping(address => ISpigot.Setting) settings; // revenue contract -> settings
Consider mixing different mappings into an struct when able in order to save gas.
>= cheaper than >Strict inequalities ( > ) are more expensive than non-strict ones ( >= ). This is due to some supplementary checks (ISZERO, 3 gas)
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L132 if (block.timestamp >= deadline && count > 0) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L398 if(facilityFee > 0) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L484 if(credit.principal > 0) { revert CloseFailedWithPrincipal(); }
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L487 if (credit.deposit + credit.interestRepaid > 0) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L119 bool is4626 = tokenAddrBytes.length > 0 && passed;
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L134 if (decimalBytes.length > 0 && successDecimals) {
Consider using >= 1 instead of > 0 to avoid some opcodes
<X> += <Y> costs more gas than <X> = <X> + <Y> for state variablesx+=y costs more gas than x=x+y for state variables. Same happens with -=
+=
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L125
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L172
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L96
-=
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L122
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/SpigotedLine.sol#L144
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L164
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/EscrowLib.sol#L202
Don't use += for state variables as it cost more gas.
abi.encode() is less gas efficient than abi.encodePacked()In general, abi.encodePacked is more gas-efficient.
Changing the abi.encode function to abi.encodePacked can save gas since the abi.encode function pads extra null bytes at the end of the call data, which is unnecessary. Also, in general, abi.encodePacked is more gas-efficient.
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/CreditLib.sol#L69 return keccak256(abi.encode(line, lender, token));
Consider changing abi.encode to abi.encodePacked
Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/modules/credit/LineOfCredit.sol#L516 function _sortIntoQ(bytes32 p) internal returns (bool) {
https://github.com/debtdao/Line-of-Credit/blob/f32cb3eeb08663f2456bf6e2fba21e964da3e8ae/contracts/utils/MutualConsent.sol#L65 function _getNonCaller(address _signerOne, address _signerTwo) internal view returns(address) {
Consider changing internal function only called once to inline code for gas savings
A value which value is already known can be used directly rather than reading it from the storage
line = LineFactoryLib.deploySecuredLine( oracle, arbiter, borrower, payable(swapTarget), s, e, ttl, split ); // give modules from address(this) to line so we can run line.init() LineFactoryLib.transferModulesToLine(address(line), s, e); emit DeployedSecuredLine(address(line), s, e, swapTarget, split);
The second parameter of SetPricePerShare can use now directly to avoid unnecessary storage read of lastPricePerShareUpdate to save some gas.
Recommendation Change to:
address _swapTarget = swapTarget; line = LineFactoryLib.deploySecuredLine( oracle, arbiter, borrower, payable(_swapTarget), s, e, ttl, split ); // give modules from address(this) to line so we can run line.init() LineFactoryLib.transferModulesToLine(address(line), s, e); emit DeployedSecuredLine(address(line), s, e, _swapTarget, split);
Set directly the value to avoid unnecessary storage read to save some gas
#0 - c4-judge
2022-11-17T23:05:54Z
dmvt marked the issue as grade-b