Migrate from ethers v5 to v6

Stanback commented 1 year ago

Description:

This pull request addresses the migration from ethers.js v5 to v6. The changes include updates to dependencies, code adjustments, and modifications to the demos to ensure compatibility with the new version.

For more information, see Migrating from v5

Changes:

Update package.json to use ethers v6
Modify the codebase to utilize the updated ethers API
- One thing to note is that it now extends AbstractProvider instead of JsonRpcProvider, this is because some of the properties (e.g. nextId) are now private, and it didn't seem necessary or beneficial to extend the RPC provider
Update the demos to match the new ethers API and fix any breaking changes
Update the documentation to reflect the new ethers version and API changes

Testing:

Both demos have been updated. Additionally, manual testing was conducted to ensure that it's functioning as expected with no regressions.

Please review the changes and provide feedback or suggestions if necessary. It's in need of a more thorough review, testing and discussion before it should be merged. It might be best to keep this in a parallel branch / release for awhile. I'm happy to make any additional updates or modifications to ensure a smooth migration.

Thank you for considering this pull request.

EdoAPP commented 1 year ago

Any updates on this? Why hasn't it been merged?

Stanback commented 11 months ago

Bump.

Should we merge this? Let me know if there's anything more I can do to support!

Ranguna commented 10 months ago

Seems like new changes have been added, which include usages of ethers v5 BigNumber class, see here for more info: https://github.com/Stanback/ethers-provider-flashbots-bundle/compare/feature/ethers6...flashbots:ethers-provider-flashbots-bundle:master

Although I do wonder how productive it is to keep patching new changes if there are no maintainers around to merge this PR sooner or later :(

We might as well just publish the changes in this PR to npm and abandon the original package.

Stanback commented 10 months ago

Thanks for the code review @Ranguna! 🙏 I'll go ahead and make those changes and publish a v6 version to npm for folks to use in the interim.

Ranguna commented 10 months ago

Man, you are a lifesaver!

Although I ended up moving to the mev share client for now, I'm sure there are many other people whole appreciate the v6 version of this package!

Stanback commented 10 months ago

Man, you are a lifesaver!

Although I ended up moving to the mev share client for now, I'm sure there are many other people whole appreciate the v6 version of this package!

You're welcome!

I've rebased all the changes from master that happened since my fork and bumped up a few dependencies. The demo worked for me, I'll conduct some more testing and fix anything that arises.

I published an npm package as well, flashbots-ethers-v6-provider-bundle (https://www.npmjs.com/package/flashbots-ethers-v6-provider-bundle). I'll consider more formally forking this project if we continue to get silence from the flashbots maintainers.

That said, it sounds like maybe we should concentrate our efforts on mev-share-client-ts. It looks pretty good - did you experience any issues or limitations while switching or was it smooth sailing?

Ranguna commented 10 months ago

Thanks for the effort @Stanback !

So far so good, although I haven't tested any frontrun strategies. The examples from mev share only include frontrunning (which makes sense, since it's the whole point of these private mempools).

But will see if I can test backruns soon.

EDIT: ended up giving up temporarily on the mev share client because it's lacking proper simulation support for public mempool transactions.

gholam-deblan commented 10 months ago

Thanks for preparing the migration that is much needed but given the lack of reaction from the owners, I will not even bother testing this library and will look into mev-share-client directly, for front-running though.

ape-dev-cs commented 9 months ago

Is there anything that's currently blocking this PR from being merged?

slavivanov commented 9 months ago

I'm not a contributor to the library, but reviewed the code and it looks good to me. Hopefully, someone from the maintainers can review it.

BlankerL commented 9 months ago

Seems @Stanback did a bunch of amazing commits. Code looks pretty neat to me, I think they should be merged and Ethers v6 is inevitable.

AmazingAng commented 7 months ago

Let it Merge

metachris commented 7 months ago

Quick update -- this PR is on our radar and a v6 update is being prepared now.

AmazingAng commented 7 months ago

Thx!

Nenad7955 commented 7 months ago

what is the ETA on this merge?

pre-vail commented 7 months ago

i would have posted this as an issue in flashbots-ethers-v6-provider-bundle but it looks like issues have not been enabled

i get an error when running src/demo.ts. first, i had to make a few changes to the script in order to make it work with ethers v6 but they involved migrating from BigNumber to BigInt so i know my changes are not the reason for the error

when i test the demo on my local (foundry) anvil fork using an alchemy rpc i get this:

userStats { error: { message: 'rpc method is not whitelisted', code: -32001 } }
userStatsV2 { error: { message: 'rpc method is not whitelisted', code: -32001 } }
Simulation Error: rpc method is not whitelisted

when i test the demo on sepolia i get:

userStats {
  error: {
    message: 'the method flashbots_getUserStats does not exist/is not available',
    code: -32601
  }
}
userStatsV2 {
  error: {
    message: 'the method flashbots_getUserStatsV2 does not exist/is not available',
    code: -32601
  }
}
Simulation Error: err: %!w(<nil>); txhash 0x2b59945369fae8e0a8dc412f7534db001cc97a6ea55a19fa43d5c695b4f4554e

fyi, i'm console logging the userStats object at line 43 and i have process.env.TEST_V2 set to true, thus the logging of both userStats and userStatsV2

what am i doing wrong?

epheph commented 7 months ago

@pre-vail you are likely using the "protect" url (rpc.flashbots.net) instead of the flashbots relay url (relay.flashbots.net)

epheph commented 7 months ago

lgtm, i'll merge it. Question though, what do you think of separating it by major version number, so v0.x.x - ethers5 v1.0.0 - ethers6 Seems like a reasonable way to separate, and we can maintain the v0 branch as well?

AmazingAng commented 7 months ago

Yes,

lgtm, i'll merge it. Question though, what do you think of separating it by major version number, so v0.x.x - ethers5 v1.0.0 - ethers6 Seems like a reasonable way to separate, and we can maintain the v0 branch as well?

lgtm

flashbots / ethers-provider-flashbots-bundle