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!: remove EIP191Signer.sol library, replace by function toDataWithIntendedValidatorHash from latest OpenZeppelin library #622

Merged
merged 4 commits into from
Jun 22, 2023

Conversation

CJ42
Copy link
Member

@CJ42 CJ42 commented Jun 20, 2023

What does this PR introduce?

BREAKING CHANGES

Release 4.9.0 of @openzeppelin/contracts includes a new function toDataWithIntendedValidatorHash(...) that uses version 0 of EIP191 (see this PR on OpenZeppelin: OpenZeppelin/openzeppelin-contracts#4063)

This makes the library EIP191Signer.sol (on the contracts/Custom/ folder and the function toDataWithIntendedValidator that we created not necessary anymore, since the same functionality is now part of OZ library.

Replace this library and its usage in the Key Manager Core _executeRelayCall(...) internal function by the same function from OZ.

Build

Upgrade @openzeppelin/contracts and @openzeppelin/contracts-upgradable from 4.8.1 to 4.9.2 (latest version).

@github-actions
Copy link
Contributor

👋 Hello
⛽ I am the Gas Bot Reporter. I keep track of the gas costs of common interactions using Universal Profiles 🆙 !
📊 Here is a summary of the gas cost with the code introduced by this PR.

⛽📊 See Gas Benchmark report of Using UniversalProfile owned by an EOA

🔀 execute scenarios

execute scenarios - 👑 UP Owner ⛽ Gas Usage
Transfer 1 LYX to an EOA without data 37570
Transfer 1 LYX to a UP without data 36672
Transfer 1 LYX to an EOA with 256 bytes of data 42195
Transfer 1 LYX to a UP with 256 bytes of data 44795
Transfer 0.1 LYX to 3x EOA without data 70967
Transfer 0.1 LYX to 3x UP without data 75785
Transfer 0.1 LYX to 3x EOA with 256 bytes of data 84919
Transfer 0.1 LYX to 3x EOA with 256 bytes of data 100147

🗄️ setData scenarios

setData scenarios - 👑 UP Owner ⛽ Gas Usage
Set a 20 bytes long value 49996
Set a 60 bytes long value 95318
Set a 160 bytes long value 164468
Set a 300 bytes long value 279705
Set a 600 bytes long value 484093
Change the value of a data key already set 32884
Remove the value of a data key already set 27348
Set 2 data keys of 20 bytes long value 78478
Set 2 data keys of 100 bytes long value 260630
Set 3 data keys of 20 bytes long value 105220
Change the value of three data keys already set of 20 bytes long value 45508
Remove the value of three data keys already set 41380

🗄️ Tokens scenarios

Tokens scenarios - 👑 UP Owner ⛽ Gas Usage
Minting a LSP7Token to a UP (No Delegate) from an EOA 91375
Minting a LSP7Token to an EOA from an EOA 59232
Transferring an LSP7Token from a UP to another UP (No Delegate) 98976
Minting a LSP8Token to a UP (No Delegate) from an EOA 158331
Minting a LSP8Token to an EOA from an EOA 126188
Transferring an LSP8Token from a UP to another UP (No Delegate) 147520

📝 Notes

  • The execute and setData scenarios are executed on a fresh UniversalProfile smart contract, deployed as standard contracts (not as proxy behind a base contract implementation).
⛽📊 See Gas Benchmark report of Using UniversalProfile owned by an LSP6KeyManager

This document contains the gas usage for common interactions and scenarios when using UniversalProfile smart contracts.

🔀 execute scenarios

👑 unrestricted controller

execute scenarios - 👑 main controller ⛽ Gas Usage
transfer LYX to an EOA 60594
transfer LYX to a UP 62196
transfer tokens (LSP7) to an EOA (no data) 107459
transfer tokens (LSP7) to a UP (no data) 243524
transfer a NFT (LSP8) to a EOA (no data) 171303
transfer a NFT (LSP8) to a UP (no data) 290721

🛃 restricted controller

execute scenarios - 🛃 restricted controller ⛽ Gas Usage
transfer some LYXes to an EOA - restricted to 1 x allowed address only (TRANSFERVALUE + 1x AllowedCalls) 72833
transfers some tokens (LSP7) to an EOA - restricted to LSP7 + 2x allowed contracts only (CALL + 2x AllowedCalls) (no data) 123268
transfers some tokens (LSP7) to an other UP - restricted to LSP7 + 2x allowed contracts only (CALL + 2x AllowedCalls) (no data) 259333
transfers a NFT (LSP8) to an EOA - restricted to LSP8 + 2x allowed contracts only (CALL + 2x AllowedCalls) (no data) 187100
transfers a NFT (LSP8) to an other UP - restricted to LSP8 + 2x allowed contracts only (CALL + 2x AllowedCalls) (no data) 306518

🗄️ setData scenarios

👑 unrestricted controller

setData scenarios - 👑 main controller ⛽ Gas Usage
updates profile details (LSP3Profile metadata) 136958
give permissions to a controller (AddressPermissions[] + AddressPermissions[index] + AddressPermissions:Permissions:) 133032
restrict a controller to some specific ERC725Y Data Keys 139305
restrict a controller to interact only with 3x specific addresses 162009
remove a controller (its permissions + its address from the AddressPermissions[] array) 67990
write 5x LSP12 Issued Assets 233454

🛃 restricted controller

setData scenarios - 🛃 restricted controller ⛽ Gas Usage
setData(bytes32,bytes) -> updates 1x data key 102687
setData(bytes32[],bytes[]) -> updates 3x data keys (first x3) 161587
setData(bytes32[],bytes[]) -> updates 3x data keys (middle x3) 145630
setData(bytes32[],bytes[]) -> updates 3x data keys (last x3) 170815
setData(bytes32[],bytes[]) -> updates 2x data keys + add 3x new controllers (including setting the array length + indexes under AddressPermissions[index]) 250159

📝 Notes

  • The execute and setData scenarios are executed on a fresh UniversalProfile and LSP6KeyManager smart contracts, deployed as standard contracts (not as proxy behind a base contract implementation).

Copy link
Member

@skimaharvey skimaharvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🖖

@YamenMerhi YamenMerhi merged commit 235048d into develop Jun 22, 2023
24 checks passed
@YamenMerhi YamenMerhi deleted the build/oz-4.9.2 branch June 22, 2023 08:26
@YamenMerhi YamenMerhi restored the build/oz-4.9.2 branch June 22, 2023 08:28
@CJ42 CJ42 mentioned this pull request Jun 22, 2023
CJ42 added a commit that referenced this pull request Jun 23, 2023
* chore: create template

* chore: setup dodoc

* docs: improve some natspec with custom tags

* docs: Improve template for SC ABI generation

Co-authored-by: Jean Cvllr <CJ42@users.noreply.github.com>

docs: update template

docs: update template

* docs: use `@b00ste/hardhat-dodoc` npm package

* docs: check and improve LSP0 core natspec

* chore: format dodoc config

* build: add list of `contract` and `library` to generate docs for

* chore: optimize dodoc config

* chore: update pr template

* chore: add suggested changes

* docs: remove indentation

* chore: fix script in `package.json`

* chore: fix template

* chore: add NatSpec guidelines to `CONTRIBUTING.md`

* chore: update prettier

* docs: update dodoc config

* fix: variable shadowing with `transferOwnership(_pendingOwner)` and `_pendingOwner` state variable

* refactor: initialize local variables to `0` (iterators) and `false`

* chore: format code

* chore: improve readability using number instead of long hex string

* docs: fix duplicate Natspec param for LSP20 error

* chore: fix typescript prettier config

* chore: add sugested changes

* docs: add info block to dodoc template (#615)

* docs: add info block to dodoc template

* chore: update `@b00ste/hardhat-dodoc` package

* ci!: remove android + iOS artifacts from Github release (#617)

* ci!: remove android + iOS artifacts generation from CI

* docs: remove references to Android + iOS artifacts

* chore: remove `scripts/java` paths from gitignore

* chore(release): 0.10.2

* chore: set single quotes for strings in prettier configs  (#619)

* ci: use npm command to check prettier

* build: use `1.1.3` for `prettier-plugin-solidity` + use minor release `^` semver

* style: run prettier to format all string text with single quotes

* build: generate Markdown table with interface IDs (#620)

* refactor!: rename ILSP20 public interface to `ILSP20CallVerifier`

Co-authored-by: Andreas Richter <richtera@users.noreply.github.com>

* docs: document contract interfaces with `@title` tags

* build: add script to generate interfaceIDs table in markdown

* chore: format Markdown in table

* test: fix wrong interface name in tests

---------

Co-authored-by: Andreas Richter <richtera@users.noreply.github.com>

* ci: add ESLint in CI + Fix eslint errors in JS / TS files across repository (#621)

* ci: add ESLint in CI

* chore: fix eslint errors via `eslint . --fix`

* chore: fix eslint errors of unused vars and functions

* refactor!: remove `EIP191Signer.sol` library, replace by function `toDataWithIntendedValidatorHash` from latest OpenZeppelin library (#622)

* build: upgrade OZ package to 4.9.2

* refactor!: remove `EIP191Signer` library to use function from OZ library

* test: remove EIP191SignerTester contracts and tests

* ci: add `--if-present` tag

---------

Co-authored-by: YamenMerhi <yamennmerhi@gmail.com>

* ci: use `hardhat compile` command to generate build (#625)

* chore: apply prettier with new rules (#623)

* build!: uprade `@erc725/smart-contracts` version to 5.1.0 + remove LSP6 function selectors from `LSP6Constants.sol` (#624)

* build: upgrade `@erc725/smart-contracts` package version to `5.1.0`

* refactor!: remove LSP6 functions selector constants and from ERC725

* chore: put back comment for three contracts at the top

---------

Co-authored-by: YamenMerhi <yamennmerhi@gmail.com>

* docs: add additional details in CHANGELOG for 0.10.2 (#626)

---------

Co-authored-by: b00ste <daniel@lukso.io>
Co-authored-by: b00ste.lyx <62855857+b00ste@users.noreply.github.com>
Co-authored-by: Andreas Richter <richtera@users.noreply.github.com>
Co-authored-by: YamenMerhi <yamennmerhi@gmail.com>
@CJ42 CJ42 deleted the build/oz-4.9.2 branch August 19, 2023 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants