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

Implement Support for ERC-4337 EntryPoint v0.7 #243

Merged
merged 3 commits into from
Feb 12, 2024

Conversation

mmv08
Copy link
Member

@mmv08 mmv08 commented Jan 29, 2024

This PR:

  • Addresses the M-01 from the audit by updating the ERC-4337 contracts to the latest ones from the development branch (presumably, they will be released as version 0.7.0). The most notable change is the new PackedUserOperation struct with the new packed gas limit field accountGasLimits that consists of two uint128 packed values (validationGasLimit, callGasLimit)
  • Implements Use the new PackedUserOperation struct with packed gas fields from entrypoint 0.7.0 #225

Forked contracts repo with build artifacts: https://github.com/5afe/account-abstraction
More about 0.7.0 changes: #215

Issues/concerns:

  • Using unaudited non-release candidate contract versions
  • The reference bundler is still in development, thus e2e tests are failing (Confirmed with ERC4337 team)

@coveralls
Copy link

coveralls commented Jan 29, 2024

Pull Request Test Coverage Report for Build 7870582983

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 76.923%

Totals Coverage Status
Change from base Build 7842856737: 0.0%
Covered Lines: 131
Relevant Lines: 155

💛 - Coveralls

@mmv08 mmv08 force-pushed the audit/m-01 branch 5 times, most recently from 3d1d8a9 to b94c828 Compare January 29, 2024 16:24
@mmv08 mmv08 added the audit review A PR to address an issue found during an audit label Jan 29, 2024
@@ -1 +1,2 @@
modules/**
Copy link
Member Author

@mmv08 mmv08 Jan 29, 2024

Choose a reason for hiding this comment

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

This and other formatting/linting-related changes were needed to fix the failing CI job. The problem was conflicting eslint-prettier-plugin rules and prettier itself due to misconfiguration.

TLDR: For linting and formatting javascript/typescript code, we use eslint-plugin-prettier, which runs prettier as ESLint rules. For checking formatting, we use a standalone prettier package instead. After updating dependencies, they introduced conflicting rules. So, ESLint would say that prettier's formatting needs to be corrected and vice versa. To fix this, I updated the prettier script to only format solidity files in the 4337 module and added the examples directory to .prettierignore for the global check (since the only app there uses eslint for formatting as well)

@mmv08 mmv08 marked this pull request as ready for review January 29, 2024 16:50
@mmv08 mmv08 requested a review from a team as a code owner January 29, 2024 16:50
@mmv08 mmv08 requested review from rmeissner, nlordell, akshay-ap and remedcu and removed request for a team January 29, 2024 16:50
modules/4337/test/e2e/run.sh Outdated Show resolved Hide resolved
modules/4337/docker/bundler/Dockerfile Outdated Show resolved Hide resolved
@mmv08 mmv08 force-pushed the audit/m-01 branch 2 times, most recently from 798faeb to 88d3daa Compare January 30, 2024 14:39
@nlordell
Copy link
Collaborator

nlordell commented Feb 2, 2024

Closing in favor of #245

@nlordell nlordell closed this Feb 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
@nlordell
Copy link
Collaborator

nlordell commented Feb 5, 2024

Re-opening, as we want to potentially include this to an entrypoint/v0.7 feature branch.

@nlordell nlordell reopened this Feb 5, 2024
@nlordell nlordell removed the audit review A PR to address an issue found during an audit label Feb 5, 2024
@nlordell nlordell changed the title [M-01] M-01 Potential Obsolescence for Safe4337Module Implement Support for ERC-4337 EntryPoint v0.7 Feb 5, 2024
@safe-global safe-global unlocked this conversation Feb 5, 2024
@nlordell nlordell changed the base branch from main to feature/entrypoint-v0.7 February 5, 2024 14:48
@nlordell nlordell force-pushed the feature/entrypoint-v0.7 branch from f955991 to 19ea9d6 Compare February 9, 2024 10:44
@nlordell
Copy link
Collaborator

Merging with failing E2E tests (see PR description).

@nlordell nlordell merged commit 7ada62b into feature/entrypoint-v0.7 Feb 12, 2024
7 of 9 checks passed
@nlordell nlordell deleted the audit/m-01 branch February 12, 2024 10:55
@github-actions github-actions bot locked and limited conversation to collaborators Feb 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants