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: specialize deploy scripts per artifact #324

Merged
merged 4 commits into from
Dec 14, 2024

Conversation

adamegyed
Copy link
Contributor

Motivation

  • We want to set different compiler settings per deployed contract.
  • CI formatting is broken since an upstream foundry update
  • We were missing the 7702 account implementation from the deploy script

Solution

  • Add the Sma7702 deployment to the DeployAccounts script.
  • Move the deployment of ExecutionInstallDelegate to the same script as the module deployments, and rename to DeployStandalones (i.e. not having dependencies).
    • Create a new foundry profile with higher optimizer runs for these contracts
  • Move the deployment of SmaStorageOnly to a new deployment script and create its own profile with the same optimizer runs as the current setting.
  • Increase the baseline optimizer runs for the profile optimized-build.
    • Update the gas profile to use the same number of runs, as it only tests contracts using these settings.
  • Adjust test cases to cover the new deploy script behavior
  • Adjust the filtering on the sizes action in CI. This is not comprehensive anymore, since there need to be multiple profiles pulled, and there's still an outstanding task to fix the formatting, but it is a first step.

Alternative considered

At first, I tried to make use of the new compilation restrictions feature in foundry, but it created duplicate compilation artifacts for the scripts, and there wasn't a mechanism to control which instance was used in the script. If this feature stabilizes, we should switch to it: foundry-rs/foundry#8668

@adamegyed adamegyed requested a review from howydev December 13, 2024 23:33
Copy link

octane-security-app bot commented Dec 13, 2024

Summary by Octane

New Contracts

  • DeploySmaStorage.s.sol: The contract deploys account implementations and an execution install delegate, utilizing specified environment variables for configuration.

Updated Contracts

  • DeployAccounts.s.sol: The smart contract modifications removed the "Execution Install Delegate" and "Storage Only" features, added "7702 Impl" feature, and included argument validation.
  • DeployFactory.s.sol: Updated the function to run only with the "optimized-build-standalone" profile.
  • GetInitcodeHash.s.sol: The smart contract now supports multiple profiles for executing tasks, adding specific initcode hash calculations for standalone and storage-focused configurations.
  • ScriptBase.sol: Added a function to retrieve the "SEMI_MODULAR_ACCOUNT_7702_IMPL" address.

🔗 Commit Hash: 9121316

Copy link

octane-security-app bot commented Dec 13, 2024

Overview

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


🔗 Commit Hash: 9121316

Copy link

github-actions bot commented Dec 13, 2024

Contract sizes:

 | Contract                      | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) |
-| AccountFactory                | 6,121            | 6,595             | 18,455             | 42,557              |
+| AccountFactory                | 6,661            | 7,135             | 17,915             | 42,017              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| AllowlistModule               | 9,602            | 9,629             | 14,974             | 39,523              |
+| AllowlistModule               | 10,262           | 10,289            | 14,314             | 38,863              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 | ExecutionInstallDelegate      | 5,947            | 5,993             | 18,629             | 43,159              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| ModularAccount                | 21,923           | 22,322            | 2,653              | 26,830              |
+| ModularAccount                | 23,273           | 23,672            | 1,303              | 25,480              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| NativeTokenLimitModule        | 4,583            | 4,610             | 19,993             | 44,542              |
+| NativeTokenLimitModule        | 5,093            | 5,120             | 19,483             | 44,032              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| PaymasterGuardModule          | 1,827            | 1,854             | 22,749             | 47,298              |
+| PaymasterGuardModule          | 2,097            | 2,124             | 22,479             | 47,028              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| SemiModularAccount7702        | 23,153           | 23,545            | 1,423              | 25,607              |
+| SemiModularAccount7702        | 23,741           | 24,133            | 835                | 25,019              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| SemiModularAccountBytecode    | 23,635           | 24,034            | 941                | 25,118              |
+| SemiModularAccountBytecode    | 24,223           | 24,622            | 353                | 24,530              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| SemiModularAccountStorageOnly | 24,168           | 24,567            | 408                | 24,585              |
+| SemiModularAccountStorageOnly | 24,756           | 25,155            | -180               | 23,997              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| SingleSignerValidationModule  | 3,646            | 3,673             | 20,930             | 45,479              |
+| SingleSignerValidationModule  | 3,976            | 4,003             | 20,600             | 45,149              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| TimeRangeModule               | 2,204            | 2,231             | 22,372             | 46,921              |
+| TimeRangeModule               | 2,504            | 2,531             | 22,072             | 46,621              |
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
 |-------------------------------+------------------+-------------------+--------------------+---------------------|
-| WebAuthnValidationModule      | 7,854            | 7,881             | 16,722             | 41,271              |
+| WebAuthnValidationModule      | 9,462            | 9,489             | 15,114             | 39,663              |

Code coverage:
| File | % Lines | % Statements | % Branches | % Funcs |
| script/Artifacts.sol | 100.00% (48/48) | 100.00% (29/29) | 100.00% (0/0) | 100.00% (24/24) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| script/DeployAccounts.s.sol | 87.88% (29/33) | 89.29% (25/28) | 33.33% (1/3) | 100.00% (6/6) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| script/DeployFactory.s.sol | 71.43% (30/42) | 71.05% (27/38) | 45.45% (5/11) | 100.00% (4/4) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| script/DeploySmaStorage.s.sol | 81.82% (18/22) | 83.33% (15/18) | 33.33% (1/3) | 100.00% (4/4) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| script/DeployStandalones.s.sol | 100.00% (29/29) | 100.00% (27/27) | 100.00% (0/0) | 100.00% (2/2) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| script/GetInitcodeHash.s.sol | 0.00% (0/58) | 0.00% (0/72) | 0.00% (0/16) | 0.00% (0/2) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| script/PredictAddresses.s.sol | 0.00% (0/53) | 0.00% (0/55) | 0.00% (0/6) | 0.00% (0/2) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| script/ScriptBase.sol | 70.59% (36/51) | 71.70% (38/53) | 37.50% (3/8) | 100.00% (12/12) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/AccountBase.sol | 100.00% (12/12) | 100.00% (7/7) | 100.00% (2/2) | 100.00% (4/4) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/AccountStorageInitializable.sol | 100.00% (21/21) | 100.00% (26/26) | 100.00% (5/5) | 100.00% (2/2) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/ModularAccount.sol | 100.00% (6/6) | 100.00% (6/6) | 100.00% (0/0) | 100.00% (3/3) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/ModularAccountBase.sol | 99.09% (326/329) | 96.24% (358/372) | 77.59% (45/58) | 100.00% (36/36) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/ModularAccountView.sol | 100.00% (32/32) | 100.00% (31/31) | 100.00% (3/3) | 100.00% (5/5) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/ModuleManagerInternals.sol | 94.03% (63/67) | 95.06% (77/81) | 63.64% (7/11) | 100.00% (4/4) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/SemiModularAccount7702.sol | 22.22% (2/9) | 16.67% (1/6) | 0.00% (0/1) | 33.33% (1/3) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/SemiModularAccountBase.sol | 90.48% (76/84) | 91.84% (90/98) | 64.71% (11/17) | 100.00% (16/16) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/SemiModularAccountBytecode.sol | 100.00% (8/8) | 100.00% (7/7) | 100.00% (1/1) | 100.00% (2/2) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/SemiModularAccountStorageOnly.sol | 77.78% (7/9) | 60.00% (6/10) | 100.00% (0/0) | 66.67% (2/3) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/account/TokenReceiver.sol | 33.33% (2/6) | 33.33% (1/3) | 100.00% (0/0) | 33.33% (1/3) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/factory/AccountFactory.sol | 79.03% (49/62) | 87.10% (54/62) | 50.00% (3/6) | 62.50% (10/16) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/helpers/ExecutionInstallDelegate.sol | 89.39% (59/66) | 89.47% (68/76) | 25.00% (2/8) | 100.00% (7/7) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/libraries/ExecutionLib.sol | 99.66% (297/298) | 98.89% (268/271) | 90.91% (30/33) | 100.00% (24/24) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/libraries/KnownSelectorsLib.sol | 100.00% (18/18) | 100.00% (34/34) | 100.00% (0/0) | 100.00% (2/2) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/libraries/LinkedListSetLib.sol | 94.83% (55/58) | 96.25% (77/80) | 66.67% (4/6) | 100.00% (8/8) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/libraries/MemManagementLib.sol | 100.00% (66/66) | 100.00% (70/70) | 100.00% (0/0) | 100.00% (12/12) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/libraries/ModuleInstallCommonsLib.sol | 64.71% (11/17) | 42.11% (8/19) | 75.00% (3/4) | 100.00% (3/3) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/libraries/ValidationLocatorLib.sol | 70.59% (72/102) | 71.74% (66/92) | 47.83% (11/23) | 85.00% (17/20) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/modules/ModuleBase.sol | 100.00% (16/16) | 94.12% (16/17) | 100.00% (2/2) | 100.00% (3/3) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/modules/permissions/AllowlistModule.sol | 81.90% (86/105) | 86.73% (98/113) | 78.26% (18/23) | 55.56% (10/18) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/modules/permissions/NativeTokenLimitModule.sol | 88.14% (52/59) | 92.06% (58/63) | 100.00% (13/13) | 75.00% (9/12) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/modules/permissions/PaymasterGuardModule.sol | 85.71% (18/21) | 84.21% (16/19) | 33.33% (1/3) | 85.71% (6/7) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/modules/permissions/TimeRangeModule.sol | 88.89% (24/27) | 85.19% (23/27) | 100.00% (5/5) | 87.50% (7/8) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/modules/validation/SingleSignerValidationModule.sol | 87.80% (36/41) | 84.21% (32/38) | 62.50% (5/8) | 100.00% (10/10) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| src/modules/validation/WebAuthnValidationModule.sol | 69.70% (23/33) | 70.37% (19/27) | 100.00% (3/3) | 70.00% (7/10) |
|---------------------------------------------------------+--------------------+--------------------+------------------+------------------|
| Total | 85.27% (1627/1908) | 84.96% (1678/1975) | 65.25% (184/282) | 88.55% (263/297) |

Copy link
Collaborator

@howydev howydev left a comment

Choose a reason for hiding this comment

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

LGTM. curious - why are we using a separate script for sma storage?

@adamegyed
Copy link
Contributor Author

adamegyed commented Dec 13, 2024

SMA-storage is ~500 bytes larger than the other variants, so I split it up to be able to bump up the optimizer runs on the other implementations

@adamegyed adamegyed force-pushed the adam/update-deploy-scripts branch from 899c959 to 9121316 Compare December 14, 2024 00:16
@adamegyed adamegyed merged commit 5344175 into develop Dec 14, 2024
6 checks passed
@adamegyed adamegyed deleted the adam/update-deploy-scripts branch December 14, 2024 00:30
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