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: create Extension4337 #735

Merged
merged 17 commits into from
Oct 9, 2023
Merged

feat: create Extension4337 #735

merged 17 commits into from
Oct 9, 2023

Conversation

skimaharvey
Copy link
Member

What does this PR introduce?

  • LSP17 4337 extension
  • Initial integration tests with Universal Profile

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

Changes to gas cost

Generated at commit: ccd283d7854bc26a100aa7bfcc8746954ee18c24, compared to commit: b8eca3c5696acf85239130ef67edec9e8c134bfa

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
LSP6ExecuteRestrictedController transferLYXToUP +11,283 ❌ +35.82%
LSP6ExecuteUnrestrictedController transferLYXToUP +10,883 ❌ +32.49%
LSP6SetDataRestrictedController execute +6,096 ❌ +21.36%
LSP6SetDataUnrestrictedController execute +6,096 ❌ +21.36%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
LSP6ExecuteRestrictedController 2,914,951 (+39,260) lsp20VerifyCall
transferLYXToEOA
transferLYXToUP
transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomEOA
transferTokensToRandomUP
14,682 (-214)
62,532 (+6,367)
42,779 (+11,283)
146,341 (+15,572)
254,966 (+15,652)
76,501 (+2,809)
211,403 (+2,244)
-1.44%
+11.34%
+35.82%
+11.91%
+6.54%
+3.81%
+1.07%
16,881 (-151)
62,532 (+6,367)
42,779 (+11,283)
146,341 (+15,572)
254,966 (+15,652)
76,501 (+2,809)
211,403 (+2,244)
-0.89%
+11.34%
+35.82%
+11.91%
+6.54%
+3.81%
+1.07%
17,614 (-131)
62,532 (+6,367)
42,779 (+11,283)
146,341 (+15,572)
254,966 (+15,652)
76,501 (+2,809)
211,403 (+2,244)
-0.74%
+11.34%
+35.82%
+11.91%
+6.54%
+3.81%
+1.07%
17,614 (-131)
62,532 (+6,367)
42,779 (+11,283)
146,341 (+15,572)
254,966 (+15,652)
76,501 (+2,809)
211,403 (+2,244)
-0.74%
+11.34%
+35.82%
+11.91%
+6.54%
+3.81%
+1.07%
8 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
LSP6ExecuteUnrestrictedController 2,914,951 (+39,260) lsp20VerifyCall
transferLYXToEOA
transferLYXToUP
transferNFTToRandomEOA
transferNFTToRandomUP
transferTokensToRandomEOA
transferTokensToRandomUP
14,682 (-214)
63,036 (+6,241)
44,379 (+10,883)
145,090 (+15,572)
253,715 (+15,652)
75,250 (+3,122)
209,839 (+2,244)
-1.44%
+10.99%
+32.49%
+12.02%
+6.57%
+4.33%
+1.08%
16,881 (-151)
63,036 (+6,241)
44,379 (+10,883)
145,090 (+15,572)
253,715 (+15,652)
75,250 (+3,122)
209,839 (+2,244)
-0.89%
+10.99%
+32.49%
+12.02%
+6.57%
+4.33%
+1.08%
17,614 (-131)
63,036 (+6,241)
44,379 (+10,883)
145,090 (+15,572)
253,715 (+15,652)
75,250 (+3,122)
209,839 (+2,244)
-0.74%
+10.99%
+32.49%
+12.02%
+6.57%
+4.33%
+1.08%
17,614 (-131)
63,036 (+6,241)
44,379 (+10,883)
145,090 (+15,572)
253,715 (+15,652)
75,250 (+3,122)
209,839 (+2,244)
-0.74%
+10.99%
+32.49%
+12.02%
+6.57%
+4.33%
+1.08%
8 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
1 (0)
LSP6SetDataRestrictedController 2,899,735 (+39,267) execute
givePermissionsToController
restrictControllerToERC725YKeys
30,774 (+12,284)
122,906 (+2,272)
141,615 (+2,731)
+66.44%
+1.88%
+1.97%
34,631 (+6,096)
122,906 (+2,272)
141,615 (+2,731)
+21.36%
+1.88%
+1.97%
34,631 (+6,096)
122,906 (+2,272)
141,615 (+2,731)
+21.36%
+1.88%
+1.97%
38,488 (-93)
122,906 (+2,272)
141,615 (+2,731)
-0.24%
+1.88%
+1.97%
2 (0)
1 (0)
1 (0)
LSP6SetDataUnrestrictedController 2,899,735 (+39,267) execute
givePermissionsToController
restrictControllerToERC725YKeys
30,774 (+12,284)
128,906 (+2,272)
150,115 (+2,731)
+66.44%
+1.79%
+1.85%
34,631 (+6,096)
128,906 (+2,272)
150,115 (+2,731)
+21.36%
+1.79%
+1.85%
34,631 (+6,096)
128,906 (+2,272)
150,115 (+2,731)
+21.36%
+1.79%
+1.85%
38,488 (-93)
128,906 (+2,272)
150,115 (+2,731)
-0.24%
+1.79%
+1.85%
2 (0)
1 (0)
1 (0)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 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 3007205 (0 )
KeyManager 3476959 (0 )
LSP1DelegateUP 1623804 (0 )
LSP7Mintable 2354082 (0 )
LSP8Mintable 2476554 (0 )

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 44867 (12 📈❌)
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 84838 (12 📈❌)
Transfer 0.1 LYX to 3x UPs with 256 bytes of data 100321 (36 📈❌)

🗄️ setData scenarios

setData scenarios - UP owned by 🔑 EOA ⛽ Gas Usage
Set a 20 bytes long value 49971 (0 )
Set a 60 bytes long value 95293 (0 )
Set a 160 bytes long value 164441 (-24 📉✅)
Set a 300 bytes long value 279688 (-12 📉✅)
Set a 600 bytes long value 484124 (0 )
Change the value of a data key already set 32859 (0 )
Remove the value of a data key already set 27333 (0 )
Set 2 data keys of 20 bytes long value 78454 (0 )
Set 2 data keys of 100 bytes long value 260618 (12 📈❌)
Set 3 data keys of 20 bytes long value 105171 (0 )
Change the value of three data keys already set of 20 bytes long value 45471 (0 )
Remove the value of three data keys already set 41360 (0 )

🗄️ Tokens scenarios

Tokens scenarios - UP owned by 🔑 EOA ⛽ Gas Usage
Minting a LSP7Token to a UP (No Delegate) from an EOA 91982 (0 )
Minting a LSP7Token to an EOA from an EOA 59289 (0 )
Transferring an LSP7Token from a UP to another UP (No Delegate) 100092 (0 )
Minting a LSP8Token to a UP (No Delegate) from an EOA 159126 (0 )
Minting a LSP8Token to an EOA from an EOA 126433 (0 )
Transferring an LSP8Token from a UP to another UP (No Delegate) 148899 (0 )
UniversalProfile owned by a 🔒📄 LSP6KeyManager

🔀 execute scenarios

execute scenarios 👑 main controller 🛃 restricted controller
LYX transfer --> to an EOA 66106 (0 ) 76899 (0 )
LYX transfer --> to a UP 67388 (0 ) 79370 (0 )
LSP7 token transfer --> to an EOA 120156 (0 ) 134847 (0 )
LSP7 token transfer --> to a UP 256711 (0 ) 271402 (0 )
LSP8 NFT transfer --> to an EOA 184399 (0 ) 199067 (0 )
LSP8 NFT transfer --> to a UP 304201 (0 ) 318869 (0 )

🗄️ setData scenarios

setData scenarios 👑 main controller 🛃 restricted controller
Update Profile details (LSP3Profile Metadata) 68714 (0 ) 78713 (0 )
Add a new controller with permission to SET_DATA + 3x allowed data keys:
AddressPermissions[]
+ AddressPermissions[index]
+ AddressPermissions:Permissions:<controller>
+ AddressPermissions:AllowedERC725YDataKeys:<controller)
212112 (0 ) 221752 (0 )
Update permissions of previous controller. Allow it now to SUPER_SETDATA 53601 (0 ) 56584 (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:
80562 (0 ) 91629 (0 )
Write 5x new LSP12 Issued Assets 68267 (0 ) 102871 (0 )
Update 3x data keys (first 3) 127555 (0 ) 161636 (0 )
Update 3x data keys (middle 3) 107643 (0 ) 145790 (0 )
Update 3x data keys (last 3) 127555 (0 ) 171123 (0 )
Set 2 x new data keys + add 3x new controllers 816000 (0 ) 876269 (0 )

@skimaharvey skimaharvey changed the title Feat/4337 extension feat: create Extension4337 Oct 4, 2023
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.

added 1st round of review. Very nice first LSP17 Extension! 🎉 🚀

contracts/LSP17Extensions/Extension4337.sol Show resolved Hide resolved
contracts/LSP17Extensions/Extension4337.sol Outdated Show resolved Hide resolved
contracts/LSP17Extensions/Extension4337.sol Outdated Show resolved Hide resolved
contracts/LSP17Extensions/Extension4337.sol Show resolved Hide resolved
contracts/LSP17Extensions/Extension4337.sol Outdated Show resolved Hide resolved
contracts/LSP17Extensions/Extension4337.sol Outdated Show resolved Hide resolved
contracts/LSP17Extensions/Extension4337.sol Outdated Show resolved Hide resolved
tests/LSP17Extentions/Extension4337/4337.test.ts Outdated Show resolved Hide resolved
contracts/LSP17Extensions/Extension4337.sol Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
contracts/LSP17Extensions/Extension4337.sol Outdated Show resolved Hide resolved
tests/LSP17Extensions/Extension4337/4337.test.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/Extension4337/4337.test.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/Extension4337/4337.test.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/Extension4337/4337.test.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/Extension4337/4337.test.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/helpers/UserOp.ts Show resolved Hide resolved
tests/LSP17Extensions/helpers/utils.ts Show resolved Hide resolved
tests/LSP17Extensions/helpers/utils.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/helpers/solidityTypes.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/helpers/UserOperation.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/helpers/utils.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/helpers/utils.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/helpers/utils.ts Outdated Show resolved Hide resolved
tests/LSP17Extensions/helpers/UserOp.ts Show resolved Hide resolved
@@ -102,16 +102,16 @@ See [`IERC165-supportsInterface`](#ierc165-supportsinterface).

- Specification details: [**LSP-17-Extensions**](https://github.com/lukso-network/lips/tree/main/LSPs/LSP-17-Extensions.md#validateuserop)
- Solidity implementation: [`Extension4337.sol`](https://github.com/lukso-network/lsp-smart-contracts/blob/develop/contracts/LSP17Extensions/Extension4337.sol)
- Function signature: `validateUserOp(UserOperation,bytes32,uint256)`
- Function selector: `0xe86fc51e`
- Function signature: `validateUserOp(UserOperation,bytes32,)`
Copy link
Member

Choose a reason for hiding this comment

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

@b00ste small bug in the template of the docs generator for param that don't have named arguments but just the types.

To be fixed in a separate PR, as the function signature is incorrect

@CJ42 CJ42 merged commit d1df1d0 into develop Oct 9, 2023
27 checks passed
@CJ42 CJ42 deleted the feat/4337-extension branch October 9, 2023 15:53
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