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

perf: save gas by not reading target storage when controller has both permissions already #588

Merged
merged 1 commit into from
May 12, 2023

Conversation

CJ42
Copy link
Member

@CJ42 CJ42 commented May 11, 2023

What does this PR introduce?

⚡️ Performance

Reduce gas cost of setting data keys related to:

  • LSP6 permissions for controllers (AddressPermissions:Permissions:<address>) or controllers list (AddressPermissions[])
  • LSP1 Delegates
  • LSP17 Extensions

This is done by avoiding multiple storage read via getData(...) if the controller making the request already has both permissions required.

Based on idea suggestion from @YamenMerhi in the LIP repo.
lukso-network/LIPs#110 (comment)

🛢️ Gas Saved for setData(...)

using the Gas Benchmark

image

🛢️ Changes in deployment cost

image

PR Checklist

  • Wrote Tests (N/A)
  • Wrote Documentation (N/A)
  • Ran npm run linter (solhint)
  • Ran npm run format (prettier)
  • Ran npm run build
  • Ran npm run test

@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 42243
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 84955
Transfer 0.1 LYX to 3x EOA with 256 bytes of data 100075

🗄️ 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 279693
Set a 600 bytes long value 484141
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 78466
Set 2 data keys of 100 bytes long value 260630
Set 3 data keys of 20 bytes long value 105196
Change the value of three data keys already set of 20 bytes long value 45520
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 91352
Minting a LSP7Token to an EOA from an EOA 59209
Transferring an LSP7Token from a UP to another UP (No Delegate) 98998
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 60786
transfer LYX to a UP 62388
transfer tokens (LSP7) to an EOA (no data) 107673
transfer tokens (LSP7) to a UP (no data) 243935
transfer a NFT (LSP8) to a EOA (no data) 171495
transfer a NFT (LSP8) to a UP (no data) 291110

🛃 restricted controller

execute scenarios - 🛃 restricted controller ⛽ Gas Usage
transfer some LYXes to an EOA - restricted to 1 x allowed address only (TRANSFERVALUE + 1x AllowedCalls) 73025
transfers some tokens (LSP7) to an EOA - restricted to LSP7 + 2x allowed contracts only (CALL + 2x AllowedCalls) (no data) 123479
transfers some tokens (LSP7) to an other UP - restricted to LSP7 + 2x allowed contracts only (CALL + 2x AllowedCalls) (no data) 259741
transfers a NFT (LSP8) to an EOA - restricted to LSP8 + 2x allowed contracts only (CALL + 2x AllowedCalls) (no data) 187289
transfers a NFT (LSP8) to an other UP - restricted to LSP8 + 2x allowed contracts only (CALL + 2x AllowedCalls) (no data) 306904

🗄️ setData scenarios

👑 unrestricted controller

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

🛃 restricted controller

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

📝 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).

@github-actions
Copy link
Contributor

Changes to gas cost

Generated at commit: 446afd5dd299568bd01d431efadf1b8d2a70bf3b, compared to commit: e1657de39dae3e31e764ef24ea3f5bd071e092f1

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
LSP6SetDataRestrictedController supportsInterface +55 ❌ +11.41%
LSP6SetDataUnrestrictedController supportsInterface +55 ❌ +11.41%
LSP6ExecuteRestrictedController supportsInterface +55 ❌ +10.89%
LSP6ExecuteUnrestrictedController supportsInterface +55 ❌ +10.89%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
LSP6SetDataRestrictedController 2,849,060 (+106,331) execute
givePermissionsToController
restrictControllerToERC725YKeys
supportsInterface
18,354 (-13)
120,527 (+259)
138,829 (+51)
537 (+55)
-0.07%
+0.22%
+0.04%
+11.41%
28,453 (+1)
120,527 (+259)
138,829 (+51)
537 (+55)
+0.00%
+0.22%
+0.04%
+11.41%
28,453 (+1)
120,527 (+259)
138,829 (+51)
537 (+55)
+0.00%
+0.22%
+0.04%
+11.41%
38,553 (+15)
120,527 (+259)
138,829 (+51)
537 (+55)
+0.04%
+0.22%
+0.04%
+11.41%
2 (0)
1 (0)
1 (0)
4 (0)
LSP6SetDataUnrestrictedController 2,849,060 (+106,331) execute
givePermissionsToController
restrictControllerToERC725YKeys
supportsInterface
18,354 (-13)
126,527 (+259)
147,329 (+51)
537 (+55)
-0.07%
+0.21%
+0.03%
+11.41%
28,453 (+1)
126,527 (+259)
147,329 (+51)
537 (+55)
+0.00%
+0.21%
+0.03%
+11.41%
28,453 (+1)
126,527 (+259)
147,329 (+51)
537 (+55)
+0.00%
+0.21%
+0.03%
+11.41%
38,553 (+15)
126,527 (+259)
147,329 (+51)
537 (+55)
+0.04%
+0.21%
+0.03%
+11.41%
2 (0)
1 (0)
1 (0)
4 (0)
LSP6ExecuteRestrictedController 2,864,276 (+106,331) lsp20VerifyCall
supportsInterface
transferLYXToEOA
transferLYXToUP
transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomEOA
transferTokensToRandomUP
15,090 (+14)
560 (+55)
57,163 (+69)
31,465 (+9)
131,016 (-200)
239,019 (+55)
77,091 (+341)
211,871 (+594)
+0.09%
+10.89%
+0.12%
+0.03%
-0.15%
+0.02%
+0.44%
+0.28%
17,227 (+38)
560 (+55)
57,163 (+69)
31,465 (+9)
131,016 (-200)
239,019 (+55)
77,091 (+341)
211,871 (+594)
+0.22%
+10.89%
+0.12%
+0.03%
-0.15%
+0.02%
+0.44%
+0.28%
17,940 (+46)
560 (+55)
57,163 (+69)
31,465 (+9)
131,016 (-200)
239,019 (+55)
77,091 (+341)
211,871 (+594)
+0.26%
+10.89%
+0.12%
+0.03%
-0.15%
+0.02%
+0.44%
+0.28%
17,940 (+46)
560 (+55)
57,163 (+69)
31,465 (+9)
131,016 (-200)
239,019 (+55)
77,091 (+341)
211,871 (+594)
+0.26%
+10.89%
+0.12%
+0.03%
-0.15%
+0.02%
+0.44%
+0.28%
8 (0)
24 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
LSP6ExecuteUnrestrictedController 2,864,276 (+106,331) lsp20VerifyCall
supportsInterface
transferLYXToEOA
transferLYXToUP
transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomEOA
transferTokensToRandomUP
15,090 (+14)
560 (+55)
56,764 (-13)
33,465 (+9)
128,853 (-294)
236,856 (-38)
74,387 (+224)
209,167 (+477)
+0.09%
+10.89%
-0.02%
+0.03%
-0.23%
-0.02%
+0.30%
+0.23%
17,227 (+38)
560 (+55)
56,764 (-13)
33,465 (+9)
128,853 (-294)
236,856 (-38)
74,387 (+224)
209,167 (+477)
+0.22%
+10.89%
-0.02%
+0.03%
-0.23%
-0.02%
+0.30%
+0.23%
17,940 (+46)
560 (+55)
56,764 (-13)
33,465 (+9)
128,853 (-294)
236,856 (-38)
74,387 (+224)
209,167 (+477)
+0.26%
+10.89%
-0.02%
+0.03%
-0.23%
-0.02%
+0.30%
+0.23%
17,940 (+46)
560 (+55)
56,764 (-13)
33,465 (+9)
128,853 (-294)
236,856 (-38)
74,387 (+224)
209,167 (+477)
+0.26%
+10.89%
-0.02%
+0.03%
-0.23%
-0.02%
+0.30%
+0.23%
8 (0)
24 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)

Copy link
Member

@YamenMerhi YamenMerhi left a comment

Choose a reason for hiding this comment

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

Nice one 👍

@CJ42 CJ42 merged commit f63ab2f into develop May 12, 2023
@CJ42 CJ42 deleted the perf/lsp6-setData branch May 12, 2023 09:36
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.

3 participants