Skip to content

Commit

Permalink
bugfix: estimateUserOperationGas gas values are overridden by the def…
Browse files Browse the repository at this point in the history
…ault values (eth-infinitism#119)

default values are for missing values, and should not override user-specified values.
(reported by: https://t.me/sherifabdelmoatty)

* update node version
  • Loading branch information
drortirosh authored Aug 3, 2023
1 parent 404002e commit d935f26
Show file tree
Hide file tree
Showing 5 changed files with 1,179 additions and 939 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
steps:
- uses: actions/setup-node@v1
with:
node-version: '14'
node-version: '18'
- uses: actions/checkout@v1
- uses: actions/cache@v2
with:
Expand All @@ -33,7 +33,7 @@ jobs:
steps:
- uses: actions/setup-node@v1
with:
node-version: '14'
node-version: '18'
- uses: actions/checkout@v1
- uses: actions/cache@v2
with:
Expand Down
2 changes: 1 addition & 1 deletion packages/bundler/localconfig/bundler.config.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"gasFactor": "1",
"port": "3000",
"network": "http://localhost:8545",
"network": "http://127.0.0.1:8545",
"entryPoint": "0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789",
"beneficiary": "0xd21934eD8eAf27a67f0A70042Af50A1D6d195E81",
"minBalance": "1",
Expand Down
6 changes: 3 additions & 3 deletions packages/bundler/src/UserOpMethodHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,18 @@ export class UserOpMethodHandler {

/**
* eth_estimateUserOperationGas RPC api.
* @param userOp1
* @param userOp1 input userOp (may have gas fields missing, so they can be estimated)
* @param entryPointInput
*/
async estimateUserOperationGas (userOp1: UserOperationStruct, entryPointInput: string): Promise<EstimateUserOpGasResult> {
const userOp = {
...await resolveProperties(userOp1),
// default values for missing fields.
paymasterAndData: '0x',
maxFeePerGas: 0,
maxPriorityFeePerGas: 0,
preVerificationGas: 0,
verificationGasLimit: 10e6
verificationGasLimit: 10e6,
...await resolveProperties(userOp1) as any
}

// todo: checks the existence of parameters, but since we hexlify the inputs, it fails to validate
Expand Down
10 changes: 9 additions & 1 deletion packages/bundler/test/UserOpMethodHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,19 @@ describe('UserOpMethodHandler', function () {
})
})
it('estimateUserOperationGas should estimate even without eth', async () => {
// fail without gas
const op = await smartAccountAPI.createSignedUserOp({
target,
data: '0xdeadface'
})
const ret = await methodHandler.estimateUserOperationGas(await resolveHexlify(op), entryPoint.address)
expect(await methodHandler.estimateUserOperationGas(await resolveHexlify(op), entryPoint.address).catch(e => e.message)).to.eql('AA21 didn\'t pay prefund')
// should estimate with gasprice=0
const op1 = await smartAccountAPI.createSignedUserOp({
maxFeePerGas: 0,
target,
data: '0xdeadface'
})
const ret = await methodHandler.estimateUserOperationGas(await resolveHexlify(op1), entryPoint.address)
// verification gas should be high - it creates this wallet
expect(ret.verificationGasLimit).to.be.closeTo(300000, 100000)
// execution should be quite low.
Expand Down
Loading

0 comments on commit d935f26

Please sign in to comment.