Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from ethers v5 to ethers v6 #178

Merged
merged 35 commits into from
Feb 7, 2024
Merged

Migrate from ethers v5 to ethers v6 #178

merged 35 commits into from
Feb 7, 2024

Conversation

Siegrift
Copy link
Collaborator

@Siegrift Siegrift commented Jan 30, 2024

Closes #177

Rationale

I followed https://docs.ethers.org/v6/migrating/ and tried to advance by small steps (that's why so many commits). Usually going from file to file, understanding the TS issue and fixing it globally. There were a few gotchas worth mentioning:

  1. Multiple things are not mentioned in the migration guide
  2. Be extra careful when deriving sponsor wallet. Especially if these are ported to commons (maybe as part of Migrate to ethers@6 commons#29)
  3. Working with Hardhat is considerably slower. Seems like an internal change in the underlying provider implementation.
  4. 379e0d0 fixes a breaking change with how ethers "ArrayObjects" work. In ethers v5 when you fetched an event or called a function, you received back for example:
Promise<[BigNumber, number] & {
    value: BigNumber;
    timestamp: number;
}>

when you did Object.keys(suchResponse) you got back ['0', '1', 'value', 'timestamp']. In ethers v6, what you get is ['0', '1'] (the named keys are not own properties). This breaks utilities such as omit.

Quality assurance

Apart from fixing all of the tests, I ran Airseeker locally for a few hours and it updated the feeds correctly so I think all is fine. I

TODO

  • Write PR description
  • Fix TODOs in code
  • Consider using BigInt literal (e.g. 123n) instead of BigInt constructor
  • Fix CI test
  • Run e2e local test flow
  • Ensure that ethers.Wallet.fromPhrase derives the same wallet as ethers.Wallet.fromMnemonic in ethers v5
  • Fix tests

@Siegrift Siegrift self-assigned this Jan 30, 2024
@Siegrift Siegrift changed the title WIP: Migrate from ethers v5 to ethers v6 Migrate from ethers v5 to ethers v6 Jan 30, 2024
@Siegrift Siegrift requested a review from aquarat January 30, 2024 18:51
Copy link
Collaborator

@aquarat aquarat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 only a very very minor comment around BigInt literals.
This was good for me as I have a better idea now of the differences between ethers v5 and v6 🚀

src/deviation-check/deviation-check.ts Outdated Show resolved Hide resolved
@Siegrift Siegrift merged commit 2fd07c5 into main Feb 7, 2024
4 checks passed
@Siegrift Siegrift deleted the ether-v6 branch February 7, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to ethers-v6
3 participants