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

feat!: Add batchCalls(...) to LSP7 & LSP8 #810

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

b00ste
Copy link
Member

@b00ste b00ste commented Nov 28, 2023

What does this PR introduce?

⚠️ BREAKING CHANGES

This PR has an impact on LSP7 and LSP8 InterfaceId, please check the latest InterfaceId in constant.ts.

🚀 Feature

This PR adds the batchCalls(...) function to both LSP7 & LSP8. Add appropriate tests for the new functionality.

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

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.

⛽📊 Gas Benchmark Report

Deployment Costs

Deployed contracts ⛽ Deployment cost
UniversalProfile 3185468 (0 )
KeyManager 3659015 (0 )
LSP1DelegateUP 1637510 (24 📈❌)
LSP7Mintable 2453846 (131,321 📈❌)
LSP8Mintable 2558785 (134,851 📈❌)

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 37560 (0 )
Transfer 1 LYX to a UP without data 46253 (0 )
Transfer 1 LYX to an EOA with 256 bytes of data 42209 (-24 📉✅)
Transfer 1 LYX to a UP with 256 bytes of data 57186 (24 📈❌)
Transfer 0.1 LYX to 3x EOA without data 70862 (0 )
Transfer 0.1 LYX to 3x UP without data 104453 (0 )
Transfer 0.1 LYX to 3x EOA with 256 bytes of data 84838 (-12 📉✅)
Transfer 0.1 LYX to 3x UPs with 256 bytes of data 137173 (36 📈❌)

🗄️ setData scenarios

setData scenarios - UP owned by 🔑 EOA ⛽ Gas Usage
Set a 20 bytes long value 49951 (0 )
Set a 60 bytes long value 95261 (-12 📉✅)
Set a 160 bytes long value 164421 (-12 📉✅)
Set a 300 bytes long value 279656 (0 )
Set a 600 bytes long value 484092 (-24 📉✅)
Change the value of a data key already set 32839 (0 )
Remove the value of a data key already set 27313 (0 )
Set 2 data keys of 20 bytes long value 78500 (0 )
Set 2 data keys of 100 bytes long value 260664 (12 📈❌)
Set 3 data keys of 20 bytes long value 105218 (0 )
Change the value of three data keys already set of 20 bytes long value 45518 (0 )
Remove the value of three data keys already set 41397 (0 )

🗄️ Tokens scenarios

Tokens scenarios - UP owned by 🔑 EOA ⛽ Gas Usage
Minting a LSP7Token to a UP (No Delegate) from an EOA 93114 (22 📈❌)
Minting a LSP7Token to an EOA from an EOA 59390 (22 📈❌)
Transferring an LSP7Token from a UP to another UP (No Delegate) 102355 (-23 📉✅)
Minting a LSP8Token to a UP (No Delegate) from an EOA 159983 (0 )
Minting a LSP8Token to an EOA from an EOA 126260 (0 )
Transferring an LSP8Token from a UP to another UP (No Delegate) 151049 (-22 📉✅)
UniversalProfile owned by a 🔒📄 LSP6KeyManager

🔀 execute scenarios

execute scenarios 👑 main controller 🛃 restricted controller
LYX transfer --> to an EOA 64356 (0 ) 75306 (0 )
LYX transfer --> to a UP 78489 (0 ) 93386 (0 )
LSP7 token transfer --> to an EOA 116848 (-1 📉✅) 131598 (-1 📉✅)
LSP7 token transfer --> to a UP 250572 (21 📈❌) 265322 (21 📈❌)
LSP8 NFT transfer --> to an EOA 180935 (-22 📉✅) 195685 (1 📈❌)
LSP8 NFT transfer --> to a UP 297861 (-22 📉✅) 312611 (1 📈❌)

🗄️ setData scenarios

setData scenarios 👑 main controller 🛃 restricted controller
Update Profile details (LSP3Profile Metadata) 67294 (0 ) 77316 (0 )
Add a new controller with permission to SET_DATA + 3x allowed data keys:
AddressPermissions[]
+ AddressPermissions[index]
+ AddressPermissions:Permissions:<controller>
+ AddressPermissions:AllowedERC725YDataKeys:<controller)
209636 (0 ) 219793 (0 )
Update permissions of previous controller. Allow it now to SUPER_SETDATA 52322 (0 ) 55328 (0 )
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:
78861 (0 ) 90160 (0 )
Write 5x new LSP12 Issued Assets 66989 (0 ) 101616 (0 )
Update 3x data keys (first 3) 125501 (0 ) 159605 (0 )
Update 3x data keys (middle 3) 105589 (0 ) 143759 (0 )
Update 3x data keys (last 3) 125501 (0 ) 169092 (0 )
Set 2 x new data keys + add 3x new controllers 810481 (0 ) 872281 (0 )

Copy link
Contributor

github-actions bot commented Nov 28, 2023

Changes to gas cost

Generated at commit: 51ddaa7b1764a90488186c34d33fff8f63da6688, compared to commit: 6594f678c362bf26882023aa1a108bfd8188924d

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
LSP6ExecuteUnrestrictedController transferTokensToRandomEOA +388 ❌ +0.53%
LSP6ExecuteRestrictedController transferTokensToRandomEOA +388 ❌ +0.52%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
LSP6ExecuteUnrestrictedController 3,047,114 (0) transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomEOA
transferTokensToRandomUP
142,168 (+414)
248,185 (+809)
74,141 (+388)
205,105 (+979)
+0.29%
+0.33%
+0.53%
+0.48%
142,168 (+414)
248,185 (+809)
74,141 (+388)
205,105 (+979)
+0.29%
+0.33%
+0.53%
+0.48%
142,168 (+414)
248,185 (+809)
74,141 (+388)
205,105 (+979)
+0.29%
+0.33%
+0.53%
+0.48%
142,168 (+414)
248,185 (+809)
74,141 (+388)
205,105 (+979)
+0.29%
+0.33%
+0.53%
+0.48%
1 (0)
1 (0)
1 (0)
1 (0)
LSP6ExecuteRestrictedController 3,047,114 (0) transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomEOA
transferTokensToRandomUP
143,424 (+414)
249,441 (+809)
75,397 (+388)
206,675 (+979)
+0.29%
+0.33%
+0.52%
+0.48%
143,424 (+414)
249,441 (+809)
75,397 (+388)
206,675 (+979)
+0.29%
+0.33%
+0.52%
+0.48%
143,424 (+414)
249,441 (+809)
75,397 (+388)
206,675 (+979)
+0.29%
+0.33%
+0.52%
+0.48%
143,424 (+414)
249,441 (+809)
75,397 (+388)
206,675 (+979)
+0.29%
+0.33%
+0.52%
+0.48%
1 (0)
1 (0)
1 (0)
1 (0)

@b00ste b00ste force-pushed the feat/add-batchCalls-to-tokens branch 3 times, most recently from 6215b96 to ad0b548 Compare November 28, 2023 12:24
@b00ste b00ste force-pushed the feat/add-batchCalls-to-tokens branch 2 times, most recently from 19dbfa9 to 2b2f5cf Compare November 28, 2023 13:37
@b00ste b00ste force-pushed the feat/add-batchCalls-to-tokens branch from 2b2f5cf to b744ee7 Compare November 28, 2023 13:38
@YamenMerhi YamenMerhi merged commit be46796 into develop Nov 28, 2023
25 checks passed
@YamenMerhi YamenMerhi deleted the feat/add-batchCalls-to-tokens branch November 28, 2023 14:20
Copy link
Member

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

LGTM ✅

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