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 function to deploy webauthn accounts from factory #275

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

howydev
Copy link
Collaborator

@howydev howydev commented Nov 5, 2024

No description provided.

This comment was marked as outdated.

Copy link

octane-security-app-dev bot commented Nov 5, 2024

Summary by Octane

New Contracts

No new contracts were added in this PR.

Updated Contracts

  • ModularAccountBenchmarkBase.sol: The smart contract now includes a WebAuthnValidationModule for enhanced validation capabilities.
  • AccountFactory.sol: Added WebAuthn validation module, new account creation methods, and improved documentation for better clarity on account functionality and features.
  • AccountTestBase.sol: The smart contract now includes a WebAuthn validation module for enhanced authentication support.

🔗 Commit Hash: 4544b2a

Copy link

octane-security-app-dev bot commented Nov 5, 2024

Overview

Octane AI analysis has finished. No vulnerabilities were found. Cheers! 🎉🎉🎉


🔗 Commit Hash: 4cc2f02

Copy link

octane-security-app-dev bot commented Nov 5, 2024

Overview

Vulnerabilities found: 1                                                                                

Detailed findings

src/factory/AccountFactory.sol


🔗 Commit Hash: 4544b2a
🛡️ Octane Dashboard: All vulnerabilities

@howydev howydev marked this pull request as ready for review November 5, 2024 21:07
@howydev howydev changed the title feat: add webauthn factory feat: add function to deploy webauthn accounts from factory Nov 5, 2024
Copy link
Collaborator

@jaypaik jaypaik left a comment

Choose a reason for hiding this comment

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

Some nits!

src/factory/AccountFactory.sol Outdated Show resolved Hide resolved
src/factory/AccountFactory.sol Outdated Show resolved Hide resolved
src/factory/AccountFactory.sol Outdated Show resolved Hide resolved
src/factory/AccountFactory.sol Outdated Show resolved Hide resolved
test/factory/AccountFactory.t.sol Outdated Show resolved Hide resolved
test/factory/AccountFactory.t.sol Outdated Show resolved Hide resolved
test/factory/AccountFactory.t.sol Outdated Show resolved Hide resolved
test/factory/AccountFactory.t.sol Outdated Show resolved Hide resolved
test/utils/AccountTestBase.sol Outdated Show resolved Hide resolved
@howydev howydev force-pushed the howy/add-webauthn-factory branch 2 times, most recently from caa5073 to 1f51c84 Compare November 6, 2024 20:09
Copy link

github-actions bot commented Nov 6, 2024

Contract sizes:

 | Contract                      | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) |
 |-------------------------------|------------------|-------------------|--------------------|---------------------|
-| AccountFactory                |            4,814 |             5,239 |             19,762 |              43,913 |
+| AccountFactory                |            5,921 |             6,386 |             18,655 |              42,766 |
 | AllowlistModule               |            9,903 |             9,930 |             14,673 |              39,222 |
 | ExecutionInstallDelegate      |            5,714 |             5,760 |             18,862 |              43,392 |
 | ModularAccount                |           21,973 |            28,676 |              2,603 |              20,476 |
 | NativeFunctionDelegate        |              434 |               461 |             24,142 |              48,691 |
 | NativeTokenLimitModule        |            4,449 |             4,476 |             20,127 |              44,676 |
 | PaymasterGuardModule          |            1,845 |             1,872 |             22,731 |              47,280 |
 | SemiModularAccountBytecode    |           23,275 |            29,978 |              1,301 |              19,174 |
 | SemiModularAccountStorageOnly |           23,769 |            30,472 |                807 |              18,680 |
 | SingleSignerValidationModule  |            3,646 |             3,673 |             20,930 |              45,479 |
 | TimeRangeModule               |            2,000 |             2,027 |             22,576 |              47,125 |
 | WebAuthnValidationModule      |            7,854 |             7,881 |             16,722 |              41,271 |

Code coverage:

File % Lines % Statements % Branches % Funcs
src/account/AccountBase.sol 100.00% (8/8) 100.00% (7/7) 100.00% (2/2) 100.00% (4/4)
src/account/AccountStorageInitializable.sol 100.00% (19/19) 100.00% (26/26) 100.00% (5/5) 100.00% (2/2)
src/account/ModularAccount.sol 100.00% (2/2) 100.00% (2/2) 100.00% (0/0) 100.00% (2/2)
src/account/ModularAccountBase.sol 98.99% (294/297) 96.29% (363/377) 77.59% (45/58) 97.30% (36/37)
src/account/ModularAccountView.sol 100.00% (24/24) 100.00% (28/28) 100.00% (2/2) 100.00% (4/4)
src/account/ModuleManagerInternals.sol 95.08% (58/61) 96.20% (76/79) 62.50% (5/8) 100.00% (3/3)
src/account/SemiModularAccountBase.sol 88.71% (55/62) 92.13% (82/89) 66.67% (10/15) 100.00% (15/15)
src/account/SemiModularAccountBytecode.sol 100.00% (6/6) 100.00% (7/7) 100.00% (1/1) 100.00% (2/2)
src/account/SemiModularAccountStorageOnly.sol 80.00% (4/5) 83.33% (5/6) 100.00% (0/0) 50.00% (1/2)
src/account/TokenReceiver.sol 33.33% (1/3) 33.33% (1/3) 100.00% (0/0) 33.33% (1/3)
src/factory/AccountFactory.sol 75.56% (34/45) 80.33% (49/61) 50.00% (3/6) 60.00% (9/15)
src/helpers/ExecutionInstallDelegate.sol 92.59% (50/54) 92.96% (66/71) 40.00% (2/5) 100.00% (7/7)
src/helpers/NativeFunctionDelegate.sol 100.00% (16/16) 100.00% (30/30) 100.00% (0/0) 100.00% (1/1)
src/libraries/ExecutionLib.sol 98.92% (274/277) 98.15% (266/271) 84.85% (28/33) 100.00% (24/24)
src/libraries/KnownSelectorsLib.sol 100.00% (16/16) 100.00% (34/34) 100.00% (0/0) 100.00% (2/2)
src/libraries/LinkedListSetLib.sol 94.00% (47/50) 96.25% (77/80) 66.67% (4/6) 100.00% (8/8)
src/libraries/MemManagementLib.sol 100.00% (54/54) 100.00% (70/70) 100.00% (0/0) 100.00% (12/12)
src/libraries/ModuleInstallCommonsLib.sol 57.14% (8/14) 42.11% (8/19) 75.00% (3/4) 100.00% (3/3)
src/modules/ModuleBase.sol 100.00% (13/13) 94.12% (16/17) 100.00% (2/2) 100.00% (3/3)
src/modules/permissions/AllowlistModule.sol 86.05% (74/86) 85.71% (96/112) 78.26% (18/23) 50.00% (9/18)
src/modules/permissions/NativeTokenLimitModule.sol 90.91% (40/44) 93.22% (55/59) 100.00% (13/13) 66.67% (8/12)
src/modules/permissions/PaymasterGuardModule.sol 83.33% (10/12) 82.35% (14/17) 66.67% (2/3) 71.43% (5/7)
src/modules/permissions/TimeRangeModule.sol 83.33% (10/12) 80.00% (16/20) 100.00% (1/1) 75.00% (6/8)
src/modules/validation/SingleSignerValidationModule.sol 92.00% (23/25) 81.58% (31/38) 62.50% (5/8) 90.00% (9/10)
src/modules/validation/WebAuthnValidationModule.sol 61.11% (11/18) 66.67% (18/27) 100.00% (3/3) 60.00% (6/10)
Total 94.11% (1151/1223) 93.10% (1443/1550) 77.78% (154/198) 85.05% (182/214)

@howydev howydev requested a review from jaypaik November 6, 2024 20:37
Copy link
Collaborator

@jaypaik jaypaik left a comment

Choose a reason for hiding this comment

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

Are we missing function getAddressWebAuthn?

src/factory/AccountFactory.sol Outdated Show resolved Hide resolved
src/factory/AccountFactory.sol Outdated Show resolved Hide resolved
src/factory/AccountFactory.sol Outdated Show resolved Hide resolved
@howydev howydev requested a review from jaypaik November 8, 2024 19:57
Copy link
Collaborator

jaypaik commented Nov 12, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @howydev and the rest of your teammates on Graphite Graphite

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.

2 participants