refactor: update ethers to v6.1 from v5.7 #670
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 313391d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
varunsrin
force-pushed
the
varunsrin/ethers-6.1
branch
from
fca54ac
to
feac708
Compare
varunsrin
commented
Mar 18, 2023
| // Remove right padding | ||
| let hex = value.toHexString(); | ||
| let hex = value.toString(16); |
There was a problem hiding this comment.
With BigInt, its possible that this value is an odd-length hex value. Is that going to create a problem here? Are there any additional test cases we should be adding?
cc @pfletcherhill and @deodad
varunsrin
force-pushed
the
varunsrin/ethers-6.1
branch
2 times, most recently
from
be945a3
to
5c18455
Compare
adityapk00
approved these changes
Mar 18, 2023
varunsrin
force-pushed
the
varunsrin/ethers-6.1
branch
from
5c18455
to
b12a28e
Compare
varunsrin
force-pushed
the
varunsrin/ethers-6.1
branch
from
b12a28e
to
313391d
Compare
2 tasks
vinliao
pushed a commit
to vinliao/hubble
that referenced
this pull request
Mar 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The downside is that we may be introducing some last minute bugs here which can go undetected, so I'd like to get some second opinions from folks working in the codebase.
Change Summary
The most significant changes being made are:
JS BigInt instead of Ethers.BigNum
BigInt seems like a drop in replacement except for the fact that serialization returns odd-length hex values like 0x3e8 instead of 0x03e8. In some cases this needs to be sanitized because parts of our codebase expect even length values.
Providers are very different under the hood
The internals of Providers have been refactored significantly. Providers now inherit from an AbstractProvider class and polling logic is handled explicitly by a Subscription. Since we used to reach into providers and modify polling behavior manually, this needs to be refactored carefully.
TypedDataSigner and verifyTypedData no longer exist
TypedDataSigner is superseded by the new Signer class and verifyTypedData no longer seems to exist for EIP712 verifications, which means we must perform signature recovery using more manual steps.
There are also some minor changes:
Wallet.createRandom()instead ofnew Wallet(utils.randomBytes(32))which has existed since v5, not sure if there's a reason we were doing this or just an omission.Merge Checklist