Skip to content

[FIX] Patch assets-controller to not use BigInt#6305

Merged
Cal-L merged 5 commits into
release/6.5.0from
fix/913-big-int
May 1, 2023
Merged

[FIX] Patch assets-controller to not use BigInt#6305
Cal-L merged 5 commits into
release/6.5.0from
fix/913-big-int

Conversation

@Cal-L

@Cal-L Cal-L commented Apr 28, 2023 •
edited by sethkfman
Loading

Copy link
Copy Markdown
Contributor

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

This PR patches @metamask/assets-controller to comment out BigInt related code for now. We will reintroduce it once RN 0.71.6 upgrade is finished.

Screenshots/Recordings

bigintfix.mov

Issue

Description

Whenever I try to make an Approve tx, the app crashes with this errror Can't find variable: BigInt

[only-android]

After first reporting it here, we've seen that it's a separate issue, happening on the RC branch 6.4

Screenshots

bigint-module-not-found.mp4

Steps to reproduce the behavior

  1. Go to the test dapp
  2. Deploy the Collectibles contract
  3. Mint a token
  4. Click Approve
  5. MM crashes with the error Can't find variable: BigInt
  • Device: Pixel
  • OS: Android
  • App Version: v6.4.0

Issue: #6307

Progresses #???

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@Cal-L Cal-L requested a review from a team as a code owner April 28, 2023 22:50
@github-actions

github-actions Bot commented Apr 28, 2023

Copy link
Copy Markdown
Contributor

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.

@Cal-L Cal-L added release-6.5.0 Issue or pull request that will be included in release 6.5.0 type-bug Something isn't working needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) team-mobile-client labels Apr 28, 2023
@Cal-L Cal-L changed the title Patch assets-controller to not use BigInt [FIX] Patch assets-controller to not use BigInt Apr 28, 2023
legobeat
legobeat reviewed May 1, 2023
View reviewed changes
Comment thread patches/@metamask+assets-controllers+4.0.0.patch Outdated
legobeat
legobeat reviewed May 1, 2023
View reviewed changes
Comment thread patches/@metamask+assets-controllers+4.0.0.patch Outdated
legobeat
legobeat reviewed May 1, 2023
View reviewed changes
Comment thread patches/@metamask+assets-controllers+4.0.0.patch Outdated
sethkfman
sethkfman reviewed May 1, 2023
View reviewed changes
Comment thread patches/@metamask+assets-controllers+4.0.0.patch Outdated
Cal-L and others added 4 commits May 1, 2023 09:07
@Cal-L @sethkfman
Update to use public link
8dd4024 
Co-authored-by: sethkfman <10342624+sethkfman@users.noreply.github.com>
@Cal-L @legobeat
Update to spell out React Native
07e1163 
Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
@Cal-L @legobeat
Update to spell out React Native
232db42 
Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
@Cal-L
Merge branch 'release/6.5.0' of https://github.com/MetaMask/metamask-…
d90baeb 
…mobile into fix/913-big-int
sethkfman
sethkfman approved these changes May 1, 2023
View reviewed changes

@sethkfman sethkfman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Cal-L Cal-L added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels May 1, 2023
@Cal-L Cal-L merged commit be8b196 into release/6.5.0 May 1, 2023
@Cal-L Cal-L deleted the fix/913-big-int branch May 1, 2023 19:00
@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 2023
@Cal-L Cal-L restored the fix/913-big-int branch May 1, 2023 19:09
@chrisleewilcox chrisleewilcox added the QA Passed QA testing has been completed and passed label May 1, 2023
@gauthierpetetin gauthierpetetin added the team-mobile-platform Mobile Platform team label Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@legobeat legobeat legobeat left review comments

@sethkfman sethkfman sethkfman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

QA Passed QA testing has been completed and passed release-6.5.0 Issue or pull request that will be included in release 6.5.0 team-mobile-platform Mobile Platform team type-bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Cal-L @sethkfman @legobeat @chrisleewilcox @gauthierpetetin