diff --git a/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol b/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol index faaa839a4..acf930d68 100644 --- a/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol +++ b/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol @@ -39,7 +39,7 @@ import { _TYPEID_LSP0_OwnershipTransferStarted, _TYPEID_LSP0_OwnershipTransferred_SenderNotification, _TYPEID_LSP0_OwnershipTransferred_RecipientNotification -} from "../LSP0ERC725Account/LSP0Constants.sol"; +} from "./LSP0Constants.sol"; import { _INTERFACEID_LSP1, _LSP1_UNIVERSAL_RECEIVER_DELEGATE_PREFIX, @@ -56,7 +56,8 @@ import { // errors import { - ERC725Y_DataKeysValuesLengthMismatch + ERC725Y_DataKeysValuesLengthMismatch, + ERC725Y_DataKeysValuesEmptyArray } from "@erc725/smart-contracts/contracts/errors.sol"; import { NoExtensionFoundForFunctionSelector @@ -191,16 +192,16 @@ abstract contract LSP0ERC725AccountCore is emit ValueReceived(msg.sender, msg.value); } - address _owner = owner(); + address accountOwner = owner(); // If the caller is the owner perform execute directly - if (msg.sender == _owner) { + if (msg.sender == accountOwner) { return ERC725XCore._execute(operationType, target, value, data); } // If the caller is not the owner, call {lsp20VerifyCall} on the owner // Depending on the magicValue returned, a second call is done after execution - bool verifyAfter = LSP20CallVerification._verifyCall(_owner); + bool verifyAfter = LSP20CallVerification._verifyCall(accountOwner); // Perform the execution bytes memory result = ERC725XCore._execute( @@ -212,7 +213,10 @@ abstract contract LSP0ERC725AccountCore is // if verifyAfter is true, Call {lsp20VerifyCallResult} on the owner if (verifyAfter) { - LSP20CallVerification._verifyCallResult(_owner, abi.encode(result)); + LSP20CallVerification._verifyCallResult( + accountOwner, + abi.encode(result) + ); } return result; @@ -228,6 +232,9 @@ abstract contract LSP0ERC725AccountCore is * - If the operation type is `CREATE` (1) or `CREATE2` (2), `target` must be `address(0)`. * - If the operation type is `STATICCALL` (3) or `DELEGATECALL` (4), `value` transfer is disallowed and must be 0. * + * @custom:warning + * - The `msg.value` should not be trusted for any method called within the batch with `operationType`: `DELEGATECALL` (4). + * * @custom:events * - {Executed} event for each call that uses under `operationType`: `CALL` (0), `STATICCALL` (3) and `DELEGATECALL` (4). (each iteration) * - {ContractCreated} event, when a contract is created under `operationType`: `CREATE` (1) and `CREATE2` (2) (each iteration) @@ -243,10 +250,10 @@ abstract contract LSP0ERC725AccountCore is emit ValueReceived(msg.sender, msg.value); } - address _owner = owner(); + address accountOwner = owner(); // If the caller is the owner perform execute directly - if (msg.sender == _owner) { + if (msg.sender == accountOwner) { return ERC725XCore._executeBatch( operationsType, @@ -258,7 +265,7 @@ abstract contract LSP0ERC725AccountCore is // If the caller is not the owner, call {lsp20VerifyCall} on the owner // Depending on the magicValue returned, a second call is done after execution - bool verifyAfter = LSP20CallVerification._verifyCall(_owner); + bool verifyAfter = LSP20CallVerification._verifyCall(accountOwner); // Perform the execution bytes[] memory results = ERC725XCore._executeBatch( @@ -271,7 +278,7 @@ abstract contract LSP0ERC725AccountCore is // if verifyAfter is true, Call {lsp20VerifyCallResult} on the owner if (verifyAfter) { LSP20CallVerification._verifyCallResult( - _owner, + accountOwner, abi.encode(results) ); } @@ -296,23 +303,23 @@ abstract contract LSP0ERC725AccountCore is emit ValueReceived(msg.sender, msg.value); } - address _owner = owner(); + address accountOwner = owner(); // If the caller is the owner perform setData directly - if (msg.sender == _owner) { + if (msg.sender == accountOwner) { return _setData(dataKey, dataValue); } // If the caller is not the owner, call {lsp20VerifyCall} on the owner // Depending on the magicValue returned, a second call is done after setting data - bool verifyAfter = _verifyCall(_owner); + bool verifyAfter = _verifyCall(accountOwner); _setData(dataKey, dataValue); // If verifyAfter is true, Call {lsp20VerifyCallResult} on the owner // The setData function does not return, second parameter of {_verifyCallResult} will be empty if (verifyAfter) { - _verifyCallResult(_owner, ""); + _verifyCallResult(accountOwner, ""); } } @@ -337,10 +344,14 @@ abstract contract LSP0ERC725AccountCore is revert ERC725Y_DataKeysValuesLengthMismatch(); } - address _owner = owner(); + if (dataKeys.length == 0) { + revert ERC725Y_DataKeysValuesEmptyArray(); + } + + address accountOwner = owner(); // If the caller is the owner perform setData directly - if (msg.sender == _owner) { + if (msg.sender == accountOwner) { for (uint256 i; i < dataKeys.length; ) { _setData(dataKeys[i], dataValues[i]); @@ -354,7 +365,7 @@ abstract contract LSP0ERC725AccountCore is // If the caller is not the owner, call {lsp20VerifyCall} on the owner // Depending on the magicValue returned, a second call is done after setting data - bool verifyAfter = _verifyCall(_owner); + bool verifyAfter = _verifyCall(accountOwner); for (uint256 i; i < dataKeys.length; ) { _setData(dataKeys[i], dataValues[i]); @@ -367,7 +378,7 @@ abstract contract LSP0ERC725AccountCore is // If verifyAfter is true, Call {lsp20VerifyCallResult} on the owner // The setData function does not return, second parameter of {_verifyCallResult} will be empty if (verifyAfter) { - _verifyCallResult(_owner, ""); + _verifyCallResult(accountOwner, ""); } } @@ -572,7 +583,7 @@ abstract contract LSP0ERC725AccountCore is * * @custom:requirements Can be only called by the {owner} or by an authorised address that pass the verification check performed on the owner. * - * @custom:danger Leaves the contract without an owner. Once ownership of the contract has been renounced, any functions that are restricted to be called by the owner will be permanently inaccessible, making these functions not callable anymore and unusable. + * @custom:danger Leaves the contract without an owner. Once ownership of the contract has been renounced, any functions that are restricted to be called by the owner or an address allowed by the owner will be permanently inaccessible, making these functions not callable anymore and unusable. * */ function renounceOwnership() @@ -580,16 +591,16 @@ abstract contract LSP0ERC725AccountCore is virtual override(LSP14Ownable2Step, OwnableUnset) { - address _owner = owner(); + address accountOwner = owner(); // If the caller is the owner perform renounceOwnership directly - if (msg.sender == _owner) { + if (msg.sender == accountOwner) { return LSP14Ownable2Step._renounceOwnership(); } // If the caller is not the owner, call {lsp20VerifyCall} on the owner // Depending on the magicValue returned, a second call is done after transferring ownership - bool verifyAfter = _verifyCall(_owner); + bool verifyAfter = _verifyCall(accountOwner); address previousOwner = owner(); LSP14Ownable2Step._renounceOwnership(); @@ -604,7 +615,7 @@ abstract contract LSP0ERC725AccountCore is // If verifyAfter is true, Call {lsp20VerifyCallResult} on the owner // The transferOwnership function does not return, second parameter of {_verifyCallResult} will be empty if (verifyAfter) { - _verifyCallResult(_owner, ""); + _verifyCallResult(accountOwner, ""); } } diff --git a/contracts/LSP14Ownable2Step/ILSP14Ownable2Step.sol b/contracts/LSP14Ownable2Step/ILSP14Ownable2Step.sol index 6b0410934..00ded5eaf 100644 --- a/contracts/LSP14Ownable2Step/ILSP14Ownable2Step.sol +++ b/contracts/LSP14Ownable2Step/ILSP14Ownable2Step.sol @@ -59,7 +59,7 @@ interface ILSP14Ownable2Step { * @dev Transfer ownership of the contract from the current {owner()} to the {pendingOwner()}. * * Once this function is called: - * - The current {owner()} will loose access to the functions restricted to the {owner()} only. + * - The current {owner()} will lose access to the functions restricted to the {owner()} only. * - The {pendingOwner()} will gain access to the functions restricted to the {owner()} only. * * @notice `msg.sender` is accepting ownership of contract: `address(this)`. diff --git a/contracts/LSP20CallVerification/LSP20CallVerification.sol b/contracts/LSP20CallVerification/LSP20CallVerification.sol index 0466a6809..9f3780ca5 100644 --- a/contracts/LSP20CallVerification/LSP20CallVerification.sol +++ b/contracts/LSP20CallVerification/LSP20CallVerification.sol @@ -8,7 +8,8 @@ import {ILSP20CallVerifier as ILSP20} from "./ILSP20CallVerifier.sol"; // errors import { LSP20InvalidMagicValue, - LSP20CallingVerifierFailed + LSP20CallingVerifierFailed, + LSP20EOACannotVerifyCall } from "./LSP20Errors.sol"; /** @@ -26,6 +27,9 @@ abstract contract LSP20CallVerification { function _verifyCall( address logicVerifier ) internal virtual returns (bool verifyAfter) { + if (logicVerifier.code.length == 0) + revert LSP20EOACannotVerifyCall(logicVerifier); + (bool success, bytes memory returnedData) = logicVerifier.call( abi.encodeWithSelector( ILSP20.lsp20VerifyCall.selector, @@ -42,7 +46,7 @@ abstract contract LSP20CallVerification { if (bytes3(magicValue) != bytes3(ILSP20.lsp20VerifyCall.selector)) revert LSP20InvalidMagicValue(false, returnedData); - return magicValue[3] == 0x01 ? true : false; + return bytes1(magicValue[3]) == 0x01; } /** diff --git a/contracts/LSP20CallVerification/LSP20Errors.sol b/contracts/LSP20CallVerification/LSP20Errors.sol index 264f2971e..4ed0d42e0 100644 --- a/contracts/LSP20CallVerification/LSP20Errors.sol +++ b/contracts/LSP20CallVerification/LSP20Errors.sol @@ -13,3 +13,9 @@ error LSP20CallingVerifierFailed(bool postCall); * @param returnedData The data returned by the call to the logic verifier */ error LSP20InvalidMagicValue(bool postCall, bytes returnedData); + +/** + * @dev Reverts when the logic verifier is an Externally Owned Account (EOA) that cannot return the LSP20 magic value. + * @param logicVerifier The address of the logic verifier + */ +error LSP20EOACannotVerifyCall(address logicVerifier); diff --git a/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol b/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol index f51fcf6c1..c8188b5e6 100644 --- a/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol +++ b/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol @@ -323,13 +323,8 @@ abstract contract LSP6KeyManagerCore is uint256 msgValue, bytes calldata data ) external virtual returns (bytes4) { - bool isSetData = false; - if ( - bytes4(data) == IERC725Y.setData.selector || - bytes4(data) == IERC725Y.setDataBatch.selector - ) { - isSetData = true; - } + bool isSetData = bytes4(data) == IERC725Y.setData.selector || + bytes4(data) == IERC725Y.setDataBatch.selector; address targetContract = _target; @@ -396,13 +391,8 @@ abstract contract LSP6KeyManagerCore is revert InvalidPayload(payload); } - bool isSetData = false; - if ( - bytes4(payload) == IERC725Y.setData.selector || - bytes4(payload) == IERC725Y.setDataBatch.selector - ) { - isSetData = true; - } + bool isSetData = bytes4(payload) == IERC725Y.setData.selector || + bytes4(payload) == IERC725Y.setDataBatch.selector; address targetContract = _target; @@ -473,13 +463,8 @@ abstract contract LSP6KeyManagerCore is LSP25MultiChannelNonce._verifyValidityTimestamps(validityTimestamps); - bool isSetData = false; - if ( - bytes4(payload) == IERC725Y.setData.selector || - bytes4(payload) == IERC725Y.setDataBatch.selector - ) { - isSetData = true; - } + bool isSetData = bytes4(payload) == IERC725Y.setData.selector || + bytes4(payload) == IERC725Y.setDataBatch.selector; uint8 reentrancyStatus = _nonReentrantBefore( targetContract, diff --git a/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol b/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol index a50ca839e..edeeebac6 100644 --- a/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol +++ b/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol @@ -273,7 +273,7 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { * It has been added in the LSP7 contract implementation so that it can be used as a prevention mechanism * against the double spending allowance vulnerability. * - * @notice Decrease the allowance of `operator` by -`substractedAmount` + * @notice Decrease the allowance of `operator` by -`subtractedAmount` * * @dev Atomically decreases the allowance granted to `operator` by the caller. * This is an alternative approach to {authorizeOperator} that can be used as a mitigation @@ -281,29 +281,29 @@ abstract contract LSP7DigitalAssetCore is ILSP7DigitalAsset { * * @custom:events * - {AuthorizedOperator} event indicating the updated allowance after decreasing it. - * - {RevokeOperator} event if `substractedAmount` is the full allowance, + * - {RevokeOperator} event if `subtractedAmount` is the full allowance, * indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. * * @param operator the operator to decrease allowance for `msg.sender` - * @param substractedAmount the amount to decrease by in the operator's allowance. + * @param subtractedAmount the amount to decrease by in the operator's allowance. * * @custom:requirements * - `operator` cannot be the zero address. - * - `operator` must have allowance for the caller of at least `substractedAmount`. + * - `operator` must have allowance for the caller of at least `subtractedAmount`. */ function decreaseAllowance( address operator, - uint256 substractedAmount, + uint256 subtractedAmount, bytes memory operatorNotificationData ) public virtual { uint256 currentAllowance = authorizedAmountFor(operator, msg.sender); - if (currentAllowance < substractedAmount) { + if (currentAllowance < subtractedAmount) { revert LSP7DecreasedAllowanceBelowZero(); } uint256 newAllowance; unchecked { - newAllowance = currentAllowance - substractedAmount; + newAllowance = currentAllowance - subtractedAmount; _updateOperator( msg.sender, operator, diff --git a/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol b/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol index 518bdb4f5..7691db56c 100644 --- a/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol +++ b/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol @@ -374,6 +374,11 @@ abstract contract LSP8IdentifiableDigitalAssetCore is _beforeTokenTransfer(address(0), to, tokenId); + // Check that `tokenId` was not minted inside the `_beforeTokenTransfer` hook + if (_exists(tokenId)) { + revert LSP8TokenIdAlreadyMinted(tokenId); + } + // token being minted ++_existingTokens; @@ -411,6 +416,10 @@ abstract contract LSP8IdentifiableDigitalAssetCore is _beforeTokenTransfer(tokenOwner, address(0), tokenId); + // Re-fetch and update `tokenOwner` in case `tokenId` + // was transferred inside the `_beforeTokenTransfer` hook + tokenOwner = tokenOwnerOf(tokenId); + // token being burned --_existingTokens; @@ -475,6 +484,10 @@ abstract contract LSP8IdentifiableDigitalAssetCore is _beforeTokenTransfer(from, to, tokenId); + // Re-fetch and update `tokenOwner` in case `tokenId` + // was transferred inside the `_beforeTokenTransfer` hook + tokenOwner = tokenOwnerOf(tokenId); + _clearOperators(from, tokenId); _ownedTokens[from].remove(tokenId); diff --git a/docs/contracts/LSP0ERC725Account/LSP0ERC725Account.md b/docs/contracts/LSP0ERC725Account/LSP0ERC725Account.md index a49f56ccc..d0b664f76 100644 --- a/docs/contracts/LSP0ERC725Account/LSP0ERC725Account.md +++ b/docs/contracts/LSP0ERC725Account/LSP0ERC725Account.md @@ -216,7 +216,7 @@ _`msg.sender` is accepting ownership of contract: `address(this)`._ Transfer ownership of the contract from the current [`owner()`](#owner) to the [`pendingOwner()`](#pendingowner). Once this function is called: -- The current [`owner()`](#owner) will loose access to the functions restricted to the [`owner()`](#owner) only. +- The current [`owner()`](#owner) will lose access to the functions restricted to the [`owner()`](#owner) only. - The [`pendingOwner()`](#pendingowner) will gain access to the functions restricted to the [`owner()`](#owner) only. @@ -350,6 +350,12 @@ Generic executor function to: ::: +:::caution Warning + +- The `msg.value` should not be trusted for any method called within the batch with `operationType`: `DELEGATECALL` (4). + +::: + ```solidity function executeBatch( uint256[] operationsType, @@ -588,7 +594,7 @@ The address that ownership of the contract is transferred to. This address may u :::danger -Leaves the contract without an owner. Once ownership of the contract has been renounced, any functions that are restricted to be called by the owner will be permanently inaccessible, making these functions not callable anymore and unusable. +Leaves the contract without an owner. Once ownership of the contract has been renounced, any functions that are restricted to be called by the owner or an address allowed by the owner will be permanently inaccessible, making these functions not callable anymore and unusable. ::: @@ -1680,6 +1686,25 @@ Reverts when the `operationTypeProvided` is none of the default operation types
+### ERC725Y_DataKeysValuesEmptyArray + +:::note References + +- Specification details: [**LSP-0-ERC725Account**](https://github.com/lukso-network/lips/tree/main/LSPs/LSP-0-ERC725Account.md#erc725y_datakeysvaluesemptyarray) +- Solidity implementation: [`LSP0ERC725Account.sol`](https://github.com/lukso-network/lsp-smart-contracts/blob/develop/contracts/LSP0ERC725Account/LSP0ERC725Account.sol) +- Error signature: `ERC725Y_DataKeysValuesEmptyArray()` +- Error hash: `0x97da5f95` + +::: + +```solidity +error ERC725Y_DataKeysValuesEmptyArray(); +``` + +Reverts when one of the array parameter provided to [`setDataBatch`](#setdatabatch) function is an empty array. + +
+ ### ERC725Y_DataKeysValuesLengthMismatch :::note References @@ -1745,6 +1770,31 @@ reverts when the call to the owner fail with no revert reason
+### LSP20EOACannotVerifyCall + +:::note References + +- Specification details: [**LSP-0-ERC725Account**](https://github.com/lukso-network/lips/tree/main/LSPs/LSP-0-ERC725Account.md#lsp20eoacannotverifycall) +- Solidity implementation: [`LSP0ERC725Account.sol`](https://github.com/lukso-network/lsp-smart-contracts/blob/develop/contracts/LSP0ERC725Account/LSP0ERC725Account.sol) +- Error signature: `LSP20EOACannotVerifyCall(address)` +- Error hash: `0x0c392301` + +::: + +```solidity +error LSP20EOACannotVerifyCall(address logicVerifier); +``` + +Reverts when the logic verifier is an Externally Owned Account (EOA) that cannot return the LSP20 magic value. + +#### Parameters + +| Name | Type | Description | +| --------------- | :-------: | --------------------------------- | +| `logicVerifier` | `address` | The address of the logic verifier | + +
+ ### LSP20InvalidMagicValue :::note References diff --git a/docs/contracts/LSP14Ownable2Step/LSP14Ownable2Step.md b/docs/contracts/LSP14Ownable2Step/LSP14Ownable2Step.md index a6e0464f0..cd4120269 100644 --- a/docs/contracts/LSP14Ownable2Step/LSP14Ownable2Step.md +++ b/docs/contracts/LSP14Ownable2Step/LSP14Ownable2Step.md @@ -98,7 +98,7 @@ _`msg.sender` is accepting ownership of contract: `address(this)`._ Transfer ownership of the contract from the current [`owner()`](#owner) to the [`pendingOwner()`](#pendingowner). Once this function is called: -- The current [`owner()`](#owner) will loose access to the functions restricted to the [`owner()`](#owner) only. +- The current [`owner()`](#owner) will lose access to the functions restricted to the [`owner()`](#owner) only. - The [`pendingOwner()`](#pendingowner) will gain access to the functions restricted to the [`owner()`](#owner) only. diff --git a/docs/contracts/LSP7DigitalAsset/LSP7DigitalAsset.md b/docs/contracts/LSP7DigitalAsset/LSP7DigitalAsset.md index eb78e271a..b40733a0f 100644 --- a/docs/contracts/LSP7DigitalAsset/LSP7DigitalAsset.md +++ b/docs/contracts/LSP7DigitalAsset/LSP7DigitalAsset.md @@ -206,12 +206,12 @@ This is a non-standard function, not part of the LSP7 standard interface. It has ```solidity function decreaseAllowance( address operator, - uint256 substractedAmount, + uint256 subtractedAmount, bytes operatorNotificationData ) external nonpayable; ``` -_Decrease the allowance of `operator` by -`substractedAmount`_ +_Decrease the allowance of `operator` by -`subtractedAmount`_ Atomically decreases the allowance granted to `operator` by the caller. This is an alternative approach to [`authorizeOperator`](#authorizeoperator) that can be used as a mitigation for the double spending allowance problem. @@ -220,7 +220,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Requirements:** - `operator` cannot be the zero address. -- `operator` must have allowance for the caller of at least `substractedAmount`. +- `operator` must have allowance for the caller of at least `subtractedAmount`. @@ -229,7 +229,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Emitted events:** - [`AuthorizedOperator`](#authorizedoperator) event indicating the updated allowance after decreasing it. -- [`RevokeOperator`](#revokeoperator) event if `substractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. +- [`RevokeOperator`](#revokeoperator) event if `subtractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. @@ -238,7 +238,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is | Name | Type | Description | | -------------------------- | :-------: | ------------------------------------------------------ | | `operator` | `address` | the operator to decrease allowance for `msg.sender` | -| `substractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | +| `subtractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | | `operatorNotificationData` | `bytes` | - |
diff --git a/docs/contracts/LSP7DigitalAsset/extensions/LSP7Burnable.md b/docs/contracts/LSP7DigitalAsset/extensions/LSP7Burnable.md index 75e286eb6..14084bc92 100644 --- a/docs/contracts/LSP7DigitalAsset/extensions/LSP7Burnable.md +++ b/docs/contracts/LSP7DigitalAsset/extensions/LSP7Burnable.md @@ -231,12 +231,12 @@ This is a non-standard function, not part of the LSP7 standard interface. It has ```solidity function decreaseAllowance( address operator, - uint256 substractedAmount, + uint256 subtractedAmount, bytes operatorNotificationData ) external nonpayable; ``` -_Decrease the allowance of `operator` by -`substractedAmount`_ +_Decrease the allowance of `operator` by -`subtractedAmount`_ Atomically decreases the allowance granted to `operator` by the caller. This is an alternative approach to [`authorizeOperator`](#authorizeoperator) that can be used as a mitigation for the double spending allowance problem. @@ -245,7 +245,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Requirements:** - `operator` cannot be the zero address. -- `operator` must have allowance for the caller of at least `substractedAmount`. +- `operator` must have allowance for the caller of at least `subtractedAmount`. @@ -254,7 +254,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Emitted events:** - [`AuthorizedOperator`](#authorizedoperator) event indicating the updated allowance after decreasing it. -- [`RevokeOperator`](#revokeoperator) event if `substractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. +- [`RevokeOperator`](#revokeoperator) event if `subtractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. @@ -263,7 +263,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is | Name | Type | Description | | -------------------------- | :-------: | ------------------------------------------------------ | | `operator` | `address` | the operator to decrease allowance for `msg.sender` | -| `substractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | +| `subtractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | | `operatorNotificationData` | `bytes` | - |
diff --git a/docs/contracts/LSP7DigitalAsset/extensions/LSP7CappedSupply.md b/docs/contracts/LSP7DigitalAsset/extensions/LSP7CappedSupply.md index 405a2233d..13868b9d0 100644 --- a/docs/contracts/LSP7DigitalAsset/extensions/LSP7CappedSupply.md +++ b/docs/contracts/LSP7DigitalAsset/extensions/LSP7CappedSupply.md @@ -204,12 +204,12 @@ This is a non-standard function, not part of the LSP7 standard interface. It has ```solidity function decreaseAllowance( address operator, - uint256 substractedAmount, + uint256 subtractedAmount, bytes operatorNotificationData ) external nonpayable; ``` -_Decrease the allowance of `operator` by -`substractedAmount`_ +_Decrease the allowance of `operator` by -`subtractedAmount`_ Atomically decreases the allowance granted to `operator` by the caller. This is an alternative approach to [`authorizeOperator`](#authorizeoperator) that can be used as a mitigation for the double spending allowance problem. @@ -218,7 +218,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Requirements:** - `operator` cannot be the zero address. -- `operator` must have allowance for the caller of at least `substractedAmount`. +- `operator` must have allowance for the caller of at least `subtractedAmount`. @@ -227,7 +227,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Emitted events:** - [`AuthorizedOperator`](#authorizedoperator) event indicating the updated allowance after decreasing it. -- [`RevokeOperator`](#revokeoperator) event if `substractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. +- [`RevokeOperator`](#revokeoperator) event if `subtractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. @@ -236,7 +236,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is | Name | Type | Description | | -------------------------- | :-------: | ------------------------------------------------------ | | `operator` | `address` | the operator to decrease allowance for `msg.sender` | -| `substractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | +| `subtractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | | `operatorNotificationData` | `bytes` | - |
diff --git a/docs/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.md b/docs/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.md index 61d68ae45..8905143e7 100644 --- a/docs/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.md +++ b/docs/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.md @@ -270,12 +270,12 @@ This is a non-standard function, not part of the LSP7 standard interface. It has ```solidity function decreaseAllowance( address operator, - uint256 substractedAmount, + uint256 subtractedAmount, bytes operatorNotificationData ) external nonpayable; ``` -_Decrease the allowance of `operator` by -`substractedAmount`_ +_Decrease the allowance of `operator` by -`subtractedAmount`_ Atomically decreases the allowance granted to `operator` by the caller. This is an alternative approach to [`authorizeOperator`](#authorizeoperator) that can be used as a mitigation for the double spending allowance problem. @@ -284,7 +284,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Requirements:** - `operator` cannot be the zero address. -- `operator` must have allowance for the caller of at least `substractedAmount`. +- `operator` must have allowance for the caller of at least `subtractedAmount`. @@ -293,7 +293,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Emitted events:** - [`AuthorizedOperator`](#authorizedoperator) event indicating the updated allowance after decreasing it. -- [`RevokeOperator`](#revokeoperator) event if `substractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. +- [`RevokeOperator`](#revokeoperator) event if `subtractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. @@ -302,7 +302,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is | Name | Type | Description | | -------------------------- | :-------: | ------------------------------------------------------ | | `operator` | `address` | the operator to decrease allowance for `msg.sender` | -| `substractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | +| `subtractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | | `operatorNotificationData` | `bytes` | - |
diff --git a/docs/contracts/LSP7DigitalAsset/presets/LSP7CompatibleERC20Mintable.md b/docs/contracts/LSP7DigitalAsset/presets/LSP7CompatibleERC20Mintable.md index cd80c23f3..a80ed6335 100644 --- a/docs/contracts/LSP7DigitalAsset/presets/LSP7CompatibleERC20Mintable.md +++ b/docs/contracts/LSP7DigitalAsset/presets/LSP7CompatibleERC20Mintable.md @@ -275,12 +275,12 @@ This is a non-standard function, not part of the LSP7 standard interface. It has ```solidity function decreaseAllowance( address operator, - uint256 substractedAmount, + uint256 subtractedAmount, bytes operatorNotificationData ) external nonpayable; ``` -_Decrease the allowance of `operator` by -`substractedAmount`_ +_Decrease the allowance of `operator` by -`subtractedAmount`_ Atomically decreases the allowance granted to `operator` by the caller. This is an alternative approach to [`authorizeOperator`](#authorizeoperator) that can be used as a mitigation for the double spending allowance problem. @@ -289,7 +289,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Requirements:** - `operator` cannot be the zero address. -- `operator` must have allowance for the caller of at least `substractedAmount`. +- `operator` must have allowance for the caller of at least `subtractedAmount`. @@ -298,7 +298,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Emitted events:** - [`AuthorizedOperator`](#authorizedoperator) event indicating the updated allowance after decreasing it. -- [`RevokeOperator`](#revokeoperator) event if `substractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. +- [`RevokeOperator`](#revokeoperator) event if `subtractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. @@ -307,7 +307,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is | Name | Type | Description | | -------------------------- | :-------: | ------------------------------------------------------ | | `operator` | `address` | the operator to decrease allowance for `msg.sender` | -| `substractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | +| `subtractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | | `operatorNotificationData` | `bytes` | - |
diff --git a/docs/contracts/LSP7DigitalAsset/presets/LSP7Mintable.md b/docs/contracts/LSP7DigitalAsset/presets/LSP7Mintable.md index 52c57a89c..ddf2e1b70 100644 --- a/docs/contracts/LSP7DigitalAsset/presets/LSP7Mintable.md +++ b/docs/contracts/LSP7DigitalAsset/presets/LSP7Mintable.md @@ -235,12 +235,12 @@ This is a non-standard function, not part of the LSP7 standard interface. It has ```solidity function decreaseAllowance( address operator, - uint256 substractedAmount, + uint256 subtractedAmount, bytes operatorNotificationData ) external nonpayable; ``` -_Decrease the allowance of `operator` by -`substractedAmount`_ +_Decrease the allowance of `operator` by -`subtractedAmount`_ Atomically decreases the allowance granted to `operator` by the caller. This is an alternative approach to [`authorizeOperator`](#authorizeoperator) that can be used as a mitigation for the double spending allowance problem. @@ -249,7 +249,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Requirements:** - `operator` cannot be the zero address. -- `operator` must have allowance for the caller of at least `substractedAmount`. +- `operator` must have allowance for the caller of at least `subtractedAmount`. @@ -258,7 +258,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is **Emitted events:** - [`AuthorizedOperator`](#authorizedoperator) event indicating the updated allowance after decreasing it. -- [`RevokeOperator`](#revokeoperator) event if `substractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. +- [`RevokeOperator`](#revokeoperator) event if `subtractedAmount` is the full allowance, indicating `operator` does not have any alauthorizedAmountForlowance left for `msg.sender`. @@ -267,7 +267,7 @@ Atomically decreases the allowance granted to `operator` by the caller. This is | Name | Type | Description | | -------------------------- | :-------: | ------------------------------------------------------ | | `operator` | `address` | the operator to decrease allowance for `msg.sender` | -| `substractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | +| `subtractedAmount` | `uint256` | the amount to decrease by in the operator's allowance. | | `operatorNotificationData` | `bytes` | - |
diff --git a/docs/contracts/LSP9Vault/LSP9Vault.md b/docs/contracts/LSP9Vault/LSP9Vault.md index f314f378e..84821b6df 100644 --- a/docs/contracts/LSP9Vault/LSP9Vault.md +++ b/docs/contracts/LSP9Vault/LSP9Vault.md @@ -202,7 +202,7 @@ _`msg.sender` is accepting ownership of contract: `address(this)`._ Transfer ownership of the contract from the current [`owner()`](#owner) to the [`pendingOwner()`](#pendingowner). Once this function is called: -- The current [`owner()`](#owner) will loose access to the functions restricted to the [`owner()`](#owner) only. +- The current [`owner()`](#owner) will lose access to the functions restricted to the [`owner()`](#owner) only. - The [`pendingOwner()`](#pendingowner) will gain access to the functions restricted to the [`owner()`](#owner) only. diff --git a/docs/contracts/UniversalProfile.md b/docs/contracts/UniversalProfile.md index dfcadcad8..1516eb39e 100644 --- a/docs/contracts/UniversalProfile.md +++ b/docs/contracts/UniversalProfile.md @@ -159,7 +159,7 @@ _`msg.sender` is accepting ownership of contract: `address(this)`._ Transfer ownership of the contract from the current [`owner()`](#owner) to the [`pendingOwner()`](#pendingowner). Once this function is called: -- The current [`owner()`](#owner) will loose access to the functions restricted to the [`owner()`](#owner) only. +- The current [`owner()`](#owner) will lose access to the functions restricted to the [`owner()`](#owner) only. - The [`pendingOwner()`](#pendingowner) will gain access to the functions restricted to the [`owner()`](#owner) only. @@ -293,6 +293,12 @@ Generic executor function to: ::: +:::caution Warning + +- The `msg.value` should not be trusted for any method called within the batch with `operationType`: `DELEGATECALL` (4). + +::: + ```solidity function executeBatch( uint256[] operationsType, @@ -531,7 +537,7 @@ The address that ownership of the contract is transferred to. This address may u :::danger -Leaves the contract without an owner. Once ownership of the contract has been renounced, any functions that are restricted to be called by the owner will be permanently inaccessible, making these functions not callable anymore and unusable. +Leaves the contract without an owner. Once ownership of the contract has been renounced, any functions that are restricted to be called by the owner or an address allowed by the owner will be permanently inaccessible, making these functions not callable anymore and unusable. ::: @@ -1623,6 +1629,25 @@ Reverts when the `operationTypeProvided` is none of the default operation types
+### ERC725Y_DataKeysValuesEmptyArray + +:::note References + +- Specification details: [**UniversalProfile**](https://github.com/lukso-network/lips/tree/main/LSPs/LSP-3-UniversalProfile-Metadata.md#erc725y_datakeysvaluesemptyarray) +- Solidity implementation: [`UniversalProfile.sol`](https://github.com/lukso-network/lsp-smart-contracts/blob/develop/contracts/UniversalProfile.sol) +- Error signature: `ERC725Y_DataKeysValuesEmptyArray()` +- Error hash: `0x97da5f95` + +::: + +```solidity +error ERC725Y_DataKeysValuesEmptyArray(); +``` + +Reverts when one of the array parameter provided to [`setDataBatch`](#setdatabatch) function is an empty array. + +
+ ### ERC725Y_DataKeysValuesLengthMismatch :::note References @@ -1688,6 +1713,31 @@ reverts when the call to the owner fail with no revert reason
+### LSP20EOACannotVerifyCall + +:::note References + +- Specification details: [**UniversalProfile**](https://github.com/lukso-network/lips/tree/main/LSPs/LSP-3-UniversalProfile-Metadata.md#lsp20eoacannotverifycall) +- Solidity implementation: [`UniversalProfile.sol`](https://github.com/lukso-network/lsp-smart-contracts/blob/develop/contracts/UniversalProfile.sol) +- Error signature: `LSP20EOACannotVerifyCall(address)` +- Error hash: `0x0c392301` + +::: + +```solidity +error LSP20EOACannotVerifyCall(address logicVerifier); +``` + +Reverts when the logic verifier is an Externally Owned Account (EOA) that cannot return the LSP20 magic value. + +#### Parameters + +| Name | Type | Description | +| --------------- | :-------: | --------------------------------- | +| `logicVerifier` | `address` | The address of the logic verifier | + +
+ ### LSP20InvalidMagicValue :::note References diff --git a/tests/LSP20CallVerification/LSP20CallVerification.behaviour.ts b/tests/LSP20CallVerification/LSP20CallVerification.behaviour.ts index 89e2c3734..6ec1ff3d8 100644 --- a/tests/LSP20CallVerification/LSP20CallVerification.behaviour.ts +++ b/tests/LSP20CallVerification/LSP20CallVerification.behaviour.ts @@ -56,8 +56,8 @@ export const shouldBehaveLikeLSP20 = (buildContext: () => Promise Promise Promise Promise Promise Promise { @@ -209,14 +209,14 @@ export const shouldBehaveLikeLSP14WithLSP20 = ( .connect(previousOwner) .execute(OPERATION_TYPES.CALL, recipient.address, amount, '0x'), ) - .to.be.revertedWithCustomError(context.contract, 'LSP20InvalidMagicValue') - .withArgs(false, '0x'); + .to.be.revertedWithCustomError(context.contract, 'LSP20EOACannotVerifyCall') + .withArgs(newOwner.address); }); it('should revert when calling `renounceOwnership(...)`', async () => { await expect(context.contract.connect(previousOwner).renounceOwnership()) - .to.be.revertedWithCustomError(context.contract, 'LSP20InvalidMagicValue') - .withArgs(false, '0x'); + .to.be.revertedWithCustomError(context.contract, 'LSP20EOACannotVerifyCall') + .withArgs(newOwner.address); }); }); @@ -257,8 +257,8 @@ export const shouldBehaveLikeLSP14WithLSP20 = ( const tx = context.contract.connect(context.accounts[5]).renounceOwnership(); await expect(tx) - .to.be.revertedWithCustomError(context.contract, 'LSP20InvalidMagicValue') - .withArgs(false, '0x'); + .to.be.revertedWithCustomError(context.contract, 'LSP20EOACannotVerifyCall') + .withArgs(newOwner.address); }); }); @@ -468,8 +468,8 @@ export const shouldBehaveLikeLSP14WithLSP20 = ( const value = ethers.utils.hexlify(ethers.utils.toUtf8Bytes('Random Value')); await expect(context.contract.connect(context.deployParams.owner).setData(key, value)) - .to.be.revertedWithCustomError(context.contract, 'LSP20InvalidMagicValue') - .withArgs(false, '0x'); + .to.be.revertedWithCustomError(context.contract, 'LSP20EOACannotVerifyCall') + .withArgs(ethers.constants.AddressZero); }); it('transfer LYX via `execute(...)`', async () => { @@ -481,8 +481,8 @@ export const shouldBehaveLikeLSP14WithLSP20 = ( .connect(context.deployParams.owner) .execute(OPERATION_TYPES.CALL, recipient, amount, '0x'), ) - .to.be.revertedWithCustomError(context.contract, 'LSP20InvalidMagicValue') - .withArgs(false, '0x'); + .to.be.revertedWithCustomError(context.contract, 'LSP20EOACannotVerifyCall') + .withArgs(ethers.constants.AddressZero); }); }); }); diff --git a/tests/LSP4DigitalAssetMetadata/LSP4DigitalAssetMetadata.behaviour.ts b/tests/LSP4DigitalAssetMetadata/LSP4DigitalAssetMetadata.behaviour.ts index d56ea1e7a..dd552e53b 100644 --- a/tests/LSP4DigitalAssetMetadata/LSP4DigitalAssetMetadata.behaviour.ts +++ b/tests/LSP4DigitalAssetMetadata/LSP4DigitalAssetMetadata.behaviour.ts @@ -24,6 +24,32 @@ export const shouldBehaveLikeLSP4DigitalAssetMetadata = ( }); describe('when setting data on ERC725Y storage', () => { + describe('when sending value while setting data', () => { + it('should revert with `setData`', async () => { + const msgValue = ethers.utils.parseEther('2'); + const key = ethers.utils.keccak256(ethers.utils.toUtf8Bytes('My Key')); + const value = ethers.utils.hexlify(ethers.utils.randomBytes(256)); + + await expect( + context.contract + .connect(context.deployParams.owner) + .setData(key, value, { value: msgValue }), + ).to.be.revertedWithCustomError(context.contract, 'ERC725Y_MsgValueDisallowed'); + }); + + it('should revert with `setDataBatch`', async () => { + const msgValue = ethers.utils.parseEther('2'); + const key = [ethers.utils.keccak256(ethers.utils.toUtf8Bytes('My Key'))]; + const value = [ethers.utils.hexlify(ethers.utils.randomBytes(256))]; + + await expect( + context.contract + .connect(context.deployParams.owner) + .setDataBatch(key, value, { value: msgValue }), + ).to.be.revertedWithCustomError(context.contract, 'ERC725Y_MsgValueDisallowed'); + }); + }); + it('should revert when trying to edit Token Name', async () => { const key = ERC725YDataKeys.LSP4['LSP4TokenName']; const value = ethers.utils.hexlify(ethers.utils.toUtf8Bytes('Overriden Token Name')); diff --git a/tests/UniversalProfile.behaviour.ts b/tests/UniversalProfile.behaviour.ts index 4f0fedaaa..1831cc4ad 100644 --- a/tests/UniversalProfile.behaviour.ts +++ b/tests/UniversalProfile.behaviour.ts @@ -133,6 +133,15 @@ export const shouldBehaveLikeLSP3 = ( '0xdaea594e385fc724449e3118b2db7e86dfba1826', ]; + it('should fail when passing empty arrays of data keys / values', async () => { + const keys = []; + const values = []; + + await expect( + context.universalProfile.setDataBatch(keys, values), + ).to.be.revertedWithCustomError(context.universalProfile, 'ERC725Y_DataKeysValuesEmptyArray'); + }); + it('should set the 3 x keys for a basic UP setup => `LSP3Profile`, `LSP12IssuedAssets[]` and `LSP1UniversalReceiverDelegate`', async () => { const keys = [ ERC725YDataKeys.LSP3.LSP3Profile, @@ -431,8 +440,8 @@ export const shouldBehaveLikeLSP3 = ( await expect( context.universalProfile.connect(context.accounts[4]).batchCalls([setDataPayload]), ) - .to.be.revertedWithCustomError(context.universalProfile, 'LSP20InvalidMagicValue') - .withArgs(false, '0x'); + .to.be.revertedWithCustomError(context.universalProfile, 'LSP20EOACannotVerifyCall') + .withArgs(context.deployParams.owner.address); }); });