Platform: Code4rena
Start Date: 30/06/2023
Pot Size: $100,000 USDC
Total HM: 8
Participants: 22
Period: 14 days
Judge: Trust
Total Solo HM: 6
Id: 253
League: ETH
Rank: 8/22
Findings: 2
Award: $454.76
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: gpersoon
Also found by: DavidGiladi, MiloTruck, Rolezn, banpaleo5, catellatech, matrix_0wl, naman1778, vnavascues
421.2796 USDC - $421.28
I reported issues that were overlooked by the winning bot. I reported only on instances that were missed
| Title | Issue | Instances |
|---|---|---|
| [L-1] Array out of bounds accesses | Array out of bounds accesses | 1 |
| [L-2] Burn functions must be protected with a modifier | Burn functions must be protected with a modifier | 9 |
| [L-3] Controlled Delegatecall | Controlled Delegatecall | 1 |
[L-4] Payable functions using delegatecall inside a loop | Payable functions using delegatecall inside a loop | 1 |
| [L-5] NFT contract redefines mint()/safeMint(), but not both | NFT contract redefines mint()/safeMint(), but not both | 1 |
| [L-6] Local variable shadowing | Local variable shadowing | 6 |
| [L-7] Contracts that lock Ether | Contracts that lock Ether | 9 |
| [L-8] Missing gap Storage Variable in Upgradeable Contract | Missing gap Storage Variable in Upgradeable Contract | 24 |
| [L-9] Missing zero address validation | Missing zero address validation | 4 |
[L-10] msg.value inside a loop | msg.value inside a loop | 4 |
| [L-11] Calls inside a loop | Calls inside a loop | 16 |
| [L-12] Reentrancy vulnerabilities | Reentrancy vulnerabilities | 4 |
| [L-13] Unbounded gas for external call | Unbounded gas for external call | 4 |
| [L-14] Unused return | Unused return | 12 |
| [L-15] Use 'abi.encodeCall()' Instead of 'abi.encodeSignature()'/abi.encodeSelector() | Use 'abi.encodeCall()' Instead of 'abi.encodeSignature()'/abi.encodeSelector() | 5 |
| [L-16] Use of transferFrom() rather than safeTransferFrom() for NFTs | Use of transferFrom() rather than safeTransferFrom() for NFTs | 2 |
Total: 16 issues
If the lengths of arrays are not checked before they're accessed, user operations may not be executed properly due to the potential issue of accessing an array out of its bounds. In Solidity, accessing an array beyond its boundaries can cause a runtime error known as an out-of-bounds exception. This error arises when attempting to read or write to an element of an array that does not exist. Therefore, it is critical to avoid out-of-bounds exceptions while accessing arrays in Solidity code to prevent unexpected halts and possible loss of funds.
<details> <summary> There are 1 instances of this issue: </summary>Line: 626 bool[] memory validatedInputKeysList
is accessed on File: contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol
Line: 729 validatedInputKeysList[ii];
index that might be out-of-bounds
</details>If burn functions are not protected by a modifier, any address may be able to burn tokens, potentially leading to financial loss. A common modifier to use is onlyOwner.
Line: 338 function _burn( address from, uint256 amount, bytes memory data ) internal virtual
Burn function without a protective modifier. https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol#L338-L384
Line: 17 function burn(address from, uint256 amount, bytes memory data) public
Burn function without a protective modifier. https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7Burnable.sol#L17-L19
Line: 19 function burn(address from, uint256 amount, bytes memory data) public
Burn function without a protective modifier. https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7BurnableInitAbstract.sol#L19-L21
Line: 137 function _burn( address from, uint256 amount, bytes memory data ) internal virtual override
Burn function without a protective modifier. https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.sol#L137-L144
Line: 145 function _burn( address from, uint256 amount, bytes memory data ) internal virtual override
Burn function without a protective modifier. https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20InitAbstract.sol#L145-L152
Line: 359 function _burn(bytes32 tokenId, bytes memory data) internal virtual
Burn function without a protective modifier. https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol#L359-L382
Line: 16 function burn(bytes32 tokenId, bytes memory data) public
Burn function without a protective modifier. https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8Burnable.sol#L16-L21
Line: 269 function _burn( bytes32 tokenId, bytes memory data ) internal virtual override
Burn function without a protective modifier. https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721.sol#L269-L277
Line: 269 function _burn( bytes32 tokenId, bytes memory data ) internal virtual override
Burn function without a protective modifier. https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721InitAbstract.sol#L269-L277
</details>This detector identifies instances where a contract's code allows a user to control the target of a delegatecall or callcode operation. Delegatecalls and callcodes are powerful features in Solidity that allow one contract to execute the code of another contract within its own context.
However, they also pose significant security risks if misused or not properly secured. When the target of a delegatecall or callcode is controlled by a user, that user may be able to manipulate the contract's state or behavior in unexpected and potentially harmful ways.
<details> <summary> There are 1 instances of this issue: </summary>Line: 225 function _executeDelegateCall( address target, bytes memory data ) internal virtual returns (bytes memory result)
uses delegatecall to a input-controlled function id - File: submodules/ERC725/implementations/contracts/ERC725XCore.sol
</details>Line: 232 (bool success, bytes memory returnData) = target.delegatecall(data)
delegatecall inside a loopDetect the use of delegatecall inside a loop in a payable function.
Line: 225 function _executeDelegateCall( address target, bytes memory data ) internal virtual returns (bytes memory result)
has delegatecall inside a loop in a payable function: File: submodules/ERC725/implementations/contracts/ERC725XCore.sol
</details>Line: 232 (bool success, bytes memory returnData) = target.delegatecall(data)
ERC721 contracts should have both _mint() and _safeMint() consistently redefined for spec compatibility. The _mint() variant is supposed to skip onERC721Received() checks, whereas _safeMint() does not. If one of these functions is re-implemented or has new arguments, the other should as well.
<details> <summary> There are 1 instances of this issue: </summary>Line: 48 abstract contract LSP8CompatibleERC721InitAbstract is ILSP8CompatibleERC721, LSP8IdentifiableDigitalAssetInitAbstract, LSP4Compatibility
Redefine _mint() without redefine _safeMint(). https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721InitAbstract.sol#L48-L351
</details>Detection of shadowing using local variables.
<details> <summary> There are 6 instances of this issue: </summary>Line: 232 address _owner = owner()
shadows: - File: submodules/ERC725/implementations/contracts/custom/OwnableUnset.sol
Line: 12 address private _owner
(state variable)
Line: 290 address _owner = owner()
shadows: - File: submodules/ERC725/implementations/contracts/custom/OwnableUnset.sol
Line: 12 address private _owner
(state variable)
Line: 347 address _owner = owner()
shadows: - File: submodules/ERC725/implementations/contracts/custom/OwnableUnset.sol
Line: 12 address private _owner
(state variable)
Line: 392 address _owner = owner()
shadows: - File: submodules/ERC725/implementations/contracts/custom/OwnableUnset.sol
Line: 12 address private _owner
(state variable)
Line: 660 address _owner = owner()
shadows: - File: submodules/ERC725/implementations/contracts/custom/OwnableUnset.sol
Line: 12 address private _owner
(state variable)
Line: 738 address _owner = owner()
shadows: - File: submodules/ERC725/implementations/contracts/custom/OwnableUnset.sol
Line: 12 address private _owner
(state variable)
</details>Contract with a payable function, but without a withdrawal capacity.
Line: 62 contract LSP1UniversalReceiverDelegateUP is ERC165, ILSP1UniversalReceiver
has payable functions: - File: contracts/LSP1UniversalReceiver/ILSP1UniversalReceiver.sol
Line: 32 function universalReceiver( bytes32 typeId, bytes calldata data ) external payable returns (bytes memory);
- File: contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol
Line: 78 function universalReceiver( bytes32 typeId, bytes memory /* data */ ) public payable virtual returns (bytes memory result)
But does not have a function to withdraw the ether
Line: 6 contract LSP7CompatibleERC20Mintable is LSP7CompatibleERC20
has payable functions: - File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 52 function setData(bytes32 dataKey, bytes memory dataValue) external payable;
- File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 67 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) external payable;
- File: submodules/ERC725/implementations/contracts/ERC725Y.sol
Line: 21 constructor(address newOwner) payable
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 62 function setData( bytes32 dataKey, bytes memory dataValue ) public payable virtual override onlyOwner
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 73 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) public payable virtual override onlyOwner
But does not have a function to withdraw the ether
Line: 9 contract LSP7CompatibleERC20MintableInit is LSP7CompatibleERC20MintableInitAbstract
has payable functions: - File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 52 function setData(bytes32 dataKey, bytes memory dataValue) external payable;
- File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 67 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) external payable;
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 62 function setData( bytes32 dataKey, bytes memory dataValue ) public payable virtual override onlyOwner
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 73 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) public payable virtual override onlyOwner
But does not have a function to withdraw the ether
Line: 16 contract LSP7Mintable is LSP7DigitalAsset, ILSP7Mintable
has payable functions: - File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 52 function setData(bytes32 dataKey, bytes memory dataValue) external payable;
- File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 67 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) external payable;
- File: submodules/ERC725/implementations/contracts/ERC725Y.sol
Line: 21 constructor(address newOwner) payable
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 62 function setData( bytes32 dataKey, bytes memory dataValue ) public payable virtual override onlyOwner
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 73 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) public payable virtual override onlyOwner
But does not have a function to withdraw the ether
Line: 11 contract LSP7MintableInit is LSP7MintableInitAbstract
has payable functions: - File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 52 function setData(bytes32 dataKey, bytes memory dataValue) external payable;
- File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 67 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) external payable;
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 62 function setData( bytes32 dataKey, bytes memory dataValue ) public payable virtual override onlyOwner
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 73 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) public payable virtual override onlyOwner
But does not have a function to withdraw the ether
Line: 6 contract LSP8CompatibleERC721Mintable is LSP8CompatibleERC721
has payable functions: - File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 52 function setData(bytes32 dataKey, bytes memory dataValue) external payable;
- File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 67 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) external payable;
- File: submodules/ERC725/implementations/contracts/ERC725Y.sol
Line: 21 constructor(address newOwner) payable
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 62 function setData( bytes32 dataKey, bytes memory dataValue ) public payable virtual override onlyOwner
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 73 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) public payable virtual override onlyOwner
But does not have a function to withdraw the ether
Line: 9 contract LSP8CompatibleERC721MintableInit is LSP8CompatibleERC721MintableInitAbstract
has payable functions: - File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 62 function setData( bytes32 dataKey, bytes memory dataValue ) public payable virtual override onlyOwner
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 73 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) public payable virtual override onlyOwner
- File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 52 function setData(bytes32 dataKey, bytes memory dataValue) external payable;
- File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 67 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) external payable;
But does not have a function to withdraw the ether
Line: 16 contract LSP8Mintable is LSP8IdentifiableDigitalAsset, ILSP8Mintable
has payable functions: - File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 52 function setData(bytes32 dataKey, bytes memory dataValue) external payable;
- File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 67 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) external payable;
- File: submodules/ERC725/implementations/contracts/ERC725Y.sol
Line: 21 constructor(address newOwner) payable
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 62 function setData( bytes32 dataKey, bytes memory dataValue ) public payable virtual override onlyOwner
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 73 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) public payable virtual override onlyOwner
But does not have a function to withdraw the ether
Line: 11 contract LSP8MintableInit is LSP8MintableInitAbstract
has payable functions: - File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 52 function setData(bytes32 dataKey, bytes memory dataValue) external payable;
- File: submodules/ERC725/implementations/contracts/interfaces/IERC725Y.sol
Line: 67 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) external payable;
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 62 function setData( bytes32 dataKey, bytes memory dataValue ) public payable virtual override onlyOwner
- File: submodules/ERC725/implementations/contracts/ERC725YCore.sol
Line: 73 function setDataBatch( bytes32[] memory dataKeys, bytes[] memory dataValues ) public payable virtual override onlyOwner
</details>But does not have a function to withdraw the ether
upgradeable contracts that are missing a '__gap' storage variable. In upgradeable contracts, it is important to reserve storage slots for future versions to introduce new storage variables. The '__gap' storage variable acts as a placeholder, allowing for seamless upgrades without affecting existing storage layout.When a contract is not designed with a '__gap' storage variable, adding new storage variables in subsequent versions becomes problematic. It can lead to storage collisions or layout incompatibilities, making it difficult to upgrade the contract without requiring costly data migrations or redeployments.
<details> <summary> There are 24 instances of this issue: </summary>Line: 40 contract LSP0ERC725AccountInit is LSP0ERC725AccountInitAbstract
missing __gap storage variable
Line: 44 abstract contract LSP0ERC725AccountInitAbstract is Initializable, LSP0ERC725AccountCore
missing __gap storage variable
Line: 26 abstract contract LSP4DigitalAssetMetadataInitAbstract is ERC725YInitAbstract
missing __gap storage variable
Line: 12 contract LSP6KeyManagerInit is LSP6KeyManagerInitAbstract
missing __gap storage variable
Line: 16 abstract contract LSP6KeyManagerInitAbstract is Initializable, LSP6KeyManagerCore
missing __gap storage variable
Line: 24 abstract contract LSP7DigitalAssetInitAbstract is LSP4DigitalAssetMetadataInitAbstract, LSP7DigitalAssetCore
missing __gap storage variable
Line: 13 abstract contract LSP7BurnableInitAbstract is LSP7DigitalAssetInitAbstract
missing __gap storage variable
Line: 13 abstract contract LSP7CappedSupplyInitAbstract is LSP7DigitalAssetInitAbstract
missing __gap storage variable
Line: 21 abstract contract LSP7CompatibleERC20InitAbstract is ILSP7CompatibleERC20, LSP4Compatibility, LSP7DigitalAssetInitAbstract
missing __gap storage variable
Line: 9 contract LSP7CompatibleERC20MintableInit is LSP7CompatibleERC20MintableInitAbstract
missing __gap storage variable
Line: 9 contract LSP7CompatibleERC20MintableInitAbstract is LSP7CompatibleERC20InitAbstract
missing __gap storage variable
Line: 11 contract LSP7MintableInit is LSP7MintableInitAbstract
missing __gap storage variable
Line: 16 abstract contract LSP7MintableInitAbstract is LSP7DigitalAssetInitAbstract, ILSP7Mintable
missing __gap storage variable
Line: 26 abstract contract LSP8IdentifiableDigitalAssetInitAbstract is LSP4DigitalAssetMetadataInitAbstract, LSP8IdentifiableDigitalAssetCore
missing __gap storage variable
Line: 13 abstract contract LSP8CappedSupplyInitAbstract is LSP8IdentifiableDigitalAssetInitAbstract
missing __gap storage variable
Line: 48 abstract contract LSP8CompatibleERC721InitAbstract is ILSP8CompatibleERC721, LSP8IdentifiableDigitalAssetInitAbstract, LSP4Compatibility
missing __gap storage variable
Line: 16 abstract contract LSP8EnumerableInitAbstract is LSP8IdentifiableDigitalAssetInitAbstract
missing __gap storage variable
Line: 9 contract LSP8CompatibleERC721MintableInit is LSP8CompatibleERC721MintableInitAbstract
missing __gap storage variable
Line: 9 contract LSP8CompatibleERC721MintableInitAbstract is LSP8CompatibleERC721InitAbstract
missing __gap storage variable
Line: 11 contract LSP8MintableInit is LSP8MintableInitAbstract
missing __gap storage variable
Line: 15 abstract contract LSP8MintableInitAbstract is LSP8IdentifiableDigitalAssetInitAbstract, ILSP8Mintable
missing __gap storage variable
Line: 12 contract UniversalProfileInit is UniversalProfileInitAbstract
missing __gap storage variable
https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/UniversalProfileInit.sol#L12-L39
Line: 20 abstract contract UniversalProfileInitAbstract is LSP0ERC725AccountInitAbstract
missing __gap storage variable
Line: 18 abstract contract ERC725YInitAbstract is Initializable, ERC725YCore
missing __gap storage variable
</details>Detect missing zero address validation.
<details> <summary> There are 4 instances of this issue: </summary>Line: 129 _pendingOwner = newOwner
state variable _pendingOwner assign with newOwner without checking
Line: 20 _target = target_
state variable _target assign with target_ without checking
Line: 22 _target = target_
state variable _target assign with target_ without checking
Line: 71 _owner = newOwner
state variable _owner assign with newOwner without checking
msg.value inside a loopDetect the use of msg.value inside a loop.
Line: 152 function executeBatch( uint256[] calldata values, bytes[] calldata payloads ) public payable virtual returns (bytes[] memory)
use msg.value in a loop: File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 164 (totalValues += values[ii]) > msg.value
Line: 152 function executeBatch( uint256[] calldata values, bytes[] calldata payloads ) public payable virtual returns (bytes[] memory)
use msg.value in a loop: File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 165 revert LSP6BatchInsufficientValueSent(totalValues, msg.value)
Line: 204 function executeRelayCallBatch( bytes[] memory signatures, uint256[] calldata nonces, uint256[] calldata validityTimestamps, uint256[] calldata values, bytes[] calldata payloads ) public payable virtual returns (bytes[] memory)
use msg.value in a loop: File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 224 (totalValues += values[ii]) > msg.value
Line: 204 function executeRelayCallBatch( bytes[] memory signatures, uint256[] calldata nonces, uint256[] calldata validityTimestamps, uint256[] calldata values, bytes[] calldata payloads ) public payable virtual returns (bytes[] memory)
use msg.value in a loop: File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
</details>Line: 225 revert LSP6BatchInsufficientValueSent(totalValues, msg.value)
Calls inside a loop might lead to a denial-of-service attack.
<details> <summary> There are 16 instances of this issue: </summary>Line: 171 function batchCalls( bytes[] calldata data ) public returns (bytes[] memory results)
has external calls inside a loop: File: contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol
Line: 176 (bool success, bytes memory result) = address(this).delegatecall( data[i] )
Line: 416 function _executePayload( uint256 msgValue, bytes calldata payload ) internal virtual returns (bytes memory)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 420 (bool success, bytes memory returnData) = _target.call{ value: msgValue, gas: gasleft() }(payload)
Line: 334 function _getPermissionToSetPermissionsArray( address controlledContract, bytes32 inputDataKey, bytes memory inputDataValue, bool hasBothAddControllerAndEditPermissions ) internal view virtual returns (bytes32)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol
Line: 349 newLength > uint128( bytes16( ERC725Y(controlledContract).getData(inputDataKey) ) )
Line: 334 function _getPermissionToSetPermissionsArray( address controlledContract, bytes32 inputDataKey, bytes memory inputDataValue, bool hasBothAddControllerAndEditPermissions ) internal view virtual returns (bytes32)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol
Line: 374 ERC725Y(controlledContract).getData(inputDataKey).length == 0
Line: 385 function _getPermissionToSetControllerPermissions( address controlledContract, bytes32 inputPermissionDataKey ) internal view virtual returns (bytes32)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol
Line: 392 bytes32( ERC725Y(controlledContract).getData(inputPermissionDataKey) ) == bytes32(0)
Line: 406 function _getPermissionToSetAllowedCalls( address controlledContract, bytes32 dataKey, bytes memory dataValue, bool hasBothAddControllerAndEditPermissions ) internal view virtual returns (bytes32)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol
Line: 426 ERC725Y(controlledContract).getData(dataKey).length == 0
Line: 438 function _getPermissionToSetAllowedERC725YDataKeys( address controlledContract, bytes32 dataKey, bytes memory dataValue, bool hasBothAddControllerAndEditPermissions ) internal view returns (bytes32)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol
Line: 461 ERC725Y(controlledContract).getData(dataKey).length == 0
Line: 474 function _getPermissionToSetLSP1Delegate( address controlledContract, bytes32 lsp1DelegateDataKey ) internal view virtual returns (bytes32)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol
Line: 479 ERC725Y(controlledContract).getData(lsp1DelegateDataKey).length == 0
Line: 490 function _getPermissionToSetLSP17Extension( address controlledContract, bytes32 lsp17ExtensionDataKey ) internal view virtual returns (bytes32)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol
Line: 495 ERC725Y(controlledContract).getData(lsp17ExtensionDataKey).length == 0
Line: 25 function getPermissionsFor( IERC725Y target, address caller ) internal view returns (bytes32)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Utils.sol
Line: 29 target.getData( LSP2Utils.generateMappingWithGroupingKey( _LSP6KEY_ADDRESSPERMISSIONS_PERMISSIONS_PREFIX, bytes20(caller) ) )
https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP6KeyManager/LSP6Utils.sol#L25-L37
Line: 58 function getAllowedERC725YDataKeysFor( IERC725Y target, address caller ) internal view returns (bytes memory)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Utils.sol
Line: 63 target.getData( LSP2Utils.generateMappingWithGroupingKey( _LSP6KEY_ADDRESSPERMISSIONS_AllowedERC725YDataKeys_PREFIX, bytes20(caller) ) )
https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP6KeyManager/LSP6Utils.sol#L58-L69
Line: 39 function getAllowedCallsFor( IERC725Y target, address from ) internal view returns (bytes memory)
has external calls inside a loop: File: contracts/LSP6KeyManager/LSP6Utils.sol
Line: 44 target.getData( LSP2Utils.generateMappingWithGroupingKey( _LSP6KEY_ADDRESSPERMISSIONS_ALLOWEDCALLS_PREFIX, bytes20(from) ) )
https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP6KeyManager/LSP6Utils.sol#L39-L50
Line: 452 function _notifyTokenSender( address from, bytes memory lsp1Data ) internal virtual
has external calls inside a loop: File: contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol
Line: 462 ILSP1UniversalReceiver(from).universalReceiver( _TYPEID_LSP7_TOKENSSENDER, lsp1Data )
Line: 475 function _notifyTokenReceiver( address to, bool allowNonLSP1Recipient, bytes memory lsp1Data ) internal virtual
has external calls inside a loop: File: contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol
Line: 486 ILSP1UniversalReceiver(to).universalReceiver( _TYPEID_LSP7_TOKENSRECIPIENT, lsp1Data )
Line: 454 function _notifyTokenSender( address from, bytes memory lsp1Data ) internal virtual
has external calls inside a loop: File: contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol
Line: 464 ILSP1UniversalReceiver(from).universalReceiver( _TYPEID_LSP8_TOKENSSENDER, lsp1Data )
Line: 477 function _notifyTokenReceiver( address to, bool allowNonLSP1Recipient, bytes memory lsp1Data ) internal virtual
has external calls inside a loop: File: contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol
</details>Line: 488 ILSP1UniversalReceiver(to).universalReceiver( _TYPEID_LSP8_TOKENSRECIPIENT, lsp1Data )
Detection of the reentrancy bug.
Only report reentrancy that acts as a double call (see reentrancy-eth, reentrancy-no-eth).
Line: 655 function renounceOwnership() public virtual override(LSP14Ownable2Step, OwnableUnset)
: External calls: - File: contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol
Line: 669 bool verifyAfter = _verifyCall(_owner)
- File: contracts/LSP20CallVerification/LSP20CallVerification.sol
Line: 26 (bool success, bytes memory returnedData) = logicVerifier.call( abi.encodeWithSelector( ILSP20.lsp20VerifyCall.selector, msg.sender, msg.value, msg.data ) )
State variables written after the call(s): - File: contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol
Line: 672 LSP14Ownable2Step._renounceOwnership()
- File: contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol
Line: 164 delete _pendingOwner
- File: contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol
Line: 672 LSP14Ownable2Step._renounceOwnership()
- File: contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol
Line: 163 _renounceOwnershipStartedAt = currentBlock
- File: contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol
Line: 177 delete _renounceOwnershipStartedAt
Line: 557 function transferOwnership( address pendingNewOwner ) public virtual override(LSP14Ownable2Step, OwnableUnset)
: External calls: - File: contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol
Line: 583 bool verifyAfter = _verifyCall(currentOwner)
- File: contracts/LSP20CallVerification/LSP20CallVerification.sol
Line: 26 (bool success, bytes memory returnedData) = logicVerifier.call( abi.encodeWithSelector( ILSP20.lsp20VerifyCall.selector, msg.sender, msg.value, msg.data ) )
State variables written after the call(s): - File: contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol
Line: 586 LSP14Ownable2Step._transferOwnership(pendingNewOwner)
- File: contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol
Line: 129 _pendingOwner = newOwner
- File: contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol
Line: 586 LSP14Ownable2Step._transferOwnership(pendingNewOwner)
- File: contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol
</details>Line: 130 delete _renounceOwnershipStartedAt
Detection of the reentrancy bug.
Do not report reentrancies that don't involve Ether (see reentrancy-no-eth)
Line: 315 function _execute( uint256 msgValue, bytes calldata payload ) internal virtual returns (bytes memory)
: External calls: - File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 336 bytes memory result = _executePayload(msgValue, payload)
- File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 420 (bool success, bytes memory returnData) = _target.call{ value: msgValue, gas: gasleft() }(payload)
State variables written after the call(s): - File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 339 _nonReentrantAfter()
- File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 553 _reentrancyStatus = false
File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 83 bool private _reentrancyStatus
can be used in cross function reentrancies: - File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 550 function _nonReentrantAfter() internal virtual
- File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 527 function _nonReentrantBefore( bool isSetData, address from ) internal virtual returns (bool isReentrantCall)
- File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 251 function lsp20VerifyCall( address caller, uint256 msgValue, bytes calldata data ) external returns (bytes4)
Line: 345 function _executeRelayCall( bytes memory signature, uint256 nonce, uint256 validityTimestamps, uint256 msgValue, bytes calldata payload ) internal virtual returns (bytes memory)
: External calls: - File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 402 bytes memory result = _executePayload(msgValue, payload)
- File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 420 (bool success, bytes memory returnData) = _target.call{ value: msgValue, gas: gasleft() }(payload)
State variables written after the call(s): - File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 405 _nonReentrantAfter()
- File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 553 _reentrancyStatus = false
File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 83 bool private _reentrancyStatus
can be used in cross function reentrancies: - File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 550 function _nonReentrantAfter() internal virtual
- File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Line: 527 function _nonReentrantBefore( bool isSetData, address from ) internal virtual returns (bool isReentrantCall)
- File: contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
</details>Line: 251 function lsp20VerifyCall( address caller, uint256 msgValue, bytes calldata data ) external returns (bytes4)
There is no limit specified on the amount of gas used, so the recipient can use up all of the transaction's gas, causing it to revert. Use addr.call{gas: <amount>}() or this library instead.
Line: 176 (bool success, bytes memory result) = address(this).delegatecall( data[i] )
Line: 742 (bool success, bytes memory result) = _owner.staticcall( abi.encodeWithSelector( IERC1271.isValidSignature.selector, dataHash, signature ) )
Line: 232 (bool success, bytes memory returnData) = target.delegatecall(data)
</details>Line: 210 (bool success, bytes memory returnData) = target.staticcall(data)
The return value of an external call is not stored in a local or state variable.
<details> <summary> There are 12 instances of this issue: </summary>Line: 33 ILSP1UniversalReceiver(lsp1Implementation).universalReceiver( typeId, data )
ignores return value
Line: 60 Address.verifyCallResult( success, result, "Call to universalReceiver failed" )
ignores return value
Line: 462 ILSP1UniversalReceiver(from).universalReceiver( _TYPEID_LSP7_TOKENSSENDER, lsp1Data )
ignores return value
Line: 486 ILSP1UniversalReceiver(to).universalReceiver( _TYPEID_LSP7_TOKENSRECIPIENT, lsp1Data )
ignores return value
Line: 334 _ownedTokens[to].add(tokenId)
ignores return value
Line: 370 _ownedTokens[tokenOwner].remove(tokenId)
ignores return value
Line: 420 _ownedTokens[from].remove(tokenId)
ignores return value
Line: 421 _ownedTokens[to].add(tokenId)
ignores return value
Line: 464 ILSP1UniversalReceiver(from).universalReceiver( _TYPEID_LSP8_TOKENSSENDER, lsp1Data )
ignores return value
Line: 488 ILSP1UniversalReceiver(to).universalReceiver( _TYPEID_LSP8_TOKENSRECIPIENT, lsp1Data )
ignores return value
Line: 314 try IERC721Receiver(to).onERC721Received( msg.sender, from, tokenId, data ) returns (bytes4 retval) { return retval == IERC721Receiver.onERC721Received.selector; } catch (bytes memory reason) { if (reason.length == 0) { revert( "LSP8CompatibleERC721: transfer to non ERC721Receiver implementer" ); } else { // solhint-disable no-inline-assembly /// @solidity memory-safe-assembly assembly { revert(add(32, reason), mload(reason)) } } }
ignores return value
Line: 314 try IERC721Receiver(to).onERC721Received( msg.sender, from, tokenId, data ) returns (bytes4 retval) { return retval == IERC721Receiver.onERC721Received.selector; } catch (bytes memory reason) { if (reason.length == 0) { revert( "LSP8CompatibleERC721: transfer to non ERC721Receiver implementer" ); } else { // solhint-disable no-inline-assembly /// @solidity memory-safe-assembly assembly { revert(add(32, reason), mload(reason)) } } }
ignores return value
</details>abi.encodeCall() has compiler type safety, whereas the other two functions do not.
<details> <summary> There are 5 instances of this issue: </summary>Line: 743 abi.encodeWithSelector( IERC1271.isValidSignature.selector, dataHash, signature )
use abi.encodeCall()
Line: 48 abi.encodeWithSelector( ILSP1UniversalReceiver.universalReceiver.selector, typeId, receivedData )
use abi.encodeCall()
Line: 27 abi.encodeWithSelector( ILSP20.lsp20VerifyCall.selector, msg.sender, msg.value, msg.data )
use abi.encodeCall()
Line: 54 abi.encodeWithSelector( ILSP20.lsp20VerifyCallResult.selector, keccak256(abi.encodePacked(msg.sender, msg.value, msg.data)), callResult )
use abi.encodeCall()
Line: 155 abi.encodeWithSelector( IERC725Y.setDataBatch.selector, keys, values )
use abi.encodeCall()
</details>Use of transferFrom() rather than safeTransferFrom() for NFTs in will lead to the loss of NFTs
The EIP-721 standard says the following about transferFrom():
/// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE /// TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING NFTS OR ELSE /// THEY MAY BE PERMANENTLY LOST /// @dev Throws unless `msg.sender` is the current owner, an authorized /// operator, or the approved address for this NFT. Throws if `_from` is /// not the current owner. Throws if `_to` is the zero address. Throws if /// `_tokenId` is not a valid NFT. /// @param _from The current owner of the NFT /// @param _to The new owner /// @param _tokenId The NFT to transfer function transferFrom(address _from, address _to, uint256 _tokenId) external payable;
https://github.com/ethereum/EIPs/blob/78e2c297611f5e92b6a5112819ab71f74041ff25/EIPS/eip-721.md?plain=1#L103-L113
Code must use the safeTransferFrom() flavor if it hasn't otherwise verified that the receiving address can handle it
Line: 173 transfer( from[i], to[i], amount[i], allowNonLSP1Recipient[i], data[i] )
</details>Line: 228 transfer( from[i], to[i], tokenId[i], allowNonLSP1Recipient[i], data[i] )
#0 - c4-pre-sort
2023-07-17T22:45:05Z
minhquanym marked the issue as high quality report
#1 - c4-sponsor
2023-07-28T08:56:06Z
CJ42 marked the issue as sponsor acknowledged
#2 - CJ42
2023-07-28T08:57:07Z
We put acknowledged because roughly 50 % of the reported points are valid and can be fixed. The rest lack of details or cannot be implemented because they are design decisions.
#3 - c4-judge
2023-08-02T09:45:43Z
trust1995 marked the issue as grade-a
🌟 Selected for report: Raihan
Also found by: DavidGiladi, LeoS, Rageur, ReyAdmirado, Rolezn, SAAJ, SAQ, SM3_SS, Sathish9098, hunter_w3b, matrix_0wl, naman1778, petrichor
33.4768 USDC - $33.48
| Title | Issue | Instances | Total Gas Saved |
|---|---|---|---|
| Multiplication and Division by 2 Should use in Bit Shifting | Multiplication and Division by 2 Should use in Bit Shifting | 3 | 60 |
| Check Arguments Early | Check Arguments Early | 2 | - |
| Potential Optimization by Combining Multiple Mappings into a Struct | Potential Optimization by Combining Multiple Mappings into a Struct | 1 | - |
| State variables that could be declared immutable | State variables that could be declared immutable | 2 | 4194 |
| Inefficient Parameter Storage | Inefficient Parameter Storage | 49 | - |
| Internal or Private Function that Called Once Should Be Inlined to Save Gas | Internal or Private Function that Called Once Should Be Inlined to Save Gas | 10 | 200 |
| State variables should be cached in stack variables rather than re-reading them from storage | State variables should be cached in stack variables rather than re-reading them from storage | 8 | 776 |
| Unnecessary Casting of Variables | Unnecessary Casting of Variables | 1 | - |
| Redundant Contract Existence Check in Consecutive External Calls | Redundant Contract Existence Check in Consecutive External Calls | 2 | 200 |
| Unused Named Return Variables | Unused Named Return Variables | 11 | - |
| Cache Array Length Outside of Loop | Cache Array Length Outside of Loop | 6 | 18 |
Total: 94 instances over 11 issues
The expressions 'x * 2' and 'x / 2' can be optimized for gas efficiency by utilizing bitwise operations. In Solidity, you can achieve the same results by using bitwise left shift (x << 1) for multiplication and bitwise right shift (x >> 1) for division.
Using bitwise shift operations (SHL and SHR) instead of multiplication (MUL) and division (DIV) opcodes can lead to significant gas savings. The MUL and DIV opcodes cost 5 gas, while the SHL and SHR opcodes incur a lower cost of only 3 gas.
By leveraging these more efficient bitwise operations, you can reduce the gas consumption of your smart contracts and enhance their overall performance.
<details> <summary> There are 3 instances of this issue: </summary>Line: 242 nbOfBytes < (offset + 32 + (arrayLength * 32))
instead 32 use bit shifting 5
Line: 581 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ) << (8 * (32 - length)); /* * tran
instead 8 use bit shifting 3
Line: 701 ffffffffffffffffffffffffffffffffffffffff ) << (8 * (32 - length)); /* * transform the allowed data key situated from
instead 8 use bit shifting 3
I have reported only on issues that were overlooked by the winning bot.
This detector identifies instances where multiple mappings of an address or ID could be combined into a single mapping to a struct. This optimisation not only saves a storage slot for each mapping that is combined, but also makes the contract more efficient in terms of gas usage. Depending on the circumstances and sizes of types, it can avoid a 20000 gas cost (Gsset) per combined mapping. Also, it can make reads and subsequent writes cheaper when a function requires both values and they both fit in the same storage slot. Lastly, if both fields are accessed in the same function, it can save approximately 42 gas per access due to not having to recalculate the key's keccak256 hash and its associated stack operations.
<details> <summary> There are 1 instances of this issue: </summary>Line: 46 mapping(bytes32 => address) private _tokenOwners
Consider to combine with <br>- Line: 52 mapping(bytes32 => EnumerableSet.AddressSet) private _operators<br>- Line: 54 mapping(address => EnumerableSet.Bytes32Set) private _tokenIdsForOperator<br><br><br>
https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol#L46
State variables that are not updated following deployment should be declared immutable to save gas.
<details> <summary> There are 2 instances of this issue: </summary>Line: 79 address internal _target
should be immutable
Line: 47 bool internal _isNonDivisible
should be immutable
</details>When passing function parameters, using the calldata area instead of memory can improve gas efficiency. Calldata is a read-only area where function arguments and external function calls' parameters are stored.
By using calldata for function parameters, you avoid unnecessary gas costs associated with copying data from calldata to memory. This is particularly beneficial when the parameter is read-only and doesn't require modification within the contract.
Using calldata for function parameters can help optimize gas usage, especially when making external function calls or when the parameter values are provided externally and don't need to be stored persistently within the contract.
Line: 281 uint256[] memory operationsType
should be declared as calldata instead
Line: 282 address[] memory targets
should be declared as calldata instead
Line: 283 uint256[] memory values
should be declared as calldata instead
Line: 284 bytes[] memory datas
should be declared as calldata instead
Line: 381 bytes32[] memory dataKeys
should be declared as calldata instead
Line: 382 bytes[] memory dataValues
should be declared as calldata instead
Line: 205 bytes[] memory signatures
should be declared as calldata instead
Line: 120 bytes32[] memory inputDataKeys
should be declared as calldata instead
Line: 121 bytes[] memory inputDataValues
should be declared as calldata instead
Line: 624 bytes memory allowedERC725YDa
should be declared as calldata instead
Line: 170 bytes32[] memory permissions
should be declared as calldata instead
https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP6KeyManager/LSP6Utils.sol#L170
Line: 190 address[] memory from
should be declared as calldata instead
Line: 191 address[] memory to
should be declared as calldata instead
Line: 192 uint256[] memory amount
should be declared as calldata instead
Line: 193 bool[] memory allowNonLSP1Recipient
should be declared as calldata instead
Line: 194 bytes[] memory data
should be declared as calldata instead
Line: 155 address[] memory from
should be declared as calldata instead
Line: 156 address[] memory to
should be declared as calldata instead
Line: 157 uint256[] memory amount
should be declared as calldata instead
Line: 158 bool[] memory allowNonLSP1Recipient
should be declared as calldata instead
Line: 159 bytes[] memory data
should be declared as calldata instead
Line: 217 address[] memory from
should be declared as calldata instead
Line: 218 address[] memory to
should be declared as calldata instead
Line: 219 bytes32[] memory tokenId
should be declared as calldata instead
Line: 220 bool[] memory allowNonLSP1Recipient
should be declared as calldata instead
Line: 221 bytes[] memory data
should be declared as calldata instead
Line: 211 address[] memory from
should be declared as calldata instead
Line: 212 address[] memory to
should be declared as calldata instead
Line: 213 bytes32[] memory tokenId
should be declared as calldata instead
Line: 214 bool[] memory allowNonLSP1Recipient
should be declared as calldata instead
Line: 215 bytes[] memory data
should be declared as calldata instead
Line: 53 uint256[] memory operationsType
should be declared as calldata instead
Line: 54 address[] memory targets
should be declared as calldata instead
Line: 55 uint256[] memory values
should be declared as calldata instead
Line: 56 bytes[] memory datas
should be declared as calldata instead
Line: 132 uint256[] memory operationsType
should be declared as calldata instead
Line: 133 address[] memory targets
should be declared as calldata instead
Line: 134 uint256[] memory values
should be declared as calldata instead
Line: 135 bytes[] memory datas
should be declared as calldata instead
Line: 43 bytes32[] memory dataKeys
should be declared as calldata instead
Line: 74 bytes32[] memory dataKeys
should be declared as calldata instead
Line: 75 bytes[] memory dataValues
should be declared as calldata instead
Line: 94 uint256[] memory operationsType
should be declared as calldata instead
Line: 95 address[] memory targets
should be declared as calldata instead
Line: 96 uint256[] memory values
should be declared as calldata instead
Line: 97 bytes[] memory datas
should be declared as calldata instead
Line: 36 bytes32[] memory dataKeys
should be declared as calldata instead
Line: 68 bytes32[] memory dataKeys
should be declared as calldata instead
Line: 69 bytes[] memory dataValues
should be declared as calldata instead
Setting the internal/private function that called once to inline would save gas
<details> <summary> There are 10 instances of this issue: </summary>Line: 32 function _extendableMsgData() internal view virtual returns (bytes calldata)
this function can be delete, the function is never called:
Line: 44 function _extendableMsgSender() internal view virtual returns (address)
this function can be delete, the function is never called:
Line: 54 function _extendableMsgValue() internal view virtual returns (uint256)
this function can be delete, the function is never called:
Line: 152 function _whenReceiving( bytes32 typeId, address notifier, bytes32 notifierMapKey, bytes4 interfaceID ) internal virtual returns (bytes memory)
should be inline to save gas:
Line: 201 function _whenSending( bytes32 typeId, address notifier, bytes32 notifierMapKey, uint128 arrayIndex ) internal virtual returns (bytes memory)
should be inline to save gas:
Line: 20 function _initialize(address target_) internal virtual onlyInitializing
should be inline to save gas:
Line: 21 function _initialize( uint256 tokenSupplyCap_ ) internal virtual onlyInitializing
should be inline to save gas:
Line: 23 function _initialize( uint256 tokenSupplyCap_ ) internal virtual onlyInitializing
should be inline to save gas:
Line: 307 function _checkOnERC721Received( address from, address to, uint256 tokenId, bytes memory data ) private returns (bool)
should be inline to save gas:
Line: 307 function _checkOnERC721Received( address from, address to, uint256 tokenId, bytes memory data ) private returns (bool)
should be inline to save gas:
</details>This detector scans for instances where a variable is casted to its own type. This is unnecessary and can be safely removed to improve code readability.
<details> <summary> There are 1 instances of this issue: </summary>Line: 76 bytes32(payload[100:132]) != bytes32(uint256(128))
Unnecessary cast: uint256(128) it cast to the same type.<br>
https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol#L76
This detector identifies instances where there are consecutive external calls to the same contract, where the subsequent calls could use a low-level call to save gas.
Note: This detector only triggers if the function call does not return any value.
Prior to 0.8.10, the compiler inserted extra code, including EXTCODESIZE (100 gas), to check for contract existence for external function calls. In more recent Solidity versions the compiler will not insert these checks if the external call has a return value. Similar behavior can be achieved in earlier versions by using low-level calls, since low-level calls never check for contract existence.
<details> <summary> There are 2 instances of this issue: </summary>Line: 636 msg.sender.tryNotifyUniversalReceiver( _TYPEID_LSP0_OwnershipTransferred_RecipientNotification, "" )
This call could be replaced with a low-level call because the contract LSP1Utils has already been checked in <br>`Line: 630 previousOwner.tryNotifyUniversalReceiver(
_TYPEID_LSP0_OwnershipTransferred_SenderNotification, "" )`<br>
Line: 97 msg.sender.tryNotifyUniversalReceiver( _TYPEID_LSP14_OwnershipTransferred_RecipientNotification, "" )
This call could be replaced with a low-level call because the contract LSP1Utils has already been checked in <br>`Line: 92 previousOwner.tryNotifyUniversalReceiver(
</details>_TYPEID_LSP14_OwnershipTransferred_SenderNotification, "" )`<br>
Checks that require() or revert() statements that check input arguments are at the top of the function. Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a gas load in a function that may ultimately revert in the unhappy case.
<details> <summary> There are 2 instances of this issue: </summary>Line: 253 require( _checkOnERC721Received(from, to, tokenId, data), "LSP8CompatibleERC721: transfer to non ERC721Receiver implementer" )
</details>Line: 253 require( _checkOnERC721Received(from, to, tokenId, data), "LSP8CompatibleERC721: transfer to non ERC721Receiver implementer" )
I have reported only on issues that were overlooked by the winning bot, specifically mappings and arrays that weren't checked but could potentially be cached.
State variables should be cached instead of re-reading them.Subsequent reads incur a 100 gas penalty.Caching such variables replaces the Gwarmaccess with much cheaper stack reads.
<details> <summary> There are 5 instances of this issue: </summary>Line: 338 function _burn( address from, uint256 amount, bytes memory data ) internal virtual
should cache the following state variables: _tokenOwnerBalances, _operatorAuthorizedAmount
Line: 399 function _transfer( address from, address to, uint256 amount, bool allowNonLSP1Recipient, bytes memory data ) internal virtual
should cache the following state variables:
_tokenOwnerBalances
Line: 394 function _transfer( address from, address to, bytes32 tokenId, bool allowNonLSP1Recipient, bytes memory data ) internal virtual
should cache the following state variables:
_ownedTokens
Line: 31 function _beforeTokenTransfer( address from, address to, bytes32 tokenId ) internal virtual override(LSP8IdentifiableDigitalAssetCore)
should cache the following state variables:
_tokenIndex,
_indexToken
Line: 33 function _beforeTokenTransfer( address from, address to, bytes32 tokenId ) internal virtual override(LSP8IdentifiableDigitalAssetCore)
should cache the following state variables:
_tokenIndex,
_indexToken
I've only reported issues that were overlooked by the winning bot, specifically on 'while' loops that could potentially be optimized with caching.
When using arrays in a for loop, calling .length on every iteration will result in unnecessary gas expenses. Instead, it is recommended to cache the array length beforehand, which saves 3 gas per iteration. For storage arrays, an additional 100 gas per iteration (except the first) can be saved by pre-caching the length
<details> <summary> There are 6 instances of this issue: </summary>Line: 334 while (pointer < compactBytesArray.length) {
caching the length of array
Line: 162 while (ii < inputDataKeys.length)
caching the length of array
Line: 542 while (pointer < allowedERC725YDataKeysCompacted.length) {
caching the length of array
Line: 662 while (pointer < allowedERC725YDataKeysCompacted.length) {
caching the length of array
Line: 96 while (pointer < allowedCallsCompacted.length) {
caching the length of array
https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP6KeyManager/LSP6Utils.sol#L96
Line: 125 while (pointer < allowedERC725YDataKeysCompacted.length) {
caching the length of array
https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP6KeyManager/LSP6Utils.sol#L125
</details>#0 - c4-judge
2023-08-02T08:25:20Z
trust1995 marked the issue as grade-b