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

♻️ improve the creation signature #51

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

qd-qd
Copy link
Member

@qd-qd qd-qd commented Mar 15, 2024

  • Take into account the prefix used to identify the signature
  • Rename loginHash to usernameHash
  • Add the address of the account concerned by the signature in the signature
  • Add the ID of the chain concerned by the creation of the account in the signature
  • Remove the _checkAccountExistence function from the AccountFactory contract

- Take into account the prefix used to identify the signature
- Rename `loginHash` to `usernameHash`
- Add the address of the `account` concerned by the signature in the signature
- Add the ID of the chain concerned by the creation of the account in the signature
- Remove the `_checkAccountExistence` function from the `AccountFactory` contract
@qd-qd qd-qd self-assigned this Mar 15, 2024
Copy link

Changes to gas cost

Generated at commit: f72f4c9b0c3b42561514dcb84815a0f49519f958, compared to commit: d968577aad122f9a8ba0a481483f0fb6953e8646

🧾 Summary (20% most significant diffs)

Contract Method Avg (+/-) %
AccountFactoryTestWrapper isSignatureLegit +25,119 ❌ +464.91%
WrapperAccount validateCreationSignature -3,499 ✅ -32.28%
ERC1967Proxy validateCreationSignature -3,499 ✅ -22.14%
MockFactory mockDeployAccount +22,194 ❌ +10.74%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
AccountFactoryTestWrapper 2,503,254 (+29,439) getAddress
isSignatureLegit
4,411 (-45)
27,086 (+24,323)
-1.01%
+880.31%
4,411 (-45)
30,522 (+25,119)
-1.01%
+464.91%
4,411 (-45)
31,262 (+24,980)
-1.01%
+397.64%
4,411 (-45)
31,514 (+25,196)
-1.01%
+398.80%
3,074 (+3,073)
2,305 (+256)
WrapperAccount 0 (0) validateCreationSignature 1,280 (0) 0.00% 7,341 (-3,499) -32.28% 8,046 (-3,194) -28.42% 10,805 (-10,355) -48.94% 2,305 (-1)
ERC1967Proxy 146,238 (0) validateCreationSignature 6,254 (0) 0.00% 12,304 (-3,499) -22.14% 13,020 (-3,194) -19.70% 15,779 (-10,355) -39.62% 2,305 (-1)
MockFactory 3,305,548 (+172,453) mockDeployAccount 228,751 (+22,194) +10.74% 228,751 (+22,194) +10.74% 228,751 (+22,194) +10.74% 228,751 (+22,194) +10.74% 10 (-1)
Signature_Test 1,987,710 (+195,888) wrappedRecover 6,253 (-178) -2.77% 6,253 (-178) -2.77% 6,253 (-178) -2.77% 6,253 (-178) -2.77% 1 (0)
Account__ValidateCreationSignature 6,542,860 (+448,509) _sliceBytes 1,072 (-22) -2.01% 1,282 (-26) -1.99% 1,300 (-22) -1.66% 1,452 (-22) -1.49% 256 (0)
AccountFactory 2,441,597 (+1,151) createAndInitAccount 31,898 (-83) -0.26% 177,801 (+79) +0.04% 225,429 (+102) +0.05% 225,429 (+102) +0.05% 8 (0)
SignerVaultWebAuthnP256R1TestWrapper 645,931 (0) remove
set
29,760 (0)
29,486 (0)
0.00%
0.00%
29,964 (-7)
83,330 (-19)
-0.02%
-0.02%
30,058 (0)
89,390 (-24)
0.00%
-0.03%
30,058 (0)
89,930 (0)
0.00%
0.00%
256 (0)
2,304 (0)
Paymaster 968,289 (0) deposit
postOp
53,327 (+2,500)
22,249 (0)
+4.92%
0.00%
53,327 (+10)
22,844 (-1)
+0.02%
-0.00%
53,327 (0)
22,441 (+6)
0.00%
+0.03%
53,327 (0)
24,097 (0)
0.00%
0.00%
259 (0)
512 (0)
SignerVaultVanillaP256K1TestWrapper 239,984 (0) remove
set
22,344 (0)
44,268 (0)
0.00%
0.00%
22,506 (+2)
44,435 (+6)
+0.01%
+0.01%
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)
Account 1,548,870 (+6,499) addFirstSigner 22,253 (0) 0.00% 59,375 (-1) -0.00% 74,293 (0) 0.00% 94,937 (-12) -0.01% 523 (0)
SmartAccountTestWrapper 1,563,433 (+6,691)
MockAccountFactory 3,890,941 (+113,356)
Account__validateWebAuthnP256R1Signature 4,975,787 (+117,006)

@qd-qd
Copy link
Member Author

qd-qd commented Mar 15, 2024

Slither sucks, we need to change.

@qd-qd qd-qd merged commit 4be4dc5 into main Mar 15, 2024
3 of 4 checks passed
@qd-qd qd-qd deleted the refacto/factory-creation-signature branch March 15, 2024 16:38
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