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

refactor: codesize reduction #231

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

Conversation

Zer0dot
Copy link
Contributor

@Zer0dot Zer0dot commented Oct 7, 2024

Motivation

We need to reduce code size to fit under the Spurious Dragon limit of 24576 bytes.

Solution

Various refactors, including:

  • Merging certain similar functions and introducing branching.
  • Removing domainSeparator() and potentially other offchain-computable functions.
  • Move certain functions to external libraries if possible and the gas impact to do so is low.

Copy link

octane-security-app-dev bot commented Oct 7, 2024

Summary by Octane

New Contracts

  • ExecutionInstallDelegate.sol: A smart contract for managing execution function installations/uninstallations, enabling modular upgrades, and ensuring function call safety.
  • ModuleInstallCommons.sol: The smart contract library manages module installation, verifying interfaces, and handling execution hooks for modular accounts.

Updated Contracts

  • ModularAccountBase.sol: Add delegate call for execution installation/uninstallation, combine create and create2, use internal domain separator.
  • ModuleManagerInternals.sol: Removed execution functions management; refactored hook installations using ModuleInstallCommons.
  • SemiModularAccountBase.sol: The smart contract now consolidates fallback signer updates and status in a single function, replacing separate functions for setting or disabling the signer.
  • SemiModularAccountStorageOnly.sol: Updated the event emission to "FallbackSignerUpdated" and removed commentary.
  • KnownSelectorsLib.sol: Removed selector checks for multiple interfaces and methods, focusing primarily on ERC-4337 functions.

🔗 Commit Hash: 8991553

Copy link

github-actions bot commented Oct 7, 2024

Contract sizes:

 | Contract                      | Size (B) | Margin (B) |
 |-------------------------------|----------|------------|
 | AccountFactory                |    4,814 |     19,762 |
 | AllowlistModule               |    9,156 |     15,420 |
 | ECDSAValidationModule         |    3,646 |     20,930 |
-| ModularAccount                |   27,034 |     -2,458 |
-| NativeTokenLimitModule        |    4,652 |     19,924 |
+| ExecutionInstallDelegate      |    6,149 |     18,427 |
+| ModularAccount                |   23,488 |      1,088 |
+| NativeTokenLimitModule        |    4,600 |     19,976 |
 | PaymasterGuardModule          |    1,718 |     22,858 |
-| SemiModularAccountBytecode    |   28,817 |     -4,241 |
-| SemiModularAccountStorageOnly |   29,352 |     -4,776 |
+| SemiModularAccountBytecode    |   24,708 |       -132 |
+| SemiModularAccountStorageOnly |   25,219 |       -643 |
 | TimeRangeModule               |    1,870 |     22,706 |
 | WebauthnValidationModule      |    7,854 |     16,722 |

Code coverage:

File % Lines % Statements % Branches % Funcs
src/account/AccountStorageInitializable.sol 89.47% (17/19) 96.15% (25/26) 80.00% (4/5) 100.00% (2/2)
src/account/BaseAccount.sol 87.50% (7/8) 85.71% (6/7) 50.00% (1/2) 100.00% (4/4)
src/account/ModularAccount.sol 100.00% (1/1) 100.00% (1/1) 100.00% (0/0) 100.00% (2/2)
src/account/ModularAccountBase.sol 95.04% (230/242) 96.15% (300/312) 86.36% (38/44) 100.00% (35/35)
src/account/ModularAccountView.sol 100.00% (26/26) 100.00% (33/33) 100.00% (2/2) 100.00% (2/2)
src/account/ModuleManagerInternals.sol 92.73% (51/55) 94.44% (68/72) 33.33% (2/6) 100.00% (3/3)
src/account/SemiModularAccountBase.sol 88.52% (54/61) 91.67% (77/84) 66.67% (10/15) 100.00% (16/16)
src/account/SemiModularAccountBytecode.sol 100.00% (5/5) 100.00% (6/6) 100.00% (1/1) 50.00% (1/2)
src/account/SemiModularAccountStorageOnly.sol 100.00% (4/4) 100.00% (5/5) 100.00% (0/0) 100.00% (2/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 70.59% (24/34) 76.09% (35/46) 40.00% (2/5) 58.33% (7/12)
src/helpers/ExecutionInstallDelegate.sol 90.74% (49/54) 90.14% (64/71) 40.00% (2/5) 100.00% (7/7)
src/libraries/ExecutionLib.sol 98.10% (207/211) 97.14% (204/210) 81.82% (18/22) 100.00% (18/18)
src/libraries/KnownSelectorsLib.sol 100.00% (13/13) 100.00% (32/32) 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% (47/47) 100.00% (63/63) 100.00% (0/0) 100.00% (9/9)
src/libraries/ModuleInstallCommons.sol 57.14% (8/14) 42.11% (8/19) 75.00% (3/4) 100.00% (3/3)
src/libraries/SparseCalldataSegmentLib.sol 100.00% (17/17) 100.00% (21/21) 100.00% (4/4) 100.00% (4/4)
src/modules/BaseModule.sol 100.00% (11/11) 93.33% (14/15) 100.00% (1/1) 100.00% (2/2)
src/modules/permissions/AllowlistModule.sol 83.72% (72/86) 83.93% (94/112) 78.26% (18/23) 44.44% (8/18)
src/modules/permissions/NativeTokenLimitModule.sol 87.18% (34/39) 89.29% (50/56) 88.89% (8/9) 54.55% (6/11)
src/modules/permissions/PaymasterGuardModule.sol 80.00% (8/10) 66.67% (10/15) 100.00% (2/2) 57.14% (4/7)
src/modules/permissions/TimeRangeModule.sol 83.33% (10/12) 80.00% (16/20) 100.00% (1/1) 62.50% (5/8)
src/modules/validation/ECDSAValidationModule.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 91.74% (977/1065) 91.63% (1259/1374) 76.79% (129/168) 83.00% (166/200)

Copy link

octane-security-app-dev bot commented Oct 7, 2024

Overview

Vulnerabilities found: 11                                                                                
Severity breakdown: 2 Critical, 2 High, 6 Medium, 1 Low

Detailed findings

src/account/ModularAccountBase.sol

src/account/SemiModularAccountBase.sol

src/account/SemiModularAccountStorageOnly.sol

  • Review potential Missing/Improper Check on the Admin Address issue that is exposed in the initialize function

src/factory/AccountFactory.sol


🔗 Commit Hash: 8991553
🛡️ Octane Dashboard: All vulnerabilities

@Zer0dot Zer0dot force-pushed the zer0dot/codesize-refactor branch 2 times, most recently from c8f1ff5 to dea2d9c Compare October 11, 2024 19:31
@Zer0dot Zer0dot marked this pull request as ready for review October 11, 2024 19:35
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