remove firewallProtectedSig from internal split #413

YouStillAlive · 2024-06-18T10:39:34Z

No description provided.

github-actions · 2024-06-18T10:41:08Z

Methods

Symbol	Meaning
◯	Execution gas for this method does not include intrinsic gas overhead
△	Cost was non-zero but below the precision setting for the currency display (see options)

	Min	Max	Avg	Calls	usd avg
DealProvider
createNewPool(address[],uint256[],bytes)	259,823	290,623	271,387	65	0.16
ERC20Token
approve(address,uint256)	-	-	46,235	1	0.03
LockDealNFT
approvePoolTransfers(bool)	22,065	43,977	33,021	8	0.02
renounceOwnership()	-	-	23,317	1	0.01
safeTransferFrom(address,address,uint256,bytes)	304,633	424,093	367,425	13	0.22
safeTransferFrom(address,address,uint256)	118,951	174,782	143,528	11	0.08
setApprovedContract(address,bool)	30,672	50,572	46,841	16	0.03
setBaseURI(string)	51,127	79,666	70,153	3	0.04
transferFrom(address,address,uint256)	101,348	119,798	110,573	2	0.07
transferOwnership(address)	-	-	28,694	1	0.02
updateAllMetadata()	-	-	24,534	2	0.01
LockDealProvider
createNewPool(address[],uint256[],bytes)	295,128	306,328	305,528	14	0.18
MockProvider
createNewPool(address[],uint256[],bytes)	-	-	378,647	5	0.22
createNewPoolWithTransfer(address[],uint256[])	-	-	346,764	1	0.20
withdraw(uint256,uint256)	68,012	72,800	70,406	2	0.04
MockTransfer
createNewPool(address[],uint256[],bytes)	-	-	271,112	1	0.16
MockVaultManager
setTransferStatus(bool)	21,703	43,615	32,659	2	0.02
TimedDealProvider
createNewPool(address[],uint256[],bytes)	355,449	384,184	367,475	32	0.22

Deployments

	Min	Max	Avg	Block %	usd avg
DealProvider	1,987,259	1,987,271	1,987,268	1.5 %	1.17
ERC20Token	-	-	790,882	0.6 %	0.47
LockDealNFT	-	-	5,460,030	4.2 %	3.21
LockDealProvider	2,127,083	2,127,095	2,127,091	1.6 %	1.25
MockProvider	-	-	1,102,276	0.8 %	0.65
MockTransfer	-	-	2,031,978	1.6 %	1.20
MockVaultManager	-	-	431,304	0.3 %	0.25
TimedDealProvider	-	-	2,370,488	1.8 %	1.39

Solidity and Network Config

Settings	Value
Solidity: version	0.8.25
Solidity: optimized	true
Solidity: runs	200
Solidity: viaIR	false
Block Limit	130,000,000
L1 Gas Price	1 gwei
Token Price	588.15 usd/bnb
Network	BINANCE
Toolchain	hardhat

github-actions · 2024-06-18T10:42:20Z

Slither report

THIS CHECKLIST IS NOT COMPLETE. Use --show-ignored-findings to show all the results.
Summary

uninitialized-local (1 results) (Medium)
unused-return (1 results) (Medium)
calls-loop (2 results) (Low)
timestamp (4 results) (Low)
dead-code (3 results) (Informational)
naming-convention (13 results) (Informational)
unused-import (2 results) (Informational)
immutable-states (3 results) (Optimization)

uninitialized-local

Impact: Medium
Confidence: Medium

ID-0
LockDealNFTInternal._split(uint256,address,uint256,address).locals is a local variable never initialized

LockDealNFT/contracts/LockDealNFT/LockDealNFTInternal.sol

Line 124 in 5677f10

SplitLocals memory locals;

unused-return

Impact: Medium
Confidence: Medium

ID-1
LockDealNFT.mintAndTransfer(address,address,uint256,IProvider) ignores return value by IERC20(token).approve(address(vaultManager),amount)

LockDealNFT/contracts/LockDealNFT/LockDealNFT.sol

Lines 25 to 42 in 5677f10

    
           function mintAndTransfer( 
        
               address owner, 
        
               address token, 
        
               uint256 amount, 
        
               IProvider provider 
        
           ) 
        
               external 
        
               firewallProtected 
        
               onlyApprovedContract(address(provider)) 
        
               notZeroAddress(owner) 
        
               notZeroAddress(token) 
        
               notZeroAmount(amount) 
        
               returns (uint256 poolId) 
        
           { 
        
               poolId = _mint(owner, provider); 
        
               IERC20(token).approve(address(vaultManager), amount); 
        
               poolIdToVaultId[poolId] = vaultManager.depositByToken(token, amount); 
        
           }

calls-loop

Impact: Low
Confidence: Medium

ID-2
LockDealNFTState._getData(uint256) has external calls inside a loop: poolInfo = BasePoolInfo(provider,provider.name(),poolId,poolIdToVaultId[poolId],ownerOf(poolId),tokenOf(poolId),provider.getParams(poolId))

LockDealNFT/contracts/LockDealNFT/LockDealNFTState.sol

Lines 43 to 54 in 5677f10

    
           function _getData(uint256 poolId) internal view returns (BasePoolInfo memory poolInfo) { 
        
               IProvider provider = poolIdToProvider[poolId]; 
        
               poolInfo = BasePoolInfo( 
        
                   provider, 
        
                   provider.name(), 
        
                   poolId, 
        
                   poolIdToVaultId[poolId], 
        
                   ownerOf(poolId), 
        
                   tokenOf(poolId), 
        
                   provider.getParams(poolId) 
        
               ); 
        
           }

ID-3
LockDealNFTState.tokenOf(uint256) has external calls inside a loop: token = vaultManager.vaultIdToTokenAddress(poolIdToVaultId[poolId])

LockDealNFT/contracts/LockDealNFT/LockDealNFTState.sol

Lines 80 to 82 in 5677f10

    
           function tokenOf(uint256 poolId) public view returns (address token) { 
        
               token = vaultManager.vaultIdToTokenAddress(poolIdToVaultId[poolId]); 
        
           }

timestamp

Impact: Low
Confidence: Medium

ID-4
LockDealNFTInternal._transfer(address,address,uint256) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(vaultManager.vaultIdToTradeStartTime(poolIdToVaultId[poolId]) < block.timestamp,Can't transfer before trade start time)

LockDealNFT/contracts/LockDealNFT/LockDealNFTInternal.sol

Lines 11 to 31 in 5677f10

    
           function _transfer(address from, address to, uint256 poolId) 
        
               internal 
        
               override 
        
               firewallProtectedSig(0x30e0789e) 
        
           { 
        
               if ( 
        
                   from != address(0) && 
        
                   ERC165Checker.supportsInterface(address(poolIdToProvider[poolId]), type(IBeforeTransfer).interfaceId) 
        
               ) { 
        
                   IBeforeTransfer(address(poolIdToProvider[poolId])).beforeTransfer(from, to, poolId); 
        
               } 
        
               // check for split and withdraw transfers 
        
               if (!(approvedContracts[to] || approvedContracts[from])) { 
        
                   require(approvedPoolUserTransfers[from], "Pool transfer not approved by user"); 
        
                   require( 
        
                       vaultManager.vaultIdToTradeStartTime(poolIdToVaultId[poolId]) < block.timestamp, 
        
                       "Can't transfer before trade start time" 
        
                   ); 
        
               } 
        
               super._transfer(from, to, poolId); 
        
           }

ID-5
LockDealProvider._registerPool(uint256,uint256[]) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(block.timestamp <= params[1],Invalid start time)

LockDealNFT/contracts/SimpleProviders/LockProvider/LockDealProvider.sol

Lines 39 to 44 in 5677f10

    
           function _registerPool(uint256 poolId, uint256[] calldata params) internal override firewallProtectedSig(0xfe3627e9) { 
        
               require(block.timestamp <= params[1], "Invalid start time"); 
        
               poolIdToTime[poolId] = params[1]; 
        
               provider.registerPool(poolId, params); 
        
           }

ID-6
TimedDealProvider.getWithdrawableAmount(uint256) uses timestamp for comparisons
Dangerous comparisons:
- block.timestamp < startTime
- finishTime <= block.timestamp

LockDealNFT/contracts/SimpleProviders/TimedDealProvider/TimedDealProvider.sol

Lines 31 to 45 in 5677f10

    
           function getWithdrawableAmount(uint256 poolId) public view override returns (uint256) { 
        
               uint256[] memory params = getParams(poolId); 
        
               uint256 leftAmount = params[0]; 
        
               uint256 startTime = params[1]; 
        
               uint256 finishTime = params[2]; 
        
               uint256 startAmount = params[3]; 
        
               if (block.timestamp < startTime) return 0; 
        
               if (finishTime <= block.timestamp) return leftAmount; 
        
               uint256 totalPoolDuration = finishTime - startTime; 
        
               uint256 timePassed = block.timestamp - startTime; 
        
               uint256 debitableAmount = (startAmount * timePassed) / totalPoolDuration; 
        
               return debitableAmount - (startAmount - leftAmount); 
        
           }

ID-7
LockDealProvider.getWithdrawableAmount(uint256) uses timestamp for comparisons
Dangerous comparisons:
- poolIdToTime[poolId] <= block.timestamp

LockDealNFT/contracts/SimpleProviders/LockProvider/LockDealProvider.sol

Lines 56 to 58 in 5677f10

    
           function getWithdrawableAmount(uint256 poolId) public view override returns (uint256) { 
        
               return poolIdToTime[poolId] <= block.timestamp ? provider.getWithdrawableAmount(poolId) : 0; 
        
           }

dead-code

Impact: Informational
Confidence: Medium

ID-8
ProviderModifiers._validProvider(uint256,IProvider) is never used and should be removed

LockDealNFT/contracts/SimpleProviders/Provider/ProviderModifiers.sol

Lines 42 to 44 in 5677f10

    
           function _validProvider(uint256 poolId, IProvider provider) internal view { 
        
               require(lockDealNFT.poolIdToProvider(poolId) == provider, "Invalid provider poolId"); 
        
           }

ID-9
ProviderModifiers._validProviderInterface(IProvider,bytes4) is never used and should be removed

LockDealNFT/contracts/SimpleProviders/Provider/ProviderModifiers.sol

Lines 58 to 60 in 5677f10

    
           function _validProviderInterface(IProvider provider, bytes4 interfaceId) internal view { 
        
               require(ERC165Checker.supportsInterface(address(provider), interfaceId), "invalid provider type"); 
        
           }

ID-10
BasicProvider._withdraw(uint256,uint256) is never used and should be removed

LockDealNFT/contracts/SimpleProviders/Provider/BasicProvider.sol

Lines 65 to 68 in 5677f10

    
           function _withdraw( 
        
               uint256 poolId, 
        
               uint256 amount 
        
           ) internal virtual returns (uint256 withdrawnAmount, bool isFinal) {}

naming-convention

Impact: Informational
Confidence: High

ID-11
Function ERC20Token.FreeTest() is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/token/ERC20Token.sol#L19-L21

ID-12
Parameter Array.KeepNElementsInArray(uint256[],uint256)._n is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L8

ID-13
Parameter Array.KeepNElementsInArray(address[],uint256)._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L22

ID-14
Parameter Array.addIfNotExsist(address[],address)._elem is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L78

ID-15
Parameter Array.addIfNotExsist(address[],address)._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L78

ID-16
Function Array.KeepNElementsInArray(address[],uint256) is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L22-L34

ID-17
Function Array.KeepNElementsInArray(uint256[],uint256) is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L8-L20

ID-18
Parameter Array.KeepNElementsInArray(address[],uint256)._n is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L22

ID-19
Parameter Array.isInArray(address[],address)._elem is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L67

ID-20
Parameter Array.KeepNElementsInArray(uint256[],uint256)._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L8

ID-21
Parameter Array.getArraySum(uint256[])._array is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L54

ID-22
Parameter Array.isArrayOrdered(uint256[])._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L37

ID-23
Parameter Array.isInArray(address[],address)._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/5677f10716681e8f0bf39cd3683c1c81f6ec5d41/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L67

unused-import

Impact: Informational
Confidence: High

ID-24
The following unused import(s) in contracts/LockDealNFT/ILockDealNFTEvents.sol should be removed:
-import "@poolzfinance/poolz-helper-v2/contracts/interfaces/IProvider.sol"; (contracts/LockDealNFT/ILockDealNFTEvents.sol#4)
ID-25
The following unused import(s) in contracts/SimpleProviders/Provider/BasicProvider.sol should be removed:
-import "@poolzfinance/poolz-helper-v2/contracts/interfaces/IProvider.sol"; (contracts/SimpleProviders/Provider/BasicProvider.sol#5)

immutable-states

Impact: Optimization
Confidence: High

ID-26
ProviderState.lockDealNFT should be immutable

LockDealNFT/contracts/SimpleProviders/Provider/ProviderState.sol

Line 9 in 5677f10

ILockDealNFT public lockDealNFT;

ID-27
LockDealState.provider should be immutable

LockDealNFT/contracts/SimpleProviders/LockProvider/LockDealState.sol

Line 9 in 5677f10

ISimpleProvider public provider;

ID-28
LockDealNFTState.vaultManager should be immutable

LockDealNFT/contracts/LockDealNFT/LockDealNFTState.sol

Line 18 in 5677f10

IVaultManager public vaultManager;

codecov · 2024-06-18T10:42:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.93%. Comparing base (e7e2b65) to head (0563df9).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #413      +/-   ##
==========================================
- Coverage   85.97%   85.93%   -0.04%     
==========================================
  Files          13       13              
  Lines         385      384       -1     
  Branches       69       68       -1     
==========================================
- Hits          331      330       -1     
  Misses         53       53              
  Partials        1        1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

remove firewallProtectedSig from internal split

0563df9

YouStillAlive linked an issue Jun 18, 2024 that may be closed by this pull request

fix size warning #412

Closed

Lomet approved these changes Jun 18, 2024

View reviewed changes

Lomet merged commit 13f36ce into master Jun 18, 2024
7 checks passed

Lomet deleted the size-fix branch June 18, 2024 11:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove firewallProtectedSig from internal split #413

remove firewallProtectedSig from internal split #413

YouStillAlive commented Jun 18, 2024

github-actions bot commented Jun 18, 2024

github-actions bot commented Jun 18, 2024

codecov bot commented Jun 18, 2024

remove firewallProtectedSig from internal split #413

remove firewallProtectedSig from internal split #413

Conversation

YouStillAlive commented Jun 18, 2024

github-actions bot commented Jun 18, 2024

Methods

Deployments

Solidity and Network Config

github-actions bot commented Jun 18, 2024

Slither report

uninitialized-local

unused-return

calls-loop

timestamp

dead-code

naming-convention

unused-import

immutable-states

codecov bot commented Jun 18, 2024

Codecov Report