add audit report to readme #421

YouStillAlive · 2024-11-25T12:30:25Z

No description provided.

github-actions · 2024-11-25T12:32:00Z

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)	260,252	291,052	271,816	65	0.18
ERC20Token
approve(address,uint256)	-	-	46,323	1	0.03
LockDealNFT
approvePoolTransfers(bool)	22,065	43,977	33,021	8	0.02
renounceOwnership()	-	-	23,317	1	0.02
safeTransferFrom(address,address,uint256,bytes)	299,328	418,787	362,119	13	0.24
safeTransferFrom(address,address,uint256)	118,841	174,672	141,771	11	0.09
setApprovedContract(address,bool)	28,072	47,972	44,241	16	0.03
setBaseURI(string)	51,127	79,666	70,153	3	0.05
transferFrom(address,address,uint256)	98,440	116,890	107,665	2	0.07
transferOwnership(address)	-	-	28,694	1	0.02
updateAllMetadata()	-	-	24,534	2	0.02
LockDealProvider
createNewPool(address[],uint256[],bytes)	295,557	306,757	305,957	14	0.20
MockProvider
createNewPool(address[],uint256[],bytes)	-	-	379,076	5	0.25
createNewPoolWithTransfer(address[],uint256[])	-	-	344,049	1	0.23
withdraw(uint256,uint256)	68,012	72,800	70,406	2	0.05
MockTransfer
createNewPool(address[],uint256[],bytes)	-	-	271,541	1	0.18
MockVaultManager
setTransferStatus(bool)	21,703	43,615	32,659	2	0.02
TimedDealProvider
createNewPool(address[],uint256[],bytes)	355,878	384,613	367,904	32	0.24

Deployments

	Min	Max	Avg	Block %	usd avg
DealProvider	1,965,666	1,965,678	1,965,675	1.5 %	1.31
ERC20Token	-	-	673,031	0.5 %	0.45
LockDealNFT	-	-	5,239,501	4 %	3.48
LockDealProvider	2,104,628	2,104,640	2,104,636	1.6 %	1.40
MockProvider	-	-	1,102,276	0.8 %	0.73
MockTransfer	-	-	2,010,387	1.5 %	1.34
MockVaultManager	-	-	431,292	0.3 %	0.29
TimedDealProvider	-	-	2,348,896	1.8 %	1.56

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	664.41 usd/bnb
Network	BINANCE
Toolchain	hardhat

codecov · 2024-11-25T12:32:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.78%. Comparing base (341f489) to head (5d1d7ff).
Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #421   +/-   ##
=======================================
  Coverage   85.78%   85.78%           
=======================================
  Files          13       13           
  Lines         380      380           
  Branches       92       92           
=======================================
  Hits          326      326           
  Misses         53       53           
  Partials        1        1

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

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

github-actions · 2024-11-25T12:33:57Z

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)
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 121 in c7ba27d

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 c7ba27d

    
           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 c7ba27d

    
           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 c7ba27d

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

timestamp

Impact: Low
Confidence: Medium

ID-4
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 c7ba27d

    
           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-5
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 c7ba27d

    
           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-6
LockDealNFTInternal._update(address,uint256,address) 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 28 in c7ba27d

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

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 c7ba27d

    
           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 c7ba27d

    
           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 c7ba27d

    
           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 c7ba27d

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