Inomurko/bump bundler by InoMurko · Pull Request #698 · bobanetwork/boba_legacy

InoMurko · 2023-04-10T10:59:14Z

This PR brings in 0.5.0 Bundler changes from upstream.
It also introduces a wrapper contract EntryPointWrapper for the purpose of EntryPoint interaction through custom errors that our l2geth currently does not support.

Overview

Re-enabled and fixed tests:
boba_aa_wallet.spec
boba_aa_sponsoring_fee.spec
boba_aa_fee_boba.spec
boba_aa_fee_alt_token.spec

Changes

Describe your changes and implementation choices. More details make PRs easier to review.

Renaming everything from Wallet to Account
I think GethTracer could be deleted, but post Bedrock it might be useful (or even with Tenderly now)
Brings in mempool support

Testing

Unit tests and integration tests are in the PR

wsdt · 2023-05-02T11:39:01Z

+        entryPoint = _entryPoint;
+    }
+
+    function simulateValidation(UserOperation calldata userOp) external returns (FailedOpStatus memory, Response memory) {


one thing I noticed when using this why we don't have the same return types for this and the actual entryPoint simulateValidation.

can you elaborate?

the return of entrypoint does not matter much, because all code paths use custom revert (ValidationResultWithAggregation, ValidationResult, FailedOp)

wsdt · 2023-05-02T11:45:49Z

+    }
+    // await EntryPoint__factory.connect(this.config.entryPoint,this.provider).callStatic.addStake(0)
+
+    // const err = await EntryPoint__factory.connect(


do we need this?

https://github.com/eth-infinitism/bundler/blob/v0.5.0/packages/bundler/src/BundlerServer.ts#L74-L79

we might want to support this via the Wrapper contract, currently we cannot run this!

https://github.com/bobanetwork/boba/issues/759

wsdt · 2023-05-02T11:46:40Z

        break
+      // custom errors post bedrock!
+      // case 'eth_estimateUserOperationGas':
+      //   result = await this.methodHandler.estimateUserOperationGas(


shouldn't we instead return an error that informs the user that the method exists but is not currently supported?

otherwise it's just the generic not supported/not exists error

i think this is the same endpoint you are working on? so we can add this back? "estimateUserOperationGas"

yeah, that can be fixed later on. for now I added a new error type and throw an error for not supported

souradeep-das

this is super, lets go 🚀

souradeep-das · 2023-05-02T12:00:35Z

+            docker-compose -f <<parameters.docker_compose_file>> -f docker-compose-side.yml up -d bobalink aa_deployer bundler
          working_directory: ops
      - run:
          name: Start background logging


L257 - lets uncomment test_flow_bundler_and_depcheck?

this actually does not exist in upstream anymore! will remove completely

souradeep-das · 2023-05-02T13:47:05Z

+  DeterministicDeployer,
+  HttpRpcClient,
+  SimpleAccountAPI,
+} from '../../../bundler_sdk/dist'


external dep (although isok, i dont think runop is used extensively)

resolved nevertheless!

souradeep-das · 2023-05-02T14:09:53Z


    const ctr = hexValue(new SampleRecipient__factory(ethers.provider.getSigner()).getDeployTransaction().data!)
-    DeterministicDeployer.init(ethers.provider, testWallet)
+    console.log(DeterministicDeployer.init(ethers.provider, testWallet))


maybe we want to remove log?
(here and L36)

souradeep-das · 2023-05-02T17:20:21Z

+      accountAPI = new SimpleAccountAPI({
+        provider: env.l2Provider,
+        entryPointAddress,
+        senderCreatorAddress: senderCreator.address,


minor: since we are passing accountAddress, do we need to pass senderCreator additionally?
( if we dont need it we could remove it from L141)
(applies to the other tests files)

souradeep-das · 2023-05-02T17:21:23Z

+
+    before('the user approves the paymaster to spend their $BOBA token', async () => {
+      // deploy a 4337 Wallet and send operation to this wallet
+      SimpleAccount__factory = new ContractFactory(


super minor: should we call it 'SimpleAccountFactory__factory'?

(applies to the other tests files)

souradeep-das · 2023-05-02T17:22:38Z

+      console.log(log.args.msgSender)
+      console.log(account)


unless this was added on purpose :D

souradeep-das · 2023-05-02T17:32:49Z

+        entryPointAddress,
+        senderCreatorAddress: senderCreator.address,
+        owner: env.l2Wallet,
+        factoryAddress: accountFactory.address,


lets remove factoryAddress and senderCreatorAddress here since we specify accountAddress?

souradeep-das · 2023-05-02T17:34:20Z

+      to: await op.sender,
+    })
+
+    //what is this supposed to be?


minor: a comment we can rem now 😄

souradeep-das · 2023-05-02T17:37:13Z

+      target: recipient.address,
+      data: recipient.interface.encodeFunctionData('something', ['hello']),
+    })
+    await env.l2Wallet.sendTransaction({


minor: we are sending eth two times? on L181 too?

* bump bundler, limit dependency bumps * uncomment bundler related stuff * build bundler in docker * build fixes * fix tests * fix bundler building * uncomment _disableInitializers * fix running DTL * v1.0.0 * fixing starting bundler and tests, default config * local unsafe, linting fix in intg tests * custom errors fixes * new api for simple account contract * use simple account factory proxy * use simple account factory proxy in SimpleAccountAPI * use simple account factory proxy in SimpleAccountAPI * use wrappers to get around custom errors * update entrypoint wrapper (#745) * sponsoring fee fixed * stricter validation for staking * remove debug namespace, fix return for unaavailable rpc methods * addressing Souradeeps comments --------- Co-authored-by: Souradeep Das <dsouradeep2@gmail.com> (cherry picked from commit 6368c72)

* Inomurko/bump bundler (#698) * bump bundler, limit dependency bumps * uncomment bundler related stuff * build bundler in docker * build fixes * fix tests * fix bundler building * uncomment _disableInitializers * fix running DTL * v1.0.0 * fixing starting bundler and tests, default config * local unsafe, linting fix in intg tests * custom errors fixes * new api for simple account contract * use simple account factory proxy * use simple account factory proxy in SimpleAccountAPI * use simple account factory proxy in SimpleAccountAPI * use wrappers to get around custom errors * update entrypoint wrapper (#745) * sponsoring fee fixed * stricter validation for staking * remove debug namespace, fix return for unaavailable rpc methods * addressing Souradeeps comments --------- Co-authored-by: Souradeep Das <dsouradeep2@gmail.com> (cherry picked from commit 6368c72) * fix: qsp30 (#773) fix: BOB1-30 (cherry picked from commit 7feda88) * run op fix (#771) (cherry picked from commit 176cd3c) * close-server (#768) (cherry picked from commit 72021af) * [AA]: fix inconsistent userOpHash (#757) * add token callback handler on SimpleAccount * fix: userOpHash packing * prevent recursive calls into handleOps * move nonce validation from individual Account to EntryPoint * add bundler changes for nonce change to EP (cherry picked from commit cc4e205) * ValidationManager account for signature expiration (#775) * resolve #753 * Update packages/boba/bundler/src/modules/ValidationManager.ts Co-authored-by: Ino Murko <ino.murko.github@protonmail.com> * fix bool * validAfter/Until integrationt tests, validAfter * cleanup * regex * integration_tests * integration & unit tests --------- Co-authored-by: Ino Murko <ino.murko.github@protonmail.com> (cherry picked from commit ef02bee) * npm release workflow for bundler-sdk (#749) (cherry picked from commit 718141f) * Fix/banxa and bridges (#772) * adding boba network * fixing boba bridge url * fixing bridge integration * replace code by selectors * remove hardcoded symbol * enable banxa only for mainnet * Available bridge inable only for mainnet * adding support for testnet * update conditional for other bridges * implemented the available bridges with typescript * unit test cases for available bridges * typo in Available bridges --------- Co-authored-by: alvaro-ricotta <alvaro.e.ricotta@gmail.com> Co-authored-by: alvaro-ricotta <81116391+alvaro-ricotta@users.noreply.github.com> Co-authored-by: Ino Murko <ino.murko.github@protonmail.com> (cherry picked from commit dcf9b7e) * add validation of entryPoint and wrapper (#779) * add validaiton of entryPoint and wrapper (cherry picked from commit 41f3150) --------- Co-authored-by: Souradeep Das <dsouradeep2@gmail.com> Co-authored-by: Riedl Kevin, Bsc <kevin.riedl@wavect.io> Co-authored-by: Sahil K <86316370+sk-enya@users.noreply.github.com>

github-actions bot added 2-reviewers M-integration M-ops labels Apr 10, 2023

InoMurko force-pushed the inomurko/bump-bundler-2 branch 5 times, most recently from de6bd09 to 0e4b621 Compare April 12, 2023 09:50

InoMurko changed the title ~~Inomurko/bump bundler 2~~ Inomurko/bump bundler Apr 20, 2023

InoMurko force-pushed the inomurko/bump-bundler-2 branch from 55249c7 to 654b5ab Compare April 24, 2023 18:46

InoMurko commented Apr 24, 2023

View reviewed changes

Comment thread integration-tests/test/eth-l2/boba_aa_wallet.spec.ts Outdated

InoMurko force-pushed the inomurko/bump-bundler-2 branch 3 times, most recently from 0c5fae4 to 4ec5d7f Compare April 25, 2023 19:40

This was referenced Apr 26, 2023

Document Bundler SDK for docs.boba.network #737

Closed

Document Bundler API for docs.boba.network #738

Closed

Publish Bundler SDK to NPM #739

Closed

InoMurko force-pushed the inomurko/bump-bundler-2 branch 2 times, most recently from d588069 to cf77369 Compare April 28, 2023 12:17

InoMurko marked this pull request as ready for review April 28, 2023 14:53

InoMurko requested review from boyuan-chen, souradeep-das and wsdt April 28, 2023 14:58

wsdt reviewed May 1, 2023

View reviewed changes

Comment thread packages/boba/bundler/contracts/tests/TestStorageAccount.sol

InoMurko requested a review from wsdt May 1, 2023 13:07

souradeep-das mentioned this pull request May 1, 2023

[AA]: fix inconsistent userOpHash #757

Merged

wsdt reviewed May 2, 2023

View reviewed changes

Comment thread packages/boba/bundler/src/GethTracer.ts

souradeep-das approved these changes May 2, 2023

View reviewed changes

InoMurko force-pushed the inomurko/bump-bundler-2 branch from 9c0ddd1 to ba17f52 Compare May 3, 2023 09:12

InoMurko and others added 21 commits May 3, 2023 11:12

bump bundler, limit dependency bumps

62b500d

uncomment bundler related stuff

9e290a2

build bundler in docker

b180c96

build fixes

5224948

fix tests

72ea874

fix bundler building

2be3b88

uncomment _disableInitializers

a0a834a

fix running DTL

8f10174

v1.0.0

67b3ea1

fixing starting bundler and tests, default config

f810f3e

local unsafe, linting fix in intg tests

68be7fb

custom errors fixes

8bff7fa

new api for simple account contract

4ffa17c

use simple account factory proxy

c772cd9

use simple account factory proxy in SimpleAccountAPI

1648055

use simple account factory proxy in SimpleAccountAPI

f2ec4fe

use wrappers to get around custom errors

6ae0ff1

update entrypoint wrapper (#745)

f2bcded

sponsoring fee fixed

3d4b3cb

stricter validation for staking

6f7b913

remove debug namespace, fix return for unaavailable rpc methods

2ae0df0

InoMurko force-pushed the inomurko/bump-bundler-2 branch from ba17f52 to 8794c88 Compare May 3, 2023 09:12

addressing Souradeeps comments

a589698

InoMurko force-pushed the inomurko/bump-bundler-2 branch from 8794c88 to a589698 Compare May 3, 2023 09:21

InoMurko merged commit 6368c72 into develop May 3, 2023

InoMurko deleted the inomurko/bump-bundler-2 branch May 3, 2023 09:58

Conversation

InoMurko commented Apr 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Changes

Testing

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

souradeep-das left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

InoMurko commented Apr 10, 2023 •

edited

Loading