Skip to content

Commit

Permalink
refactor!: add extra parameter requestor to lsp20VerifyCall (#753)
Browse files Browse the repository at this point in the history
* refactor!: add `requestor` parameter to `lsp20VerifyCall`

* test: update tests

* docs: update auto-generated docs

* refactor!: update LSP6 + LSP20 Call Verifier interface IDs

* chore: update param name in LSP20 interface
  • Loading branch information
CJ42 authored Oct 17, 2023
1 parent 804779a commit f82626d
Show file tree
Hide file tree
Showing 16 changed files with 82 additions and 46 deletions.
12 changes: 6 additions & 6 deletions constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const INTERFACE_IDS = {
LSP0ERC725Account: '0x24871b3d',
LSP1UniversalReceiver: '0x6bb56a14',
LSP1UniversalReceiverDelegate: '0xa245bbda',
LSP6KeyManager: '0xe7424397',
LSP6KeyManager: '0x23f34c62',
LSP7DigitalAsset: '0x05519512',
LSP8IdentifiableDigitalAsset: '0x1ae9ba1f',
LSP9Vault: '0x28af17e6',
Expand All @@ -36,7 +36,7 @@ export const INTERFACE_IDS = {
LSP17Extendable: '0xa918fa6b',
LSP17Extension: '0xcee78b40',
LSP20CallVerification: '0x1a0eb6a5',
LSP20CallVerifier: '0xc9dfc532',
LSP20CallVerifier: '0x0d6ecac7',
LSP25ExecuteRelayCall: '0x5ac79908',
};

Expand All @@ -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',
Expand Down
5 changes: 3 additions & 2 deletions contracts/LSP17Extensions/Extension4337.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 10 additions & 8 deletions contracts/LSP20CallVerification/ILSP20CallVerifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
3 changes: 2 additions & 1 deletion contracts/LSP20CallVerification/LSP20CallVerification.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity ^0.8.4;

// interfaces

import {ILSP20CallVerifier as ILSP20} from "./ILSP20CallVerifier.sol";

// errors
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -64,6 +64,7 @@ abstract contract LSP20CallVerification {
ILSP20.lsp20VerifyCallResult.selector,
keccak256(
abi.encodePacked(
msg.sender,
address(this),
msg.sender,
msg.value,
Expand Down
8 changes: 4 additions & 4 deletions contracts/LSP20CallVerification/LSP20Constants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
2 changes: 1 addition & 1 deletion contracts/LSP6KeyManager/LSP6Constants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 9 additions & 8 deletions contracts/LSP6KeyManager/LSP6KeyManagerCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions contracts/Mocks/ERC165Interfaces.sol
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ contract CalculateLSPInterfaces {
type(IERC1271).interfaceId ^
calculateInterfaceLSP20CallVerifier() ^
calculateInterfaceLSP25ExecuteRelayCall();

require(
interfaceId == _INTERFACEID_LSP6,
"hardcoded _INTERFACEID_LSP6 does not match type(ILSP6).interfaceId"
Expand All @@ -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"
Expand All @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ contract FirstCallReturnFailValue {
address public target;

function lsp20VerifyCall(
address,
address,
address,
uint256,
Expand Down
4 changes: 2 additions & 2 deletions docs/_interface_ids_table.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand All @@ -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` | |
20 changes: 11 additions & 9 deletions docs/contracts/LSP6KeyManager/LSP6KeyManager.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

:::

Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,13 @@ export const testSecurityScenarios = (buildContext: () => Promise<LSP6TestContex
it('Should revert when caller calls the KeyManager through `ERC725X.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(
Expand Down Expand Up @@ -297,6 +303,7 @@ export const testSecurityScenarios = (buildContext: () => Promise<LSP6TestContex
]);

const tx = await context.keyManager.lsp20VerifyCall(
context.mainController.address,
context.universalProfile.address,
context.mainController.address,
0,
Expand Down
8 changes: 7 additions & 1 deletion tests/LSP6KeyManager/Interactions/PermissionCall.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,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
);

const executePayload = context.universalProfile.interface.encodeFunctionData('execute', [
Expand Down
2 changes: 2 additions & 0 deletions tests/foundry/LSP6KeyManager/LSP6SetDataTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ contract LSP6SetDataTest is Test {

// CHECK the LSP20 verification function reverts as well
keyManager.lsp20VerifyCall(
malicious,
address(universalProfile),
malicious,
0,
Expand Down Expand Up @@ -228,6 +229,7 @@ contract LSP6SetDataTest is Test {

// CHECK the LSP20 verification function reverts as well
keyManager.lsp20VerifyCall(
malicious,
address(universalProfile),
malicious,
0,
Expand Down

0 comments on commit f82626d

Please sign in to comment.