-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix(deps): ethereumjs-util@6->^7.1.5 #11932
base: main
Are you sure you want to change the base?
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/bindings@1.5.0, npm/bip66@1.1.5, npm/drbg.js@1.0.1, npm/ethereumjs-abi@0.6.6, npm/file-uri-to-path@1.0.0 |
This comment was marked as outdated.
This comment was marked as outdated.
app/store/migrations/029.ts
Outdated
@@ -466,7 +465,7 @@ export default async function migrate(stateAsync: unknown) { | |||
if (Array.isArray(transactionControllerState.transactions)) { | |||
transactionControllerState.transactions.forEach( | |||
(transaction: TransactionParams, index: number) => { | |||
if (transaction && !isHexString(transaction.chainId)) { | |||
if (transaction && !transaction.chainId || (transaction.chainId && !isHexString(transaction.chainId))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not change the resulting logic and is just validating transaction.chainId
to account for that isHexString
does not accept arbitrary input.
I am not sure if it is correct and intended that this should evaluate to true
when transaction.chainId
is undefined
(or otherwise not sane to pass to toHex
), or if explicit handling for that should actually be added.
5afd1f3
to
c1fe27f
Compare
281315e
to
0688d0b
Compare
## **Description** - fix: migrate from legacy `ethjs-` packages to maintained and already used `@metamask/ethjs-` forks - Removes dependency on deprecated `babel-runtime` v6 - Bundle size reduction ## **Related issues** - #11952 - #11932 #### Blocking - #11968 ## **Manual testing steps** ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
754bf84
to
df69679
Compare
Bitrise✅✅✅ Commit hash: df69679 Note
|
df69679
to
5ff36d8
Compare
Bitrise✅✅✅ Commit hash: eb0c3e4 Note
|
eb0c3e4
to
10bcec0
Compare
10bcec0
to
8f84b8d
Compare
Bitrise✅✅✅ Commit hash: 8f84b8d Note
|
0299a2e
to
7818d3f
Compare
- to get correct types - to prepare upgrading to versions which don't provide the re-export - fix: vendor BNToHex and hexToBN in order to use bn.js v4 - fix: explicitly use either bn.js@4 and bn.js@5 - Why mixing v4 and v5 is A Bad Thing: ethereumjs/ethereumjs-util#250
7818d3f
to
7e3ed75
Compare
Quality Gate passedIssues Measures |
Description
Bump deprecated dependencies to (still deprecated but) lated versions with various fixes and improvements:
ethereumjs-util
from6.1.0
to^7.1.5
Related issues
.toString(16)
indutny/bn.js#295Based on
Blocking
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist