From f1686d51fbe8cdd253fa678bd8a656e07514059f Mon Sep 17 00:00:00 2001 From: Yamen Merhi Date: Wed, 27 Sep 2023 10:36:54 +0300 Subject: [PATCH] chore: [C4] Add first batch of Gas optimization (#695) * chore: [C4] Add first batch of Gas optimization * chore: replace `= 0` assignment in iterators of LSP7/8 Core * chore: add more gas optimizations * docs: generate docs * chore: run prettier * docs: add latest docs * chore: resolve failing tesst * chore: suggested changes --------- Co-authored-by: CJ42 --- .../LSP0ERC725AccountCore.sol | 8 +- .../LSP1UniversalReceiverDelegateUP.sol | 2 +- .../LSP1UniversalReceiverDelegateVault.sol | 2 +- .../LSP20CallVerification.sol | 2 +- .../LSP25MultiChannelNonce.sol | 3 +- contracts/LSP2ERC725YJSONSchema/LSP2Utils.sol | 68 ------------- .../LSP6KeyManager/LSP6KeyManagerCore.sol | 95 ++++++++++++------- .../LSP6Modules/LSP6ExecuteModule.sol | 2 +- contracts/LSP6KeyManager/LSP6Utils.sol | 2 +- .../LSP7DigitalAsset/LSP7DigitalAssetCore.sol | 38 ++++---- .../LSP8IdentifiableDigitalAssetCore.sol | 35 +++---- .../LSP8CompatibleERC721InitAbstract.sol | 8 +- contracts/LSP9Vault/LSP9VaultCore.sol | 4 +- .../KeyManager/KeyManagerInternalsTester.sol | 2 +- .../LSP6KeyManager/LSP6KeyManager.md | 25 +++-- .../LSP2ERC725YJSONSchema/LSP2Utils.md | 44 --------- 16 files changed, 127 insertions(+), 213 deletions(-) diff --git a/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol b/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol index 44335ecd7..faaa839a4 100644 --- a/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol +++ b/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol @@ -146,7 +146,7 @@ abstract contract LSP0ERC725AccountCore is if (!success) { // Look for revert reason and bubble it up if present - if (result.length > 0) { + if (result.length != 0) { // The easiest way to bubble the revert reason is using memory via assembly // solhint-disable no-inline-assembly /// @solidity memory-safe-assembly @@ -341,7 +341,7 @@ abstract contract LSP0ERC725AccountCore is // If the caller is the owner perform setData directly if (msg.sender == _owner) { - for (uint256 i = 0; i < dataKeys.length; ) { + for (uint256 i; i < dataKeys.length; ) { _setData(dataKeys[i], dataValues[i]); unchecked { @@ -356,7 +356,7 @@ abstract contract LSP0ERC725AccountCore is // Depending on the magicValue returned, a second call is done after setting data bool verifyAfter = _verifyCall(_owner); - for (uint256 i = 0; i < dataKeys.length; ) { + for (uint256 i; i < dataKeys.length; ) { _setData(dataKeys[i], dataValues[i]); unchecked { @@ -671,7 +671,7 @@ abstract contract LSP0ERC725AccountCore is address _owner = owner(); // If owner is a contract - if (_owner.code.length > 0) { + if (_owner.code.length != 0) { (bool success, bytes memory result) = _owner.staticcall( abi.encodeWithSelector( IERC1271.isValidSignature.selector, diff --git a/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol b/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol index 8527981a3..9746e154d 100644 --- a/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol +++ b/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol @@ -172,7 +172,7 @@ contract LSP1UniversalReceiverDelegateUP is ERC165, ILSP1UniversalReceiver { ) internal returns (bytes memory) { // CHECK balance only when the Token contract is already deployed, // not when tokens are being transferred on deployment through the `constructor` - if (notifier.code.length > 0) { + if (notifier.code.length != 0) { // if the amount sent is 0, then do not update the keys try ILSP7DigitalAsset(notifier).balanceOf(msg.sender) returns ( uint256 balance diff --git a/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateVault/LSP1UniversalReceiverDelegateVault.sol b/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateVault/LSP1UniversalReceiverDelegateVault.sol index 34f6144bd..e36960943 100644 --- a/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateVault/LSP1UniversalReceiverDelegateVault.sol +++ b/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateVault/LSP1UniversalReceiverDelegateVault.sol @@ -148,7 +148,7 @@ contract LSP1UniversalReceiverDelegateVault is ERC165, ILSP1UniversalReceiver { ) internal returns (bytes memory) { // CHECK balance only when the Token contract is already deployed, // not when tokens are being transferred on deployment through the `constructor` - if (notifier.code.length > 0) { + if (notifier.code.length != 0) { // if the amount sent is 0, then do not update the keys try ILSP7DigitalAsset(notifier).balanceOf(msg.sender) returns ( uint256 balance diff --git a/contracts/LSP20CallVerification/LSP20CallVerification.sol b/contracts/LSP20CallVerification/LSP20CallVerification.sol index 5c1477d24..0466a6809 100644 --- a/contracts/LSP20CallVerification/LSP20CallVerification.sol +++ b/contracts/LSP20CallVerification/LSP20CallVerification.sol @@ -89,7 +89,7 @@ abstract contract LSP20CallVerification { bytes memory returnedData ) internal pure virtual { // Look for revert reason and bubble it up if present - if (returnedData.length > 0) { + if (returnedData.length != 0) { // The easiest way to bubble the revert reason is using memory via assembly // solhint-disable no-inline-assembly /// @solidity memory-safe-assembly diff --git a/contracts/LSP25ExecuteRelayCall/LSP25MultiChannelNonce.sol b/contracts/LSP25ExecuteRelayCall/LSP25MultiChannelNonce.sol index 15dbc1cb7..2d6c33598 100644 --- a/contracts/LSP25ExecuteRelayCall/LSP25MultiChannelNonce.sol +++ b/contracts/LSP25ExecuteRelayCall/LSP25MultiChannelNonce.sol @@ -56,8 +56,7 @@ abstract contract LSP25MultiChannelNonce { address from, uint128 channelId ) internal view virtual returns (uint256 idx) { - uint256 nonceInChannel = _nonceStore[from][channelId]; - return (uint256(channelId) << 128) | nonceInChannel; + return (uint256(channelId) << 128) | _nonceStore[from][channelId]; } /** diff --git a/contracts/LSP2ERC725YJSONSchema/LSP2Utils.sol b/contracts/LSP2ERC725YJSONSchema/LSP2Utils.sol index ff5d87553..06fd72f1d 100644 --- a/contracts/LSP2ERC725YJSONSchema/LSP2Utils.sol +++ b/contracts/LSP2ERC725YJSONSchema/LSP2Utils.sol @@ -308,74 +308,6 @@ library LSP2Utils { return true; } - /** - * @dev Verify if `data` is an abi-encoded array of addresses (`address[]`) encoded according to the ABI specs. - * - * @param data The bytes value to verify. - * - * @return `true` if the `data` represents an abi-encoded array of addresses, `false` otherwise. - */ - function isEncodedArrayOfAddresses( - bytes memory data - ) internal pure returns (bool) { - if (!isEncodedArray(data)) return false; - - uint256 offset = uint256(bytes32(data)); - uint256 arrayLength = data.toUint256(offset); - - uint256 pointer = offset + 32; - - for (uint256 ii = 0; ii < arrayLength; ) { - bytes32 key = data.toBytes32(pointer); - - // check that the leading bytes are zero bytes "00" - // NB: address type is padded on the left (unlike bytes20 type that is padded on the right) - if (bytes12(key) != bytes12(0)) return false; - - // increment the pointer - pointer += 32; - - unchecked { - ++ii; - } - } - - return true; - } - - /** - * @dev Verify if `data` is an abi-array of `bytes4` values (`bytes4[]`) encoded according to the ABI specs. - * - * @param data The bytes value to verify. - * - * @return `true` if the `data` represents an abi-encoded array of `bytes4`, `false` otherwise. - */ - function isBytes4EncodedArray( - bytes memory data - ) internal pure returns (bool) { - if (!isEncodedArray(data)) return false; - - uint256 offset = uint256(bytes32(data)); - uint256 arrayLength = data.toUint256(offset); - uint256 pointer = offset + 32; - - for (uint256 ii = 0; ii < arrayLength; ) { - bytes32 key = data.toBytes32(pointer); - - // check that the trailing bytes are zero bytes "00" - if (uint224(uint256(key)) != 0) return false; - - // increment the pointer - pointer += 32; - - unchecked { - ++ii; - } - } - - return true; - } - /** * @dev Verify if `data` is a valid array of value encoded as a `CompactBytesArray` according to the LSP2 `CompactBytesArray` valueType specification. * diff --git a/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol b/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol index 369a390ca..f51fcf6c1 100644 --- a/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol +++ b/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol @@ -94,7 +94,9 @@ abstract contract LSP6KeyManagerCore is // Variables, methods and modifier used for ReentrancyGuard are taken from the link below and modified accordingly. // https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.8/contracts/security/ReentrancyGuard.sol - bool internal _reentrancyStatus; + uint8 internal _reentrancyStatus; + uint8 internal constant _NOT_ENTERED = 1; + uint8 internal constant _ENTERED = 2; /** * @inheritdoc ILSP6 @@ -195,7 +197,7 @@ abstract contract LSP6KeyManagerCore is bytes[] memory results = new bytes[](payloads.length); uint256 totalValues; - for (uint256 ii = 0; ii < payloads.length; ) { + for (uint256 ii; ii < payloads.length; ) { if ((totalValues += values[ii]) > msg.value) { revert LSP6BatchInsufficientValueSent(totalValues, msg.value); } @@ -278,7 +280,7 @@ abstract contract LSP6KeyManagerCore is bytes[] memory results = new bytes[](payloads.length); uint256 totalValues; - for (uint256 ii = 0; ii < payloads.length; ) { + for (uint256 ii; ii < payloads.length; ) { if ((totalValues += values[ii]) > msg.value) { revert LSP6BatchInsufficientValueSent(totalValues, msg.value); } @@ -329,37 +331,43 @@ abstract contract LSP6KeyManagerCore is isSetData = true; } + address targetContract = _target; + // If target is invoking the verification, emit the event and change the reentrancy guard - if (msg.sender == _target) { - bool isReentrantCall = _nonReentrantBefore(isSetData, caller); + if (msg.sender == targetContract) { + uint8 reentrancyStatus = _nonReentrantBefore( + targetContract, + isSetData, + caller + ); - _verifyPermissions(caller, msgValue, data); + _verifyPermissions(targetContract, caller, msgValue, data); emit PermissionsVerified(caller, msgValue, bytes4(data)); // if it's a setData call, do not invoke the `lsp20VerifyCallResult(..)` function return - isSetData || isReentrantCall + isSetData || (reentrancyStatus == _ENTERED) ? _LSP20_VERIFY_CALL_MAGIC_VALUE_WITHOUT_POST_VERIFICATION : _LSP20_VERIFY_CALL_MAGIC_VALUE_WITH_POST_VERIFICATION; } /// @dev If a different address is invoking the verification, /// do not change the state or emit the event to allow read-only verification else { - bool isReentrantCall = _reentrancyStatus; + uint8 reentrancyStatus = _reentrancyStatus; - if (isReentrantCall) { + if (reentrancyStatus == _ENTERED) { _requirePermissions( caller, - ERC725Y(_target).getPermissionsFor(caller), + ERC725Y(targetContract).getPermissionsFor(caller), _PERMISSION_REENTRANCY ); } - _verifyPermissions(caller, msgValue, data); + _verifyPermissions(targetContract, caller, msgValue, data); // if it's a setData call, do not invoke the `lsp20VerifyCallResult(..)` function return - isSetData || isReentrantCall + isSetData || (reentrancyStatus == _ENTERED) ? _LSP20_VERIFY_CALL_MAGIC_VALUE_WITHOUT_POST_VERIFICATION : _LSP20_VERIFY_CALL_MAGIC_VALUE_WITH_POST_VERIFICATION; } @@ -396,14 +404,24 @@ abstract contract LSP6KeyManagerCore is isSetData = true; } - bool isReentrantCall = _nonReentrantBefore(isSetData, msg.sender); + address targetContract = _target; + + uint8 reentrancyStatus = _nonReentrantBefore( + targetContract, + isSetData, + msg.sender + ); - _verifyPermissions(msg.sender, msgValue, payload); + _verifyPermissions(targetContract, msg.sender, msgValue, payload); emit PermissionsVerified(msg.sender, msgValue, bytes4(payload)); - bytes memory result = _executePayload(msgValue, payload); + bytes memory result = _executePayload( + targetContract, + msgValue, + payload + ); - if (!isReentrantCall && !isSetData) { + if (reentrancyStatus == _NOT_ENTERED && !isSetData) { _nonReentrantAfter(); } @@ -435,6 +453,8 @@ abstract contract LSP6KeyManagerCore is revert InvalidPayload(payload); } + address targetContract = _target; + address signer = LSP25MultiChannelNonce ._recoverSignerFromLSP25Signature( signature, @@ -461,14 +481,22 @@ abstract contract LSP6KeyManagerCore is isSetData = true; } - bool isReentrantCall = _nonReentrantBefore(isSetData, signer); + uint8 reentrancyStatus = _nonReentrantBefore( + targetContract, + isSetData, + signer + ); - _verifyPermissions(signer, msgValue, payload); + _verifyPermissions(targetContract, signer, msgValue, payload); emit PermissionsVerified(signer, msgValue, bytes4(payload)); - bytes memory result = _executePayload(msgValue, payload); + bytes memory result = _executePayload( + targetContract, + msgValue, + payload + ); - if (!isReentrantCall && !isSetData) { + if (reentrancyStatus == _NOT_ENTERED && !isSetData) { _nonReentrantAfter(); } @@ -481,10 +509,11 @@ abstract contract LSP6KeyManagerCore is * @return bytes The data returned by the call made to the linked {target} contract. */ function _executePayload( + address targetContract, uint256 msgValue, bytes calldata payload ) internal virtual returns (bytes memory) { - (bool success, bytes memory returnData) = _target.call{ + (bool success, bytes memory returnData) = targetContract.call{ value: msgValue, gas: gasleft() }(payload); @@ -503,11 +532,12 @@ abstract contract LSP6KeyManagerCore is * @param payload The abi-encoded function call to execute on the {target} contract. */ function _verifyPermissions( + address targetContract, address from, uint256 msgValue, bytes calldata payload ) internal view virtual { - bytes32 permissions = ERC725Y(_target).getPermissionsFor(from); + bytes32 permissions = ERC725Y(targetContract).getPermissionsFor(from); if (permissions == bytes32(0)) revert NoPermissionsSet(from); bytes4 erc725Function = bytes4(payload); @@ -521,7 +551,7 @@ abstract contract LSP6KeyManagerCore is ); LSP6SetDataModule._verifyCanSetData( - _target, + targetContract, from, permissions, inputKey, @@ -535,7 +565,7 @@ abstract contract LSP6KeyManagerCore is .decode(payload[4:], (bytes32[], bytes[])); LSP6SetDataModule._verifyCanSetData( - _target, + targetContract, from, permissions, inputKeys, @@ -552,7 +582,7 @@ abstract contract LSP6KeyManagerCore is ) = abi.decode(payload[4:], (uint256, address, uint256, bytes)); LSP6ExecuteModule._verifyCanExecute( - _target, + targetContract, from, permissions, operationType, @@ -574,7 +604,7 @@ abstract contract LSP6KeyManagerCore is * @dev Initialise _reentrancyStatus to _NOT_ENTERED. */ function _setupLSP6ReentrancyGuard() internal virtual { - _reentrancyStatus = false; + _reentrancyStatus = 1; } /** @@ -583,20 +613,21 @@ abstract contract LSP6KeyManagerCore is * Used in the beginning of the `nonReentrant` modifier, before the method execution starts. */ function _nonReentrantBefore( + address targetContract, bool isSetData, address from - ) internal virtual returns (bool isReentrantCall) { - isReentrantCall = _reentrancyStatus; - if (isReentrantCall) { + ) internal virtual returns (uint8 reentrancyStatus) { + reentrancyStatus = _reentrancyStatus; + if (reentrancyStatus == _ENTERED) { // CHECK the caller has REENTRANCY permission _requirePermissions( from, - ERC725Y(_target).getPermissionsFor(from), + ERC725Y(targetContract).getPermissionsFor(from), _PERMISSION_REENTRANCY ); } else { if (!isSetData) { - _reentrancyStatus = true; + _reentrancyStatus = _ENTERED; } } } @@ -608,7 +639,7 @@ abstract contract LSP6KeyManagerCore is function _nonReentrantAfter() internal virtual { // By storing the original value once again, a refund is triggered (see // https://eips.ethereum.org/EIPS/eip-2200) - _reentrancyStatus = false; + _reentrancyStatus = _NOT_ENTERED; } /** diff --git a/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol b/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol index 1747ca931..f3aab5d24 100644 --- a/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol +++ b/contracts/LSP6KeyManager/LSP6Modules/LSP6ExecuteModule.sol @@ -261,7 +261,7 @@ abstract contract LSP6ExecuteModule { isEmptyCall ); - for (uint256 ii = 0; ii < allowedCalls.length; ii += 34) { + for (uint256 ii; ii < allowedCalls.length; ii += 34) { /// @dev structure of an AllowedCall // /// AllowedCall = 0x00200000000ncafecafecafecafecafecafecafecafecafecafe5a5a5a5af1f1f1f1 diff --git a/contracts/LSP6KeyManager/LSP6Utils.sol b/contracts/LSP6KeyManager/LSP6Utils.sol index ea76fe73b..ccc9a2f38 100644 --- a/contracts/LSP6KeyManager/LSP6Utils.sol +++ b/contracts/LSP6KeyManager/LSP6Utils.sol @@ -213,7 +213,7 @@ library LSP6Utils { bytes32[] memory permissions ) internal pure returns (bytes32) { bytes32 result; - for (uint256 i = 0; i < permissions.length; i++) { + for (uint256 i; i < permissions.length; i++) { result |= permissions[i]; } return result; diff --git a/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol b/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol index ee1bca998..a50ca839e 100644 --- a/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol +++ b/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol @@ -176,19 +176,21 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { ) public virtual { if (from == to) revert LSP7CannotSendToSelf(); - address operator = msg.sender; - if (operator != from) { - uint256 operatorAmount = _operatorAuthorizedAmount[from][operator]; + // if the caller is an operator + if (msg.sender != from) { + uint256 operatorAmount = _operatorAuthorizedAmount[from][ + msg.sender + ]; if (amount > operatorAmount) { revert LSP7AmountExceedsAuthorizedAmount( from, operatorAmount, - operator, + msg.sender, amount ); } - _updateOperator(from, operator, operatorAmount - amount, ""); + _updateOperator(from, msg.sender, operatorAmount - amount, ""); } _transfer(from, to, amount, force, data); @@ -214,7 +216,7 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { revert LSP7InvalidTransferBatch(); } - for (uint256 i = 0; i < fromLength; ) { + for (uint256 i; i < fromLength; ) { // using the public transfer function to handle updates to operator authorized amounts transfer(from[i], to[i], amount[i], force[i], data[i]); @@ -388,8 +390,6 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { revert LSP7CannotSendWithAddressZero(); } - address operator = msg.sender; - _beforeTokenTransfer(address(0), to, amount); // tokens being minted @@ -397,7 +397,7 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { _tokenOwnerBalances[to] += amount; - emit Transfer(operator, address(0), to, amount, force, data); + emit Transfer(msg.sender, address(0), to, amount, force, data); bytes memory lsp1Data = abi.encode(address(0), to, amount, data); _notifyTokenReceiver(to, force, lsp1Data); @@ -440,20 +440,22 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { revert LSP7AmountExceedsBalance(balance, from, amount); } - address operator = msg.sender; - if (operator != from) { + // if the caller is an operator + if (msg.sender != from) { uint256 authorizedAmount = _operatorAuthorizedAmount[from][ - operator + msg.sender ]; if (amount > authorizedAmount) { revert LSP7AmountExceedsAuthorizedAmount( from, authorizedAmount, - operator, + msg.sender, amount ); } - _operatorAuthorizedAmount[from][operator] -= amount; + _operatorAuthorizedAmount[from][msg.sender] = + authorizedAmount - + amount; } _beforeTokenTransfer(from, address(0), amount); @@ -463,7 +465,7 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { _tokenOwnerBalances[from] -= amount; - emit Transfer(operator, from, address(0), amount, false, data); + emit Transfer(msg.sender, from, address(0), amount, false, data); bytes memory lsp1Data = abi.encode(from, address(0), amount, data); _notifyTokenSender(from, lsp1Data); @@ -508,14 +510,12 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { revert LSP7AmountExceedsBalance(balance, from, amount); } - address operator = msg.sender; - _beforeTokenTransfer(from, to, amount); _tokenOwnerBalances[from] -= amount; _tokenOwnerBalances[to] += amount; - emit Transfer(operator, from, to, amount, force, data); + emit Transfer(msg.sender, from, to, amount, force, data); bytes memory lsp1Data = abi.encode(from, to, amount, data); @@ -618,7 +618,7 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { lsp1Data ); } else if (!force) { - if (to.code.length > 0) { + if (to.code.length != 0) { revert LSP7NotifyTokenReceiverContractMissingLSP1Interface(to); } else { revert LSP7NotifyTokenReceiverIsEOA(to); diff --git a/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol b/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol index 7a41e367b..518bdb4f5 100644 --- a/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol +++ b/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol @@ -222,9 +222,8 @@ abstract contract LSP8IdentifiableDigitalAssetCore is address caller, bytes32 tokenId ) internal view virtual returns (bool) { - address tokenOwner = tokenOwnerOf(tokenId); - - return (caller == tokenOwner || _operators[tokenId].contains(caller)); + return (caller == tokenOwnerOf(tokenId) || + _operators[tokenId].contains(caller)); } // --- Transfer functionality @@ -239,10 +238,8 @@ abstract contract LSP8IdentifiableDigitalAssetCore is bool force, bytes memory data ) public virtual { - address operator = msg.sender; - - if (!_isOperatorOrOwner(operator, tokenId)) { - revert LSP8NotTokenOperator(tokenId, operator); + if (!_isOperatorOrOwner(msg.sender, tokenId)) { + revert LSP8NotTokenOperator(tokenId, msg.sender); } _transfer(from, to, tokenId, force, data); @@ -268,7 +265,7 @@ abstract contract LSP8IdentifiableDigitalAssetCore is revert LSP8InvalidTransferBatch(); } - for (uint256 i = 0; i < fromLength; ) { + for (uint256 i; i < fromLength; ) { transfer(from[i], to[i], tokenId[i], force[i], data[i]); unchecked { @@ -317,9 +314,10 @@ abstract contract LSP8IdentifiableDigitalAssetCore is ]; uint256 operatorListLength = operatorsForTokenId.length(); - for (uint256 i = 0; i < operatorListLength; ) { + address operator; + for (uint256 i; i < operatorListLength; ) { // we are emptying the list, always remove from index 0 - address operator = operatorsForTokenId.at(0); + operator = operatorsForTokenId.at(0); _revokeOperator(operator, tokenOwner, tokenId, ""); unchecked { @@ -374,17 +372,15 @@ abstract contract LSP8IdentifiableDigitalAssetCore is revert LSP8TokenIdAlreadyMinted(tokenId); } - address operator = msg.sender; - _beforeTokenTransfer(address(0), to, tokenId); // token being minted - _existingTokens += 1; + ++_existingTokens; _ownedTokens[to].add(tokenId); _tokenOwners[tokenId] = to; - emit Transfer(operator, address(0), to, tokenId, force, data); + emit Transfer(msg.sender, address(0), to, tokenId, force, data); bytes memory lsp1Data = abi.encode(address(0), to, tokenId, data); _notifyTokenReceiver(to, force, lsp1Data); @@ -412,19 +408,18 @@ abstract contract LSP8IdentifiableDigitalAssetCore is */ function _burn(bytes32 tokenId, bytes memory data) internal virtual { address tokenOwner = tokenOwnerOf(tokenId); - address operator = msg.sender; _beforeTokenTransfer(tokenOwner, address(0), tokenId); // token being burned - _existingTokens -= 1; + --_existingTokens; _clearOperators(tokenOwner, tokenId); _ownedTokens[tokenOwner].remove(tokenId); delete _tokenOwners[tokenId]; - emit Transfer(operator, tokenOwner, address(0), tokenId, false, data); + emit Transfer(msg.sender, tokenOwner, address(0), tokenId, false, data); bytes memory lsp1Data = abi.encode( tokenOwner, @@ -478,8 +473,6 @@ abstract contract LSP8IdentifiableDigitalAssetCore is revert LSP8CannotSendToAddressZero(); } - address operator = msg.sender; - _beforeTokenTransfer(from, to, tokenId); _clearOperators(from, tokenId); @@ -488,7 +481,7 @@ abstract contract LSP8IdentifiableDigitalAssetCore is _ownedTokens[to].add(tokenId); _tokenOwners[tokenId] = to; - emit Transfer(operator, from, to, tokenId, force, data); + emit Transfer(msg.sender, from, to, tokenId, force, data); bytes memory lsp1Data = abi.encode(from, to, tokenId, data); @@ -581,7 +574,7 @@ abstract contract LSP8IdentifiableDigitalAssetCore is lsp1Data ); } else if (!force) { - if (to.code.length > 0) { + if (to.code.length != 0) { revert LSP8NotifyTokenReceiverContractMissingLSP1Interface(to); } else { revert LSP8NotifyTokenReceiverIsEOA(to); diff --git a/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721InitAbstract.sol b/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721InitAbstract.sol index b524cd111..42cd8f02e 100644 --- a/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721InitAbstract.sol +++ b/contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8CompatibleERC721InitAbstract.sol @@ -289,13 +289,11 @@ abstract contract LSP8CompatibleERC721InitAbstract is bool force, bytes memory data ) internal virtual override { - address operator = msg.sender; - if ( - !isApprovedForAll(from, operator) && - !_isOperatorOrOwner(operator, tokenId) + !isApprovedForAll(from, msg.sender) && + !_isOperatorOrOwner(msg.sender, tokenId) ) { - revert LSP8NotTokenOperator(tokenId, operator); + revert LSP8NotTokenOperator(tokenId, msg.sender); } emit Transfer(from, to, uint256(tokenId)); diff --git a/contracts/LSP9Vault/LSP9VaultCore.sol b/contracts/LSP9Vault/LSP9VaultCore.sol index 225518887..15f8853da 100644 --- a/contracts/LSP9Vault/LSP9VaultCore.sol +++ b/contracts/LSP9Vault/LSP9VaultCore.sol @@ -92,7 +92,7 @@ contract LSP9VaultCore is * @custom:events {ValueReceived} when receiving native tokens. */ receive() external payable virtual { - if (msg.value > 0) emit ValueReceived(msg.sender, msg.value); + if (msg.value != 0) emit ValueReceived(msg.sender, msg.value); } /** @@ -147,7 +147,7 @@ contract LSP9VaultCore is if (!success) { // Look for revert reason and bubble it up if present - if (result.length > 0) { + if (result.length != 0) { // The easiest way to bubble the revert reason is using memory via assembly // solhint-disable no-inline-assembly /// @solidity memory-safe-assembly diff --git a/contracts/Mocks/KeyManager/KeyManagerInternalsTester.sol b/contracts/Mocks/KeyManager/KeyManagerInternalsTester.sol index dde9bfa47..007a58dfb 100644 --- a/contracts/Mocks/KeyManager/KeyManagerInternalsTester.sol +++ b/contracts/Mocks/KeyManager/KeyManagerInternalsTester.sol @@ -118,7 +118,7 @@ contract KeyManagerInternalTester is LSP6KeyManager { uint256 msgValue, bytes calldata payload ) public { - super._verifyPermissions(from, msgValue, payload); + super._verifyPermissions(target(), from, msgValue, payload); // This event is emitted just for a sake of not marking this function as `view`, // as Hardhat has a bug that does not catch error that occured from failed `abi.decode` diff --git a/docs/contracts/LSP6KeyManager/LSP6KeyManager.md b/docs/contracts/LSP6KeyManager/LSP6KeyManager.md index 95e13300c..024890611 100644 --- a/docs/contracts/LSP6KeyManager/LSP6KeyManager.md +++ b/docs/contracts/LSP6KeyManager/LSP6KeyManager.md @@ -1124,6 +1124,7 @@ and conform to the signature format according to the LSP25 standard. ```solidity function _executePayload( + address targetContract, uint256 msgValue, bytes payload ) internal nonpayable returns (bytes); @@ -1133,10 +1134,11 @@ _Execute the `payload` passed to `execute(...)` or `executeRelayCall(...)`_ #### Parameters -| Name | Type | Description | -| ---------- | :-------: | ------------------------------------------------------------------ | -| `msgValue` | `uint256` | - | -| `payload` | `bytes` | The abi-encoded function call to execute on the {target} contract. | +| Name | Type | Description | +| ---------------- | :-------: | ------------------------------------------------------------------ | +| `targetContract` | `address` | - | +| `msgValue` | `uint256` | - | +| `payload` | `bytes` | The abi-encoded function call to execute on the {target} contract. | #### Returns @@ -1150,6 +1152,7 @@ _Execute the `payload` passed to `execute(...)` or `executeRelayCall(...)`_ ```solidity function _verifyPermissions( + address targetContract, address from, uint256 msgValue, bytes payload @@ -1160,11 +1163,12 @@ Verify if the `from` address is allowed to execute the `payload` on the [`target #### Parameters -| Name | Type | Description | -| ---------- | :-------: | ------------------------------------------------------------------- | -| `from` | `address` | Either the caller of {execute} or the signer of {executeRelayCall}. | -| `msgValue` | `uint256` | - | -| `payload` | `bytes` | The abi-encoded function call to execute on the {target} contract. | +| Name | Type | Description | +| ---------------- | :-------: | ------------------------------------------------------------------- | +| `targetContract` | `address` | - | +| `from` | `address` | Either the caller of {execute} or the signer of {executeRelayCall}. | +| `msgValue` | `uint256` | - | +| `payload` | `bytes` | The abi-encoded function call to execute on the {target} contract. |
@@ -1182,9 +1186,10 @@ Initialise \_reentrancyStatus to \_NOT_ENTERED. ```solidity function _nonReentrantBefore( + address targetContract, bool isSetData, address from -) internal nonpayable returns (bool isReentrantCall); +) internal nonpayable returns (uint8 reentrancyStatus); ``` Update the status from `_NON_ENTERED` to `_ENTERED` and checks if diff --git a/docs/libraries/LSP2ERC725YJSONSchema/LSP2Utils.md b/docs/libraries/LSP2ERC725YJSONSchema/LSP2Utils.md index 516377228..34debe873 100644 --- a/docs/libraries/LSP2ERC725YJSONSchema/LSP2Utils.md +++ b/docs/libraries/LSP2ERC725YJSONSchema/LSP2Utils.md @@ -361,50 +361,6 @@ Verify if `data` is an abi-encoded array.
-### isEncodedArrayOfAddresses - -```solidity -function isEncodedArrayOfAddresses(bytes data) internal pure returns (bool); -``` - -Verify if `data` is an abi-encoded array of addresses (`address[]`) encoded according to the ABI specs. - -#### Parameters - -| Name | Type | Description | -| ------ | :-----: | -------------------------- | -| `data` | `bytes` | The bytes value to verify. | - -#### Returns - -| Name | Type | Description | -| ---- | :----: | ------------------------------------------------------------------------------------- | -| `0` | `bool` | `true` if the `data` represents an abi-encoded array of addresses, `false` otherwise. | - -
- -### isBytes4EncodedArray - -```solidity -function isBytes4EncodedArray(bytes data) internal pure returns (bool); -``` - -Verify if `data` is an abi-array of `bytes4` values (`bytes4[]`) encoded according to the ABI specs. - -#### Parameters - -| Name | Type | Description | -| ------ | :-----: | -------------------------- | -| `data` | `bytes` | The bytes value to verify. | - -#### Returns - -| Name | Type | Description | -| ---- | :----: | ------------------------------------------------------------------------------------ | -| `0` | `bool` | `true` if the `data` represents an abi-encoded array of `bytes4`, `false` otherwise. | - -
- ### isCompactBytesArray ```solidity