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

♻️ remove the upgradeability of the AccountFactory #84

Merged
merged 3 commits into from
Apr 4, 2024

Conversation

qd-qd
Copy link
Member

@qd-qd qd-qd commented Apr 4, 2024

To avoid stacking as stated in the 4337 EIP, we remove the upgradeability of the AccountFactory contract.

qd-qd added 3 commits April 4, 2024 18:18
To avoid stacking as stated in the 4337 EIP, we remove the upgradeability
of the AccountFactory contract.
Now that the factory is immutable, we can simplify the tests.
Following recent factory updates, the factory scripts have been simplified.
All scripts related to the upgradeability have been removed.
@qd-qd qd-qd self-assigned this Apr 4, 2024
Copy link

github-actions bot commented Apr 4, 2024

Changes to gas cost

Generated at commit: 3bd2be43d32d1ecb0945b0aff7ffe30b0443fbed, compared to commit: 4c84455bb95bac8a7a1725b2dcab425700c8eb83

🧾 Summary (20% most significant diffs)

Contract Method Avg (+/-) %
AccountFactory createAndInitAccount +22,876 ❌ +51.81%
AccountFactoryHarness exposed_isSignatureLegit -1,771 ✅ -25.61%
ERC1967Proxy exposed_validateCreationSignature -4,030 ✅ -14.72%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
AccountFactory 1,153,485 (-256,319) createAndInitAccount
getAddress
owner
36,307 (+25,498)
7,965 (+22)
190 (-173)
+235.90%
+0.28%
-47.66%
67,033 (+22,876)
7,965 (+22)
190 (-1,030)
+51.81%
+0.28%
-84.43%
39,240 (+22,931)
7,965 (+22)
190 (-173)
+140.60%
+0.28%
-47.66%
251,304 (+23,383)
7,965 (+22)
190 (-2,173)
+10.26%
+0.28%
-91.96%
296 (0)
295 (-2)
1 (-6)
AccountFactoryHarness 1,203,680 (-257,712) exposed_isSignatureLegit
getAddress
3,804 (-178)
7,965 (+22)
-4.47%
+0.28%
5,144 (-1,771)
7,965 (+22)
-25.61%
+0.28%
3,810 (-2,166)
7,965 (+22)
-36.24%
+0.28%
7,361 (-2,172)
7,965 (+22)
-22.78%
+0.28%
1,282 (0)
1,026 (0)
ERC1967Proxy 0 (0) exposed_validateCreationSignature
isValidSignature
6,311 (0)
32,678 (0)
0.00%
0.00%
23,349 (-4,030)
169,091 (-15,246)
-14.72%
-8.27%
24,576 (-6,220)
45,852 (0)
-20.20%
0.00%
34,427 (-7,050)
540,343 (-91,495)
-17.00%
-14.48%
1,795 (0)
12 (0)
SignerVaultVanillaP256K1TestWrapper 239,984 (0) remove
set
22,344 (0)
44,268 (0)
0.00%
0.00%
22,501 (-13)
44,433 (-13)
-0.06%
-0.03%
22,572 (0)
44,496 (0)
0.00%
0.00%
22,572 (0)
44,496 (0)
0.00%
0.00%
256 (0)
1,024 (0)
SignerVaultWebAuthnP256R1Harness 799,090 (0) exposed_get
exposed_has(bytes)
exposed_remove
exposed_set
exposed_tryGet
1,877 (-12)
1,637 (-12)
29,743 (0)
24,633 (0)
8,199 (+91)
-0.64%
-0.73%
0.00%
0.00%
+1.12%
4,896 (0)
3,655 (0)
29,946 (-14)
82,259 (-6)
8,257 (-2)
0.00%
0.00%
-0.05%
-0.01%
-0.02%
4,907 (+6)
3,666 (+6)
30,040 (0)
89,409 (0)
8,202 (0)
+0.12%
+0.16%
0.00%
0.00%
0.00%
7,925 (0)
5,683 (0)
30,040 (0)
89,937 (0)
8,482 (0)
0.00%
0.00%
0.00%
0.00%
0.00%
512 (0)
512 (0)
256 (0)
2,304 (0)
256 (0)
Paymaster 1,187,223 (0) postOp
withdrawTo(address,uint256)
22,227 (0)
23,950 (0)
0.00%
0.00%
22,834 (-10)
36,871 (-7)
-0.04%
-0.02%
22,425 (+6)
24,190 (0)
+0.03%
0.00%
24,075 (0)
62,571 (0)
0.00%
0.00%
512 (0)
770 (0)
SmartAccountERC1271__EIP191 7,218,947 (-1,263,109)
SmartAccountERC1271__EIP712 7,245,988 (-1,263,112)
SmartAccount__ValidateCreationSignature 7,202,316 (-1,179,363)
AccountFactory__GetAddress 4,003,258 (-1,169,195)

@qd-qd qd-qd merged commit e3ce147 into main Apr 4, 2024
4 checks passed
@qd-qd qd-qd deleted the refacto/factory-immutable branch April 4, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant