Skip to content

Commit

Permalink
Add a new logUserOperationGas function and improve user operation gas…
Browse files Browse the repository at this point in the history
… estimation (#446)

This PR fixes #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.
  • Loading branch information
mmv08 authored Jun 26, 2024
1 parent 1e57772 commit b4a0b60
Show file tree
Hide file tree
Showing 21 changed files with 1,897 additions and 1,073 deletions.
12 changes: 6 additions & 6 deletions examples/4337-passkeys/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,22 @@
"@account-abstraction/contracts": "0.7.0",
"@safe-global/safe-4337": "0.3.0",
"@safe-global/safe-contracts": "^1.4.1-build.0",
"@safe-global/safe-deployments": "^1.36.0",
"@safe-global/safe-deployments": "^1.37.0",
"@safe-global/safe-modules-deployments": "^2.2.0",
"@safe-global/safe-passkey": "0.2.0",
"@web3modal/ethers": "^4.1.11",
"ethers": "^6.12.1",
"ethers": "^6.13.1",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-router-dom": "^6.23.1"
},
"devDependencies": {
"@types/react": "^18.3.2",
"@types/react": "^18.3.3",
"@types/react-dom": "^18.3.0",
"@vitejs/plugin-react-swc": "^3.6.0",
"@vitejs/plugin-react-swc": "^3.7.0",
"react-router": "^6.23.1",
"typescript": "^5.4.5",
"vite": "^5.2.11",
"typescript": "^5.5.2",
"vite": "^5.3.1",
"vite-plugin-commonjs": "^0.10.1"
}
}
27 changes: 27 additions & 0 deletions examples/4337-passkeys/tsconfig.app.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"compilerOptions": {
"composite": true,
"tsBuildInfoFile": "./node_modules/.tmp/tsconfig.app.tsbuildinfo",
"target": "ES2020",
"useDefineForClassFields": true,
"lib": ["ES2020", "DOM", "DOM.Iterable"],
"module": "ESNext",
"skipLibCheck": true,

/* Bundler mode */
"moduleResolution": "bundler",
"allowImportingTsExtensions": true,
"resolveJsonModule": true,
"isolatedModules": true,
"moduleDetection": "force",
"noEmit": true,
"jsx": "react-jsx",

/* Linting */
"strict": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noFallthroughCasesInSwitch": true
},
"include": ["src"]
}
32 changes: 9 additions & 23 deletions examples/4337-passkeys/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,25 +1,11 @@
{
"compilerOptions": {
"target": "ES2020",
"useDefineForClassFields": true,
"lib": ["ES2020", "DOM", "DOM.Iterable"],
"module": "ESNext",
"skipLibCheck": true,

/* Bundler mode */
"moduleResolution": "bundler",
"allowImportingTsExtensions": true,
"resolveJsonModule": true,
"isolatedModules": true,
"noEmit": true,
"jsx": "react-jsx",

/* Linting */
"strict": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noFallthroughCasesInSwitch": true
},
"include": ["src"],
"references": [{ "path": "./tsconfig.node.json" }]
"files": [],
"references": [
{
"path": "./tsconfig.app.json"
},
{
"path": "./tsconfig.node.json"
}
]
}
5 changes: 4 additions & 1 deletion examples/4337-passkeys/tsconfig.node.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
{
"compilerOptions": {
"composite": true,
"tsBuildInfoFile": "./node_modules/.tmp/tsconfig.node.tsbuildinfo",
"skipLibCheck": true,
"module": "ESNext",
"moduleResolution": "bundler",
"allowSyntheticDefaultImports": true
"allowSyntheticDefaultImports": true,
"strict": true,
"noEmit": true
},
"include": ["vite.config.ts"]
}
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/Safe4337Module.conf
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
"verify": "Safe4337Module:certora/specs/Safe4337Module.spec",
"packages": [
"@account-abstraction=../../node_modules/.pnpm/@account-abstraction+contracts@0.7.0/node_modules/@account-abstraction",
"@safe-global=../../node_modules/.pnpm/@safe-global+safe-contracts@1.4.1-build.0_ethers@6.12.1_bufferutil@4.0.8_utf-8-validate@5.0.10_/node_modules/@safe-global"
"@safe-global=../../node_modules/.pnpm/@safe-global+safe-contracts@1.4.1-build.0_ethers@6.13.1_bufferutil@4.0.8_utf-8-validate@5.0.10_/node_modules/@safe-global"
]
}
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/TransactionExecutionMethods.conf
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
"verify": "Safe4337Module:certora/specs/TransactionExecutionMethods.spec",
"packages": [
"@account-abstraction=../../node_modules/.pnpm/@account-abstraction+contracts@0.7.0/node_modules/@account-abstraction",
"@safe-global=../../node_modules/.pnpm/@safe-global+safe-contracts@1.4.1-build.0_ethers@6.12.1_bufferutil@4.0.8_utf-8-validate@5.0.10_/node_modules/@safe-global"
"@safe-global=../../node_modules/.pnpm/@safe-global+safe-contracts@1.4.1-build.0_ethers@6.13.1_bufferutil@4.0.8_utf-8-validate@5.0.10_/node_modules/@safe-global"
]
}
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/ValidationDataLastBitOne.conf
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
"verify": "Safe4337Module:certora/specs/ValidationDataLastBitOne.spec",
"packages": [
"@account-abstraction=../../node_modules/.pnpm/@account-abstraction+contracts@0.7.0/node_modules/@account-abstraction",
"@safe-global=../../node_modules/.pnpm/@safe-global+safe-contracts@1.4.1-build.0_ethers@6.12.1_bufferutil@4.0.8_utf-8-validate@5.0.10_/node_modules/@safe-global"
"@safe-global=../../node_modules/.pnpm/@safe-global+safe-contracts@1.4.1-build.0_ethers@6.13.1_bufferutil@4.0.8_utf-8-validate@5.0.10_/node_modules/@safe-global"
]
}
11 changes: 9 additions & 2 deletions modules/4337/contracts/test/SafeMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,15 @@ contract Safe4337Mock is SafeMock, IAccount {
*/
function _validateSignatures(PackedUserOperation calldata userOp) internal view returns (uint256 validationData) {
(bytes memory operationData, uint48 validAfter, uint48 validUntil, bytes calldata signatures) = _getSafeOp(userOp);
checkSignatures(keccak256(operationData), operationData, signatures);
validationData = _packValidationData(false, validUntil, validAfter);

bytes32 dataHash = keccak256(operationData);
uint8 v;
bytes32 r;
bytes32 s;
(v, r, s) = _signatureSplit(signatures);
bool validSignature = owner == ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);

validationData = _packValidationData(!validSignature, validUntil, validAfter);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ import "@safe-global/safe-contracts/contracts/libraries/MultiSend.sol";
import "@safe-global/safe-contracts/contracts/libraries/SafeStorage.sol";
import "@safe-global/safe-contracts/contracts/proxies/SafeProxyFactory.sol";
import "@safe-global/safe-contracts/contracts/SafeL2.sol";
// Named import for EntryPointSimulations needed because it also defines an interface for IERC165, which
// conflicts with the same interface defined in the Safe contracts.
/* solhint-disable-next-line no-unused-import */
import {EntryPointSimulations} from "@account-abstraction/contracts/core/EntryPointSimulations.sol";
14 changes: 7 additions & 7 deletions modules/4337/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,27 +50,27 @@
"@account-abstraction/contracts": "^0.7.0",
"@noble/curves": "^1.4.0",
"@nomicfoundation/hardhat-ethers": "^3.0.6",
"@nomicfoundation/hardhat-network-helpers": "^1.0.10",
"@nomicfoundation/hardhat-network-helpers": "^1.0.11",
"@nomicfoundation/hardhat-toolbox": "^5.0.0",
"@openzeppelin/contracts": "^5.0.2",
"@safe-global/safe-4337-local-bundler": "workspace:^0.0.0",
"@safe-global/safe-4337-provider": "workspace:^0.0.0",
"@simplewebauthn/server": "10.0.0",
"@types/chai": "^4.3.16",
"@types/mocha": "^10.0.6",
"@types/node": "^20.14.0",
"@types/mocha": "^10.0.7",
"@types/node": "^20.14.8",
"@types/yargs": "^17.0.32",
"cbor": "^9.0.2",
"debug": "^4.3.4",
"debug": "^4.3.5",
"dotenv": "^16.4.5",
"ethers": "^6.12.1",
"hardhat": "^2.22.3",
"ethers": "^6.13.1",
"hardhat": "^2.22.5",
"hardhat-deploy": "^0.12.4",
"husky": "^9.0.11",
"solc": "^0.8.25",
"solhint": "^5.0.1",
"ts-node": "^10.9.2",
"typescript": "^5.4.5",
"typescript": "^5.5.2",
"yargs": "^17.7.2"
},
"dependencies": {
Expand Down
19 changes: 19 additions & 0 deletions modules/4337/src/deploy/entrypointHelpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { DeployFunction } from 'hardhat-deploy/types'

const deploy: DeployFunction = async ({ deployments, getNamedAccounts, network }) => {
if (!network.tags.test) {
return
}

const { deployer } = await getNamedAccounts()
const { deploy } = deployments

await deploy('EntryPointSimulations', {
from: deployer,
args: [],
log: true,
deterministicDeployment: true,
})
}

export default deploy
48 changes: 48 additions & 0 deletions modules/4337/src/utils/execution.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { BigNumberish, Signer, TransactionResponse, ethers } from 'ethers'
import { IEntryPoint } from '../../typechain-types'

export const EIP_DOMAIN = {
EIP712Domain: [
Expand Down Expand Up @@ -137,9 +138,56 @@ export const buildSignatureBytes = (signatures: SafeSignature[]): string => {
export const logGas = async (message: string, tx: Promise<TransactionResponse>, skip?: boolean): Promise<TransactionResponse> => {
return tx.then(async (result) => {
const receipt = await result.wait()

if (!receipt?.gasUsed) throw new Error('No gas used in receipt')

if (!skip) console.log(` Used ${receipt.gasUsed} gas for >${message}<`)
return result
})
}

type UserOperationGasLog = {
actualGasUsed: bigint
actualGasCost: bigint
transactionResponse: TransactionResponse
}

/**
* Logs the gas used by a user operation and returns the gas log.
*
* @param message - The message associated with the user operation.
* @param entryPoint - The entry point object.
* @param tx - The transaction promise.
* @param skip - Optional flag to skip logging.
* @returns {UserOperationGasLog} A promise that resolves to the user operation gas log.
* @throws An error if the receipt is not available, gas used is not available in the receipt,
* gas used or gas cost is not available in the UserOperationEvent, or UserOperationEvent is not emitted.
*/
export const logUserOperationGas = async (
message: string,
entryPoint: IEntryPoint,
tx: Promise<TransactionResponse>,
skip?: boolean,
): Promise<UserOperationGasLog> => {
return tx.then(async (transactionResponse) => {
const receipt = await transactionResponse.wait()
if (!receipt) throw new Error('No receipt')

const userOperationEvent = await entryPoint.queryFilter(entryPoint.filters.UserOperationEvent(), receipt.blockNumber)
const parsedUserOperationEvent = entryPoint.interface.parseLog(userOperationEvent[0])

if (!receipt?.gasUsed) throw new Error('No gas used in receipt')
if (!parsedUserOperationEvent?.args.actualGasUsed || !parsedUserOperationEvent?.args.actualGasCost)
throw new Error('No gas used or gas cost in UserOperationEvent or UserOperationEvent not emitted')

if (!skip) {
console.log(` Used ${parsedUserOperationEvent.args.actualGasUsed} gas (Account or Paymaster) for >${message}<`)
console.log(` Used ${receipt.gasUsed} gas (Transaction) for >${message}<`)
}
return {
actualGasUsed: parsedUserOperationEvent.args.actualGasUsed,
actualGasCost: parsedUserOperationEvent.args.actualGasCost,
transactionResponse,
}
})
}
8 changes: 5 additions & 3 deletions modules/4337/src/utils/userOp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ export { PackedUserOperation, UserOperation }
type OptionalExceptFor<T, TRequired extends keyof T = keyof T> = Partial<Pick<T, Exclude<keyof T, TRequired>>> &
Required<Pick<T, TRequired>>

export const PLACEHOLDER_SIGNATURE =
'0x9c8ecb7ad80d2dd4411c8827079cda17095236ee3cba1c9b81153d52af17bc9d0701228dc95a75136a3e3a0130988ba4053cc15d3805db49e2cc08d9c99562191b'

export type SafeUserOperation = {
safe: string
entryPoint: string
Expand Down Expand Up @@ -172,11 +175,10 @@ export const getRequiredGas = (userOp: PackedUserOperation): string => {
return (BigInt(callGasLimit) + BigInt(verificationGasLimit) * multiplier + BigInt(userOp.preVerificationGas)).toString()
}

export const getRequiredPrefund = (userOp: PackedUserOperation): string => {
export const getRequiredPrefund = (userOp: PackedUserOperation): bigint => {
const requiredGas = getRequiredGas(userOp)
const { maxFeePerGas } = unpackGasParameters(userOp)
const requiredPrefund = (BigInt(requiredGas) * BigInt(maxFeePerGas)).toString()
console.log({ requiredGas, requiredPrefund })
const requiredPrefund = BigInt(requiredGas) * BigInt(maxFeePerGas)

return requiredPrefund
}
Expand Down
Loading

0 comments on commit b4a0b60

Please sign in to comment.