Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: mark setData(..) as payable #197

Merged
merged 8 commits into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 7 additions & 13 deletions docs/ERC-725.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ The initial motivation came out of the need to create a smart contract account s

This standard consists of two sub standards, a generic data key/value store (ERC725Y) and a generic execute function (ERC725X). Both of which in combination allow for a very flexible and long lasting account system. The account version of ERC725 is standardised under [LSP0 ERC725Account](https://github.com/lukso-network/LIPs/blob/master/LSPs/LSP-0-ERC725Account.md).

These standareds (ERC725 X and Y) can also be used seperately to enhance NFTs and Token meta data or other types of smart contracts. ERC725X allows for a generic exection through a smart contract, functioning as an account or actor.
These standards (ERC725 X and Y) can also be used separately to enhance NFTs and Token meta data or other types of smart contracts. ERC725X allows for a generic execution through a smart contract, functioning as an account or actor.

## Specification

Expand Down Expand Up @@ -59,23 +59,20 @@ Function Selector: `0x44c028fe`

Executes a call on any other smart contracts or address, transfers the blockchains native token, or deploys a new smart contract.


_Parameters:_

- `operationType`: the operation type used to execute.
- `target`: the smart contract or address to call. `target` will be unused if a contract is created (operation types 1 and 2).
- `value`: the amount of native tokens to transfer (in Wei).
- `data`: the call data, or the creation bytecode of the contract to deploy.


_Requirements:_

- MUST only be called by the current owner of the contract.
- MUST revert when the execution or the contract creation fails.
- `target` SHOULD be address(0) in case of contract creation with `CREATE` and `CREATE2` (operation types 1 and 2).
- `value` SHOULD be zero in case of `STATICCALL` or `DELEGATECALL` (operation types 3 and 4).


_Returns:_ `bytes` , the returned data of the called function, or the address of the contract deployed (operation types 1 and 2).

**Triggers Event:** [ContractCreated](#contractcreated), [Executed](#executed)
Expand Down Expand Up @@ -137,7 +134,6 @@ _Returns:_ `bytes[]` , array list of returned data of the called function, or th

**Triggers Event:** [ContractCreated](#contractcreated), [Executed](#executed) on each call iteration


**Note:** The `execute()` functions use function overloading, therefore it is better to reference them by the given function signature as follows:

```js
Expand All @@ -146,15 +142,15 @@ _Returns:_ `bytes[]` , array list of returned data of the called function, or th
// execute
myContract.methods['execute(uint256,address,uint256,bytes)'](OPERATION_CALL, target.address, 2WEI, "0x").send();

// execute Array
// execute Array
myContract.methods['execute(uint256[],address[],uint256[],bytes[])']([OPERATION_CALL, OPERATION_CREATE], [target.address, ZERO_ADDRESS], [2WEI, 0WEI], ["0x", CONTRACT_BYTECODE]).send()

// OR

// execute
myContract.methods['0x44c028fe'](OPERATION_CALL, target.address, 2WEI, "0x").send();

// execute Array
// execute Array
myContract.methods['0x13ced88d']([OPERATION_CALL, OPERATION_CREATE], [target.address, ZERO_ADDRESS], [2WEI, 0WEI], ["0x", CONTRACT_BYTECODE]).send()
```

Expand Down Expand Up @@ -223,12 +219,12 @@ _Returns:_ `bytes[]` , array of data values for the requested data keys.
#### setData

```solidity
function setData(bytes32 dataKey, bytes memory dataValue) external
function setData(bytes32 dataKey, bytes memory dataValue) external payable
```

Function Selector: `0x7f23690c`

Sets data as bytes in the storage for a single data key.
Sets data as bytes in the storage for a single data key.

_Parameters:_

Expand All @@ -244,7 +240,7 @@ _Requirements:_
#### setData (Array)

```solidity
function setData(bytes32[] memory dataKeys, bytes[] memory dataValues) external
function setData(bytes32[] memory dataKeys, bytes[] memory dataValues) external payable
```

Function Selector: `0x14a6e293`
Expand Down Expand Up @@ -366,6 +362,4 @@ interface IERC725 /* is IERC725X, IERC725Y */ {

Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).



[ERC165]: <https://eips.ethereum.org/EIPS/eip-165>
[erc165]: https://eips.ethereum.org/EIPS/eip-165
42 changes: 20 additions & 22 deletions implementations/contracts/ERC725YCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,18 @@ abstract contract ERC725YCore is OwnableUnset, ERC165, IERC725Y {
/**
* @inheritdoc IERC725Y
*/
function getData(bytes32 dataKey) public view virtual override returns (bytes memory dataValue) {
function getData(
bytes32 dataKey
) public view virtual override returns (bytes memory dataValue) {
dataValue = _getData(dataKey);
}

/**
* @inheritdoc IERC725Y
*/
function getData(bytes32[] memory dataKeys)
public
view
virtual
override
returns (bytes[] memory dataValues)
{
function getData(
bytes32[] memory dataKeys
) public view virtual override returns (bytes[] memory dataValues) {
dataValues = new bytes[](dataKeys.length);

for (uint256 i = 0; i < dataKeys.length; i = _uncheckedIncrementERC725Y(i)) {
Expand All @@ -56,19 +54,23 @@ abstract contract ERC725YCore is OwnableUnset, ERC165, IERC725Y {
/**
* @inheritdoc IERC725Y
*/
function setData(bytes32 dataKey, bytes memory dataValue) public virtual override onlyOwner {
function setData(
bytes32 dataKey,
bytes memory dataValue
) public payable virtual override onlyOwner {
if (msg.value != 0) revert ERC725Y_MsgValueDisallowed();
_setData(dataKey, dataValue);
}

/**
* @inheritdoc IERC725Y
*/
function setData(bytes32[] memory dataKeys, bytes[] memory dataValues)
public
virtual
override
onlyOwner
{
function setData(
bytes32[] memory dataKeys,
bytes[] memory dataValues
) public payable virtual override onlyOwner {
if (msg.value != 0) revert ERC725Y_MsgValueDisallowed();
CJ42 marked this conversation as resolved.
Show resolved Hide resolved

if (dataKeys.length != dataValues.length) {
revert ERC725Y_DataKeysValuesLengthMismatch(dataKeys.length, dataValues.length);
}
Expand Down Expand Up @@ -100,13 +102,9 @@ abstract contract ERC725YCore is OwnableUnset, ERC165, IERC725Y {
/**
* @inheritdoc ERC165
*/
function supportsInterface(bytes4 interfaceId)
public
view
virtual
override(IERC165, ERC165)
returns (bool)
{
function supportsInterface(
bytes4 interfaceId
) public view virtual override(IERC165, ERC165) returns (bool) {
return interfaceId == _INTERFACEID_ERC725Y || super.supportsInterface(interfaceId);
}
}
5 changes: 5 additions & 0 deletions implementations/contracts/errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,8 @@ error ERC725X_ExecuteParametersLengthMismatch();
* @param dataValuesLength the number of data value in the bytes[] dataValue
*/
error ERC725Y_DataKeysValuesLengthMismatch(uint256 dataKeysLength, uint256 dataValuesLength);

/**
* @dev reverts when sending value to the `setData(..)` functions
*/
error ERC725Y_MsgValueDisallowed();
14 changes: 12 additions & 2 deletions implementations/contracts/interfaces/IERC725Y.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,27 @@ interface IERC725Y is IERC165 {
* @param dataValue The value to set
* SHOULD only be callable by the owner of the contract set via ERC173
*
* The function was marked as payable to enable flexibility on child contracts
CJ42 marked this conversation as resolved.
Show resolved Hide resolved
*
* If the function is not intended to receive value, an additional check
* should be implemented to check that value equal 0.
*
* Emits a {DataChanged} event.
*/
function setData(bytes32 dataKey, bytes memory dataValue) external;
function setData(bytes32 dataKey, bytes memory dataValue) external payable;

/**
* @param dataKeys The array of data keys for values to set
* @param dataValues The array of values to set
* @dev Sets array of data for multiple given `dataKeys`
* SHOULD only be callable by the owner of the contract set via ERC173
*
* The function was marked as payable to enable flexibility on child contracts
CJ42 marked this conversation as resolved.
Show resolved Hide resolved
*
* If the function is not intended to receive value, an additional check
* should be implemented to check that value equal 0.
*
* Emits a {DataChanged} event.
*/
function setData(bytes32[] memory dataKeys, bytes[] memory dataValues) external;
function setData(bytes32[] memory dataKeys, bytes[] memory dataValues) external payable;
}
46 changes: 46 additions & 0 deletions implementations/test/ERC725Y.behaviour.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,52 @@ export const shouldBehaveLikeERC725Y = (
});

describe("When testing setting data", () => {
describe("When sending value to setData", () => {
it("should revert when sending value to setData(..)", async () => {
let value = 100;
const txParams = {
dataKey: ethers.utils.solidityKeccak256(["string"], ["FirstDataKey"]),
dataValue: "0xaabbccdd",
};

await expect(
context.erc725Y
.connect(context.accounts.owner)
["setData(bytes32,bytes)"](txParams.dataKey, txParams.dataValue, {
value: value,
})
).to.be.revertedWithCustomError(
context.erc725Y,
"ERC725Y_MsgValueDisallowed"
);
});

it("should revert when sending value to setData(..) Array", async () => {
let value = 100;
const txParams = {
dataKey: [
ethers.utils.solidityKeccak256(["string"], ["FirstDataKey"]),
],
dataValue: ["0xaabbccdd"],
};

await expect(
context.erc725Y
.connect(context.accounts.owner)
["setData(bytes32[],bytes[])"](
txParams.dataKey,
txParams.dataValue,
{
value: value,
}
)
).to.be.revertedWithCustomError(
context.erc725Y,
"ERC725Y_MsgValueDisallowed"
);
});
});

describe("When using setData(bytes32,bytes)", () => {
describe("When owner is setting data", () => {
it("should pass and emit DataChanged event", async () => {
Expand Down