diff --git a/constants.ts b/constants.ts index eed5bfc63..18c3aaebf 100644 --- a/constants.ts +++ b/constants.ts @@ -27,7 +27,7 @@ export const INTERFACE_IDS = { LSP0ERC725Account: '0x24871b3d', LSP1UniversalReceiver: '0x6bb56a14', LSP1UniversalReceiverDelegate: '0xa245bbda', - LSP6KeyManager: '0xe7424397', + LSP6KeyManager: '0x23f34c62', LSP7DigitalAsset: '0x05519512', LSP8IdentifiableDigitalAsset: '0x1ae9ba1f', LSP9Vault: '0x28af17e6', @@ -36,7 +36,7 @@ export const INTERFACE_IDS = { LSP17Extendable: '0xa918fa6b', LSP17Extension: '0xcee78b40', LSP20CallVerification: '0x1a0eb6a5', - LSP20CallVerifier: '0xc9dfc532', + LSP20CallVerifier: '0x0d6ecac7', LSP25ExecuteRelayCall: '0x5ac79908', }; @@ -61,10 +61,10 @@ export const ERC1271_VALUES = { */ export const LSP20_SUCCESS_VALUES = { VERIFY_CALL: { - // bytes3(keccak256("lsp20VerifyCall(address,address,uint256,bytes)")) + "0x00" - NO_POST_VERIFICATION: '0x1a238000', - // bytes3(keccak256("lsp20VerifyCall(address,address,uint256,bytes)")) + "0x01" - WITH_POST_VERIFICATION: '0x1a238001', + // bytes3(keccak256("lsp20VerifyCall(address,address,address,uint256,bytes)")) + "0x00" + NO_POST_VERIFICATION: '0xde928f00', + // bytes3(keccak256("lsp20VerifyCall(address,address,address,uint256,bytes)")) + "0x01" + WITH_POST_VERIFICATION: '0xde928f01', }, // bytes4(keccak256("lsp20VerifyCallResult(bytes32,bytes)")) VERIFY_CALL_RESULT: '0xd3fc45d3', diff --git a/contracts/LSP17Extensions/Extension4337.sol b/contracts/LSP17Extensions/Extension4337.sol index d5fd38331..133abe0ad 100644 --- a/contracts/LSP17Extensions/Extension4337.sol +++ b/contracts/LSP17Extensions/Extension4337.sol @@ -72,10 +72,11 @@ contract Extension4337 is LSP17Extension, IAccount { // verify that the recovered address can execute the userOp.callData bytes4 returnedStatus = ILSP20CallVerifier(owner).lsp20VerifyCall({ - callee: msg.sender, + requestor: _ENTRY_POINT, + target: msg.sender, caller: recovered, value: 0, - receivedCalldata: userOp.callData + callData: userOp.callData }); // if the returnedStatus is a value different than the success value, return signature validation failed diff --git a/contracts/LSP20CallVerification/ILSP20CallVerifier.sol b/contracts/LSP20CallVerification/ILSP20CallVerifier.sol index 2b1b97f88..f23eee193 100644 --- a/contracts/LSP20CallVerification/ILSP20CallVerifier.sol +++ b/contracts/LSP20CallVerification/ILSP20CallVerifier.sol @@ -12,26 +12,28 @@ interface ILSP20CallVerifier { * the function is allowed, concatened with a byte that determines if the lsp20VerifyCallResult function should * be called after the original function call. The byte that invoke the lsp20VerifyCallResult function is strictly `0x01`. * - * @param callee The address of the contract that implements the `LSP20CallVerification` interface - * @param caller The address who called the function on the msg.sender + * @param requestor The address that requested to make the call to `target`. + * @param target The address of the contract that implements the `LSP20CallVerification` interface. + * @param caller The address who called the function on the `target` contract. * @param value The value sent by the caller to the function called on the msg.sender - * @param receivedCalldata The receivedCalldata sent by the caller to the msg.sender + * @param callData The calldata sent by the caller to the msg.sender */ function lsp20VerifyCall( - address callee, + address requestor, + address target, address caller, uint256 value, - bytes memory receivedCalldata + bytes memory callData ) external returns (bytes4 returnedStatus); /** * @return MUST return the lsp20VerifyCallResult function selector if the call to the function is allowed * - * @param callHash The keccak256 of the parameters of {lsp20VerifyCall} concatenated - * @param result The value result of the function called on the msg.sender + * @param callHash The keccak256 hash of the parameters of {lsp20VerifyCall} concatenated + * @param callResult The value result of the function called on the msg.sender */ function lsp20VerifyCallResult( bytes32 callHash, - bytes memory result + bytes memory callResult ) external returns (bytes4); } diff --git a/contracts/LSP20CallVerification/LSP20CallVerification.sol b/contracts/LSP20CallVerification/LSP20CallVerification.sol index d23ac7810..6a7732222 100644 --- a/contracts/LSP20CallVerification/LSP20CallVerification.sol +++ b/contracts/LSP20CallVerification/LSP20CallVerification.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.4; // interfaces - import {ILSP20CallVerifier as ILSP20} from "./ILSP20CallVerifier.sol"; // errors @@ -33,6 +32,7 @@ abstract contract LSP20CallVerification { (bool success, bytes memory returnedData) = logicVerifier.call( abi.encodeWithSelector( ILSP20.lsp20VerifyCall.selector, + msg.sender, address(this), msg.sender, msg.value, @@ -64,6 +64,7 @@ abstract contract LSP20CallVerification { ILSP20.lsp20VerifyCallResult.selector, keccak256( abi.encodePacked( + msg.sender, address(this), msg.sender, msg.value, diff --git a/contracts/LSP20CallVerification/LSP20Constants.sol b/contracts/LSP20CallVerification/LSP20Constants.sol index d36869a57..9837c368c 100644 --- a/contracts/LSP20CallVerification/LSP20Constants.sol +++ b/contracts/LSP20CallVerification/LSP20Constants.sol @@ -4,14 +4,14 @@ pragma solidity ^0.8.4; // bytes4(keccak256("LSP20CallVerification")) bytes4 constant _INTERFACEID_LSP20_CALL_VERIFICATION = 0x1a0eb6a5; -// `lsp20VerifyCall(address,address,uint256,bytes)` selector XOR `lsp20VerifyCallResult(bytes32,bytes)` selector -bytes4 constant _INTERFACEID_LSP20_CALL_VERIFIER = 0xc9dfc532; +// `lsp20VerifyCall(address,address,address,uint256,bytes)` selector XOR `lsp20VerifyCallResult(bytes32,bytes)` selector +bytes4 constant _INTERFACEID_LSP20_CALL_VERIFIER = 0x0d6ecac7; // bytes4(bytes.concat(bytes3(ILSP20.lsp20VerifyCall.selector), hex"01")) -bytes4 constant _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITH_POST_VERIFICATION = 0x1a238001; +bytes4 constant _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITH_POST_VERIFICATION = 0xde928f01; // bytes4(bytes.concat(bytes3(ILSP20.lsp20VerifyCall.selector), hex"00")) -bytes4 constant _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITHOUT_POST_VERIFICATION = 0x1a238000; +bytes4 constant _LSP20_VERIFY_CALL_SUCCESS_VALUE_WITHOUT_POST_VERIFICATION = 0xde928f00; // bytes4(ILSP20.lsp20VerifyCallResult.selector) bytes4 constant _LSP20_VERIFY_CALL_RESULT_SUCCESS_VALUE = 0xd3fc45d3; diff --git a/contracts/LSP6KeyManager/LSP6Constants.sol b/contracts/LSP6KeyManager/LSP6Constants.sol index a7d26ede9..79d3fd7e1 100644 --- a/contracts/LSP6KeyManager/LSP6Constants.sol +++ b/contracts/LSP6KeyManager/LSP6Constants.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.4; // --- ERC165 interface ids -bytes4 constant _INTERFACEID_LSP6 = 0xe7424397; +bytes4 constant _INTERFACEID_LSP6 = 0x23f34c62; // --- ERC725Y Data Keys diff --git a/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol b/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol index 2fc1c117f..0a25eec83 100644 --- a/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol +++ b/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol @@ -318,13 +318,14 @@ abstract contract LSP6KeyManagerCore is * - `0x1a238001`: LSP20 success value **with** post-verification (last byte is `0x01`). */ function lsp20VerifyCall( + address /* requestor */, address targetContract, address caller, uint256 msgValue, - bytes calldata data + bytes calldata callData ) external virtual override returns (bytes4) { - bool isSetData = bytes4(data) == IERC725Y.setData.selector || - bytes4(data) == IERC725Y.setDataBatch.selector; + bool isSetData = bytes4(callData) == IERC725Y.setData.selector || + bytes4(callData) == IERC725Y.setDataBatch.selector; // If target is invoking the verification, emit the event and change the reentrancy guard if (msg.sender == targetContract) { @@ -334,9 +335,9 @@ abstract contract LSP6KeyManagerCore is caller ); - _verifyPermissions(targetContract, caller, false, data); + _verifyPermissions(targetContract, caller, false, callData); - emit PermissionsVerified(caller, msgValue, bytes4(data)); + emit PermissionsVerified(caller, msgValue, bytes4(callData)); // if it's a setData call, do not invoke the `lsp20VerifyCallResult(..)` function return @@ -357,7 +358,7 @@ abstract contract LSP6KeyManagerCore is ); } - _verifyPermissions(targetContract, caller, false, data); + _verifyPermissions(targetContract, caller, false, callData); // if it's a setData call, do not invoke the `lsp20VerifyCallResult(..)` function return @@ -371,8 +372,8 @@ abstract contract LSP6KeyManagerCore is * @inheritdoc ILSP20 */ function lsp20VerifyCallResult( - bytes32 /*callHash*/, - bytes memory /*result*/ + bytes32 /* callHash */, + bytes memory /* callResult */ ) external virtual override returns (bytes4) { // If it's the target calling, set back the reentrancy guard // to false, if not return the success value diff --git a/contracts/Mocks/ERC165Interfaces.sol b/contracts/Mocks/ERC165Interfaces.sol index ca4480dba..4fcd847b3 100644 --- a/contracts/Mocks/ERC165Interfaces.sol +++ b/contracts/Mocks/ERC165Interfaces.sol @@ -121,6 +121,7 @@ contract CalculateLSPInterfaces { type(IERC1271).interfaceId ^ calculateInterfaceLSP20CallVerifier() ^ calculateInterfaceLSP25ExecuteRelayCall(); + require( interfaceId == _INTERFACEID_LSP6, "hardcoded _INTERFACEID_LSP6 does not match type(ILSP6).interfaceId" @@ -133,6 +134,7 @@ contract CalculateLSPInterfaces { bytes4 interfaceId = type(ILSP7).interfaceId ^ type(IERC725Y).interfaceId ^ calculateInterfaceLSP17Extendable(); + require( interfaceId == _INTERFACEID_LSP7, "hardcoded _INTERFACEID_LSP7 does not match type(ILSP7).interfaceId" @@ -145,6 +147,7 @@ contract CalculateLSPInterfaces { bytes4 interfaceId = type(ILSP8).interfaceId ^ type(IERC725Y).interfaceId ^ calculateInterfaceLSP17Extendable(); + require( interfaceId == _INTERFACEID_LSP8, "hardcoded _INTERFACEID_LSP8 does not match type(ILSP8).interfaceId" diff --git a/contracts/Mocks/LSP20Owners/FirstCallReturnExpandedInvalidValue.sol b/contracts/Mocks/LSP20Owners/FirstCallReturnExpandedInvalidValue.sol index cb3d3d617..320c08f45 100644 --- a/contracts/Mocks/LSP20Owners/FirstCallReturnExpandedInvalidValue.sol +++ b/contracts/Mocks/LSP20Owners/FirstCallReturnExpandedInvalidValue.sol @@ -14,14 +14,18 @@ contract FirstCallReturnExpandedFailValue { address public target; function lsp20VerifyCall( - address callee, + address requestor, + address targetContract, address caller, uint256 value, bytes memory data ) external returns (bytes32) { emit CallVerified(); - return keccak256(abi.encode(callee, caller, value, data)); + return + keccak256( + abi.encode(requestor, targetContract, caller, value, data) + ); } function acceptOwnership(address newTarget) external { diff --git a/contracts/Mocks/LSP20Owners/FirstCallReturnInvalidMagicValue.sol b/contracts/Mocks/LSP20Owners/FirstCallReturnInvalidMagicValue.sol index e1ccdb45c..c9e6b60a2 100644 --- a/contracts/Mocks/LSP20Owners/FirstCallReturnInvalidMagicValue.sol +++ b/contracts/Mocks/LSP20Owners/FirstCallReturnInvalidMagicValue.sol @@ -13,6 +13,7 @@ contract FirstCallReturnFailValue { address public target; function lsp20VerifyCall( + address, address, address, uint256, diff --git a/docs/_interface_ids_table.mdx b/docs/_interface_ids_table.mdx index 242e85d86..fbfb9856d 100644 --- a/docs/_interface_ids_table.mdx +++ b/docs/_interface_ids_table.mdx @@ -7,7 +7,7 @@ | **LSP0ERC725Account** | `0x24871b3d` | Interface of the [LSP-0-ERC725Account] standard, an account based smart contract that represents an identity on-chain. | | **LSP1UniversalReceiver** | `0x6bb56a14` | Interface of the LSP1 - Universal Receiver standard, an entry function for a contract to receive arbitrary information. | | **LSP1UniversalReceiverDelegate** | `0xa245bbda` | Interface of the LSP1 - Universal Receiver Delegate standard. | -| **LSP6KeyManager** | `0xe7424397` | Interface of the LSP6 - Key Manager standard, a contract acting as a controller of an ERC725 Account using predfined permissions. | +| **LSP6KeyManager** | `0x23f34c62` | Interface of the LSP6 - Key Manager standard, a contract acting as a controller of an ERC725 Account using predfined permissions. | | **LSP7DigitalAsset** | `0x05519512` | Interface of the LSP7 - Digital Asset standard, a fungible digital asset. | | **LSP8IdentifiableDigitalAsset** | `0x1ae9ba1f` | Interface of the LSP8 - Identifiable Digital Asset standard, a non-fungible digital asset. | | **LSP9Vault** | `0x28af17e6` | Interface of LSP9 - Vault standard, a blockchain vault that can hold assets and interact with other smart contracts. | @@ -16,5 +16,5 @@ | **LSP17Extendable** | `0xa918fa6b` | Module to add more functionalities to a contract using extensions. | | **LSP17Extension** | `0xcee78b40` | Module to create a contract that can act as an extension. | | **LSP20CallVerification** | `0x1a0eb6a5` | Implementation of a contract calling the verification functions according to LSP20 - Call Verification standard. | -| **LSP20CallVerifier** | `0xc9dfc532` | Interface for the LSP20 Call Verification standard, a set of functions intended to perform verifications on behalf of another contract. | +| **LSP20CallVerifier** | `0x0d6ecac7` | Interface for the LSP20 Call Verification standard, a set of functions intended to perform verifications on behalf of another contract. | | **LSP25ExecuteRelayCall** | `0x5ac79908` | | diff --git a/docs/contracts/LSP6KeyManager/LSP6KeyManager.md b/docs/contracts/LSP6KeyManager/LSP6KeyManager.md index fe2fd187a..a1b0b76df 100644 --- a/docs/contracts/LSP6KeyManager/LSP6KeyManager.md +++ b/docs/contracts/LSP6KeyManager/LSP6KeyManager.md @@ -332,8 +332,8 @@ Checks if a signature was signed by a controller that has the permission `SIGN`. - Specification details: [**LSP-6-KeyManager**](https://github.com/lukso-network/lips/tree/main/LSPs/LSP-6-KeyManager.md#lsp20verifycall) - Solidity implementation: [`LSP6KeyManager.sol`](https://github.com/lukso-network/lsp-smart-contracts/blob/develop/contracts/LSP6KeyManager/LSP6KeyManager.sol) -- Function signature: `lsp20VerifyCall(address,address,uint256,bytes)` -- Function selector: `0x1a2380e1` +- Function signature: `lsp20VerifyCall(address,address,address,uint256,bytes)` +- Function selector: `0xde928f14` ::: @@ -348,21 +348,23 @@ This function can call by any other address than the {`target`}. This allows to ```solidity function lsp20VerifyCall( + address, address targetContract, address caller, uint256 msgValue, - bytes data + bytes callData ) external nonpayable returns (bytes4); ``` #### Parameters -| Name | Type | Description | -| ---------------- | :-------: | ----------------------------------------------------- | -| `targetContract` | `address` | - | -| `caller` | `address` | The address who called the function on the msg.sender | -| `msgValue` | `uint256` | - | -| `data` | `bytes` | - | +| Name | Type | Description | +| ---------------- | :-------: | ------------------------------------------------------------- | +| `_0` | `address` | - | +| `targetContract` | `address` | - | +| `caller` | `address` | The address who called the function on the `target` contract. | +| `msgValue` | `uint256` | - | +| `callData` | `bytes` | The calldata sent by the caller to the msg.sender | #### Returns diff --git a/tests/LSP20CallVerification/LSP6/Interactions/PermissionCall.test.ts b/tests/LSP20CallVerification/LSP6/Interactions/PermissionCall.test.ts index ec906ccb9..a2aa9b93f 100644 --- a/tests/LSP20CallVerification/LSP6/Interactions/PermissionCall.test.ts +++ b/tests/LSP20CallVerification/LSP6/Interactions/PermissionCall.test.ts @@ -499,7 +499,13 @@ export const shouldBehaveLikePermissionCall = ( it('Should revert when caller calls the KeyManager through execute', async () => { const lsp20VerifyCallPayload = context.keyManager.interface.encodeFunctionData( 'lsp20VerifyCall', - [context.keyManager.address, context.accounts[2].address, 0, '0xaabbccdd'], // random arguments + [ + context.accounts[2].address, + context.keyManager.address, + context.accounts[2].address, + 0, + '0xaabbccdd', + ], // random arguments ); await expect( diff --git a/tests/LSP20CallVerification/LSP6/Interactions/Security.test.ts b/tests/LSP20CallVerification/LSP6/Interactions/Security.test.ts index 00cd0946a..8b39ce1a5 100644 --- a/tests/LSP20CallVerification/LSP6/Interactions/Security.test.ts +++ b/tests/LSP20CallVerification/LSP6/Interactions/Security.test.ts @@ -107,7 +107,13 @@ export const testSecurityScenarios = (buildContext: () => Promise { const lsp20VerifyCallPayload = context.keyManager.interface.encodeFunctionData( 'lsp20VerifyCall', - [context.keyManager.address, context.accounts[2].address, 0, '0xaabbccdd'], // random arguments + [ + context.accounts[2].address, + context.keyManager.address, + context.accounts[2].address, + 0, + '0xaabbccdd', + ], // random arguments ); await expect( @@ -297,6 +303,7 @@ export const testSecurityScenarios = (buildContext: () => Promise { const lsp20VerifyCallPayload = context.keyManager.interface.encodeFunctionData( 'lsp20VerifyCall', - [context.keyManager.address, context.accounts[2].address, 0, '0xaabbccdd'], // random arguments + [ + context.accounts[2].address, + context.keyManager.address, + context.accounts[2].address, + 0, + '0xaabbccdd', + ], // random arguments ); const executePayload = context.universalProfile.interface.encodeFunctionData('execute', [ diff --git a/tests/foundry/LSP6KeyManager/LSP6SetDataTest.t.sol b/tests/foundry/LSP6KeyManager/LSP6SetDataTest.t.sol index 0f8ef3959..729b284e3 100644 --- a/tests/foundry/LSP6KeyManager/LSP6SetDataTest.t.sol +++ b/tests/foundry/LSP6KeyManager/LSP6SetDataTest.t.sol @@ -103,6 +103,7 @@ contract LSP6SetDataTest is Test { // CHECK the LSP20 verification function reverts as well keyManager.lsp20VerifyCall( + malicious, address(universalProfile), malicious, 0, @@ -228,6 +229,7 @@ contract LSP6SetDataTest is Test { // CHECK the LSP20 verification function reverts as well keyManager.lsp20VerifyCall( + malicious, address(universalProfile), malicious, 0,