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.

Sign up for GitHub

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

Jump to bottom

✨ Account: Initialize functions #18

Merged
merged 4 commits into from
Jan 12, 2024
Merged

✨ Account: Initialize functions #18

merged 4 commits into from
Jan 12, 2024

Conversation

qd-qd
Copy link
Member

@qd-qd qd-qd commented Jan 11, 2024

No description provided.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 11, 2024 •
edited
Loading

Changes to gas cost

Generated at commit: 88a4b0edd1a35d70b03f3052333e54795f6185e6, compared to commit: f6c539b688851bb828dfc05671196c608664cf9d

🧾 Summary (20% most significant diffs)

Contract Method Avg (+/-) %
Account addFirstSigner
initialize		 +46,264 ❌
+44,709 ❌		 +10326.79%
+21189.10%
ERC1967Proxy addFirstSigner +38,673 ❌ +4587.54%
AccountFactory createAndInitAccount +68,559 ❌ +82.01%
Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
Account 400,831 (+308,542) addFirstSigner
initialize		 1,226 (+778)
594 (+383)		 +173.66%
+181.52%		 46,712 (+46,264)
44,920 (+44,709)		 +10326.79%
+21189.10%		 47,027 (+46,579)
46,640 (+46,429)		 +10397.10%
+22004.27%		 70,096 (+69,648)
46,640 (+46,429)		 +15546.43%
+22004.27%		 18 (+9)
28 (+10)
ERC1967Proxy 99,957 (+46,283) addFirstSigner 30,664 (+29,823) +3546.14% 39,516 (+38,673) +4587.54% 30,664 (+29,823) +3546.14% 70,501 (+69,648) +8165.06% 9 (0)
AccountFactory 1,188,953 (+301,847) createAndInitAccount
getAddress		 5,227 (-195)
4,365 (-195)		 -3.60%
-4.28%		 152,162 (+68,559)
4,365 (-195)		 +82.01%
-4.28%		 178,579 (+75,791)
4,365 (-195)		 +73.74%
-4.28%		 218,940 (+115,616)
4,365 (-195)		 +111.90%
-4.28%		 10 (0)
12 (0)
AccountFactoryMultiSteps 1,264,470 (+298,430) createAccount
createAndInitAccount
getAddress		 4,665 (-195)
178,557 (+75,791)
4,388 (-195)		 -4.01%
+73.75%
-4.25%		 127,223 (+41,352)
178,557 (+75,791)
4,388 (-195)		 +48.16%
+73.75%
-4.25%		 141,397 (+45,969)
178,557 (+75,791)
4,388 (-195)		 +48.17%
+73.75%
-4.25%		 141,397 (+45,969)
178,557 (+75,791)
4,388 (-195)		 +48.17%
+73.75%
-4.25%		 10 (0)
1 (0)
3 (0)
AccountFactoryTestWrapper 1,234,422 (+301,830) checkAccountExistence
getAddress		 4,634 (-195)
4,389 (-195)		 -4.04%
-4.25%		 6,307 (-195)
4,389 (-195)		 -3.00%
-4.25%		 7,144 (-195)
4,389 (-195)		 -2.66%
-4.25%		 7,144 (-195)
4,389 (-195)		 -2.66%
-4.25%		 3 (0)
1 (0)
SignerVaultWebAuthnP256R1TestWrapper 1,461,927 (-16,614) get
tryGet		 1,899 (-159)
8,235 (-223)		 -7.73%
-2.64%		 4,899 (-159)
8,235 (-223)		 -3.14%
-2.64%		 4,899 (-159)
8,235 (-223)		 -3.14%
-2.64%		 7,899 (-159)
8,235 (-223)		 -1.97%
-2.64%		 2 (0)
1 (0)

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 11, 2024 •
edited
Loading

LCOV of commit 7213d01 during Tests #39

Summary coverage rate:
  lines......: 80.6% (50 of 62 lines)
  functions..: 100.0% (20 of 20 functions)
  branches...: no data found

Files changed coverage rate:
                                   |Lines       |Functions  |Branches    
  Filename                         |Rate     Num|Rate    Num|Rate     Num
  =======================================================================
  src/Account.sol                  | 100%      6| 100%     3|    -      0
  src/AccountFactory.sol           |93.3%     15| 100%     4|    -      0
  src/AccountFactoryMultiSteps.sol |83.3%      6| 100%     1|    -      0
  src/SignerVaultWebAuthnP256R1.sol|67.9%     28| 100%     8|    -      0

@qd-qd qd-qd marked this pull request as ready for review January 12, 2024 16:43
@qd-qd qd-qd self-assigned this Jan 12, 2024
@qd-qd qd-qd changed the title ✨ Develop the constructor of the Account ✨ Account: Initialize functions Jan 12, 2024
@qd-qd
Copy link
Member Author

qd-qd commented Jan 12, 2024

This pull request triggers a weird Slither issue. I have shared it on Github here

@qd-qd qd-qd mentioned this pull request Jan 12, 2024
Closed
qd-qd added a commit that referenced this pull request Jan 12, 2024
@qd-qd
👷 Temporary disable Slither's workflow
de425db 
Since the PR #18, Slither throws an error related to IR when analyzing
our repository. I do not know where this issue comes from and I have the
feeling the issue is in Slither's hands. That's why I posted a comment to
their repository
[here](crytic/slither#2217 (comment))

Until we find the fix, I'm disabling the CI workflow that runs Slither.
@qd-qd qd-qd mentioned this pull request Jan 12, 2024
Merged
qd-qd added 4 commits January 12, 2024 18:28
@qd-qd
✨ Develop the constructor of the Account
582f58c 
This commit implements the constructor of the Account contract, which is
responsible for setting some immutable variables.
Additionnaly, a new utility function is added to the BaseTest contract to
fuzz addresses not taken by any precompute contract.
@qd-qd
✨ Develop the initialize function of the Account
a531820 
This commit implements the initialize functio of the Account contract.
This function can only be called once and it is responsible of setting
a fuse for the first signer addition
@qd-qd
✨ Develop the addFirstSigner function
8e71423 
This commit includes the function used to add the first and only the first
signer of the account.
In addition to that, this commit incudes a new function in the TestBase
contract to bound a coordinate on the secp256r1 curve.
@qd-qd
✨ Account - get signer functions
7213d01 
Implements two new functions in the Account contract to fetch a stored
signer using either the credId or the credIdHash.
Introduces breaking changes in the API of the SignerVaultWebAuthnP256R1
library by removing the signer struct in favor of tuples.
@qd-qd
Copy link
Member Author

qd-qd commented Jan 12, 2024

This PR is the one that throws the Slither issue described in #19

@qd-qd qd-qd merged commit 7213d01 into main Jan 12, 2024
5 checks passed
@qd-qd qd-qd deleted the feature/account branch January 12, 2024 17:38
qd-qd added a commit that referenced this pull request Dec 19, 2024
@qd-qd
👷 Temporary disable Slither's workflow
281d1d2 
Since the PR #18, Slither throws an error related to IR when analyzing
our repository. I do not know where this issue comes from and I have the
feeling the issue is in Slither's hands. That's why I posted a comment to
their repository
[here](crytic/slither#2217 (comment))

Until we find the fix, I'm disabling the CI workflow that runs Slither.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@qd-qd qd-qd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@qd-qd