Skip to content

Instantly share code, notes, and snippets.

@ChaseTheLight01
Created January 28, 2025 13:03
LightChaserV3_Cantina_Daaoai

LightChaser-V3

Generated for: Cantina : Daaoai

Generated on: 2025-01-28

Total findings: 115

Total HIGH findings: 1

Total Medium findings: 3

Total Low findings: 30

Total Gas findings: 43

Total NonCritical findings: 38

Summary for High findings

Number Details Instances
[High-1] Permanent DoS due to non-shrinking array usage in an unbounded loop 1

Summary for Medium findings

Number Details Instances
[Medium-1] Uniswap Slot0 value used in accounting arithmetic instead of TWAP value 1
[Medium-2] Privileged functions can create points of failure 17
[Medium-3] Using transfer functions to a payable address 2

Summary for Low findings

Number Details Instances
[Low-1] Code does not follow the best practice of check-effects-interaction 2
[Low-2] Solidity version 0.8.23 won't work on all chains due to MCOPY 2
[Low-3] Some tokens may revert when zero value transfers are made 1
[Low-4] No limits when setting min/max amounts 2
[Low-5] Minting to the zero address should be avoided 1
[Low-6] Missing zero address check in constructor 1
[Low-7] Use of onlyOwner functions can be lost 2
[Low-8] approve()/safeApprove() may revert if the current approval is not zero 1
[Low-9] Constant decimal values 1
[Low-10] Arrays can grow in size without a way to shrink them 1
[Low-11] Large approvals such as type(uint256).max may not work with some tokens 1
[Low-12] Return values not checked for approve() 3
[Low-13] External call recipient may consume all transaction gas 3
[Low-14] Events may be emitted out of order due to code not follow the best practice of check-effects-interaction 2
[Low-15] Critical functions should have a timelock 4
[Low-16] Consider implementing two-step procedure for updating protocol addresses 2
[Low-17] Prefer skip over revert model in iteration 2
[Low-18] Missing contract-existence checks before low-level calls 1
[Low-19] approve will always revert as the IERC20 interface mismatch 1
[Low-20] Constructors missing validation 1
[Low-21] State variables not capped at reasonable values 2
[Low-22] Use forceApprove in place of approve 4
[Low-23] Functions calling contracts/addresses with transfer hooks are missing reentrancy guards 2
[Low-24] Solidity version 0.8.20 won't work on all chains due to PUSH0 2
[Low-25] Missing events in functions that are either setters, privileged or voting related 12
[Low-26] Unsafe use of transfer()/transferFrom() with IERC20 4
[Low-27] Avoid floating pragma in non interface/library files 3
[Low-28] Read only reentrancy risk detected 1
[Low-29] Common tokens such as WETH9 work differently on chains such a Blast which isn't taken into account during transfer calls. 1
[Low-30] Consider a uptime feed on L2 deployments to prevent issues caused by downtime 3

Summary for NonCritical findings

Number Details Instances
[NonCritical-1] Addresses shouldn't be hard-coded 1
[NonCritical-2] Greater than comparisons made on state uints that can be set to max 3
[NonCritical-3] Floating pragma should be avoided 2
[NonCritical-4] Require statements should have error string 1
[NonCritical-5] Events regarding state variable changes should emit the previous state variable value 1
[NonCritical-6] In functions which accept an address as a parameter, there should be a zero address check to prevent bugs 6
[NonCritical-7] Default int values are manually set 3
[NonCritical-8] Ownable2Step should be used in place of Ownable 2
[NonCritical-9] Specific imports should be used where possible so only used code is imported 4
[NonCritical-10] Old Solidity version 2
[NonCritical-11] Not all event definitions are utilizing indexed variables. 11
[NonCritical-12] Contracts should have all public/external functions exposed by interfaces 19
[NonCritical-13] Functions within contracts are not ordered according to the solidity style guide 2
[NonCritical-14] Emits without msg.sender parameter 3
[NonCritical-15] Unused state variables present 5
[NonCritical-16] Constants should be on the left side of the comparison 5
[NonCritical-17] Both immutable and constant state variables should be CONSTANT_CASE 1
[NonCritical-18] Using zero as a parameter 1
[NonCritical-19] Use of non-named numeric constants 2
[NonCritical-20] Unused structs present 2
[NonCritical-21] Empty bytes check is missing 2
[NonCritical-22] Whitespace in expressions 5
[NonCritical-23] call bypasses function existence check, type checking and argument packing 1
[NonCritical-24] Cyclomatic complexity in functions 1
[NonCritical-25] Unused events present 4
[NonCritical-26] Unused import 1
[NonCritical-27] A event should be emitted if a non immutable state variable is set in a constructor 1
[NonCritical-28] Non constant/immutable state variables are missing a setter post deployment 4
[NonCritical-29] Use transfer libraries instead of low level calls 4
[NonCritical-30] Long numbers should include underscores to improve readability and prevent typos 1
[NonCritical-31] Constructors should emit an event 1
[NonCritical-32] Contract and Abstract files should have a fixed compiler version 5
[NonCritical-33] Token minting is centralized 1
[NonCritical-34] For loop iterates on arrays not indexed 2
[NonCritical-35] Constructor with array/string/bytes parameters has no empty array checks 2
[NonCritical-36] Avoid using 'owner' or '_owner' as a parameter name 1
[NonCritical-37] ERC777 tokens can introduce reentrancy risks 2
[NonCritical-38] It is best practise to initialize a local variable when they are defined 2

Summary for Gas findings

Number Details Instances Gas
[Gas-1] Mappings are cheaper than arrays for state variable iteration 2 4000
[Gas-2] The result of a function call should be cached rather than re-calling the function 1 100
[Gas-3] x + y is more efficient than using += for state variables (likewise for -=) 1 5
[Gas-4] There is a 32 byte length threshold for error strings, strings longer than this consume more gas 3 126
[Gas-5] Public functions not used internally can be marked as external to save gas 4 0.0
[Gas-6] Usage of smaller uint/int types causes overhead 23 29095
[Gas-7] Use != 0 instead of > 0 12 432
[Gas-8] Default int values are manually reset 1 0.0
[Gas-9] Function calls within for loops 2 0.0
[Gas-10] For loops in public or external functions should be avoided due to high gas costs and possible DOS 3 0.0
[Gas-11] Use assembly to check for the zero address 5 0.0
[Gas-12] Some error strings are not descriptive 1 0.0
[Gas-13] State variables which are not modified within functions should be set as constants or immutable for values set at deployment 6 0.0
[Gas-14] Structs can be packed into fewer storage slots 2 10000
[Gas-15] Use selfBalance() in place of address(this).balance 2 3200
[Gas-16] Use assembly to emit events 17 10982
[Gas-17] Use solady library where possible to save gas 4 16000
[Gas-18] Use assembly in place of abi.decode to extract calldata values more efficiently 1 0.0
[Gas-19] Struct variables can be packed into fewer storage slots by truncating timestamp bytes 1 2500
[Gas-20] Using private rather than public for constants and immutables, saves gas 8 0.0
[Gas-21] Identical Deployments Should be Replaced with Clone 1 0.0
[Gas-22] Mark Functions That Revert For Normal Users As payable 11 3025
[Gas-23] Lack of unchecked in loops 6 2160
[Gas-24] Where a value is casted more than once, consider caching the result to save gas 1 0.0
[Gas-25] Use assembly to validate msg.sender 3 0.0
[Gas-26] Simple checks for zero uint can be done using assembly to save gas 4 96
[Gas-27] Using nested if to save gas 3 54
[Gas-28] Optimize Storage with Byte Truncation for Time Related State Variables 1 2000
[Gas-29] Using delete instead of setting mapping to 0 saves gas 1 5
[Gas-30] Low level call can be optimized with assembly 9 20088
[Gas-31] Solidity versions 0.8.19 and above are more gas efficient 1 1000
[Gas-32] Calling .length in a for loop wastes gas 6 3492
[Gas-33] Internal functions only used once can be inlined to save gas 1 30
[Gas-34] Constructors can be marked as payable to save deployment gas 2 0.0
[Gas-35] It is a waste of GAS to emit variable literals 5 200
[Gas-36] Use OZ Array.unsafeAccess() to avoid repeated array length checks 6 75600
[Gas-37] State variable read in a loop 2 23176
[Gas-38] Use uint256(1)/uint256(2) instead of true/false to save gas for changes 2 68000
[Gas-39] Avoid emitting events in loops 8 24000
[Gas-40] Call msg.XYZ directly rather than caching the value to save gas 1 0.0
[Gas-41] Write direct outcome, instead of performing mathematical operations for constant state variables 1 0.0
[Gas-42] Consider pre-calculating the address of address(this) to save gas 12 0.0
[Gas-43] Using named returns for pure and view functions is cheaper than using regular returns 3 234

[High-1] Permanent DoS due to non-shrinking array usage in an unbounded loop

Resolution

Using non-shrinking state variable arrays in unbounded loops within smart contracts can lead to a permanent Denial of Service (DoS) vulnerability. When a loop iterates over a continually growing state variable array without any bound, the gas cost for each iteration increases. Eventually, the loop may require more gas than the block gas limit allows, making the function and possibly the entire contract unusable. To prevent this, it's essential to design contracts with mechanisms that either limit the size of these arrays or process data in smaller, manageable chunks. This approach ensures that the contract functions remain gas-efficient and operable, safeguarding against unmanageable growth in storage and execution costs.

Num of instances: 1

Findings

Click to show findings

['287']

287:        for (uint256 i = 0; i < contributors.length; i++) { // <= FOUND
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

[Medium-1] Uniswap Slot0 value used in accounting arithmetic instead of TWAP value

Resolution

Relying on Uniswap’s slot0 value for accounting arithmetic instead of using the Time-Weighted Average Price (TWAP) can introduce significant risks and inaccuracies. The slot0 value represents the current price of the token pair in the pool, but it is highly volatile and can be easily manipulated, especially in short timeframes or during low liquidity periods. This volatility makes slot0 unsuitable for critical accounting operations, such as determining fair value, calculating collateralization, or settling trades, where stability and accuracy are paramount.

In contrast, TWAP provides a more reliable and robust pricing mechanism by averaging prices over a specified time window. This averaging process smooths out short-term fluctuations and reduces the impact of market manipulation. TWAP is particularly valuable in DeFi protocols where price accuracy and resistance to manipulation are crucial for maintaining security and fairness.

Using slot0 instead of TWAP in accounting can lead to skewed financial results, opening the door to potential exploits, such as flash loan attacks or other forms of market manipulation. To mitigate these risks and ensure the integrity of financial calculations, it's essential to use TWAP values, which provide a more stable and trustworthy price reference for sensitive operations. This approach safeguards the protocol against volatility-induced errors and malicious activities.

Num of instances: 1

Findings

Click to show findings

['57']

57:     function getSwapResult( // <= FOUND
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio // <= FOUND
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount);
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0(); // <= FOUND
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount);
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

[Medium-2] Privileged functions can create points of failure

Resolution

Ensure such accounts are protected and consider implementing multi sig to prevent a single point of failure

Num of instances: 17

Findings

Click to show findings

['382']

382:     function setDaoToken(address _daoToken) external onlyOwner  // <= FOUND

['388']

388:     function setSecondToken(address _daoToken) external onlyOwner  // <= FOUND

['412']

412:     function execute(
413:         address[] calldata contracts,
414:         bytes[] calldata data,
415:         uint256[] calldata msgValues
416:     ) external onlyOwner  // <= FOUND

['429']

429:     function extendFundExpiry(uint256 newFundExpiry) external onlyOwner  // <= FOUND

['13']

13:     function mint(address _to, uint256 _amount) external onlyOwner  // <= FOUND

['183']

183:     function addToWhitelist(
184:         address[] calldata _addresses,
185:         WhitelistTier[] calldata _tiers
186:     ) external {
187:         require(
188:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
189:             "Must be owner or protocolAdmin"
190:         );
191:         require(_addresses.length == _tiers.length, "Arrays length mismatch");
192:         require(_addresses.length > 0, "Empty arrays");
193: 
194:         for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }
206:     }

['208']

208:     function updateTierLimit(WhitelistTier _tier, uint256 _newLimit) external {
209:         require(
210:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
211:             "Not authorized"
212:         );
213:         require(_tier != WhitelistTier.None, "Invalid tier");
214:         require(_newLimit > 0, "Invalid limit");
215: 
216:         tierLimits[_tier] = _newLimit;
217: 
218:         if (_tier == WhitelistTier.Gold) {
219:             GOLD_DEFAULT_LIMIT = _newLimit;
220:         } else if (_tier == WhitelistTier.Silver) {
221:             SILVER_DEFAULT_LIMIT = _newLimit;
222:         } else {
223:             PLATINUM_DEFAULT_LIMIT = _newLimit;
224:         }
225: 
226:         emit TierLimitUpdated(_tier, _newLimit);
227:     }

['233']

233:     function removeFromWhitelist(address removedAddress) external {
234:         require(
235:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
236:             "Must be owner or protocolAdmin"
237:         );
238: 
239:         require(removedAddress != address(0), "Invalid address");
240:         require(
241:             userTiers[removedAddress] != WhitelistTier.None,
242:             "Address not whitelisted"
243:         );
244:         userTiers[removedAddress] = WhitelistTier.None;
245: 
246:         
247: 
248:         
249:         
250:         
251:         
252:         
253:         
254:         
255: 
256:         emit RemoveWhitelist(removedAddress);
257:     }

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public {
260:         require(
261:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
262:             "Must be owner or protocolAdmin"
263:         );
264:         maxWhitelistAmount = _maxWhitelistAmount;
265:     }

['267']

267:     function setMaxPublicContributionAmount(
268:         uint256 _maxPublicContributionAmount
269:     ) public {
270:         require(
271:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
272:             "Must be owner or protocolAdmin"
273:         );
274:         maxPublicContributionAmount = _maxPublicContributionAmount;
275:     }

['435']

435:     function extendFundraisingDeadline(
436:         uint256 newFundraisingDeadline
437:     ) external {
438:         require(
439:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
440:             "Must be owner or protocolAdmin"
441:         );
442:         require(!goalReached, "Fundraising goal was reached");
443:         require(
444:             newFundraisingDeadline > fundraisingDeadline,
445:             "new fundraising deadline must be > old one"
446:         );
447:         fundraisingDeadline = newFundraisingDeadline;
448:     }

['183']

183:     function addToWhitelist(
184:         address[] calldata _addresses,
185:         WhitelistTier[] calldata _tiers
186:     ) external {
187:         require(
188:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
189:             "Must be owner or protocolAdmin"
190:         );
191:         require(_addresses.length == _tiers.length, "Arrays length mismatch");
192:         require(_addresses.length > 0, "Empty arrays");
193: 
194:         for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }
206:     }

['208']

208:     function updateTierLimit(WhitelistTier _tier, uint256 _newLimit) external {
209:         require(
210:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
211:             "Not authorized"
212:         );
213:         require(_tier != WhitelistTier.None, "Invalid tier");
214:         require(_newLimit > 0, "Invalid limit");
215: 
216:         tierLimits[_tier] = _newLimit;
217: 
218:         if (_tier == WhitelistTier.Gold) {
219:             GOLD_DEFAULT_LIMIT = _newLimit;
220:         } else if (_tier == WhitelistTier.Silver) {
221:             SILVER_DEFAULT_LIMIT = _newLimit;
222:         } else {
223:             PLATINUM_DEFAULT_LIMIT = _newLimit;
224:         }
225: 
226:         emit TierLimitUpdated(_tier, _newLimit);
227:     }

['233']

233:     function removeFromWhitelist(address removedAddress) external {
234:         require(
235:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
236:             "Must be owner or protocolAdmin"
237:         );
238: 
239:         require(removedAddress != address(0), "Invalid address");
240:         require(
241:             userTiers[removedAddress] != WhitelistTier.None,
242:             "Address not whitelisted"
243:         );
244:         userTiers[removedAddress] = WhitelistTier.None;
245: 
246:         
247: 
248:         
249:         
250:         
251:         
252:         
253:         
254:         
255: 
256:         emit RemoveWhitelist(removedAddress);
257:     }

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public {
260:         require(
261:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
262:             "Must be owner or protocolAdmin"
263:         );
264:         maxWhitelistAmount = _maxWhitelistAmount;
265:     }

['267']

267:     function setMaxPublicContributionAmount(
268:         uint256 _maxPublicContributionAmount
269:     ) public {
270:         require(
271:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
272:             "Must be owner or protocolAdmin"
273:         );
274:         maxPublicContributionAmount = _maxPublicContributionAmount;
275:     }

['435']

435:     function extendFundraisingDeadline(
436:         uint256 newFundraisingDeadline
437:     ) external {
438:         require(
439:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
440:             "Must be owner or protocolAdmin"
441:         );
442:         require(!goalReached, "Fundraising goal was reached");
443:         require(
444:             newFundraisingDeadline > fundraisingDeadline,
445:             "new fundraising deadline must be > old one"
446:         );
447:         fundraisingDeadline = newFundraisingDeadline;
448:     }

[Medium-3] Using transfer functions to a payable address

Resolution

Transferring tokens to a payable address using transfer or transferFrom in Solidity can be problematic because these functions are not aware of the payable status of the address. Payable addresses are typically contracts expecting to receive Ether and might contain a receive or fallback function to handle incoming transactions. When ERC20 tokens are sent to these addresses, these functions are not triggered, potentially leading to loss of tokens if the contract lacks a way to retrieve them.

In Ethereum, when a contract sends Ether to another contract, the receiving contract's fallback function gets triggered. This function has a stipend of 2300 gas to work with. This stipend is intentionally limited to prevent the recipient from performing other complex operations during the transaction, which would add to the sending contract's gas costs unpredictably.

2300 gas is just enough to perform basic logging by triggering an event. But it is not enough to perform state changes, as these operations cost significantly more gas.

The 2300 gas stipend is only applicable if Ether is sent through a simple send() or transfer() function call. If a contract calls another contract function that changes the contract state, the gas provided for the function call must be explicitly specified and can be higher than 2300 gas.

This limitation is one reason why it's recommended to avoid using send() or transfer() in favor of the call{value: x}("") pattern. The latter allows sending arbitrary amounts of gas along with the Ether, reducing the chance of out-of-gas errors in the receiving contract.

Num of instances: 2

Findings

Click to show findings

['135']

135:     function contribute() public payable nonReentrant { // <= FOUND
136:         require(!goalReached, "Goal already reached");
137:         require(block.timestamp < fundraisingDeadline, "Deadline hit");
138:         require(msg.value > 0, "Contribution must be greater than 0");
139: 
140:         
141:         WhitelistTier userTier = userTiers[msg.sender];
142:         require(userTier != WhitelistTier.None, "Not whitelisted");
143:         
144:         uint256 userLimit = tierLimits[userTier];
145:         require(
146:             contributions[msg.sender] + msg.value <= userLimit,
147:             "Exceeding tier limit"
148:         );
149: 
150:         if (maxWhitelistAmount > 0) {
151:             require(
152:                 contributions[msg.sender] + msg.value <= maxWhitelistAmount,
153:                 "Exceeding maxWhitelistAmount"
154:             );
155:         } else if (maxPublicContributionAmount > 0) {
156:             require(
157:                 contributions[msg.sender] + msg.value <=
158:                     maxPublicContributionAmount,
159:                 "Exceeding maxPublicContributionAmount"
160:             );
161:         }
162: 
163:         uint256 effectiveContribution = msg.value;
164:         if (totalRaised + msg.value > fundraisingGoal) {
165:             effectiveContribution = fundraisingGoal - totalRaised;
166:             payable(msg.sender).transfer(msg.value - effectiveContribution); // <= FOUND
167:         }
168: 
169:         if (contributions[msg.sender] == 0) {
170:             contributors.push(msg.sender);
171:         }
172: 
173:         contributions[msg.sender] += effectiveContribution;
174:         totalRaised += effectiveContribution;
175: 
176:         if (totalRaised == fundraisingGoal) {
177:             goalReached = true;
178:         }
179: 
180:         emit Contribution(msg.sender, effectiveContribution);
181:     }

['395']

395:     function refund() external nonReentrant { // <= FOUND
396:         require(!goalReached, "Fundraising goal was reached");
397:         require(
398:             block.timestamp > fundraisingDeadline,
399:             "Deadline not reached yet"
400:         );
401:         require(contributions[msg.sender] > 0, "No contributions to refund");
402: 
403:         uint256 contributedAmount = contributions[msg.sender];
404:         contributions[msg.sender] = 0;
405: 
406:         payable(msg.sender).transfer(contributedAmount); // <= FOUND
407: 
408:         emit Refund(msg.sender, contributedAmount);
409:     }

[Low-1] Code does not follow the best practice of check-effects-interaction

Resolution

The "check-effects-interaction" pattern is a best practice in smart contract development, emphasizing the order of operations in functions to prevent reentrancy attacks. Violations arise when a function interacts with external contracts before settling internal state changes or checks. This misordering can expose the contract to potential threats. To adhere to this pattern, first ensure all conditions or checks are satisfied, then update any internal states, and only after these steps, interact with external contracts or addresses. Rearranging operations to this recommended sequence bolsters contract security and aligns with established best practices in the Ethereum community.

Num of instances: 2

Findings

Click to show findings

['57']

57:     function getSwapResult( // <= FOUND
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount); // <= FOUND
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount);
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external { // <= FOUND
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set");
282:         emit DebugLog("Starting finalizeFundraising");
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true);
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96");
311: 
312:         uint256 amountToken0ForLP;
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP);
330: 
331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP);
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP);
350:         emit DebugLog("Minted additional tokens for LP");
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params);
353:         emit LPTokenMinted(tokenId);
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER),
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress);
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress);
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId); // <= FOUND
376:         emit LockerInitialized(tokenId);
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete");
380:     }

[Low-2] Solidity version 0.8.23 won't work on all chains due to MCOPY

Resolution

Solidity version 0.8.23 introduces the MCOPY opcode, this may not be implemented on all chains and L2 thus reducing the portability and compatibility of the code. Consider using a earlier solidity version.

Num of instances: 2

Findings

Click to show findings

['2']

2: pragma solidity ^0.8.0; // <= FOUND

['2']

2: pragma solidity ^0.8.18; // <= FOUND

[Low-3] Some tokens may revert when zero value transfers are made

Resolution

Reason: In Solidity, ERC20 token transfers of value 0 can sometimes lead to unexpected issues. This is particularly relevant when dealing with fractional token amounts that round to 0 when less than 1 of the smallest unit is transferred, leading to an effective transfer of nothing while still consuming gas. Furthermore, some ERC20 token implementations may revert on attempts to transfer a value of 0. However, note that this issue doesn't generally apply to wrapper native tokens like WETH.

Resolution: It's advisable to include a condition before any transfer operation to bypass the transaction if the transfer amount is 0. This saves unnecessary gas expenditure and prevents potential function reverts. For handling fractions, ensure token decimals are appropriately assigned and contemplate setting a minimum transfer threshold to avoid rounding down to 0. When dealing with wrapped tokens like WETH, special consideration should be given to their unique characteristics.

Num of instances: 1

Findings

Click to show findings

['57']

57:     function getSwapResult( // <= FOUND
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount);
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount); // <= FOUND
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

[Low-4] No limits when setting min/max amounts

Resolution

When settings min/max state variables, ensure there a require checks in place to prevent incorrect values from being set

Num of instances: 2

Findings

Click to show findings

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public { // <= FOUND
260:         require(
261:             msg.sender == owner() || msg.sender == protocolAdmin,
262:             "Must be owner or protocolAdmin"
263:         );
264:         maxWhitelistAmount = _maxWhitelistAmount; // <= FOUND
265:     }

['267']

267:     function setMaxPublicContributionAmount( // <= FOUND
268:         uint256 _maxPublicContributionAmount // <= FOUND
269:     ) public {
270:         require(
271:             msg.sender == owner() || msg.sender == protocolAdmin,
272:             "Must be owner or protocolAdmin"
273:         );
274:         maxPublicContributionAmount = _maxPublicContributionAmount; // <= FOUND
275:     }

[Low-5] Minting to the zero address should be avoided

Resolution

Minting tokens to the zero address in Solidity is a potential pitfall that should be carefully guarded against. The zero address (0x0) is generally used as a default value and represents an uninitialized or null address. Minting tokens to this address effectively burns them, making them inaccessible and permanently removing them from the total supply. This could lead to unintended token loss if performed accidentally. To prevent this, it's important to include a check in the minting function to ensure that the target address is not the zero address. Using a library like OpenZeppelin's Address can provide utility functions like requireNonZero, which simplifies this check and enhances the security of the minting function.

Num of instances: 1

Findings

Click to show findings

['13']

13:     function mint(address _to, uint256 _amount) external onlyOwner {
14:         _mint(_to, _amount);
15:     }

[Low-6] Missing zero address check in constructor

Resolution

In Solidity, constructors often take address parameters to initialize important components of a contract, such as owner or linked contracts. However, without a check, there's a risk that an address parameter could be mistakenly set to the zero address (0x0). This could occur due to a mistake or oversight during contract deployment. A zero address in a crucial role can cause serious issues, as it cannot perform actions like a normal address, and any funds sent to it are irretrievable. Therefore, it's crucial to include a zero address check in constructors to prevent such potential problems. If a zero address is detected, the constructor should revert the transaction.

Num of instances: 1

Findings

Click to show findings

['95']

95:     constructor(
96:         uint256 _fundraisingGoal,
97:         string memory _name,
98:         string memory _symbol,
99:         uint256 _fundraisingDeadline,
100:         uint256 _fundExpiry,
101:         address _daoManager, // <= FOUND
102:         address _liquidityLockerFactory, // <= FOUND
103:         uint256 _maxWhitelistAmount,
104:         address _protocolAdmin, // <= FOUND
105:         uint256 _maxPublicContributionAmount
106:     ) Ownable(_daoManager) {
107:         require(
108:             _fundraisingGoal > 0,
109:             "Fundraising goal must be greater than 0"
110:         );
111:         require(
112:             _fundraisingDeadline > block.timestamp,
113:             "_fundraisingDeadline > block.timestamp"
114:         );
115:         require(
116:             _fundExpiry > fundraisingDeadline,
117:             "_fundExpiry > fundraisingDeadline"
118:         );
119:         name = _name;
120:         symbol = _symbol;
121:         fundraisingGoal = _fundraisingGoal;
122:         fundraisingDeadline = _fundraisingDeadline;
123:         fundExpiry = _fundExpiry;
124:         liquidityLockerFactory = ILockerFactory(_liquidityLockerFactory);
125:         maxWhitelistAmount = _maxWhitelistAmount;
126:         protocolAdmin = _protocolAdmin;
127:         maxPublicContributionAmount = _maxPublicContributionAmount;
128: 
129:         
130:         tierLimits[WhitelistTier.Platinum] = PLATINUM_DEFAULT_LIMIT;
131:         tierLimits[WhitelistTier.Gold] = GOLD_DEFAULT_LIMIT;
132:         tierLimits[WhitelistTier.Silver] = SILVER_DEFAULT_LIMIT;
133:     }

[Low-7] Use of onlyOwner functions can be lost

Resolution

In Solidity, renouncing ownership of a contract essentially transfers ownership to the zero address. This is an irreversible operation and has considerable security implications. If the renounceOwnership function is used, the contract will lose the ability to perform any operations that are limited to the owner. This can be problematic if there are any bugs, flaws, or unexpected events that require owner intervention to resolve. Therefore, in some instances, it is better to disable or omit the renounceOwnership function, and instead implement a secure transferOwnership function. This way, if necessary, ownership can be transferred to a new, trusted party without losing the potential for administrative intervention.

Num of instances: 2

Findings

Click to show findings

['7']

7: contract DaaoToken is ERC20, Ownable  // <= FOUND

['16']

16: contract Daao is Ownable, ReentrancyGuard  // <= FOUND

[Low-8] approve()/safeApprove() may revert if the current approval is not zero

Resolution

Reason: ERC20 token standard has an inherent race condition problem in its approve function. This issue particularly arises when you try to change an existing non-zero allowance. For tokens like USDT, which enforce anti-race condition checks, trying to change a non-zero allowance directly might fail.

Resolution: Always set the allowance to zero before setting a new value. By approving the spender to 0 first, it mitigates the risk of the race condition. This ensures you have a consistent allowance setup and prevent any potential transaction failure or mishap due to tokens enforcing anti-race condition checks.

Num of instances: 1

Findings

Click to show findings

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external {
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set");
282:         emit DebugLog("Starting finalizeFundraising");
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true);
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96");
311: 
312:         uint256 amountToken0ForLP;
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP);
330: 
331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP); // <= FOUND
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP); // <= FOUND
350:         emit DebugLog("Minted additional tokens for LP");
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params);
353:         emit LPTokenMinted(tokenId);
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER),
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress);
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress);
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId);
376:         emit LockerInitialized(tokenId);
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete");
380:     }

[Low-9] Constant decimal values

Resolution

The use of fixed decimal values such as 1e18 or 1e8 in Solidity contracts can lead to inaccuracies, bugs, and vulnerabilities, particularly when interacting with tokens having different decimal configurations. Not all ERC20 tokens follow the standard 18 decimal places, and assumptions about decimal places can lead to miscalculations.

Resolution: Always retrieve and use the decimals() function from the token contract itself when performing calculations involving token amounts. This ensures that your contract correctly handles tokens with any number of decimal places, mitigating the risk of numerical errors or under/overflows that could jeopardize contract integrity and user funds.

Num of instances: 1

Findings

Click to show findings

['49']

49:     uint256 public constant SUPPLY_TO_FUNDRAISERS = 1_000_000_000 * 1e18; // <= FOUND

[Low-10] Arrays can grow in size without a way to shrink them

Resolution

It's a good practice to maintain control over the size of array state variables in Solidity, especially if they are dynamically updated. If a contract includes a mechanism to push items into an array, it should ideally also provide a mechanism to remove items. This is because Solidity arrays don't automatically shrink when items are deleted - their length needs to be manually adjusted.

Ignoring this can lead to bloated and inefficient contracts. For instance, iterating over a large array can cause your contract to hit the block gas limit. Additionally, if entries are only marked for deletion but never actually removed, you may end up dealing with stale or irrelevant data, which can cause logical errors.

Therefore, implementing a method to 'pop' items from arrays helps manage contract's state, improve efficiency and prevent potential issues related to gas limits or stale data. Always ensure to handle potential underflow conditions when popping elements from the array.

Num of instances: 1

Findings

Click to show findings

['69']

69: address[] public contributors; // <= FOUND

[Low-11] Large approvals such as type(uint256).max may not work with some tokens

Resolution

The approve function in ERC-20 tokens allows a user to permit another user or contract to spend tokens on their behalf. Setting the approval to type(uint256).max is often used as a way to grant an indefinite approval, as this value is the maximum possible value of a uint256 variable in Solidity.

However, some tokens may not function as expected when type(uint256).max is used. These tokens may have an atypical implementation of the transferFrom function, which is used in combination with approve. This function might behave differently when confronted with such a high allowance, possibly due to custom logic in the contract that wasn't designed to handle these edge cases.

Moreover, tokens that have a built-in burning or fees mechanism could behave unpredictably when the maximum allowance is set. This can lead to potential vulnerabilities or misinterpretations of contract behavior.

Resolution: It's advisable to be conservative with the approve function and only approve the specific amount of tokens that need to be spent for the specific operation you're performing. If you need to provide an extensive allowance, ensure you've thoroughly analyzed the token contract to understand how it behaves with high allowances. Alternatively, consider implementing a mechanism in your contract to handle token allowances in a more dynamic way, adjusting them as needed for each operation, rather than relying on a single indefinite approval.

Num of instances: 1

Findings

Click to show findings

['36']

36:     function _handleApproval(address token, uint256 amount) internal { // <= FOUND
37:         IERC20Minimal tokenContract = IERC20Minimal(token);
38:         uint256 currentAllowance = tokenContract.allowance(
39:             msg.sender,
40:             address(this)
41:         );
42: 
43:         if (currentAllowance < amount) {
44:             if (currentAllowance > 0) {
45:                 tokenContract.approve(address(this), 0);
46:             }
47: 
48:             require( // <= FOUND
49:                 tokenContract.approve(address(this), amount),
50:                 "Approval failed"
51:             );
52: 
53:             emit ApprovalHandled(token, msg.sender, amount);
54:         }
55:     }

[Low-12] Return values not checked for approve()

Resolution

The ERC-20 token standard does not dictate that the approve function must return a value. The function signature in the ERC-20 standard is function approve(address spender, uint tokens) public returns (bool success);. However, a well-implemented ERC-20 token contract will typically have approve return a boolean value indicating whether or not the operation was successful.

It's crucial to note that not all token contracts follow this practice. Some might not return a value, or they might return a value in a non-standard way. This inconsistency among token contracts is one reason why it's important to handle token interactions carefully in your smart contracts and to check the return value of approve when possible.

Num of instances: 3

Findings

Click to show findings

['45']

45:                 tokenContract.approve(address(this), 0); // <= FOUND

['348']

348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP); // <= FOUND

['349']

349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP); // <= FOUND

[Low-13] External call recipient may consume all transaction gas

Resolution

When making external calls, the called contract can intentionally or unintentionally consume all provided gas, leading to unintended transaction reversion. To mitigate this risk, it's crucial to specify a gas limit when making the call. By using addr.call{gas: <amount>}(""), you allocate a specific amount of gas to the external call, ensuring the parent transaction has gas left for post-call operations. This approach safeguards against malevolent contracts aiming to exhaust gas and provides greater control over transaction execution.

Num of instances: 3

Findings

Click to show findings

['302']

302:         
303:         (bool success, ) = owner().call{value: amountForTreasury}(""); // <= FOUND

['424']

424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]); // <= FOUND

['453']

453:         (bool success, ) = protocolAdmin.call{value: address(this).balance}(""); // <= FOUND

[Low-14] Events may be emitted out of order due to code not follow the best practice of check-effects-interaction

Resolution

The "check-effects-interaction" pattern also impacts event ordering. When a contract doesn't adhere to this pattern, events might be emitted in a sequence that doesn't reflect the actual logical flow of operations. This can cause confusion during event tracking, potentially leading to erroneous off-chain interpretations. To rectify this, always ensure that checks are performed first, state modifications come next, and interactions with external contracts or addresses are done last. This will ensure events are emitted in a logical, consistent manner, providing a clear and accurate chronological record of on-chain actions for off-chain systems and observers.

Num of instances: 2

Findings

Click to show findings

['57']

57:     function getSwapResult( // <= FOUND
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount); // <= FOUND
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount);
111:         }
112: 
113:         emit SwapExecuted( // <= FOUND
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external { // <= FOUND
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set");
282:         emit DebugLog("Starting finalizeFundraising"); // <= FOUND
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint); // <= FOUND
293: 
294:             token.mint(contributor, tokensToMint);
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true); // <= FOUND
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96"); // <= FOUND
311: 
312:         uint256 amountToken0ForLP;
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP);
330: 
331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP);
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP);
350:         emit DebugLog("Minted additional tokens for LP"); // <= FOUND
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params);
353:         emit LPTokenMinted(tokenId); // <= FOUND
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER),
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress); // <= FOUND
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress); // <= FOUND
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId); // <= FOUND
376:         emit LockerInitialized(tokenId); // <= FOUND
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete"); // <= FOUND
380:     }

[Low-15] Critical functions should have a timelock

Resolution

Critical functions, especially those affecting protocol parameters or user funds, are potential points of failure or exploitation. To mitigate risks, incorporating a timelock on such functions can be beneficial. A timelock requires a waiting period between the time an action is initiated and when it's executed, giving stakeholders time to react, potentially vetoing malicious or erroneous changes. To implement, integrate a smart contract like OpenZeppelin's TimelockController or build a custom mechanism. This ensures governance decisions or administrative changes are transparent and allows for community or multi-signature interventions, enhancing protocol security and trustworthiness.

Num of instances: 4

Findings

Click to show findings

['382']

382:     function setDaoToken(address _daoToken) external onlyOwner  // <= FOUND

['388']

388:     function setSecondToken(address _daoToken) external onlyOwner  // <= FOUND

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public  // <= FOUND

['267']

267:     function setMaxPublicContributionAmount( // <= FOUND
268:         uint256 _maxPublicContributionAmount
269:     ) public 

[Low-16] Consider implementing two-step procedure for updating protocol addresses

Resolution

Implementing a two-step procedure for updating protocol addresses adds an extra layer of security. In such a system, the first step initiates the change, and the second step, after a predefined delay, confirms and finalizes it. This delay allows stakeholders or monitoring tools to observe and react to unintended or malicious changes. If an unauthorized change is detected, corrective actions can be taken before the change is finalized. To achieve this, introduce a "proposed address" state variable and a "delay period". Upon an update request, set the "proposed address". After the delay, if not contested, the main protocol address can be updated.

Num of instances: 2

Findings

Click to show findings

['382']

382:     function setDaoToken(address _daoToken) external onlyOwner { // <= FOUND
383:         require(_daoToken != address(0), "Invalid DAO token address");
384:         require(daoToken == address(0), "DAO token already set");
385:         daoToken = _daoToken;
386:     }

['388']

388:     function setSecondToken(address _daoToken) external onlyOwner { // <= FOUND
389:         require(_daoToken != address(0), "Invalid second token address");
390:         require(secondToken == address(0), "DAO token already set");
391:         secondToken = _daoToken;
392:     }

[Low-17] Prefer skip over revert model in iteration

Resolution

It is preferable to skip operations on an array index when a condition is not met rather than reverting the whole transaction as reverting can introduce the possiblity of malicous actors purposefully introducing array objects which fail conditional checks within for/while loops so group operations fail. As such it is recommended to simply skip such array indices over reverting unless there is a valid security or logic reason behind not doing so.

Num of instances: 2

Findings

Click to show findings

['194']

194:        for (uint256 i = 0; i < _addresses.length; i++) { // <= FOUND
195:             require(_addresses[i] != address(0), "Invalid address"); // <= FOUND
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }

['194']

194:         for (uint256 i = 0; i < _addresses.length; i++) { // <= FOUND
195:             require(_addresses[i] != address(0), "Invalid address"); // <= FOUND
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }

[Low-18] Missing contract-existence checks before low-level calls

Resolution

Low-level calls in Solidity, when made to addresses without contract code, don't fail but return a successful status. This behavior can be misleading, leading to unintended consequences in dApps. Ignoring this can potentially mean acting on false positive results. To address this, apart from the conventional zero-address check, developers should verify the existence of contract code at the target address by ensuring that the code length at the specified address (<address>.code.length) is greater than zero. By doing so, it provides a more robust validation before executing low-level calls, safeguarding against unintentional interactions with empty addresses.

Num of instances: 1

Findings

Click to show findings

['412']

412:     function execute(
413:         address[] calldata contracts,
414:         bytes[] calldata data,
415:         uint256[] calldata msgValues
416:     ) external onlyOwner {
417:         require(fundraisingFinalized);
418:         require(
419:             contracts.length == data.length && data.length == msgValues.length,
420:             "Array lengths mismatch"
421:         );
422: 
423:         for (uint256 i = 0; i < contracts.length; i++) {
424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]); // <= FOUND
425:             require(success, "Call failed");
426:         }
427:     }

[Low-19] approve will always revert as the IERC20 interface mismatch

Resolution

In Solidity, using the ERC20 approve function can be problematic with tokens like USDT, which may not fully adhere to the standard interface, potentially causing transaction reverts. To avoid issues, it’s crucial to interact directly with the token's specific ABI rather than the generic IERC20 interface. Before integrating any token, thoroughly review its contract to ensure compatibility, especially for the transfer and approve methods.

Num of instances: 1

Findings

Click to show findings

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external { // <= FOUND
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set");
282:         emit DebugLog("Starting finalizeFundraising");
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true);
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96");
311: 
312:         uint256 amountToken0ForLP;
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP);
330: 
331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP); // <= FOUND
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP);
350:         emit DebugLog("Minted additional tokens for LP");
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params);
353:         emit LPTokenMinted(tokenId);
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER),
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress);
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress);
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId);
376:         emit LockerInitialized(tokenId);
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete");
380:     }

[Low-20] Constructors missing validation

Resolution

In Solidity, when values are being assigned in constructors to unsigned or integer variables, it's crucial to ensure the provided values adhere to the protocol's specific operational boundaries as laid out in the project specifications and documentation. If the constructors lack appropriate validation checks, there's a risk of setting state variables with values that could cause unexpected and potentially detrimental behavior within the contract's operations, violating the intended logic of the protocol. This can compromise the contract's security and impact the maintainability and reliability of the system. In order to avoid such issues, it is recommended to incorporate rigorous validation checks in constructors. These checks should align with the project's defined rules and constraints, making use of Solidity's built-in require function to enforce these conditions. If the validation checks fail, the require function will cause the transaction to revert, ensuring the integrity and adherence to the protocol's expected behavior.

Num of instances: 1

Findings

Click to show findings

['95']

95:     constructor(
96:         uint256 _fundraisingGoal,
97:         string memory _name,
98:         string memory _symbol,
99:         uint256 _fundraisingDeadline,
100:         uint256 _fundExpiry,
101:         address _daoManager,
102:         address _liquidityLockerFactory,
103:         uint256 _maxWhitelistAmount,
104:         address _protocolAdmin,
105:         uint256 _maxPublicContributionAmount
106:     ) Ownable(_daoManager) {
107:         require(
108:             _fundraisingGoal > 0,
109:             "Fundraising goal must be greater than 0"
110:         );
111:         require(
112:             _fundraisingDeadline > block.timestamp,
113:             "_fundraisingDeadline > block.timestamp"
114:         );
115:         require(
116:             _fundExpiry > fundraisingDeadline,
117:             "_fundExpiry > fundraisingDeadline"
118:         );
119:         name = _name; // <= FOUND
120:         symbol = _symbol;
121:         fundraisingGoal = _fundraisingGoal;
122:         fundraisingDeadline = _fundraisingDeadline;
123:         fundExpiry = _fundExpiry;
124:         liquidityLockerFactory = ILockerFactory(_liquidityLockerFactory);
125:         maxWhitelistAmount = _maxWhitelistAmount;
126:         protocolAdmin = _protocolAdmin;
127:         maxPublicContributionAmount = _maxPublicContributionAmount;
128: 
129:         
130:         tierLimits[WhitelistTier.Platinum] = PLATINUM_DEFAULT_LIMIT;
131:         tierLimits[WhitelistTier.Gold] = GOLD_DEFAULT_LIMIT;
132:         tierLimits[WhitelistTier.Silver] = SILVER_DEFAULT_LIMIT;
133:     }

[Low-21] State variables not capped at reasonable values

Resolution

Setting boundaries on state variables in smart contracts is essential for maintaining system integrity and user protection. Without caps on values, variables could reach extremes that exploit or disrupt contract functionality, leading to potential loss or unintended consequences for users. Implementing checks for minimum and maximum permissible values can prevent such issues, ensuring variables remain within a safe and reasonable range. This practice guards against attacks aimed at destabilizing the contract, such as griefing, where attackers intentionally cause distress by exploiting vulnerabilities. Proper validation promotes contract reliability, user trust, and a healthier ecosystem by mitigating risks associated with unbounded state changes.

Num of instances: 2

Findings

Click to show findings

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public {
260:         require(
261:             msg.sender == owner() || msg.sender == protocolAdmin,
262:             "Must be owner or protocolAdmin"
263:         );
264:         maxWhitelistAmount = _maxWhitelistAmount; // <= FOUND
265:     }

['267']

267:     function setMaxPublicContributionAmount(
268:         uint256 _maxPublicContributionAmount
269:     ) public {
270:         require(
271:             msg.sender == owner() || msg.sender == protocolAdmin,
272:             "Must be owner or protocolAdmin"
273:         );
274:         maxPublicContributionAmount = _maxPublicContributionAmount; // <= FOUND
275:     }

[Low-22] Use forceApprove in place of approve

Resolution

The forceApprove function is a specialized solution addressing the issues that arise with non-standard ERC-20 tokens, such as USDT, which exhibit non-standard behavior in their approve function. These tokens may necessitate setting the allowance to 0 before changing it, may not return a boolean value from approve calls, or may return false instead of reverting on failure. OpenZeppelin's SafeERC20 library includes a forceApprove method to safely handle these peculiarities, ensuring that smart contracts can interact with such tokens without transaction failures. It rectifies inconsistencies by ensuring that all token interactions, including approvals, adhere to expected standards, even when the underlying tokens do not.

Num of instances: 4

Findings

Click to show findings

['45']

45:                 tokenContract.approve(address(this), 0); // <= FOUND

['48']

48:             require(
49:                 tokenContract.approve(address(this), amount), // <= FOUND
50:                 "Approval failed"
51:             );

['348']

348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP); // <= FOUND

['349']

349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP); // <= FOUND

[Low-23] Functions calling contracts/addresses with transfer hooks are missing reentrancy guards

Resolution

While adherence to the check-effects-interaction pattern is commendable, the absence of a reentrancy guard in functions, especially where transfer hooks might be present, can expose the protocol users to risks of read-only reentrancies. Such reentrancy vulnerabilities can be exploited to execute malicious actions even without altering the contract state. Without a reentrancy guard, the only potential mitigation would be to blocklist the entire protocol - an extreme and disruptive measure. Therefore, incorporating a reentrancy guard into these functions is vital to bolster security, as it helps protect against both traditional reentrancy attacks and read-only reentrancies, ensuring robust and safe protocol operations.

Num of instances: 2

Findings

Click to show findings

['57']

57:     function getSwapResult( // <= FOUND
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount);
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta"); // <= FOUND
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta"); // <= FOUND
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require( // <= FOUND
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount);
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

['57']

57:     function getSwapResult(
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount);
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount); // <= FOUND
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

[Low-24] Solidity version 0.8.20 won't work on all chains due to PUSH0

Resolution

Solidity version 0.8.20 uses the new Shanghai EVM which introduces the PUSH0 opcode, this may not be implemented on all chains and L2 thus reducing the portability and compatability of the code. Consider using a earlier solidity version.

Num of instances: 2

Findings

Click to show findings

['2']

2: pragma solidity ^0.8.0; // <= FOUND

['2']

2: pragma solidity ^0.8.18; // <= FOUND

[Low-25] Missing events in functions that are either setters, privileged or voting related

Resolution

Sensitive setter functions in smart contracts often alter critical state variables. Without events emitted in these functions, external observers or dApps cannot easily track or react to these state changes. Missing events can obscure contract activity, hampering transparency and making integration more challenging. To resolve this, incorporate appropriate event emissions within these functions. Events offer an efficient way to log crucial changes, aiding in real-time tracking and post-transaction verification.

Num of instances: 12

Findings

Click to show findings

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public 

['267']

267:     function setMaxPublicContributionAmount(
268:         uint256 _maxPublicContributionAmount
269:     ) public 

['382']

382:     function setDaoToken(address _daoToken) external onlyOwner 

['388']

388:     function setSecondToken(address _daoToken) external onlyOwner 

['412']

412:     function execute(
413:         address[] calldata contracts,
414:         bytes[] calldata data,
415:         uint256[] calldata msgValues
416:     ) external onlyOwner 

['429']

429:     function extendFundExpiry(uint256 newFundExpiry) external onlyOwner 

['13']

13:     function mint(address _to, uint256 _amount) external onlyOwner 

['435']

435:     function extendFundraisingDeadline(
436:         uint256 newFundraisingDeadline
437:     ) external 

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public { // <= FOUND
260:         require(
261:             msg.sender == owner() || msg.sender == protocolAdmin,
262:             "Must be owner or protocolAdmin"
263:         );
264:         maxWhitelistAmount = _maxWhitelistAmount;
265:     }

['267']

267:     function setMaxPublicContributionAmount( // <= FOUND
268:         uint256 _maxPublicContributionAmount
269:     ) public {
270:         require(
271:             msg.sender == owner() || msg.sender == protocolAdmin,
272:             "Must be owner or protocolAdmin"
273:         );
274:         maxPublicContributionAmount = _maxPublicContributionAmount;
275:     }

['382']

382:     function setDaoToken(address _daoToken) external onlyOwner { // <= FOUND
383:         require(_daoToken != address(0), "Invalid DAO token address");
384:         require(daoToken == address(0), "DAO token already set");
385:         daoToken = _daoToken;
386:     }

['388']

388:     function setSecondToken(address _daoToken) external onlyOwner { // <= FOUND
389:         require(_daoToken != address(0), "Invalid second token address");
390:         require(secondToken == address(0), "DAO token already set");
391:         secondToken = _daoToken;
392:     }

[Low-26] Unsafe use of transfer()/transferFrom() with IERC20

Resolution

SafeTransfer should be used in place of Transfer for Solidity contracts to ensure robust security and error handling. Unlike the basic Transfer function, SafeTransfer incorporates safeguards against potential smart contract vulnerabilities, such as reentrancy attacks and unexpected token loss. By automatically validating the recipient's ability to receive tokens and reverting transactions in case of failures,

Num of instances: 4

Findings

Click to show findings

['57']

57:     function getSwapResult( // <= FOUND
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount);
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount); // <= FOUND
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

['125']

125:     function uniswapV3SwapCallback( // <= FOUND
126:         int256 amount0Delta,
127:         int256 amount1Delta,
128:         bytes calldata data
129:     ) external override {
130:         address sender = abi.decode(data, (address));
131: 
132:         if (amount0Delta > 0) {
133:             IERC20Minimal(ICLPool(msg.sender).token0()).transferFrom( // <= FOUND
134:                 sender,
135:                 msg.sender,
136:                 uint256(amount0Delta)
137:             );
138:         } else if (amount1Delta > 0) {
139:             IERC20Minimal(ICLPool(msg.sender).token1()).transferFrom(
140:                 sender,
141:                 msg.sender,
142:                 uint256(amount1Delta)
143:             );
144:         }
145:     }

['135']

135:     function contribute() public payable nonReentrant { // <= FOUND
136:         require(!goalReached, "Goal already reached");
137:         require(block.timestamp < fundraisingDeadline, "Deadline hit");
138:         require(msg.value > 0, "Contribution must be greater than 0");
139: 
140:         
141:         WhitelistTier userTier = userTiers[msg.sender];
142:         require(userTier != WhitelistTier.None, "Not whitelisted");
143:         
144:         uint256 userLimit = tierLimits[userTier];
145:         require(
146:             contributions[msg.sender] + msg.value <= userLimit,
147:             "Exceeding tier limit"
148:         );
149: 
150:         if (maxWhitelistAmount > 0) {
151:             require(
152:                 contributions[msg.sender] + msg.value <= maxWhitelistAmount,
153:                 "Exceeding maxWhitelistAmount"
154:             );
155:         } else if (maxPublicContributionAmount > 0) {
156:             require(
157:                 contributions[msg.sender] + msg.value <=
158:                     maxPublicContributionAmount,
159:                 "Exceeding maxPublicContributionAmount"
160:             );
161:         }
162: 
163:         uint256 effectiveContribution = msg.value;
164:         if (totalRaised + msg.value > fundraisingGoal) {
165:             effectiveContribution = fundraisingGoal - totalRaised;
166:             payable(msg.sender).transfer(msg.value - effectiveContribution); // <= FOUND
167:         }
168: 
169:         if (contributions[msg.sender] == 0) {
170:             contributors.push(msg.sender);
171:         }
172: 
173:         contributions[msg.sender] += effectiveContribution;
174:         totalRaised += effectiveContribution;
175: 
176:         if (totalRaised == fundraisingGoal) {
177:             goalReached = true;
178:         }
179: 
180:         emit Contribution(msg.sender, effectiveContribution);
181:     }

['395']

395:     function refund() external nonReentrant { // <= FOUND
396:         require(!goalReached, "Fundraising goal was reached");
397:         require(
398:             block.timestamp > fundraisingDeadline,
399:             "Deadline not reached yet"
400:         );
401:         require(contributions[msg.sender] > 0, "No contributions to refund");
402: 
403:         uint256 contributedAmount = contributions[msg.sender];
404:         contributions[msg.sender] = 0;
405: 
406:         payable(msg.sender).transfer(contributedAmount); // <= FOUND
407: 
408:         emit Refund(msg.sender, contributedAmount);
409:     }

[Low-27] Avoid floating pragma in non interface/library files

Resolution

Avoid using floating pragmas in non-interface and non-library files to ensure contract compatibility and security. Floating pragmas like pragma solidity ^0.8.0; allow any compiler version that matches the specified range. While this can provide flexibility, it risks introducing unexpected behavior or vulnerabilities from future compiler versions. Instead, specify a fixed pragma version, such as pragma solidity 0.8.0;, to guarantee consistent behavior and security across deployments. This practice ensures that your contracts are tested and verified against a specific compiler version, reducing the risk of unforeseen issues and maintaining code reliability.

Num of instances: 3

Findings

Click to show findings

['8']

8: contract CLPoolRouter is ICLSwapCallback 

['16']

16: contract Daao is Ownable, ReentrancyGuard 

['7']

7: contract DaaoToken is ERC20, Ownable 

[Low-28] Read only reentrancy risk detected

Resolution

A read-only reentrancy vulnerability arises when a contract's view or pure function is re-entered during its execution, potentially leading to inconsistent state readings or unintended behaviors. Although these functions don't modify the contract's state, they can still be exploited if they depend on external calls that may re-enter the contract. This can result in the function returning outdated or manipulated data, which, when relied upon by other contracts or systems, can cause incorrect operations or financial discrepancies.

To mitigate this risk, it's essential to implement reentrancy guards even in read-only functions, especially when they involve external calls. Additionally, adhering to the checks-effects-interactions pattern and minimizing external calls within view functions can further reduce the potential for such vulnerabilities. Regular security audits and thorough testing are also crucial to identify and address any reentrancy issues before deployment.

Num of instances: 1

Findings

Click to show findings

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external {
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set");
282:         emit DebugLog("Starting finalizeFundraising");
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint); // <= FOUND
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true);
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96");
311: 
312:         uint256 amountToken0ForLP;
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP); // <= FOUND
330: 
331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP);
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP);
350:         emit DebugLog("Minted additional tokens for LP");
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params); // <= FOUND
353:         emit LPTokenMinted(tokenId);
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER),
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress);
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress);
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId);
376:         emit LockerInitialized(tokenId);
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete");
380:     }

[Low-29] Common tokens such as WETH9 work differently on chains such a Blast which isn't taken into account during transfer calls.

Resolution

There is a difference on chains such as Blast on how WETH9 is implemented. On most chains the WETH9 contract contains handling for the case where src == msg.sender, however on chains such as Blast, Arbitrum and Fantom. This isn’t the case. Failing to take this discrepancy into account can results in the protocol not functioning as intended on these chains which can have drastic results. Particularly in cases where the contract interacts with it’s own WETH allowance through transferFrom calls, in such cases these can fail if the contract fails to approve itself to use it’s WETH balance which normally isn't done as most WETH contracts do not require approval for instances where src is msg.sender.

Num of instances: 1

Findings

Click to show findings

['57']

57:     function getSwapResult( // <= FOUND
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount); // <= FOUND
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount);
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

[Low-30] Consider a uptime feed on L2 deployments to prevent issues caused by downtime

Resolution

In L2 deployments, incorporating an uptime feed is crucial to mitigate issues arising from sequencer downtime. Downtime can disrupt services, leading to transaction failures or incorrect data readings, affecting overall system reliability. By integrating an uptime feed, you gain insight into the operational status of the L2 network, enabling proactive measures like halting sensitive operations or alerting users. This approach ensures that your contract behaves predictably and securely during network outages, enhancing the robustness and reliability of your decentralized application, which is especially important in mission-critical or high-stakes environments.

Num of instances: 3

Findings

Click to show findings

['8']

8: contract CLPoolRouter is ICLSwapCallback  // <= FOUND

['16']

16: contract Daao is Ownable, ReentrancyGuard  // <= FOUND

['7']

7: contract DaaoToken is ERC20, Ownable  // <= FOUND

[NonCritical-1] Addresses shouldn't be hard-coded

Num of instances: 1

Findings

Click to show findings

['38']

38:         INonfungiblePositionManager(0x991d5546C4B442B4c5fdc4c8B8b8d131DEB24702);

[NonCritical-2] Greater than comparisons made on state uints that can be set to max

Resolution

When state variables (uints) that can be set to their maximum value (type(uint256).max for example) are used in "greater than" comparisons, it introduces a risk of logic errors. If the state variable ever reaches this max value, any comparison expecting it to increment further will fail. This can halt or disrupt contract functionality. To avoid this, implement checks to ensure that the state variable doesn't exceed a certain threshold below the max value.

Num of instances: 3

Findings

Click to show findings

['395']

395:     function refund() external nonReentrant {
396:         require(!goalReached, "Fundraising goal was reached");
397:         require(
398:             block.timestamp > fundraisingDeadline, // <= FOUND
399:             "Deadline not reached yet"
400:         );
401:         require(contributions[msg.sender] > 0, "No contributions to refund");
402: 
403:         uint256 contributedAmount = contributions[msg.sender];
404:         contributions[msg.sender] = 0;
405: 
406:         payable(msg.sender).transfer(contributedAmount);
407: 
408:         emit Refund(msg.sender, contributedAmount);
409:     }

['435']

435:     function extendFundraisingDeadline(
436:         uint256 newFundraisingDeadline
437:     ) external {
438:         require(
439:             msg.sender == owner() || msg.sender == protocolAdmin,
440:             "Must be owner or protocolAdmin"
441:         );
442:         require(!goalReached, "Fundraising goal was reached");
443:         require(
444:             newFundraisingDeadline > fundraisingDeadline, // <= FOUND
445:             "new fundraising deadline must be > old one"
446:         );
447:         fundraisingDeadline = newFundraisingDeadline;
448:     }

['429']

429:     function extendFundExpiry(uint256 newFundExpiry) external onlyOwner {
430:         require(newFundExpiry > fundExpiry, "Must choose later fund expiry"); // <= FOUND
431:         fundExpiry = newFundExpiry;
432:         ILocker(liquidityLocker).extendFundExpiry(newFundExpiry);
433:     }

[NonCritical-3] Floating pragma should be avoided

Num of instances: 2

Findings

Click to show findings

['2']

2: pragma solidity ^0.8.0; // <= FOUND

['2']

2: pragma solidity ^0.8.18; // <= FOUND

[NonCritical-4] Require statements should have error string

Resolution

Adding error strings to require statements in Solidity contracts, although not mandatory, enhances error handling, debugging, and overall contract maintainability. Providing a descriptive error message with each require statement helps identify the specific reason for a transaction failure, making it easier for developers to troubleshoot issues and for users to understand the cause of a revert. Including error strings improves code readability and fosters transparency, as the logic and conditions behind each requirement are clearly communicated

Num of instances: 1

Findings

Click to show findings

['417']

417: require(fundraisingFinalized);

[NonCritical-5] Events regarding state variable changes should emit the previous state variable value

Resolution

Modify such events to contain the previous value of the state variable as demonstrated in the example below

Num of instances: 1

Findings

Click to show findings

['84']

84: event TierLimitUpdated(WhitelistTier indexed teir, uint256 _newLimit);

[NonCritical-6] In functions which accept an address as a parameter, there should be a zero address check to prevent bugs

Resolution

In smart contract development, especially with Solidity, it's crucial to validate inputs to functions. When a function accepts an Ethereum address as a parameter, implementing a zero address check (i.e., ensuring the address is not 0x0) is a best practice to prevent potential bugs and vulnerabilities. The zero address (0x0) is a default value and generally indicates an uninitialized or invalid state. Passing the zero address to certain functions can lead to unintended behaviors, like funds getting locked permanently or transactions failing silently. By checking for and rejecting the zero address, developers can ensure that the function operates as intended and interacts only with valid Ethereum addresses. This check enhances the contract's robustness and security.

Num of instances: 6

Findings

Click to show findings

['29']

29:     function checkApproval(
30:         address token,
31:         address owner
32:     ) external view returns (uint256) 

['36']

36:     function _handleApproval(address token, uint256 amount) internal 

['57']

57:     function getSwapResult(
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     

['412']

412:     function execute(
413:         address[] calldata contracts,
414:         bytes[] calldata data,
415:         uint256[] calldata msgValues
416:     ) external onlyOwner 

['464']

464:     function onERC721Received(
465:         address,
466:         address,
467:         uint256,
468:         bytes calldata
469:     ) external pure returns (bytes4) 

['13']

13:     function mint(address _to, uint256 _amount) external onlyOwner 

[NonCritical-7] Default int values are manually set

Resolution

In instances where a new variable is defined, there is no need to set it to it's default value.

Num of instances: 3

Findings

Click to show findings

['194']

194:         for (uint256 i = 0; i < _addresses.length; i++) { // <= FOUND

['287']

287:         
288:         for (uint256 i = 0; i < contributors.length; i++) { // <= FOUND

['423']

423:         for (uint256 i = 0; i < contracts.length; i++) { // <= FOUND

[NonCritical-8] Ownable2Step should be used in place of Ownable

Resolution

Ownable2Step further prevents risks posed by centralised privileges as there is a smaller likelihood of the owner being wrongfully changed

Num of instances: 2

Findings

Click to show findings

['7']

7: contract DaaoToken is ERC20, Ownable  // <= FOUND

['16']

16: contract Daao is Ownable, ReentrancyGuard  // <= FOUND

[NonCritical-9] Specific imports should be used where possible so only used code is imported

Resolution

In many cases only some functionality is used from an import. In such cases it makes more sense to use {} to specify what to import and thus save gas whilst improving readability

Num of instances: 4

Findings

Click to show findings

['4']

4: import "./interfaces/ICLPool.sol";

['5']

5: import "./interfaces/IERC20Minimal.sol";

['6']

6: import "./interfaces/callback/ICLSwapCallback.sol";

['7']

7: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

[NonCritical-10] Old Solidity version

Resolution

Using outdated Solidity versions can lead to security risks and inefficiencies. It's recommended to adopt newer versions, ideally the latest, which as of now is 0.8.24. This ensures access to the latest bug fixes, features, and optimizations, particularly crucial for Layer 2 deployments. Regular updates to versions like 0.8.19 or later, up to 0.8.24, enhance contract security and performance.

Num of instances: 2

Findings

Click to show findings

['2']

2: pragma solidity ^0.8.0; // <= FOUND

['2']

2: pragma solidity ^0.8.18;

[NonCritical-11] Not all event definitions are utilizing indexed variables.

Resolution

Try to index as much as three variables in event declarations as this is more gas efficient when done on value type variables (uint, address etc) however not for bytes and string variables

Num of instances: 11

Findings

Click to show findings

['12']

12: event SwapExecuted( // <= FOUND
13:         address indexed pool,
14:         address indexed sender,
15:         bool zeroForOne,
16:         int256 amountSpecified,
17:         uint160 sqrtPriceLimitX96,
18:         int256 amount0Delta,
19:         int256 amount1Delta,
20:         uint256 outputAmount
21:     );

['23']

23: event ApprovalHandled( // <= FOUND
24:         address indexed token,
25:         address indexed owner,
26:         uint256 amount
27:     );

['73']

73: event LPTokenMinted(uint256 tokenId); // <= FOUND

['75']

75: event LockerInitialized(uint256 tokenId); // <= FOUND

['76']

76: event FundraisingFinalized(bool success); // <= FOUND

['79']

79: event Refund(address indexed contributor, uint256 amount); // <= FOUND

['80']

80: event TokenApproved(address indexed token, uint256 amount); // <= FOUND

['82']

82: event Contribution(address indexed contributor, uint256 amount); // <= FOUND

['83']

83: event MintDetails(address indexed contributor, uint256 tokensToMint); // <= FOUND

['85']

85: event TokenTransferredToLocker(uint256 tokenId, address lockerAddress); // <= FOUND

['86']

86: event MintParamsCreated( // <= FOUND
87:         uint256 tokenId,
88:         address token0,
89:         address token1,
90:         uint256 liquidity
91:     );

[NonCritical-12] Contracts should have all public/external functions exposed by interfaces

Resolution

Contracts should expose all public and external functions through interfaces. This practice ensures a clear and consistent definition of how the contract can be interacted with, promoting better transparency and integration.

Num of instances: 19

Findings

Click to show findings

['29']

29:     function checkApproval(
30:         address token,
31:         address owner
32:     ) external view returns (uint256) 

['57']

57:     function getSwapResult(
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     

['183']

183:     function addToWhitelist(
184:         address[] calldata _addresses,
185:         WhitelistTier[] calldata _tiers
186:     ) external 

['208']

208:     function updateTierLimit(WhitelistTier _tier, uint256 _newLimit) external 

['233']

233:     function removeFromWhitelist(address removedAddress) external 

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external 

['382']

382:     function setDaoToken(address _daoToken) external onlyOwner 

['388']

388:     function setSecondToken(address _daoToken) external onlyOwner 

['395']

395:     function refund() external nonReentrant 

['412']

412:     function execute(
413:         address[] calldata contracts,
414:         bytes[] calldata data,
415:         uint256[] calldata msgValues
416:     ) external onlyOwner 

['429']

429:     function extendFundExpiry(uint256 newFundExpiry) external onlyOwner 

['435']

435:     function extendFundraisingDeadline(
436:         uint256 newFundraisingDeadline
437:     ) external 

['450']

450:     function emergencyEscape() external 

['464']

464:     function onERC721Received(
465:         address,
466:         address,
467:         uint256,
468:         bytes calldata
469:     ) external pure returns (bytes4) 

['13']

13:     function mint(address _to, uint256 _amount) external onlyOwner 

['135']

135:     function contribute() public payable nonReentrant 

['229']

229:     function getWhitelistLength() public view returns (uint256) 

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public 

['267']

267:     function setMaxPublicContributionAmount(
268:         uint256 _maxPublicContributionAmount
269:     ) public 

[NonCritical-13] Functions within contracts are not ordered according to the solidity style guide

Resolution

The following order should be used within contracts

constructor

receive function (if exists)

fallback function (if exists)

external

public

internal

private

Rearrange the contract functions and contructors to fit this ordering

Num of instances: 2

Findings

Click to show findings

['8']

8: contract CLPoolRouter is ICLSwapCallback  // <= FOUND

['16']

16: contract Daao is Ownable, ReentrancyGuard  // <= FOUND

[NonCritical-14] Emits without msg.sender parameter

Resolution

In Solidity, when msg.sender plays a crucial role in a function's logic, it's important for transparency and auditability that any events emitted by this function include msg.sender as a parameter. This practice enhances the traceability and accountability of transactions, allowing users and external observers to easily track who initiated a particular action. Including msg.sender in event logs helps in creating a clear and verifiable record of interactions with the contract, thereby increasing user trust and facilitating easier debugging and analysis of contract behavior. It's a key aspect of writing clear, transparent, and user-friendly smart contracts.

Num of instances: 3

Findings

Click to show findings

['183']

183:     function addToWhitelist(
184:         address[] calldata _addresses,
185:         WhitelistTier[] calldata _tiers
186:     ) external {
187:         require(
188:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
189:             "Must be owner or protocolAdmin"
190:         );
191:         require(_addresses.length == _tiers.length, "Arrays length mismatch");
192:         require(_addresses.length > 0, "Empty arrays");
193: 
194:         for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }
206:     }

['208']

208:     function updateTierLimit(WhitelistTier _tier, uint256 _newLimit) external {
209:         require(
210:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
211:             "Not authorized"
212:         );
213:         require(_tier != WhitelistTier.None, "Invalid tier");
214:         require(_newLimit > 0, "Invalid limit");
215: 
216:         tierLimits[_tier] = _newLimit;
217: 
218:         if (_tier == WhitelistTier.Gold) {
219:             GOLD_DEFAULT_LIMIT = _newLimit;
220:         } else if (_tier == WhitelistTier.Silver) {
221:             SILVER_DEFAULT_LIMIT = _newLimit;
222:         } else {
223:             PLATINUM_DEFAULT_LIMIT = _newLimit;
224:         }
225: 
226:         emit TierLimitUpdated(_tier, _newLimit);
227:     }

['233']

233:     function removeFromWhitelist(address removedAddress) external {
234:         require(
235:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
236:             "Must be owner or protocolAdmin"
237:         );
238: 
239:         require(removedAddress != address(0), "Invalid address");
240:         require(
241:             userTiers[removedAddress] != WhitelistTier.None,
242:             "Address not whitelisted"
243:         );
244:         userTiers[removedAddress] = WhitelistTier.None;
245: 
246:         
247: 
248:         
249:         
250:         
251:         
252:         
253:         
254:         
255: 
256:         emit RemoveWhitelist(removedAddress);
257:     }

[NonCritical-15] Unused state variables present

Resolution

If these serve no purpose, they should be safely removed

Num of instances: 5

Findings

Click to show findings

['9']

9:  int256 private _amount0Delta; // <= FOUND

['10']

10:  int256 private _amount1Delta; // <= FOUND

[]

enum WhitelistTier {
 None, Platinum, Gold, Silver }
 uint24 public constant UNI_V3_FEE = 500; // <= FOUND

['30']

30:  uint256 public constant TREASURY_PERCENTAGE = 90; // <= FOUND

[]

 IVelodromeFactory public constant Velodrome_factory = IVelodromeFactory(0x04625B046C69577EfC40e6c0Bb83CDBAfab5a55F); // <= FOUND

[NonCritical-16] Constants should be on the left side of the comparison

Resolution

Putting constants on the left side of a comparison operator like == or < is a best practice known as "Yoda conditions", which can help prevent accidental assignment instead of comparison. In some programming languages, if a variable is mistakenly put on the left with a single = instead of ==, it assigns the constant's value to the variable without any compiler error. However, doing this with the constant on the left would generate an error, as constants cannot be assigned values. Although Solidity's static typing system prevents accidental assignments within conditionals, adopting this practice enhances code readability and consistency, especially when developers are working across multiple languages that support this convention.

Num of instances: 5

Findings

Click to show findings

['169']

169:         if (contributions[msg.sender] == 0)  // <= FOUND

['102']

102:         if (outputAmount > 0)  // <= FOUND

['132']

132:         if (amount0Delta > 0)  // <= FOUND

['150']

150:         if (maxWhitelistAmount > 0)  // <= FOUND

['44']

44:           if (currentAllowance > 0)  // <= FOUND

[NonCritical-17] Both immutable and constant state variables should be CONSTANT_CASE

Resolution

Make found instants CAPITAL_CASE

Num of instances: 1

Findings

Click to show findings

[]

IVelodromeFactory public constant Velodrome_factory = IVelodromeFactory(0x04625B046C69577EfC40e6c0Bb83CDBAfab5a55F); // <= FOUND

[NonCritical-18] Using zero as a parameter

Resolution

Taking 0 as a valid argument in Solidity without checks can lead to severe security issues. A historical example is the infamous 0x0 address bug where numerous tokens were lost. This happens because '0' can be interpreted as an uninitialized address, leading to transfers to the '0x0' address, effectively burning tokens. Moreover, 0 as a denominator in division operations would cause a runtime exception. It's also often indicative of a logical error in the caller's code. It's important to always validate input and handle edge cases like 0 appropriately. Use require() statements to enforce conditions and provide clear error messages to facilitate debugging and safer code.

Num of instances: 1

Findings

Click to show findings

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external { // <= FOUND
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set");
282:         emit DebugLog("Starting finalizeFundraising");
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true);
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96");
311: 
312:         uint256 amountToken0ForLP;
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP);
330: 
331:         INonfungiblePositionManager.MintParams // <= FOUND
332:             memory params = INonfungiblePositionManager.MintParams( // <= FOUND
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP);
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP);
350:         emit DebugLog("Minted additional tokens for LP");
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params);
353:         emit LPTokenMinted(tokenId);
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER),
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress);
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress);
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId);
376:         emit LockerInitialized(tokenId);
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete");
380:     }

[NonCritical-19] Use of non-named numeric constants

Resolution

Magic numbers should be avoided in Solidity code to enhance readability, maintainability, and reduce the likelihood of errors. Magic numbers are hard-coded values with no clear meaning or context, which can create confusion and make the code harder to understand for developers. Using well-defined constants or variables with descriptive names instead of magic numbers not only clarifies the purpose and significance of the value but also simplifies code updates and modifications.

Num of instances: 2

Findings

Click to show findings

['299']

299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;

['329']

329:         token.mint(address(this), 4 * amountForLP);

[NonCritical-20] Unused structs present

Resolution

If these serve no purpose, they should be safely removed

Num of instances: 2

Findings

Click to show findings

['83']

83: struct ExactInputSingleParams { // <= FOUND
84:     address tokenIn;
85:     address tokenOut;
86:     uint24 fee;
87:     address recipient;
88:     uint256 amountIn;
89:     uint256 amountOutMinimum;
90:     uint160 sqrtPriceLimitX96;
91: }

['20']

20:     struct CollectParams { // <= FOUND
21:         uint256 tokenId;
22:         address recipient;
23:         uint128 amount0Max;
24:         uint128 amount1Max;
25:     }

[NonCritical-21] Empty bytes check is missing

Resolution

When developing smart contracts in Solidity, it's crucial to validate the inputs of your functions. This includes ensuring that the bytes parameters are not empty, especially when they represent crucial data such as addresses, identifiers, or raw data that the contract needs to process.

Missing empty bytes checks can lead to unexpected behaviour in your contract. For instance, certain operations might fail, produce incorrect results, or consume unnecessary gas when performed with empty bytes. Moreover, missing input validation can potentially expose your contract to malicious activity, including exploitation of unhandled edge cases.

To mitigate these issues, always validate that bytes parameters are not empty when the logic of your contract requires it.

Num of instances: 2

Findings

Click to show findings

['125']

125:     function uniswapV3SwapCallback(
126:         int256 amount0Delta,
127:         int256 amount1Delta,
128:         bytes calldata data
129:     ) external override {
130:         address sender = abi.decode(data, (address));
131: 
132:         if (amount0Delta > 0) {
133:             IERC20Minimal(ICLPool(msg.sender).token0()).transferFrom(
134:                 sender,
135:                 msg.sender,
136:                 uint256(amount0Delta)
137:             );
138:         } else if (amount1Delta > 0) {
139:             IERC20Minimal(ICLPool(msg.sender).token1()).transferFrom(
140:                 sender,
141:                 msg.sender,
142:                 uint256(amount1Delta)
143:             );
144:         }
145:     }

['464']

464:     function onERC721Received(
465:         address,
466:         address,
467:         uint256,
468:         bytes calldata
469:     ) external pure returns (bytes4) {
470:         return IERC721Receiver.onERC721Received.selector;
471:     }

[NonCritical-22] Whitespace in expressions

Resolution

Avoid unnecessary whitespace in contract lines such as ' ;' and ', )'

Num of instances: 5

Findings

Click to show findings

['92']

92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0(); // <= FOUND

['302']

302:         
303:         (bool success, ) = owner().call{value: amountForTreasury}(""); // <= FOUND

['352']

352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params); // <= FOUND

['424']

424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]); // <= FOUND

['453']

453:         (bool success, ) = protocolAdmin.call{value: address(this).balance}(""); // <= FOUND

[NonCritical-23] call bypasses function existence check, type checking and argument packing

Resolution

Using the .call method in Solidity enables direct communication with an address, bypassing function existence checks, type checking, and argument packing. While this can save gas and provide flexibility, it can also introduce security risks and potential errors. The absence of these checks can lead to unexpected behavior if the callee contract's interface changes or if the input parameters are not crafted with care. The resolution to these issues is to use Solidity's high-level interface for calling functions when possible, as it automatically manages these aspects. If using .call is necessary, ensure that the inputs are carefully validated and that awareness of the called contract's behavior is maintained.

Num of instances: 1

Findings

Click to show findings

['424']

424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]); // <= FOUND

[NonCritical-24] Cyclomatic complexity in functions

Resolution

Cyclomatic complexity is a software metric used to measure the complexity of a program. It quantifies the number of linearly independent paths through a program's source code, giving an idea of how complex the control flow is. High cyclomatic complexity may indicate a higher risk of defects and can make the code harder to understand, test, and maintain. It often suggests that a function or method is trying to do too much, and a refactor might be needed. By breaking down complex functions into smaller, more focused pieces, you can improve readability, ease of testing, and overall maintainability.

Num of instances: 1

Findings

Click to show findings

['135']

135:     function contribute() public payable nonReentrant { // <= FOUND
136:         require(!goalReached, "Goal already reached");
137:         require(block.timestamp < fundraisingDeadline, "Deadline hit");
138:         require(msg.value > 0, "Contribution must be greater than 0");
139: 
140:         
141:         WhitelistTier userTier = userTiers[msg.sender];
142:         require(userTier != WhitelistTier.None, "Not whitelisted");
143:         
144:         uint256 userLimit = tierLimits[userTier];
145:         require(
146:             contributions[msg.sender] + msg.value <= userLimit,
147:             "Exceeding tier limit"
148:         );
149: 
150:         if (maxWhitelistAmount > 0) {
151:             require(
152:                 contributions[msg.sender] + msg.value <= maxWhitelistAmount,
153:                 "Exceeding maxWhitelistAmount"
154:             );
155:         } else if (maxPublicContributionAmount > 0) {
156:             require(
157:                 contributions[msg.sender] + msg.value <=
158:                     maxPublicContributionAmount,
159:                 "Exceeding maxPublicContributionAmount"
160:             );
161:         }
162: 
163:         uint256 effectiveContribution = msg.value;
164:         if (totalRaised + msg.value > fundraisingGoal) {
165:             effectiveContribution = fundraisingGoal - totalRaised;
166:             payable(msg.sender).transfer(msg.value - effectiveContribution);
167:         }
168: 
169:         if (contributions[msg.sender] == 0) {
170:             contributors.push(msg.sender);
171:         }
172: 
173:         contributions[msg.sender] += effectiveContribution;
174:         totalRaised += effectiveContribution;
175: 
176:         if (totalRaised == fundraisingGoal) {
177:             goalReached = true;
178:         }
179: 
180:         emit Contribution(msg.sender, effectiveContribution);
181:     }

[NonCritical-25] Unused events present

Resolution

If these serve no purpose, they should be safely removed

Num of instances: 4

Findings

Click to show findings

['74']

74: event PoolCreated(address indexed pool); // <= FOUND

['77']

77: event PoolInitialized(uint160 sqrtPriceX96); // <= FOUND

['80']

80: event TokenApproved(address indexed token, uint256 amount); // <= FOUND

['86']

86: event MintParamsCreated( // <= FOUND
87:         uint256 tokenId,
88:         address token0,
89:         address token1,
90:         uint256 liquidity
91:     );

[NonCritical-26] Unused import

Resolution

If these serve no purpose, they should be safely removed

Num of instances: 1

Findings

Click to show findings

['6']

6: import {ICLFactory} from "./interfaces/ICLFactory.sol"; // <= FOUND

[NonCritical-27] A event should be emitted if a non immutable state variable is set in a constructor

Num of instances: 1

Findings

Click to show findings

['95']

95:     constructor(
96:         uint256 _fundraisingGoal,
97:         string memory _name,
98:         string memory _symbol,
99:         uint256 _fundraisingDeadline,
100:         uint256 _fundExpiry,
101:         address _daoManager,
102:         address _liquidityLockerFactory,
103:         uint256 _maxWhitelistAmount,
104:         address _protocolAdmin,
105:         uint256 _maxPublicContributionAmount
106:     ) Ownable(_daoManager) {
107:         require(
108:             _fundraisingGoal > 0,
109:             "Fundraising goal must be greater than 0"
110:         );
111:         require(
112:             _fundraisingDeadline > block.timestamp,
113:             "_fundraisingDeadline > block.timestamp"
114:         );
115:         require(
116:             _fundExpiry > fundraisingDeadline,
117:             "_fundExpiry > fundraisingDeadline"
118:         );
119:         name = _name;
120:         symbol = _symbol;
121:         fundraisingGoal = _fundraisingGoal;
122:         fundraisingDeadline = _fundraisingDeadline;
123:         fundExpiry = _fundExpiry;
124:         liquidityLockerFactory = ILockerFactory(_liquidityLockerFactory); // <= FOUND
125:         maxWhitelistAmount = _maxWhitelistAmount;
126:         protocolAdmin = _protocolAdmin;
127:         maxPublicContributionAmount = _maxPublicContributionAmount;
128: 
129:         
130:         tierLimits[WhitelistTier.Platinum] = PLATINUM_DEFAULT_LIMIT;
131:         tierLimits[WhitelistTier.Gold] = GOLD_DEFAULT_LIMIT;
132:         tierLimits[WhitelistTier.Silver] = SILVER_DEFAULT_LIMIT;
133:     }

[NonCritical-28] Non constant/immutable state variables are missing a setter post deployment

Resolution

Non-constant or non-immutable state variables lacking a setter function can create inflexibility in contract operations. If there's no way to update these variables post-deployment, the contract might not adapt to changing conditions or requirements, which can be a significant drawback, especially in upgradable or long-lived contracts. To resolve this, implement setter functions guarded by appropriate access controls, like onlyOwner or similar modifiers, so that these variables can be updated as required while maintaining security. This enables smoother contract maintenance and feature upgrades.

Num of instances: 4

Findings

Click to show findings

['9']

9:  int256 private _amount0Delta;

['10']

10:  int256 private _amount1Delta;

['44']

44:  uint256 public fundraisingGoal;

['50']

50:  uint8 public lpFeesCut = 60;

[NonCritical-29] Use transfer libraries instead of low level calls

Resolution

Using transfer libraries like OpenZeppelin's Address.sendValue is preferred over low-level calls for transferring Ether in Solidity. These libraries provide clearer, more semantically meaningful methods compared to low-level call() functions. They encapsulate best practices for error handling and gas management, enhancing the security and readability of your code. Low-level calls lack these built-in safety checks and can be more error-prone, especially when dealing with Ether transfers.

Num of instances: 4

Findings

Click to show findings

['302']

302:         
303:         (bool success, ) = owner().call{value: amountForTreasury}(""); // <= FOUND

['453']

453:         (bool success, ) = protocolAdmin.call{value: address(this).balance}(""); // <= FOUND

['302']

302:         
303:         (bool success, ) = owner().call{value: amountForTreasury}(""); // <= FOUND

['453']

453:         (bool success, ) = protocolAdmin.call{value: address(this).balance}(""); // <= FOUND

[NonCritical-30] Long numbers should include underscores to improve readability and prevent typos

Resolution

A large number such as 2000000 is far more readable as 2_000_000, this will help prevent unintended bugs in the code

Num of instances: 1

Findings

Click to show findings

['308']

308:         int24 iprice = -13862; // <= FOUND

[NonCritical-31] Constructors should emit an event

Resolution

Emitting an event in a constructor of a smart contract provides transparency and traceability in blockchain applications. This event logs the contract’s creation, aiding in monitoring and verifying contract deployment. Although constructors are executed only once, the emitted event ensures the contract's initialization is recorded on the blockchain.

Num of instances: 1

Findings

Click to show findings

['95']

95:     constructor( // <= FOUND
96:         uint256 _fundraisingGoal,
97:         string memory _name,
98:         string memory _symbol,
99:         uint256 _fundraisingDeadline,
100:         uint256 _fundExpiry,
101:         address _daoManager,
102:         address _liquidityLockerFactory,
103:         uint256 _maxWhitelistAmount,
104:         address _protocolAdmin,
105:         uint256 _maxPublicContributionAmount
106:     ) Ownable(_daoManager) {
107:         require(
108:             _fundraisingGoal > 0,
109:             "Fundraising goal must be greater than 0"
110:         );
111:         require(
112:             _fundraisingDeadline > block.timestamp,
113:             "_fundraisingDeadline > block.timestamp"
114:         );
115:         require(
116:             _fundExpiry > fundraisingDeadline,
117:             "_fundExpiry > fundraisingDeadline"
118:         );
119:         name = _name;
120:         symbol = _symbol;
121:         fundraisingGoal = _fundraisingGoal;
122:         fundraisingDeadline = _fundraisingDeadline;
123:         fundExpiry = _fundExpiry;
124:         liquidityLockerFactory = ILockerFactory(_liquidityLockerFactory);
125:         maxWhitelistAmount = _maxWhitelistAmount;
126:         protocolAdmin = _protocolAdmin;
127:         maxPublicContributionAmount = _maxPublicContributionAmount;
128: 
129:         
130:         tierLimits[WhitelistTier.Platinum] = PLATINUM_DEFAULT_LIMIT;
131:         tierLimits[WhitelistTier.Gold] = GOLD_DEFAULT_LIMIT;
132:         tierLimits[WhitelistTier.Silver] = SILVER_DEFAULT_LIMIT;
133:     }

[NonCritical-32] Contract and Abstract files should have a fixed compiler version

Resolution

Using a fixed compiler version in Solidity contract and abstract files ensures consistency and predictability in smart contract behavior. A fixed version prevents unforeseen issues arising from compiler updates, which might introduce breaking changes or new bugs. It guarantees that the contract's behavior remains stable and consistent with the version used during its development and testing.

Num of instances: 5

Findings

Click to show findings

['8']

8: contract CLPoolRouter is ICLSwapCallback 

['16']

16: contract Daao is Ownable, ReentrancyGuard 

['7']

7: contract DaaoToken is ERC20, Ownable 

['2']

2: pragma solidity ^0.8.0; // <= FOUND

['2']

2: pragma solidity ^0.8.18; // <= FOUND

[NonCritical-33] Token minting is centralized

Resolution

Centralized ERC20 token minting, where only a select few addresses have control over the mint function, can be viewed negatively. It introduces risks of manipulation, as the power to alter token supply is concentrated in the hands of a small group, potentially affecting token value unfairly. Such centralization can also introduce risks if the admin account(s) is compromised. Consider implementing a timelock feature so users can decide to opt out of the protocol if they do not agree with actions taken.

Num of instances: 1

Findings

Click to show findings

['13']

13:     function mint(address _to, uint256 _amount) external onlyOwner { // <= FOUND
14:         _mint(_to, _amount);
15:     }

[NonCritical-34] For loop iterates on arrays not indexed

Resolution

Ensuring matching input array lengths in functions is crucial to prevent logical errors and inconsistencies in smart contracts. Mismatched array lengths can lead to incomplete operations or incorrect data handling, particularly if one array's length is assumed to reflect another's. For instance, iterating over a shorter array than intended could result in unprocessed elements from a longer array, potentially causing unintended consequences in contract execution. Validating that all input arrays have intended lengths before performing operations helps safeguard against such errors, ensuring that all elements are appropriately processed and the contract behaves as expected.

Num of instances: 2

Findings

Click to show findings

['194']

194:        for (uint256 i = 0; i < _addresses.length; i++) { // <= FOUND
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier"); // <= FOUND
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }

['194']

194:         for (uint256 i = 0; i < _addresses.length; i++) { // <= FOUND
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier"); // <= FOUND
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }

[NonCritical-35] Constructor with array/string/bytes parameters has no empty array checks

Resolution

Failing to validate for empty array inputs in constructors can lead to vulnerabilities or logical errors in smart contracts. Constructors often initialize key contract settings, and arrays may represent crucial data like access controls, initial states, or configuration parameters. Without empty array checks, a contract might be deployed in an unintended state, lacking necessary data for its operations or security measures.

Num of instances: 2

Findings

Click to show findings

['95']

95:     constructor(
96:         uint256 _fundraisingGoal,
97:         string memory _name,
98:         string memory _symbol,
99:         uint256 _fundraisingDeadline,
100:         uint256 _fundExpiry,
101:         address _daoManager,
102:         address _liquidityLockerFactory,
103:         uint256 _maxWhitelistAmount,
104:         address _protocolAdmin,
105:         uint256 _maxPublicContributionAmount
106:     ) Ownable(_daoManager) {
107:         require(
108:             _fundraisingGoal > 0,
109:             "Fundraising goal must be greater than 0"
110:         );
111:         require(
112:             _fundraisingDeadline > block.timestamp,
113:             "_fundraisingDeadline > block.timestamp"
114:         );
115:         require(
116:             _fundExpiry > fundraisingDeadline,
117:             "_fundExpiry > fundraisingDeadline"
118:         );
119:         name = _name;
120:         symbol = _symbol;
121:         fundraisingGoal = _fundraisingGoal;
122:         fundraisingDeadline = _fundraisingDeadline;
123:         fundExpiry = _fundExpiry;
124:         liquidityLockerFactory = ILockerFactory(_liquidityLockerFactory);
125:         maxWhitelistAmount = _maxWhitelistAmount;
126:         protocolAdmin = _protocolAdmin;
127:         maxPublicContributionAmount = _maxPublicContributionAmount;
128: 
129:         
130:         tierLimits[WhitelistTier.Platinum] = PLATINUM_DEFAULT_LIMIT;
131:         tierLimits[WhitelistTier.Gold] = GOLD_DEFAULT_LIMIT;
132:         tierLimits[WhitelistTier.Silver] = SILVER_DEFAULT_LIMIT;
133:     }

['8']

8:     constructor(
9:         string memory _name,
10:         string memory _symbol
11:     ) ERC20(_name, _symbol) Ownable(msg.sender) {}

[NonCritical-36] Avoid using 'owner' or '_owner' as a parameter name

Resolution

Using 'owner' or '_owner' as a parameter name in functions, especially in contracts inheriting from or interacting with OpenZeppelin's Ownable contract, can lead to confusion and potential bugs. These contracts often have a state variable named owner, managed by the Ownable pattern for access control. Using the same name for function parameters may obscure the contract's owner state variable, complicating code readability and maintenance. Prefer alternative descriptive names for parameters to maintain clarity and prevent conflicts with established ownership patterns.

Num of instances: 1

Findings

Click to show findings

['29']

29:     function checkApproval(
30:         address token,
31:         address owner // <= FOUND
32:     ) external view returns (uint256) 

[NonCritical-37] ERC777 tokens can introduce reentrancy risks

Resolution

ERC777 is an advanced token standard that introduces hooks, allowing operators to execute additional logic during transfers. While this feature offers greater flexibility, it also opens up the possibility of reentrancy attacks. Specifically, when tokens are sent, the receiving contract's tokensReceived hook gets called, and this external call can execute arbitrary code. An attacker can exploit this feature to re-enter the original function, potentially leading to double-spending or other types of financial manipulation.

To mitigate reentrancy risks with ERC777, it's crucial to adopt established security measures, such as utilizing reentrancy guards or following the check-effects-interactions pattern. Some developers opt to stick with the simpler ERC20 standard, which does not have these hooks, to minimize this risk. If you do choose to use ERC777, extreme caution and thorough auditing are advised to secure against potential reentrancy vulnerabilities.

Num of instances: 2

Findings

Click to show findings

['57']

57:     function getSwapResult( // <= FOUND
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount); // <= FOUND
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount);
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

['125']

125:     function uniswapV3SwapCallback( // <= FOUND
126:         int256 amount0Delta,
127:         int256 amount1Delta,
128:         bytes calldata data
129:     ) external override {
130:         address sender = abi.decode(data, (address));
131: 
132:         if (amount0Delta > 0) {
133:             IERC20Minimal(ICLPool(msg.sender).token0()).transferFrom( // <= FOUND
134:                 sender,
135:                 msg.sender,
136:                 uint256(amount0Delta)
137:             );
138:         } else if (amount1Delta > 0) {
139:             IERC20Minimal(ICLPool(msg.sender).token1()).transferFrom(
140:                 sender,
141:                 msg.sender,
142:                 uint256(amount1Delta)
143:             );
144:         }
145:     }

[NonCritical-38] It is best practise to initialize a local variable when they are defined

Num of instances: 2

Findings

Click to show findings

['57']

57:     function getSwapResult(
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount);
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount; // <= FOUND
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount);
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external {
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set");
282:         emit DebugLog("Starting finalizeFundraising");
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true);
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96");
311: 
312:         uint256 amountToken0ForLP; // <= FOUND
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP);
330: 
331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP);
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP);
350:         emit DebugLog("Minted additional tokens for LP");
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params);
353:         emit LPTokenMinted(tokenId);
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER),
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress);
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress);
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId);
376:         emit LockerInitialized(tokenId);
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete");
380:     }

[Gas-1] Mappings are cheaper than arrays for state variable iteration

Resolution

Where possible use mappings in place of arrays as they are far cheaper during iterations and are easier to use. There are instances such as struct arrays where arrays make more sense.

Num of instances: 2

Findings

Click to show findings

['287']

287:        for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i]; // <= FOUND
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

['287']

287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i]; // <= FOUND
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

[Gas-2] The result of a function call should be cached rather than re-calling the function

Resolution

External calls in Solidity are costly in terms of gas usage. This can significantly impact contract efficiency and cost. Functions that make repetitive calls to fetch the same data from other contracts can cause unnecessary gas expenditure. To optimize this, it's advisable to store the returned value of these function calls in a state variable, essentially caching the data. This data can be updated at regular intervals or under specific conditions instead of fetching it from the external contract on every invocation. Be sure to analyze the frequency of data change in the external contract to balance data freshness with gas efficiency when implementing caching.

Num of instances: 1

Findings

Click to show findings

['57']

57:     function getSwapResult(
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0() // <= FOUND
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0(); // <= FOUND
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount);
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount);
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

[Gas-3] x + y is more efficient than using += for state variables (likewise for -=)

Resolution

In instances found where either += or -= are used against state variables use x = x + y instead

Num of instances: 1

Findings

Click to show findings

['135']

135:     function contribute() public payable nonReentrant {
136:         require(!goalReached, "Goal already reached");
137:         require(block.timestamp < fundraisingDeadline, "Deadline hit");
138:         require(msg.value > 0, "Contribution must be greater than 0");
139: 
140:         
141:         WhitelistTier userTier = userTiers[msg.sender];
142:         require(userTier != WhitelistTier.None, "Not whitelisted");
143:         
144:         uint256 userLimit = tierLimits[userTier];
145:         require(
146:             contributions[msg.sender] + msg.value <= userLimit,
147:             "Exceeding tier limit"
148:         );
149: 
150:         if (maxWhitelistAmount > 0) {
151:             require(
152:                 contributions[msg.sender] + msg.value <= maxWhitelistAmount,
153:                 "Exceeding maxWhitelistAmount"
154:             );
155:         } else if (maxPublicContributionAmount > 0) {
156:             require(
157:                 contributions[msg.sender] + msg.value <=
158:                     maxPublicContributionAmount,
159:                 "Exceeding maxPublicContributionAmount"
160:             );
161:         }
162: 
163:         uint256 effectiveContribution = msg.value;
164:         if (totalRaised + msg.value > fundraisingGoal) {
165:             effectiveContribution = fundraisingGoal - totalRaised;
166:             payable(msg.sender).transfer(msg.value - effectiveContribution);
167:         }
168: 
169:         if (contributions[msg.sender] == 0) {
170:             contributors.push(msg.sender);
171:         }
172: 
173:         contributions[msg.sender] += effectiveContribution;
174:         totalRaised += effectiveContribution; // <= FOUND
175: 
176:         if (totalRaised == fundraisingGoal) {
177:             goalReached = true;
178:         }
179: 
180:         emit Contribution(msg.sender, effectiveContribution);
181:     }

[Gas-4] There is a 32 byte length threshold for error strings, strings longer than this consume more gas

Resolution

In require statements found which are longer than 32 characters, shorten these to 32 character or less

Num of instances: 3

Findings

Click to show findings

['57']

57:     function getSwapResult(
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio
68:         )
69:     {
70:         address tokenIn = zeroForOne
71:             ? ICLPool(pool).token0()
72:             : ICLPool(pool).token1();
73:         address tokenOut = zeroForOne
74:             ? ICLPool(pool).token1()
75:             : ICLPool(pool).token0();
76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified
78:         );
79: 
80:         _handleApproval(tokenIn, amount);
81: 
82:         IERC20Minimal(tokenIn).transferFrom(msg.sender, pool, amount);
83: 
84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this),
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );
91: 
92:         (nextSqrtRatio, , , , , ) = ICLPool(pool).slot0();
93:         uint256 outputAmount;
94:         if (zeroForOne) {
95:             require(amount1Delta < 0, "Invalid amount1Delta");
96:             outputAmount = uint256(-amount1Delta);
97:         } else {
98:             require(amount0Delta < 0, "Invalid amount0Delta");
99:             outputAmount = uint256(-amount0Delta);
100:         }
101: 
102:         if (outputAmount > 0) {
103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this)
105:             );
106:             require(
107:                 poolBalance >= outputAmount,
108:                 "Insufficient pool balance for output"
109:             );
110:             IERC20Minimal(tokenOut).transfer(msg.sender, outputAmount);
111:         }
112: 
113:         emit SwapExecuted(
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );
123:     }

['135']

135:     function contribute() public payable nonReentrant {
136:         require(!goalReached, "Goal already reached");
137:         require(block.timestamp < fundraisingDeadline, "Deadline hit");
138:         require(msg.value > 0, "Contribution must be greater than 0"); // <= FOUND
139: 
140:         
141:         WhitelistTier userTier = userTiers[msg.sender];
142:         require(userTier != WhitelistTier.None, "Not whitelisted");
143:         
144:         uint256 userLimit = tierLimits[userTier];
145:         require(
146:             contributions[msg.sender] + msg.value <= userLimit,
147:             "Exceeding tier limit"
148:         );
149: 
150:         if (maxWhitelistAmount > 0) {
151:             require(
152:                 contributions[msg.sender] + msg.value <= maxWhitelistAmount,
153:                 "Exceeding maxWhitelistAmount"
154:             );
155:         } else if (maxPublicContributionAmount > 0) {
156:             require(
157:                 contributions[msg.sender] + msg.value <=
158:                     maxPublicContributionAmount,
159:                 "Exceeding maxPublicContributionAmount"
160:             );
161:         }
162: 
163:         uint256 effectiveContribution = msg.value;
164:         if (totalRaised + msg.value > fundraisingGoal) {
165:             effectiveContribution = fundraisingGoal - totalRaised;
166:             payable(msg.sender).transfer(msg.value - effectiveContribution);
167:         }
168: 
169:         if (contributions[msg.sender] == 0) {
170:             contributors.push(msg.sender);
171:         }
172: 
173:         contributions[msg.sender] += effectiveContribution;
174:         totalRaised += effectiveContribution;
175: 
176:         if (totalRaised == fundraisingGoal) {
177:             goalReached = true;
178:         }
179: 
180:         emit Contribution(msg.sender, effectiveContribution);
181:     }

['435']

435:     function extendFundraisingDeadline(
436:         uint256 newFundraisingDeadline
437:     ) external {
438:         require(
439:             msg.sender == owner() || msg.sender == protocolAdmin,
440:             "Must be owner or protocolAdmin"
441:         );
442:         require(!goalReached, "Fundraising goal was reached");
443:         require(
444:             newFundraisingDeadline > fundraisingDeadline,
445:             "new fundraising deadline must be > old one"
446:         );
447:         fundraisingDeadline = newFundraisingDeadline;
448:     }

[Gas-5] Public functions not used internally can be marked as external to save gas

Resolution

Public functions that aren't used internally in Solidity contracts should be made external to optimize gas usage and improve contract efficiency. External functions can only be called from outside the contract, and their arguments are directly read from the calldata, which is more gas-efficient than loading them into memory, as is the case for public functions. By using external visibility, developers can reduce gas consumption for external calls and ensure that the contract operates more cost-effectively for users. Moreover, setting the appropriate visibility level for functions also enhances code readability and maintainability, promoting a more secure and well-structured contract design.

Num of instances: 4

Findings

Click to show findings

['135']

135:     function contribute() public payable nonReentrant 

['229']

229:     function getWhitelistLength() public view returns (uint256) 

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public 

['267']

267:     function setMaxPublicContributionAmount(
268:         uint256 _maxPublicContributionAmount
269:     ) public 

[Gas-6] Usage of smaller uint/int types causes overhead

Resolution

When using a smaller int/uint type it first needs to be converted to it's 256 bit counterpart to be operated, this increases the gas cost and thus should be avoided. However it does make sense to use smaller int/uint values within structs provided you pack the struct properly.

Num of instances: 23

Findings

Click to show findings

['50']

50:     uint8 public lpFeesCut = 60; // <= FOUND

['50']

50: uint8 public lpFeesCut = 60; // <= FOUND

['27']

27:     uint24 public constant UNI_V3_FEE = 500; // <= FOUND

['28']

28:     int24 public constant TICKING_SPACE = 100; // <= FOUND

['278']

278:     
279:     function finalizeFundraising(int24 initialTick, int24 upperTick) external { // <= FOUND

['308']

308:         int24 iprice = -13862; // <= FOUND

['8']

8:         int24 tickSpacing; // <= FOUND

['9']

9:         int24 tickLower; // <= FOUND

['10']

10:         int24 tickUpper; // <= FOUND

['39']

39:     function createAndInitializePoolIfNecessary(
40:         address token0,
41:         address token1,
42:         uint24 fee, // <= FOUND
43:         uint160 sqrtPriceX96 // <= FOUND
44:     ) external payable returns (address pool);

['58']

58:     function createPool(
59:         address tokenA,
60:         address tokenB,
61:         int24 tickSpacing, // <= FOUND
62:         uint160 sqrtPriceX96 // <= FOUND
63:     ) external returns (address pool);

['86']

86:     uint24 fee; // <= FOUND

['27']

27: uint24 public constant UNI_V3_FEE = 500; // <= FOUND

['28']

28: int24 public constant TICKING_SPACE = 100; // <= FOUND

['23']

23:         uint128 amount0Max; // <= FOUND

['24']

24:         uint128 amount1Max; // <= FOUND

['27']

27:     function mint(
28:         MintParams calldata params
29:     )
30:         external
31:         payable
32:         returns (
33:             uint256 tokenId,
34:             uint128 liquidity, // <= FOUND
35:             uint256 amount0,
36:             uint256 amount1
37:         );

['12']

12:     event SwapExecuted(
13:         address indexed pool,
14:         address indexed sender,
15:         bool zeroForOne,
16:         int256 amountSpecified,
17:         uint160 sqrtPriceLimitX96, // <= FOUND
18:         int256 amount0Delta,
19:         int256 amount1Delta,
20:         uint256 outputAmount
21:     );

['57']

57:     function getSwapResult(
58:         address pool,
59:         bool zeroForOne,
60:         int256 amountSpecified,
61:         uint160 sqrtPriceLimitX96 // <= FOUND
62:     )
63:         external
64:         returns (
65:             int256 amount0Delta,
66:             int256 amount1Delta,
67:             uint160 nextSqrtRatio // <= FOUND
68:         )
69:     {

['77']

77:     event PoolInitialized(uint160 sqrtPriceX96); // <= FOUND

['309']

309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice); // <= FOUND

['17']

17:         uint160 sqrtPriceX96; // <= FOUND

['90']

90:     uint160 sqrtPriceLimitX96; // <= FOUND

[Gas-7] Use != 0 instead of > 0

Resolution

Replace spotted instances with != 0 for uints as this uses less gas

Num of instances: 12

Findings

Click to show findings

['44']

44:             if (currentAllowance > 0) { // <= FOUND

['76']

76:         uint256 amount = uint256(
77:             amountSpecified > 0 ? amountSpecified : -amountSpecified // <= FOUND
78:         );

['102']

102:         if (outputAmount > 0) { // <= FOUND

['132']

132:         if (amount0Delta > 0) { // <= FOUND

['138']

138:         } else if (amount1Delta > 0) { // <= FOUND

['107']

107:         require(
108:             _fundraisingGoal > 0, // <= FOUND
109:             "Fundraising goal must be greater than 0"
110:         );

['138']

138:         require(msg.value > 0, "Contribution must be greater than 0"); // <= FOUND

['150']

150:         if (maxWhitelistAmount > 0) { // <= FOUND

['155']

155:         } else if (maxPublicContributionAmount > 0) { // <= FOUND

['192']

192:         require(_addresses.length > 0, "Empty arrays"); // <= FOUND

['214']

214:         require(_newLimit > 0, "Invalid limit"); // <= FOUND

['401']

401:         require(contributions[msg.sender] > 0, "No contributions to refund"); // <= FOUND

[Gas-8] Default int values are manually reset

Resolution

Using .delete is better than resetting a Solidity variable to its default value manually because it frees up storage space on the Ethereum blockchain, resulting in gas cost savings.

Num of instances: 1

Findings

Click to show findings

['404']

404:         contributions[msg.sender] = 0;

[Gas-9] Function calls within for loops

Resolution

Making function calls or external calls within loops in Solidity can lead to inefficient gas usage, potential bottlenecks, and increased vulnerability to attacks. Each function call or external call consumes gas, and when executed within a loop, the gas cost multiplies, potentially causing the transaction to run out of gas or exceed block gas limits. This can result in transaction failure or unpredictable behavior.

Num of instances: 2

Findings

Click to show findings

['287']

287:        for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint); // <= FOUND
295:         }

['287']

287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint); // <= FOUND
295:         }

[Gas-10] For loops in public or external functions should be avoided due to high gas costs and possible DOS

Resolution

In Solidity, for loops can potentially cause Denial of Service (DoS) attacks if not handled carefully. DoS attacks can occur when an attacker intentionally exploits the gas cost of a function, causing it to run out of gas or making it too expensive for other users to call. Below are some scenarios where for loops can lead to DoS attacks: Nested for loops can become exceptionally gas expensive and should be used sparingly

Num of instances: 3

Findings

Click to show findings

['183']

183:     function addToWhitelist(
184:         address[] calldata _addresses,
185:         WhitelistTier[] calldata _tiers
186:     ) external {
187:         require(
188:             msg.sender == owner() || msg.sender == protocolAdmin,
189:             "Must be owner or protocolAdmin"
190:         );
191:         require(_addresses.length == _tiers.length, "Arrays length mismatch");
192:         require(_addresses.length > 0, "Empty arrays");
193: 
194:         for (uint256 i = 0; i < _addresses.length; i++) { // <= FOUND
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }
206:     }

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external {
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set");
282:         emit DebugLog("Starting finalizeFundraising");
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) { // <= FOUND
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true);
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96");
311: 
312:         uint256 amountToken0ForLP;
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP);
330: 
331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP);
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP);
350:         emit DebugLog("Minted additional tokens for LP");
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params);
353:         emit LPTokenMinted(tokenId);
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER),
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress);
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress);
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId);
376:         emit LockerInitialized(tokenId);
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete");
380:     }

['412']

412:     function execute(
413:         address[] calldata contracts,
414:         bytes[] calldata data,
415:         uint256[] calldata msgValues
416:     ) external onlyOwner {
417:         require(fundraisingFinalized);
418:         require(
419:             contracts.length == data.length && data.length == msgValues.length,
420:             "Array lengths mismatch"
421:         );
422: 
423:         for (uint256 i = 0; i < contracts.length; i++) { // <= FOUND
424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]);
425:             require(success, "Call failed");
426:         }
427:     }

[Gas-11] Use assembly to check for the zero address

Resolution

Using assembly for address comparisons in Solidity can save gas because it allows for more direct access to the Ethereum Virtual Machine (EVM), reducing the overhead of higher-level operations. Solidity's high-level abstraction simplifies coding but can introduce additional gas costs. Using assembly for simple operations like address comparisons can be more gas-efficient.

Num of instances: 5

Findings

Click to show findings

['382']

382:     function setDaoToken(address _daoToken) external onlyOwner {
383:         require(_daoToken != address(0), "Invalid DAO token address"); // <= FOUND
384:         require(daoToken == address(0), "DAO token already set"); // <= FOUND
385:         daoToken = _daoToken;
386:     }

['388']

388:     function setSecondToken(address _daoToken) external onlyOwner {
389:         require(_daoToken != address(0), "Invalid second token address"); // <= FOUND
390:         require(secondToken == address(0), "DAO token already set"); // <= FOUND
391:         secondToken = _daoToken;
392:     }

['183']

183:     function addToWhitelist(
184:         address[] calldata _addresses,
185:         WhitelistTier[] calldata _tiers
186:     ) external {
187:         require(
188:             msg.sender == owner() || msg.sender == protocolAdmin,
189:             "Must be owner or protocolAdmin"
190:         );
191:         require(_addresses.length == _tiers.length, "Arrays length mismatch");
192:         require(_addresses.length > 0, "Empty arrays");
193: 
194:         for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address"); // <= FOUND
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }
206:     }

['233']

233:     function removeFromWhitelist(address removedAddress) external {
234:         require(
235:             msg.sender == owner() || msg.sender == protocolAdmin,
236:             "Must be owner or protocolAdmin"
237:         );
238: 
239:         require(removedAddress != address(0), "Invalid address"); // <= FOUND
240:         require(
241:             userTiers[removedAddress] != WhitelistTier.None,
242:             "Address not whitelisted"
243:         );
244:         userTiers[removedAddress] = WhitelistTier.None;
245: 
246:         
247: 
248:         
249:         
250:         
251:         
252:         
253:         
254:         
255: 
256:         emit RemoveWhitelist(removedAddress);
257:     }

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external {
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set"); // <= FOUND
282:         emit DebugLog("Starting finalizeFundraising");
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true);
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96");
311: 
312:         uint256 amountToken0ForLP;
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP);
330: 
331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP);
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP);
350:         emit DebugLog("Minted additional tokens for LP");
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params);
353:         emit LPTokenMinted(tokenId);
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER),
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress);
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress);
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId);
376:         emit LockerInitialized(tokenId);
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete");
380:     }

[Gas-12] Some error strings are not descriptive

Resolution

Consider adding more detail to these error strings

Num of instances: 1

Findings

Click to show findings

['412']

412:     function execute(
413:         address[] calldata contracts,
414:         bytes[] calldata data,
415:         uint256[] calldata msgValues
416:     ) external onlyOwner {
417:         require(fundraisingFinalized); // <= FOUND
418:         require(
419:             contracts.length == data.length && data.length == msgValues.length,
420:             "Array lengths mismatch"
421:         );
422: 
423:         for (uint256 i = 0; i < contracts.length; i++) {
424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]);
425:             require(success, "Call failed");
426:         }
427:     }

[Gas-13] State variables which are not modified within functions should be set as constants or immutable for values set at deployment

Resolution

Set state variables listed below as constant or immutable for values set at deployment. Ensure it is safe to do so

Num of instances: 6

Findings

Click to show findings

['9']

9:  int256 private _amount0Delta;

['10']

10:  int256 private _amount1Delta;

['44']

44:  uint256 public fundraisingGoal;

['50']

50:  uint8 public lpFeesCut = 60;

['52']

52:  string public name;

['53']

53:  string public symbol;

[Gas-14] Structs can be packed into fewer storage slots

Resolution

In Solidity, each storage slot has a size of 32 bytes. If a struct contains multiple uint values, it's efficient to pack these into as few storage slots as possible to optimize gas usage. The EVM (Ethereum Virtual Machine) charges gas for each storage operation, so minimizing the number of slots used can result in substantial gas savings. This can be achieved by ordering struct fields according to their size or by using smaller data types where possible. However, developers must balance these optimizations with the need for code clarity and the precision requirements of their application. Always ensure that data packing does not compromise the functionality or security of the contract.

Num of instances: 2

Findings

Click to show findings

['83']

83: struct ExactInputSingleParams { // <= FOUND
84:     address tokenIn;
85:     address tokenOut;
86:     uint24 fee;
87:     address recipient;
88:     uint256 amountIn; // <= FOUND
89:     uint256 amountOutMinimum; // <= FOUND
90:     uint160 sqrtPriceLimitX96;
91: }

['5']

5:     struct MintParams { // <= FOUND
6:         address token0;
7:         address token1;
8:         int24 tickSpacing;
9:         int24 tickLower;
10:         int24 tickUpper;
11:         uint256 amount0Desired; // <= FOUND
12:         uint256 amount1Desired; // <= FOUND
13:         uint256 amount0Min; // <= FOUND
14:         uint256 amount1Min; // <= FOUND
15:         address recipient;
16:         uint256 deadline; // <= FOUND
17:         uint160 sqrtPriceX96;
18:     }

[Gas-15] Use selfBalance() in place of address(this).balance

Resolution

In Solidity, using selfBalance instead of address(this).balance is advantageous for gas optimization. address(this).balance performs an external call to retrieve the contract's balance, which costs extra gas. As a resolution, defining a selfBalance state variable that's updated accordingly provides a more gas-efficient approach. Using selfBalance instead of address(this).balance can save around 800 gas per call. This is because address(this).balance makes an EXTBAL operation, which costs 700 gas, and additionally, the operation is a SLOAD, which costs around 800 gas. Please note that these estimates are based on the Ethereum gas schedule and might vary depending on the Ethereum network state and its future upgrades.

Num of instances: 2

Findings

Click to show findings

['298']

298:         
299:         uint256 totalCollected = address(this).balance; // <= FOUND

['453']

453:         (bool success, ) = protocolAdmin.call{value: address(this).balance}(""); // <= FOUND

[Gas-16] Use assembly to emit events

Resolution

With the use of inline assembly in Solidity, we can take advantage of low-level features like scratch space and the free memory pointer, offering more gas-efficient ways of emitting events. The scratch space is a certain area of memory where we can temporarily store data, and the free memory pointer indicates the next available memory slot. Using these, we can efficiently assemble event data without incurring additional memory expansion costs. However, safety is paramount: to avoid overwriting or leakage, we must cache the free memory pointer before use and restore it afterward, ensuring that it points to the correct memory location post-operation.

Num of instances: 17

Findings

Click to show findings

['53']

53:             emit ApprovalHandled(token, msg.sender, amount); // <= FOUND

['113']

113:         emit SwapExecuted( // <= FOUND
114:             pool,
115:             msg.sender,
116:             zeroForOne,
117:             amountSpecified,
118:             sqrtPriceLimitX96,
119:             amount0Delta,
120:             amount1Delta,
121:             outputAmount
122:         );

['180']

180:         emit Contribution(msg.sender, effectiveContribution); // <= FOUND

['204']

204:             
205:             
206:             
207:             
208:             emit AddWhitelist(_addresses[i], _tiers[i]); // <= FOUND

['226']

226:         emit TierLimitUpdated(_tier, _newLimit); // <= FOUND

['256']

256:         
257: 
258:         
259:         
260:         
261:         
262:         
263:         
264:         
265: 
266:         emit RemoveWhitelist(removedAddress); // <= FOUND

['282']

282:         emit DebugLog("Starting finalizeFundraising"); // <= FOUND

['292']

292:             emit MintDetails(contributor, tokensToMint); // <= FOUND

['305']

305:         emit FundraisingFinalized(true); // <= FOUND

['310']

310:         emit DebugLog("Calculated sqrtPriceX96"); // <= FOUND

['350']

350:         emit DebugLog("Minted additional tokens for LP"); // <= FOUND

['353']

353:         emit LPTokenMinted(tokenId); // <= FOUND

['364']

364:         emit LockerDeployed(lockerAddress); // <= FOUND

['372']

372:         emit TokenTransferredToLocker(tokenId, lockerAddress); // <= FOUND

['376']

376:         emit LockerInitialized(tokenId); // <= FOUND

['379']

379:         emit DebugLog("Finalize fundraising complete"); // <= FOUND

['408']

408:         emit Refund(msg.sender, contributedAmount); // <= FOUND

[Gas-17] Use solady library where possible to save gas

Resolution

The following OpenZeppelin imports have a Solady equivalent, as such they can be used to save GAS as Solady modules have been specifically designed to be as GAS efficient as possible

Num of instances: 4

Findings

Click to show findings

['4']

4: import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; // <= FOUND

['7']

7: import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; // <= FOUND

['5']

5: import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; // <= FOUND

['13']

13: import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; // <= FOUND

[Gas-18] Use assembly in place of abi.decode to extract calldata values more efficiently

Resolution

Using inline assembly to extract calldata values can be more gas-efficient than using abi.decode in Solidity. Inline assembly gives more direct access to EVM operations, enabling optimized usage of calldata. However, assembly should be used judiciously as it's more prone to errors. Opt for this approach when performance is critical and the complexity it introduces is manageable.

Num of instances: 1

Findings

Click to show findings

['130']

130:         address sender = abi.decode(data, (address)); // <= FOUND

[Gas-19] Struct variables can be packed into fewer storage slots by truncating timestamp bytes

Resolution

Struct uint variables in Solidity are typically stored in 32-byte storage slots. When dealing with timestamps, which generally fit into a smaller byte size, it can be beneficial to truncate these bytes and pack them with other variables. This reduces the number of required storage slots, saving both storage space and associated gas costs. For example, a timestamp generally fits into a uint32, so it can be combined with other small variables within a single storage slot. When designing a contract, carefully structuring struct variables to utilize truncation and packing can lead to a more efficient and cost-effective implementation.

Num of instances: 1

Findings

Click to show findings

['5']

5:     struct MintParams { // <= FOUND
6:         address token0;
7:         address token1;
8:         int24 tickSpacing;
9:         int24 tickLower;
10:         int24 tickUpper;
11:         uint256 amount0Desired;
12:         uint256 amount1Desired;
13:         uint256 amount0Min;
14:         uint256 amount1Min;
15:         address recipient;
16:         uint256 deadline; // <= FOUND
17:         uint160 sqrtPriceX96;
18:     }

[Gas-20] Using private rather than public for constants and immutables, saves gas

Resolution

Using private visibility for constants and immutables in Solidity instead of public can save gas. This is because private elements are not included in the contract's ABI, reducing the deployment and interaction costs. To achieve better efficiency, it is recommended to use private visibility when external access is not needed.

Num of instances: 8

Findings

Click to show findings

['27']

27: uint24 public constant UNI_V3_FEE = 500; // <= FOUND

['28']

28: int24 public constant TICKING_SPACE = 100; // <= FOUND

['29']

29: uint256 public constant LP_PERCENTAGE = 10; // <= FOUND

['30']

30: uint256 public constant TREASURY_PERCENTAGE = 90; // <= FOUND

[]

IVelodromeFactory public constant Velodrome_factory = IVelodromeFactory(0x04625B046C69577EfC40e6c0Bb83CDBAfab5a55F); // <= FOUND

[]

INonfungiblePositionManager public constant POSITION_MANAGER = INonfungiblePositionManager(0x991d5546C4B442B4c5fdc4c8B8b8d131DEB24702); // <= FOUND

['39']

39: address public constant MODE = 0x4200000000000000000000000000000000000006; // <= FOUND

['49']

49: uint256 public constant SUPPLY_TO_FUNDRAISERS = 1_000_000_000 * 1e18; // <= FOUND

[Gas-21] Identical Deployments Should be Replaced with Clone

Resolution

In the context of smart contracts, deploying multiple identical contracts can lead to inefficient use of gas and unnecessarily duplicate code on the blockchain. A more gas-efficient approach is to use a "clone" pattern. By deploying a master contract and then creating clones of it, only the differences between the instances are stored for each clone. This approach leverages the EIP-1167 standard, which defines a minimal proxy contract that points to the implementation contract. Clones can be far cheaper to deploy compared to full instances. So, the resolution is to replace identical deployments with clones, saving on gas and storage space.

Num of instances: 1

Findings

Click to show findings

['356']

356:         
357:         address lockerAddress = liquidityLockerFactory.deploy( // <= FOUND
358:             address(POSITION_MANAGER),
359:             owner(),
360:             uint64(fundExpiry),
361:             tokenId,
362:             lpFeesCut,
363:             address(this)
364:         );

[Gas-22] Mark Functions That Revert For Normal Users As payable

Resolution

In Solidity, marking functions as payable allows them to accept Ether. If a function is known to revert for regular users (non-admin or specific roles) but needs to be accessible to others, marking it as payable can be beneficial. This ensures that even if a regular user accidentally sends Ether to the function, the Ether won't be trapped, as the function reverts, returning the funds. This can save gas by avoiding unnecessary failure handling in the function itself. Resolution: Carefully assess the roles and access patterns, and mark functions that should revert for regular users as payable to handle accidental Ether transfers.

Num of instances: 11

Findings

Click to show findings

['382']

382:     function setDaoToken(address _daoToken) external onlyOwner {
383:         require(_daoToken != address(0), "Invalid DAO token address");
384:         require(daoToken == address(0), "DAO token already set");
385:         daoToken = _daoToken;
386:     }

['388']

388:     function setSecondToken(address _daoToken) external onlyOwner {
389:         require(_daoToken != address(0), "Invalid second token address");
390:         require(secondToken == address(0), "DAO token already set");
391:         secondToken = _daoToken;
392:     }

['412']

412:     function execute(
413:         address[] calldata contracts,
414:         bytes[] calldata data,
415:         uint256[] calldata msgValues
416:     ) external onlyOwner {
417:         require(fundraisingFinalized);
418:         require(
419:             contracts.length == data.length && data.length == msgValues.length,
420:             "Array lengths mismatch"
421:         );
422: 
423:         for (uint256 i = 0; i < contracts.length; i++) {
424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]);
425:             require(success, "Call failed");
426:         }
427:     }

['429']

429:     function extendFundExpiry(uint256 newFundExpiry) external onlyOwner {
430:         require(newFundExpiry > fundExpiry, "Must choose later fund expiry");
431:         fundExpiry = newFundExpiry;
432:         ILocker(liquidityLocker).extendFundExpiry(newFundExpiry);
433:     }

['13']

13:     function mint(address _to, uint256 _amount) external onlyOwner {
14:         _mint(_to, _amount);
15:     }

['183']

183:     function addToWhitelist(
184:         address[] calldata _addresses,
185:         WhitelistTier[] calldata _tiers
186:     ) external {
187:         require(
188:             msg.sender == owner() || msg.sender == protocolAdmin,
189:             "Must be owner or protocolAdmin"
190:         );
191:         require(_addresses.length == _tiers.length, "Arrays length mismatch");
192:         require(_addresses.length > 0, "Empty arrays");
193: 
194:         for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }
206:     }

['208']

208:     function updateTierLimit(WhitelistTier _tier, uint256 _newLimit) external {
209:         require(
210:             msg.sender == owner() || msg.sender == protocolAdmin,
211:             "Not authorized"
212:         );
213:         require(_tier != WhitelistTier.None, "Invalid tier");
214:         require(_newLimit > 0, "Invalid limit");
215: 
216:         tierLimits[_tier] = _newLimit;
217: 
218:         if (_tier == WhitelistTier.Gold) {
219:             GOLD_DEFAULT_LIMIT = _newLimit;
220:         } else if (_tier == WhitelistTier.Silver) {
221:             SILVER_DEFAULT_LIMIT = _newLimit;
222:         } else {
223:             PLATINUM_DEFAULT_LIMIT = _newLimit;
224:         }
225: 
226:         emit TierLimitUpdated(_tier, _newLimit);
227:     }

['233']

233:     function removeFromWhitelist(address removedAddress) external {
234:         require(
235:             msg.sender == owner() || msg.sender == protocolAdmin,
236:             "Must be owner or protocolAdmin"
237:         );
238: 
239:         require(removedAddress != address(0), "Invalid address");
240:         require(
241:             userTiers[removedAddress] != WhitelistTier.None,
242:             "Address not whitelisted"
243:         );
244:         userTiers[removedAddress] = WhitelistTier.None;
245: 
246:         
247: 
248:         
249:         
250:         
251:         
252:         
253:         
254:         
255: 
256:         emit RemoveWhitelist(removedAddress);
257:     }

['259']

259:     function setMaxWhitelistAmount(uint256 _maxWhitelistAmount) public {
260:         require(
261:             msg.sender == owner() || msg.sender == protocolAdmin,
262:             "Must be owner or protocolAdmin"
263:         );
264:         maxWhitelistAmount = _maxWhitelistAmount;
265:     }

['267']

267:     function setMaxPublicContributionAmount(
268:         uint256 _maxPublicContributionAmount
269:     ) public {
270:         require(
271:             msg.sender == owner() || msg.sender == protocolAdmin,
272:             "Must be owner or protocolAdmin"
273:         );
274:         maxPublicContributionAmount = _maxPublicContributionAmount;
275:     }

['435']

435:     function extendFundraisingDeadline(
436:         uint256 newFundraisingDeadline
437:     ) external {
438:         require(
439:             msg.sender == owner() || msg.sender == protocolAdmin,
440:             "Must be owner or protocolAdmin"
441:         );
442:         require(!goalReached, "Fundraising goal was reached");
443:         require(
444:             newFundraisingDeadline > fundraisingDeadline,
445:             "new fundraising deadline must be > old one"
446:         );
447:         fundraisingDeadline = newFundraisingDeadline;
448:     }

[Gas-23] Lack of unchecked in loops

Resolution

In Solidity, the unchecked block allows arithmetic operations to not revert on overflow. Without using unchecked in loops, extra gas is consumed due to overflow checks. If it's certain that overflows won't occur within the loop, using unchecked can make the loop more gas-efficient by skipping unnecessary checks.

Num of instances: 6

Findings

Click to show findings

['194']

194:        for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }

['287']

287:        for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

['423']

423:        for (uint256 i = 0; i < contracts.length; i++) {
424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]);
425:             require(success, "Call failed");
426:         }

['194']

194:         for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]);
205:         }

['287']

287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

['423']

423:         for (uint256 i = 0; i < contracts.length; i++) {
424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]);
425:             require(success, "Call failed");
426:         }

[Gas-24] Where a value is casted more than once, consider caching the result to save gas

Resolution

Casting values multiple times in Solidity can be gas-inefficient. When a value undergoes repeated type conversions, the EVM must execute additional operations for each cast, consuming more gas than necessary. To optimize for gas efficiency, cache the result of the initial cast in a local variable and reuse it, rather than performing multiple casts. This not only conserves gas but also enhances code readability, reducing potential error points. For example, instead of repeatedly casting an address to uint256, cast once, store the result in a local variable, and reference that variable in subsequent operations.

Num of instances: 1

Findings

Click to show findings

['278']

278:     function finalizeFundraising(int24 initialTick, int24 upperTick) external {
279:         require(goalReached, "Fundraising goal not reached");
280:         require(!fundraisingFinalized, "DAO tokens already minted");
281:         require(daoToken != address(0), "Token not set");
282:         emit DebugLog("Starting finalizeFundraising");
283:         DaaoToken token = DaaoToken(daoToken);
284:         daoToken = address(token);
285: 
286:         
287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }
296: 
297:         
298:         uint256 totalCollected = address(this).balance;
299:         uint256 amountForLP = (totalCollected * LP_PERCENTAGE) / 100;
300:         uint256 amountForTreasury = totalCollected - amountForLP;
301:         
302:         (bool success, ) = owner().call{value: amountForTreasury}("");
303:         require(success, "Treasury transfer failed");
304: 
305:         emit FundraisingFinalized(true);
306:         fundraisingFinalized = true;
307: 
308:         int24 iprice = -13862;
309:         uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(iprice);
310:         emit DebugLog("Calculated sqrtPriceX96");
311: 
312:         uint256 amountToken0ForLP;
313:         uint256 amountToken1ForLP;
314: 
315:         if (daoToken < MODE) {
316:             token0 = daoToken;
317:             token1 = MODE;
318:             
319:             amountToken0ForLP = 4 * amountForLP;
320:             amountToken1ForLP = amountForLP;
321:         } else {
322:             token0 = MODE;
323:             token1 = daoToken;
324:             
325:             amountToken0ForLP = amountForLP;
326:             amountToken1ForLP = 4 * amountForLP;
327:         }
328: 
329:         token.mint(address(this), 4 * amountForLP);
330: 
331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this),
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );
346: 
347:         token.renounceOwnership();
348:         IERC20(token0).approve(address(POSITION_MANAGER), amountToken0ForLP); // <= FOUND 'address(POSITION_MANAGER)'
349:         IERC20(token1).approve(address(POSITION_MANAGER), amountToken1ForLP); // <= FOUND 'address(POSITION_MANAGER)'
350:         emit DebugLog("Minted additional tokens for LP");
351: 
352:         (uint256 tokenId, , , ) = POSITION_MANAGER.mint(params);
353:         emit LPTokenMinted(tokenId);
354: 
355:         
356:         address lockerAddress = liquidityLockerFactory.deploy(
357:             address(POSITION_MANAGER), // <= FOUND 'address(POSITION_MANAGER)'
358:             owner(),
359:             uint64(fundExpiry),
360:             tokenId,
361:             lpFeesCut,
362:             address(this)
363:         );
364:         emit LockerDeployed(lockerAddress);
365: 
366:         
367:         POSITION_MANAGER.safeTransferFrom(
368:             address(this),
369:             lockerAddress,
370:             tokenId
371:         );
372:         emit TokenTransferredToLocker(tokenId, lockerAddress);
373: 
374:         
375:         ILocker(lockerAddress).initializer(tokenId);
376:         emit LockerInitialized(tokenId);
377: 
378:         liquidityLocker = lockerAddress;
379:         emit DebugLog("Finalize fundraising complete");
380:     }

[Gas-25] Use assembly to validate msg.sender

Resolution

Utilizing assembly for validating msg.sender can potentially save gas as it allows for more direct and efficient access to Ethereum’s EVM opcodes, bypassing some of the overhead introduced by Solidity’s higher-level abstractions. However, this practice requires deep expertise in EVM, as incorrect implementation can introduce critical vulnerabilities. It is a trade-off between gas efficiency and code safety.

Num of instances: 3

Findings

Click to show findings

['187']

187:         require( // <= FOUND
188:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
189:             "Must be owner or protocolAdmin"
190:         );

['209']

209:         require( // <= FOUND
210:             msg.sender == owner() || msg.sender == protocolAdmin, // <= FOUND
211:             "Not authorized"
212:         );

['451']

451:         require(msg.sender == protocolAdmin, "must be protocol admin"); // <= FOUND

[Gas-26] Simple checks for zero uint can be done using assembly to save gas

Resolution

Using assembly for simple zero checks on unsigned integers can save gas due to lower-level, optimized operations.

Resolution: Implement inline assembly with Solidity's assembly block to perform zero checks. Ensure thorough testing and verification, as assembly lacks the safety checks of high-level Solidity, potentially introducing vulnerabilities if not used carefully.

Num of instances: 4

Findings

Click to show findings

['107']

107:         require(
108:             _fundraisingGoal > 0,
109:             "Fundraising goal must be greater than 0"
110:         );

['214']

214:         require(_newLimit > 0, "Invalid limit");

['95']

95:             require(amount1Delta < 0, "Invalid amount1Delta");

['98']

98:             require(amount0Delta < 0, "Invalid amount0Delta");

[Gas-27] Using nested if to save gas

Resolution

Using nested if statements instead of logical AND (&&) operators can potentially save gas in Solidity contracts. When a series of conditions are connected with &&, all conditions must be evaluated even if the first one fails. In contrast, nested if statements allow for short-circuiting; if the first condition fails, the rest are skipped, saving gas. This approach is more gas-efficient, especially when dealing with complex or gas-intensive conditions. However, it's crucial to balance gas savings with code readability and maintainability, ensuring that the code remains clear and easy to understand.

Num of instances: 3

Findings

Click to show findings

['459']

459:        if (!goalReached && block.timestamp < fundraisingDeadline) { // <= FOUND
460:             contribute();
461:         }

['418']

418:         require(
419:             contracts.length == data.length && data.length == msgValues.length, // <= FOUND
420:             "Array lengths mismatch"
421:         );

['459']

459:         if (!goalReached && block.timestamp < fundraisingDeadline) { // <= FOUND

[Gas-28] Optimize Storage with Byte Truncation for Time Related State Variables

Resolution

Storage optimization in Solidity contracts is vital for reducing gas costs, especially when storing time-related state variables. Using uint32 for storing time values like timestamps is often sufficient, given it can represent dates up to the year 2106. By truncating larger default integer types to uint32, you significantly save on storage space and consequently on gas costs for deployment and state modifications. However, ensure that the truncation does not lead to overflow issues and that the variable's size is adequate for the application's expected lifespan and precision requirements. Adopting this optimization practice contributes to more efficient and cost-effective smart contract development.

Num of instances: 1

Findings

Click to show findings

['48']

48: uint256 public fundExpiry; // <= FOUND

[Gas-29] Using delete instead of setting mapping to 0 saves gas

Num of instances: 1

Findings

Click to show findings

['404']

404:         contributions[msg.sender] = 0; // <= FOUND

[Gas-30] Low level call can be optimized with assembly

Resolution

Optimizing low-level calls using assembly in Solidity can be beneficial, particularly when dealing with function return data. Typically, even if return data from a low-level call is not used, Solidity still allocates memory to store it, which incurs gas costs. By using assembly, developers can bypass the automatic memory allocation for unused return data. This manual optimization involves handling the call at the assembly level and deliberately choosing not to store the return data in memory when it's not needed.

Num of instances: 9

Findings

Click to show findings

['302']

302:         
303:         (bool success, ) = owner().call{value: amountForTreasury}(""); // <= FOUND

['424']

424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]); // <= FOUND

['453']

453:         (bool success, ) = protocolAdmin.call{value: address(this).balance}(""); // <= FOUND

['302']

302:         
303:         (bool success, ) = owner().call{value: amountForTreasury}(""); // <= FOUND

['424']

424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]); // <= FOUND

['453']

453:         (bool success, ) = protocolAdmin.call{value: address(this).balance}(""); // <= FOUND

['302']

302:         
303:         (bool success, ) = owner().call{value: amountForTreasury}(""); // <= FOUND

['424']

424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]); // <= FOUND

['453']

453:         (bool success, ) = protocolAdmin.call{value: address(this).balance}(""); // <= FOUND

[Gas-31] Solidity versions 0.8.19 and above are more gas efficient

Resolution

Solidity version 0.8.19 introduced a array of gas optimizations which make contracts which use it more efficient. Provided compatability it may be beneficial to upgrade to this version

Num of instances: 1

Findings

Click to show findings

['2']

2: pragma solidity ^0.8.0;

[Gas-32] Calling .length in a for loop wastes gas

Resolution

Rather than calling .length for an array in a for loop declaration, it is far more gas efficient to cache this length before and use that instead. This will prevent the array length from being called every loop iteration

Num of instances: 6

Findings

Click to show findings

['194']

194: for (uint256 i = 0; i < _addresses.length; i++)  // <= FOUND

['287']

287: for (uint256 i = 0; i < contributors.length; i++)  // <= FOUND

['423']

423: for (uint256 i = 0; i < contracts.length; i++)  // <= FOUND

['194']

194: for (uint256 i = 0; i < _addresses.length; i++)  // <= FOUND

['287']

287: for (uint256 i = 0; i < contributors.length; i++)  // <= FOUND

['423']

423: for (uint256 i = 0; i < contracts.length; i++)  // <= FOUND

[Gas-33] Internal functions only used once can be inlined to save gas

Resolution

If a internal function is only used once it doesn't make sense to modularise it unless the function which does call the function would be overly long and complex otherwise

Num of instances: 1

Findings

Click to show findings

['36']

36:     function _handleApproval(address token, uint256 amount) internal  // <= FOUND

[Gas-34] Constructors can be marked as payable to save deployment gas

Num of instances: 2

Findings

Click to show findings

['95']

95:     constructor(
96:         uint256 _fundraisingGoal,
97:         string memory _name,
98:         string memory _symbol,
99:         uint256 _fundraisingDeadline,
100:         uint256 _fundExpiry,
101:         address _daoManager,
102:         address _liquidityLockerFactory,
103:         uint256 _maxWhitelistAmount,
104:         address _protocolAdmin,
105:         uint256 _maxPublicContributionAmount
106:     ) Ownable(_daoManager) {
107:         require(
108:             _fundraisingGoal > 0,
109:             "Fundraising goal must be greater than 0"
110:         );
111:         require(
112:             _fundraisingDeadline > block.timestamp,
113:             "_fundraisingDeadline > block.timestamp"
114:         );
115:         require(
116:             _fundExpiry > fundraisingDeadline,
117:             "_fundExpiry > fundraisingDeadline"
118:         );
119:         name = _name;
120:         symbol = _symbol;
121:         fundraisingGoal = _fundraisingGoal;
122:         fundraisingDeadline = _fundraisingDeadline;
123:         fundExpiry = _fundExpiry;
124:         liquidityLockerFactory = ILockerFactory(_liquidityLockerFactory);
125:         maxWhitelistAmount = _maxWhitelistAmount;
126:         protocolAdmin = _protocolAdmin;
127:         maxPublicContributionAmount = _maxPublicContributionAmount;
128: 
129:         
130:         tierLimits[WhitelistTier.Platinum] = PLATINUM_DEFAULT_LIMIT;
131:         tierLimits[WhitelistTier.Gold] = GOLD_DEFAULT_LIMIT;
132:         tierLimits[WhitelistTier.Silver] = SILVER_DEFAULT_LIMIT;
133:     }

['8']

8:     constructor(
9:         string memory _name,
10:         string memory _symbol
11:     ) ERC20(_name, _symbol) Ownable(msg.sender) {}

[Gas-35] It is a waste of GAS to emit variable literals

Resolution

Emitting variable literals (true, false, 'hello', 1 etc...) in events is inefficient, as it consumes extra gas without providing added value. These literals are fixed values that can be accessed or hardcoded elsewhere in the smart contract or application, making their inclusion in events redundant and an unnecessary drain on resources during transaction execution.

Num of instances: 5

Findings

Click to show findings

['305']

305:         emit FundraisingFinalized(true); // <= FOUND

['282']

282:         emit DebugLog("Starting finalizeFundraising"); // <= FOUND

['310']

310:         emit DebugLog("Calculated sqrtPriceX96"); // <= FOUND

['350']

350:         emit DebugLog("Minted additional tokens for LP"); // <= FOUND

['379']

379:         emit DebugLog("Finalize fundraising complete"); // <= FOUND

[Gas-36] Use OZ Array.unsafeAccess() to avoid repeated array length checks

Resolution

The OpenZeppelin Array.unsafeAccess() method is a optimization strategy for Solidity, aimed at reducing gas costs by bypassing automatic length checks on storage array accesses. In Solidity, every access to an array element involves a hidden gas cost due to a length check, ensuring that accesses do not exceed the array bounds. However, if a developer has already verified the array's bounds earlier in the function or knows through logic that the access is safe, directly accessing the array elements without redundant length checks can save gas. This approach requires careful consideration to avoid out-of-bounds errors, as it trades off safety checks for efficiency.

Num of instances: 6

Findings

Click to show findings

['195']

195:             require(_addresses[i] != address(0), "Invalid address"); // <= FOUND

['196']

196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier"); // <= FOUND

['198']

198:             userTiers[_addresses[i]] = _tiers[i]; // <= FOUND

['204']

204:             
205:             
206:             
207:             
208:             emit AddWhitelist(_addresses[i], _tiers[i]); // <= FOUND

['288']

288:             address contributor = contributors[i]; // <= FOUND

['424']

424:             (bool success, ) = contracts[i].call{value: msgValues[i]}(data[i]); // <= FOUND

[Gas-37] State variable read in a loop

Resolution

Reading a state variable inside a loop in a smart contract can unnecessarily increase gas consumption, as each read operation from the blockchain state is costly. This inefficiency becomes pronounced in loops with many iterations. To optimize gas usage, it's advisable to read the state variable once before the loop starts, store its value in a local (memory) variable, and then use this local variable within the loop. This approach minimizes the number of state read operations, thereby reducing the gas cost associated with executing the contract function, making the smart contract more efficient and cost-effective to run.

Num of instances: 2

Findings

Click to show findings

['287']

287:        for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

['287']

287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint);
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

[Gas-38] Use uint256(1)/uint256(2) instead of true/false to save gas for changes

Resolution

In Solidity, the use of uint256 values instead of boolean for certain state variables can result in gas savings. This is due to how Ethereum's storage optimization works: changing a variable from 0 to a non-zero value (like flipping false to true) incurs a higher gas cost compared to modifying an already non-zero value. By using uint256 with values 1 and 2 instead of true and false, you avoid the higher cost associated with the 0 to non-zero change, since 1 and 2 are both non-zero. This approach is notably used in OpenZeppelin's ReentrancyGuard as a gas optimization technique. However, this should be applied where it makes sense and where gas optimization is critical, as it can decrease code readability.

Num of instances: 2

Findings

Click to show findings

['177']

177:             goalReached = true; // <= FOUND

['306']

306:         fundraisingFinalized = true; // <= FOUND

[Gas-39] Avoid emitting events in loops

Resolution

Emitting events inside loops can significantly increase gas costs in Ethereum smart contracts, as each event emission consumes gas. This practice can quickly escalate transaction fees, especially with a high number of iterations. To optimize for efficiency and cost, it's advisable to minimize event emissions within loops, possibly aggregating data to emit a single event post-loop or reconsidering the design to reduce looped emissions. This approach helps maintain manageable transaction costs and enhances contract performance.

Num of instances: 8

Findings

Click to show findings

['194']

194:        for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]); // <= FOUND
205:         }

['287']

287:        for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint); // <= FOUND
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

['194']

194:         for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]); // <= FOUND
205:         }

['287']

287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint); // <= FOUND
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

['194']

194:        for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]); // <= FOUND
205:         }

['287']

287:        for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint); // <= FOUND
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

['194']

194:         for (uint256 i = 0; i < _addresses.length; i++) {
195:             require(_addresses[i] != address(0), "Invalid address");
196:             require(_tiers[i] != WhitelistTier.None, "Invalid tier");
197: 
198:             userTiers[_addresses[i]] = _tiers[i];
199: 
200:             
201:             
202:             
203:             
204:             emit AddWhitelist(_addresses[i], _tiers[i]); // <= FOUND
205:         }

['287']

287:         for (uint256 i = 0; i < contributors.length; i++) {
288:             address contributor = contributors[i];
289:             uint256 contribution = contributions[contributor];
290:             uint256 tokensToMint = (contribution * SUPPLY_TO_FUNDRAISERS) / totalRaised;
291: 
292:             emit MintDetails(contributor, tokensToMint); // <= FOUND
293: 
294:             token.mint(contributor, tokensToMint);
295:         }

[Gas-40] Call msg.XYZ directly rather than caching the value to save gas

Num of instances: 1

Findings

Click to show findings

['163']

163:         uint256 effectiveContribution = msg.value; // <= FOUND

[Gas-41] Write direct outcome, instead of performing mathematical operations for constant state variables

Resolution

In Solidity, it's highly efficient to directly assign constant values to state variables when these values are known at compile time and will not change. This practice avoids unnecessary computational operations and reduces gas costs for deploying and interacting with smart contracts.

Num of instances: 1

Findings

Click to show findings

['49']

49:  uint256 public constant SUPPLY_TO_FUNDRAISERS = 1_000_000_000 * 1e18; // <= FOUND

[Gas-42] Consider pre-calculating the address of address(this) to save gas

Resolution

Consider saving the address(this) value within a constant using foundry's script.sol or solady's LibRlp.sol to save gas

Num of instances: 12

Findings

Click to show findings

['33']

33:         return IERC20Minimal(token).allowance(owner, address(this)); // <= FOUND

['38']

38:         uint256 currentAllowance = tokenContract.allowance(
39:             msg.sender,
40:             address(this) // <= FOUND
41:         );

['45']

45:                 tokenContract.approve(address(this), 0); // <= FOUND

['48']

48:             require(
49:                 tokenContract.approve(address(this), amount), // <= FOUND
50:                 "Approval failed"
51:             );

['84']

84:         (amount0Delta, amount1Delta) = ICLPool(pool).swap(
85:             address(this), // <= FOUND
86:             zeroForOne,
87:             amountSpecified,
88:             sqrtPriceLimitX96,
89:             abi.encode(msg.sender)
90:         );

['103']

103:             uint256 poolBalance = IERC20Minimal(tokenOut).balanceOf(
104:                 address(this) // <= FOUND
105:             );

['298']

298:         
299:         uint256 totalCollected = address(this).balance; // <= FOUND

['329']

329:         token.mint(address(this), 4 * amountForLP); // <= FOUND

['331']

331:         INonfungiblePositionManager.MintParams
332:             memory params = INonfungiblePositionManager.MintParams(
333:                 token0,
334:                 token1,
335:                 TICKING_SPACE,
336:                 initialTick,
337:                 upperTick,
338:                 amountToken0ForLP,
339:                 amountToken1ForLP,
340:                 0,
341:                 0,
342:                 address(this), // <= FOUND
343:                 block.timestamp,
344:                 sqrtPriceX96
345:             );

['356']

356:         
357:         address lockerAddress = liquidityLockerFactory.deploy(
358:             address(POSITION_MANAGER),
359:             owner(),
360:             uint64(fundExpiry),
361:             tokenId,
362:             lpFeesCut,
363:             address(this) // <= FOUND
364:         );

['367']

367:         
368:         POSITION_MANAGER.safeTransferFrom(
369:             address(this), // <= FOUND
370:             lockerAddress,
371:             tokenId
372:         );

['453']

453:         (bool success, ) = protocolAdmin.call{value: address(this).balance}(""); // <= FOUND

[Gas-43] Using named returns for pure and view functions is cheaper than using regular returns

Num of instances: 3

Findings

Click to show findings

['29']

29:     function checkApproval(
30:         address token,
31:         address owner
32:     ) external view returns (uint256) {
33:         return IERC20Minimal(token).allowance(owner, address(this)); // <= FOUND
34:     }

['229']

229:     function getWhitelistLength() public view returns (uint256) {
230:         return whitelistArray.length; // <= FOUND
231:     }

['464']

464:     function onERC721Received(
465:         address,
466:         address,
467:         uint256,
468:         bytes calldata
469:     ) external pure returns (bytes4) {
470:         return IERC721Receiver.onERC721Received.selector; // <= FOUND
471:     }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment