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: optimize runtime gas cost for LSP6 + LSP20 #744

Closed
wants to merge 19 commits into from
Closed

Conversation

CJ42
Copy link
Member

@CJ42 CJ42 commented Oct 11, 2023

What does this PR introduce?

⚡️ Performance

Optimize for runtime cost around LSP20 + LSP6.

PR Checklist

  • Wrote Tests
  • Wrote & Generated Documentation (readme/natspec/dodoc)
  • Ran npm run lint && npm run lint:solidity (solhint)
  • Ran npm run format (prettier)
  • Ran npm run build
  • Ran npm run test

@Hugoo
Copy link
Contributor

Hugoo commented Oct 11, 2023

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2023

👋 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.

⛽📊 Gas Benchmark Report

Deployment Costs

Deployed contracts ⛽ Deployment cost
UniversalProfile 3007181 (-12,736 📉✅)
KeyManager 3411252 (-62,243 📉✅)
LSP1DelegateUP 1623792 (0 )
LSP7Mintable 2314282 (-52,755 📉✅)
LSP8Mintable 2420830 (-61,455 📉✅)

Runtime Costs

UniversalProfile owned by an 🔑 EOA

🔀 execute scenarios

execute scenarios - UP owned by 🔑 EOA ⛽ Gas Usage
Transfer 1 LYX to an EOA without data 37537 (0 )
Transfer 1 LYX to a UP without data 36639 (0 )
Transfer 1 LYX to an EOA with 256 bytes of data 42198 (-12 📉✅)
Transfer 1 LYX to a UP with 256 bytes of data 44572 (-24 📉✅)
Transfer 0.1 LYX to 3x EOA without data 70862 (0 )
Transfer 0.1 LYX to 3x UP without data 75680 (0 )
Transfer 0.1 LYX to 3x EOA with 256 bytes of data 84826 (-24 📉✅)
Transfer 0.1 LYX to 3x UPs with 256 bytes of data 99544 (0 )

🗄️ setData scenarios

setData scenarios - UP owned by 🔑 EOA ⛽ Gas Usage
Set a 20 bytes long value 49970 (-1 📉✅)
Set a 60 bytes long value 95292 (-1 📉✅)
Set a 160 bytes long value 164452 (-13 📉✅)
Set a 300 bytes long value 278705 (-983 📉✅)
Set a 600 bytes long value 483141 (-983 📉✅)
Change the value of a data key already set 32858 (-1 📉✅)
Remove the value of a data key already set 27332 (-1 📉✅)
Set 2 data keys of 20 bytes long value 78428 (-14 📉✅)
Set 2 data keys of 100 bytes long value 260616 (10 📈❌)
Set 3 data keys of 20 bytes long value 105156 (-15 📉✅)
Change the value of three data keys already set of 20 bytes long value 45468 (-3 📉✅)
Remove the value of three data keys already set 41357 (-3 📉✅)

🗄️ Tokens scenarios

Tokens scenarios - UP owned by 🔑 EOA ⛽ Gas Usage
Minting a LSP7Token to a UP (No Delegate) from an EOA 91440 (0 )
Minting a LSP7Token to an EOA from an EOA 59289 (0 )
Transferring an LSP7Token from a UP to another UP (No Delegate) 99008 (0 )
Minting a LSP8Token to a UP (No Delegate) from an EOA 158380 (0 )
Minting a LSP8Token to an EOA from an EOA 126229 (0 )
Transferring an LSP8Token from a UP to another UP (No Delegate) 147815 (0 )
UniversalProfile owned by a 🔒📄 LSP6KeyManager

🔀 execute scenarios

execute scenarios 👑 main controller 🛃 restricted controller
LYX transfer --> to an EOA 62423 (-3,926 📉✅) 72752 (-4,637 📉✅)
LYX transfer --> to a UP 63704 (-3,927 📉✅) 74432 (-5,493 📉✅)
LSP7 token transfer --> to an EOA 115178 (-5,334 📉✅) 129216 (-6,296 📉✅)
LSP7 token transfer --> to a UP 249189 (-7,672 📉✅) 263227 (-8,634 📉✅)
LSP8 NFT transfer --> to an EOA 179421 (-5,334 📉✅) 193436 (-6,296 📉✅)
LSP8 NFT transfer --> to a UP 297273 (-7,672 📉✅) 311288 (-8,634 📉✅)

🗄️ setData scenarios

setData scenarios 👑 main controller 🛃 restricted controller
Update Profile details (LSP3Profile Metadata) 67362 (-1,636 📉✅) 77487 (-1,819 📉✅)
Add a new controller with permission to SET_DATA + 3x allowed data keys:
AddressPermissions[]
+ AddressPermissions[index]
+ AddressPermissions:Permissions:<controller>
+ AddressPermissions:AllowedERC725YDataKeys:<controller)
209640 (-2,756 📉✅) 219215 (-2,756 📉✅)
Update permissions of previous controller. Allow it now to SUPER_SETDATA 52390 (-1,495 📉✅) 55682 (-1,495 📉✅)
Remove a controller:
1. decrease AddressPermissions[] Array length
2. remove the controller address at AddressPermissions[index]
3. set "0x" for the controller permissions under AddressPermissions:Permissions:
78866 (-1,923 📉✅) 91102 (-1,923 📉✅)
Write 5x new LSP12 Issued Assets 67056 (-1,495 📉✅) 100941 (-2,523 📉✅)
Update 3x data keys (first 3) 125507 (-2,332 📉✅) 159376 (-2,853 📉✅)
Update 3x data keys (middle 3) 105595 (-2,332 📉✅) 143023 (-3,360 📉✅)
Update 3x data keys (last 3) 125507 (-2,332 📉✅) 167680 (-4,036 📉✅)
Set 2 x new data keys + add 3x new controllers 810468 (-5,816 📉✅) 869856 (-6,168 📉✅)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2023

Changes to gas cost

Generated at commit: c56a48336e121f586c186f983f7db72c8dbec03c, compared to commit: e2ec65aeba2aca8351d9a9e71e08e34a004ae483

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
LSP6ExecuteRestrictedController lsp20VerifyCall
transferLYXToEOA
transferLYXToUP
transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomEOA
transferTokensToRandomUP
-999 ✅
-308 ✅
-2 ✅
-4,780 ✅
-6,729 ✅
-308 ✅
-2,820 ✅
-5.87%
-0.49%
-0.00%
-3.31%
-2.68%
-0.41%
-1.36%
LSP6ExecuteUnrestrictedController lsp20VerifyCall
transferLYXToEOA
transferLYXToUP
transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomUP
-999 ✅
-2 ✅
-2 ✅
-4,475 ✅
-6,424 ✅
-2,438 ✅
-5.87%
-0.00%
-0.00%
-3.12%
-2.57%
-1.19%
LSP6SetDataRestrictedController execute -1 ✅ -0.00%
LSP6SetDataUnrestrictedController execute -1 ✅ -0.00%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
LSP6ExecuteRestrictedController 3,000,452 (+94,509) lsp20VerifyCall
transferLYXToEOA
transferLYXToUP
transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomEOA
transferTokensToRandomUP
11,346 (-3,471)
62,202 (-308)
42,755 (-2)
139,824 (-4,780)
244,802 (-6,729)
74,336 (-308)
204,029 (-2,820)
-23.43%
-0.49%
-0.00%
-3.31%
-2.68%
-0.41%
-1.36%
16,017 (-999)
62,202 (-308)
42,755 (-2)
139,824 (-4,780)
244,802 (-6,729)
74,336 (-308)
204,029 (-2,820)
-5.87%
-0.49%
-0.00%
-3.31%
-2.68%
-0.41%
-1.36%
17,574 (-175)
62,202 (-308)
42,755 (-2)
139,824 (-4,780)
244,802 (-6,729)
74,336 (-308)
204,029 (-2,820)
-0.99%
-0.49%
-0.00%
-3.31%
-2.68%
-0.41%
-1.36%
17,574 (-175)
62,202 (-308)
42,755 (-2)
139,824 (-4,780)
244,802 (-6,729)
74,336 (-308)
204,029 (-2,820)
-0.99%
-0.49%
-0.00%
-3.31%
-2.68%
-0.41%
-1.36%
8 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
LSP6ExecuteUnrestrictedController 3,000,452 (+94,509) lsp20VerifyCall
transferLYXToEOA
transferLYXToUP
transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomEOA
transferTokensToRandomUP
11,346 (-3,471)
63,012 (-2)
44,355 (-2)
138,878 (-4,475)
243,856 (-6,424)
73,391 (-2)
202,847 (-2,438)
-23.43%
-0.00%
-0.00%
-3.12%
-2.57%
-0.00%
-1.19%
16,017 (-999)
63,012 (-2)
44,355 (-2)
138,878 (-4,475)
243,856 (-6,424)
73,391 (-2)
202,847 (-2,438)
-5.87%
-0.00%
-0.00%
-3.12%
-2.57%
-0.00%
-1.19%
17,574 (-175)
63,012 (-2)
44,355 (-2)
138,878 (-4,475)
243,856 (-6,424)
73,391 (-2)
202,847 (-2,438)
-0.99%
-0.00%
-0.00%
-3.12%
-2.57%
-0.00%
-1.19%
17,574 (-175)
63,012 (-2)
44,355 (-2)
138,878 (-4,475)
243,856 (-6,424)
73,391 (-2)
202,847 (-2,438)
-0.99%
-0.00%
-0.00%
-3.12%
-2.57%
-0.00%
-1.19%
8 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
LSP6SetDataRestrictedController 2,985,236 (+94,516) execute 30,768 (-2) -0.01% 34,615 (-1) -0.00% 34,615 (-1) -0.00% 38,463 (0) 0.00% 2 (0)
LSP6SetDataUnrestrictedController 2,985,236 (+94,516) execute 30,768 (-2) -0.01% 34,615 (-1) -0.00% 34,615 (-1) -0.00% 38,463 (0) 0.00% 2 (0)

Copy link
Member Author

@CJ42 CJ42 left a comment

Choose a reason for hiding this comment

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

We can review this PR, but let's not merge it until the code moved in the assembly blocks has been tested through fuzzing.

@CJ42
Copy link
Member Author

CJ42 commented Oct 27, 2023

Closing this PR as we are only picking the optimisations for LSP0.

Replaced by #768

Since UP users can upgrade their Key Manager, we can gas optimise the Key Manager further in the future, release a new version of it and users could upgrade to a new more optimised version.

@CJ42 CJ42 closed this Oct 27, 2023
@Hugoo Hugoo deleted the DEV-3683 branch November 29, 2023 14:00
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