Issue 393 version 5

[YouStillAlive]

No description provided.

[github-actions]

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)	258,262	289,062	269,821	66	0.76
ERC20Token
approve(address,uint256)	-	-	45,928	1	0.13
LockDealNFT
approvePoolTransfers(bool)	22,706	44,618	33,662	8	0.09
renounceOwnership()	-	-	23,722	1	0.07
safeTransferFrom(address,address,uint256,bytes)	482,309	721,390	607,831	16	1.71
safeTransferFrom(address,address,uint256)	114,466	419,277	257,144	14	0.72
setApprovedContract(address,bool)	28,264	48,164	44,432	16	0.12
setBaseURI(string)	50,883	79,069	69,674	3	0.20
transferFrom(address,address,uint256)	96,627	114,960	105,794	2	0.30
transferOwnership(address)	-	-	29,439	1	0.08
updateAllMetadata()	-	-	24,683	2	0.07
LockDealProvider
createNewPool(address[],uint256[],bytes)	293,559	304,759	304,170	19	0.85
MockProvider
createNewPool(address[],uint256[],bytes)	-	-	377,637	5	1.06
createNewPoolWithTransfer(address[],uint256[])	-	-	340,743	1	0.96
withdraw(uint256,uint256)	74,299	76,477	75,388	2	0.21
MockTransfer
createNewPool(address[],uint256[],bytes)	-	-	269,462	1	0.76
MockVaultManager
setTransferStatus(bool)	21,710	43,622	32,666	2	0.09
TimedDealProvider
createNewPool(address[],uint256[],bytes)	354,367	383,102	366,281	37	1.03

Deployments

	Min	Max	Avg	Block %	usd avg
DealProvider	-	-	1,730,358	1.3 %	4.85
ERC20Token	-	-	658,352	0.5 %	1.85
LockDealNFT	-	-	4,361,099	3.4 %	12.23
LockDealProvider	-	-	1,667,308	1.3 %	4.68
MockProvider	-	-	860,176	0.7 %	2.41
MockTransfer	-	-	1,759,560	1.4 %	4.94
MockVaultManager	-	-	349,644	0.3 %	0.98
TimedDealProvider	2,018,768	2,018,780	2,018,774	1.6 %	5.66

Solidity and Network Config

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.97%. Comparing base (f348001) to head (1d003d1).
Report is 5 commits behind head on issue-393.

Additional details and impacted files

@@              Coverage Diff              @@
##           issue-393     #432      +/-   ##
=============================================
+ Coverage      88.77%   88.97%   +0.19%     
=============================================
  Files             13       13              
  Lines            401      408       +7     
  Branches          96       97       +1     
=============================================
+ Hits             356      363       +7     
  Misses            44       44              
  Partials           1        1              

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

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Slither report

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

• incorrect-equality (1 results) (Medium)
• 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)

incorrect-equality

Impact: Medium
Confidence: High

• ID-0
  TimedDealProvider._withdraw(uint256,uint256) uses a dangerous strict equality:
  • amount == 0

LockDealNFT/contracts/SimpleProviders/TimedDealProvider/TimedDealProvider.sol

Lines 25 to 52 in 343a4e9

	function _withdraw(
	uint256 poolId,
	uint256 amount
	) internal override firewallProtectedSig(0x9e2bf22c) returns (uint256 withdrawnAmount, bool isFinal) {
	if (amount == 0) return (0, false);
	isFinal = true;
	withdrawnAmount = amount;
	uint256[] memory params = provider.getParams(poolId);
	uint256 remainingAmount = params[0] - amount;
	if (remainingAmount > 0 && lastPoolOwner[poolId] != address(0)) {
	// create immutable NFT
	uint256 newPoolId = lockDealNFT.mintForProvider(
	lastPoolOwner[poolId],
	lockDealNFT.poolIdToProvider(poolId)
	);
	// clone vault id
	lockDealNFT.cloneVaultId(newPoolId, poolId);
	// register new pool
	params[0] = remainingAmount;
	provider.registerPool(newPoolId, params);
	poolIdToTime[newPoolId] = poolIdToTime[poolId];
	poolIdToAmount[newPoolId] = poolIdToAmount[poolId];
	delete lastPoolOwner[poolId];
	}
	// Reset and update the original pool
	params[0] = 0;
	provider.registerPool(poolId, params);
	}

uninitialized-local

Impact: Medium
Confidence: Medium

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

LockDealNFT/contracts/LockDealNFT/LockDealNFTInternal.sol

Line 121 in 343a4e9

SplitLocals memory locals;

unused-return

Impact: Medium
Confidence: Medium

• ID-2
  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 343a4e9

	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-3
  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 343a4e9

	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-4
  LockDealNFTState.tokenOf(uint256) has external calls inside a loop: token = vaultManager.vaultIdToTokenAddress(poolIdToVaultId[poolId])

LockDealNFT/contracts/LockDealNFT/LockDealNFTState.sol

Lines 80 to 82 in 343a4e9

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

timestamp

Impact: Low
Confidence: Medium

• ID-5
  TimedDealProvider._withdraw(uint256,uint256) uses timestamp for comparisons
  Dangerous comparisons:
  • amount == 0
  • remainingAmount > 0 && lastPoolOwner[poolId] != address(0)

LockDealNFT/contracts/SimpleProviders/TimedDealProvider/TimedDealProvider.sol

Lines 25 to 52 in 343a4e9

	function _withdraw(
	uint256 poolId,
	uint256 amount
	) internal override firewallProtectedSig(0x9e2bf22c) returns (uint256 withdrawnAmount, bool isFinal) {
	if (amount == 0) return (0, false);
	isFinal = true;
	withdrawnAmount = amount;
	uint256[] memory params = provider.getParams(poolId);
	uint256 remainingAmount = params[0] - amount;
	if (remainingAmount > 0 && lastPoolOwner[poolId] != address(0)) {
	// create immutable NFT
	uint256 newPoolId = lockDealNFT.mintForProvider(
	lastPoolOwner[poolId],
	lockDealNFT.poolIdToProvider(poolId)
	);
	// clone vault id
	lockDealNFT.cloneVaultId(newPoolId, poolId);
	// register new pool
	params[0] = remainingAmount;
	provider.registerPool(newPoolId, params);
	poolIdToTime[newPoolId] = poolIdToTime[poolId];
	poolIdToAmount[newPoolId] = poolIdToAmount[poolId];
	delete lastPoolOwner[poolId];
	}
	// Reset and update the original pool
	params[0] = 0;
	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 54 to 68 in 343a4e9

	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
  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 343a4e9

	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-8
  LockDealProvider.getWithdrawableAmount(uint256) uses timestamp for comparisons
  Dangerous comparisons:
  • poolIdToTime[poolId] <= block.timestamp

LockDealNFT/contracts/SimpleProviders/LockProvider/LockDealProvider.sol

Lines 56 to 58 in 343a4e9

	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-9
  ProviderModifiers._validProvider(uint256,IProvider) is never used and should be removed

LockDealNFT/contracts/SimpleProviders/Provider/ProviderModifiers.sol

Lines 42 to 44 in 343a4e9

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

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

LockDealNFT/contracts/SimpleProviders/Provider/ProviderModifiers.sol

Lines 58 to 60 in 343a4e9

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

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

LockDealNFT/contracts/SimpleProviders/Provider/BasicProvider.sol

Lines 65 to 68 in 343a4e9

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

naming-convention

Impact: Informational
Confidence: High

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/token/ERC20Token.sol#L22-L24

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L11

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L25

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L81

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L81

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

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

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L11-L23

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L25

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L70

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L11

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L57

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L40

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

https://github.com/The-Poolz/LockDealNFT/blob/343a4e9a2b16e46d2fe57974a3170213f8817190/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L70

immutable-states

Impact: Optimization
Confidence: High

• ID-25
  ProviderState.lockDealNFT should be immutable

LockDealNFT/contracts/SimpleProviders/Provider/ProviderState.sol

Line 9 in 343a4e9

ILockDealNFT public lockDealNFT;

• ID-26
  LockDealState.provider should be immutable

LockDealNFT/contracts/SimpleProviders/LockProvider/LockDealState.sol

Line 9 in 343a4e9

ISimpleProvider public provider;

• ID-27
  LockDealNFTState.vaultManager should be immutable

LockDealNFT/contracts/LockDealNFT/LockDealNFTState.sol

Line 18 in 343a4e9

IVaultManager public vaultManager;