search

safe-global / safe-modules

A collection of modules that can be used with the Safe contract
GNU Lesser General Public License v3.0
140 stars 73 forks source link

Add a new logUserOperationGas function and improve user operation gas estimation #446

Closed mmv08 closed 2 months ago

mmv08 commented 2 months ago

This PR fixes https://github.com/safe-global/safe-modules/issues/425 by introducing a new estimateUserOperationGas function, which runs the simulation using AA's reference EntryPointSimulations contract to ensure we use correct estimations for our test user operations. Previously, we set them to arbitrarily high numbers to make the tests pass. I also improved a couple of tests where we set maxFeePerGas to 0 by actually obtaining the prefund value the account paid for the operation.

The code was mostly borrowed from the account-abstraction reference bundler and is not suitable for use in a production environment.

It also includes the logUserOperationGas function, which fixes the core issue of displaying the actual gas used by the user operation.

Example log:

  Gas Metering
    Safe Deployment + Enabling 4337 Module
           Used 418349 gas (Account or Paymaster) for >Safe with 4337 Module Deployment<
           Used 415210 gas (Transaction) for >Safe with 4337 Module Deployment<
      ✔ Safe with 4337 Module Deployment (137ms)
    Safe Deployment + Enabling 4337 Module + Native Transfers
           Used 449725 gas (Account or Paymaster) for >Safe with 4337 Module Deployment + Native Transfer<
           Used 447665 gas (Transaction) for >Safe with 4337 Module Deployment + Native Transfer<
      ✔ Safe with 4337 Module Deployment + Native Transfer
           Used 191691 gas (Account or Paymaster) for >Safe with 4337 Module Native Transfer<
           Used 182078 gas (Transaction) for >Safe with 4337 Module Native Transfer<
      ✔ Safe with 4337 Module Native Transfer
    Safe Deployment + Enabling 4337 Module + Token Operations
           Used 435081 gas (Account or Paymaster) for >Safe with 4337 Module Deployment + ERC20 Transfer<
           Used 426148 gas (Transaction) for >Safe with 4337 Module Deployment + ERC20 Transfer<
      ✔ Safe with 4337 Module Deployment + ERC20 Token Transfer
           Used 469349 gas (Account or Paymaster) for >Safe with 4337 Module Deployment + ERC721 Transfer<
           Used 467959 gas (Transaction) for >Safe with 4337 Module Deployment + ERC721 Transfer<
      ✔ Safe with 4337 Module Deployment + ERC721 Token Minting
    Token Operations Only
           Used 176295 gas (Account or Paymaster) for >Safe with 4337 Module ERC20 Transfer<
           Used 160584 gas (Transaction) for >Safe with 4337 Module ERC20 Transfer<
      ✔ Safe with 4337 Module ERC20 Token Transfer
           Used 211369 gas (Account or Paymaster) for >Safe with 4337 Module ERC721 Transfer<
           Used 202383 gas (Transaction) for >Safe with 4337 Module ERC721 Transfer<
      ✔ Safe with 4337 Module ERC721 Token Minting

I ran into some types issues on CI and just working on the issue. I'll add a GitHub comment to each one.

coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9583033693

Details

Totals Coverage Status
Change from base Build 9578863041: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9583999402

Details

Totals Coverage Status
Change from base Build 9578863041: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9616845351

Details

Totals Coverage Status
Change from base Build 9615652927: -37.3%
Covered Lines: 29
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9646722637

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9646759500

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9646992241

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9647336964

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9647423005

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9647468294

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9647573606

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9647650551

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9647772115

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9667332437

Details

Totals Coverage Status
Change from base Build 9615652927: 0.0%
Covered Lines: 39
Relevant Lines: 39
💛 - Coveralls