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

feat: Update controller utils to 5.0.2 #9149

Merged
merged 16 commits into from
Apr 12, 2024

Conversation

tommasini
Copy link
Contributor

@tommasini tommasini commented Apr 5, 2024

Description

Updated the patch accordingly, and introduced networkIds list hardcoded for we keep the ens legacy functions working

  • The patch controller-utils would be able to be removed on v8 and v9 of controller-utils
  • Added a temporary resolution for the dependency of transaction controller controller utils be bumped to version 5 to keep linea sepolia transactions working. It will be removed when transaction controller be updated to v13
  • This PR needs assets controller PR v14 be merged to have auto nfts detection working

Related issues

Fixes: https://github.com/MetaMask/mobile-planning/issues/1622

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

ENS solving:

Screen.Recording.2024-04-11.at.11.15.53.mov

Sepolia linea network:

Uploading Screen Recording 2024-04-11 at 11.16.38.mov…

NFT auto detection working

Screen.Recording.2024-04-12.at.14.23.43.mov

Before

After

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've completed the PR template to the best of my ability
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). 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.

Copy link
Contributor

github-actions bot commented Apr 5, 2024

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.

@tommasini tommasini marked this pull request as ready for review April 11, 2024 10:23
@tommasini tommasini requested review from a team as code owners April 11, 2024 10:23
@tommasini tommasini added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) Run Smoke E2E Triggers smoke e2e on Bitrise labels Apr 11, 2024
Copy link
Contributor

github-actions bot commented Apr 11, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 570d896
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c2441e11-f9f7-4f50-a2dc-e1f77458c51e

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@tommasini tommasini added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Apr 11, 2024
Copy link
Contributor

github-actions bot commented Apr 11, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: f67d448
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/bc153ebb-a144-49e6-8378-e8d9b4b5a59f

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

tommasini added a commit that referenced this pull request Apr 12, 2024
## **Description**
This PR addresses the update to the Asset Controller v14

Created Core branch to address patch changes:
patch/mobile-assets-controllers-v-14-0-0

Fixed issue of token being automatically imported when requested by a
dapp.

Changed breaking changes on addToken and watchAsset function signature.

Important to note that this version of assets controller is pointing to
version 5 of controller utils, therefore the auto detect tokens will
only be available when this
[PR](#9149) is merged

## **Related issues**

Fixes: MetaMask/mobile-planning#1620

## **Manual testing steps**

1. imported tokens on mainnet
2. Imported tokens on custom network
3. Auto-detect tokens on mainnet & custom network
4. Import token via dapp
5.

## **Screenshots/Recordings**

Auto detect tokens, import token via dapp:


https://github.com/MetaMask/metamask-mobile/assets/46944231/daed21fe-1bbe-432e-9c1d-71e7ccc3baa9


Add custom token/ hide token:



https://github.com/MetaMask/metamask-mobile/assets/46944231/a1911455-136f-436b-924f-01f8d5bcd5a9




### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] 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**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.
@tommasini tommasini removed the Run Smoke E2E Triggers smoke e2e on Bitrise label Apr 12, 2024
@tommasini tommasini added the Run Smoke E2E Triggers smoke e2e on Bitrise label Apr 12, 2024
Copy link
Contributor

github-actions bot commented Apr 12, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 67e71fb
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/732d3a02-d010-407d-a2af-795e44ad9f10

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

tommasini added a commit that referenced this pull request Apr 12, 2024
## **Description**
This PR addresses the update to the Asset Controller v14

Created Core branch to address patch changes:
patch/mobile-assets-controllers-v-14-0-0

Fixed issue of token being automatically imported when requested by a
dapp.

Changed breaking changes on addToken and watchAsset function signature.

Important to note that this version of assets controller is pointing to
version 5 of controller utils, therefore the auto detect tokens will
only be available when this
[PR](#9149) is merged

## **Related issues**

Fixes: MetaMask/mobile-planning#1620

## **Manual testing steps**

1. imported tokens on mainnet
2. Imported tokens on custom network
3. Auto-detect tokens on mainnet & custom network
4. Import token via dapp
5.

## **Screenshots/Recordings**

Auto detect tokens, import token via dapp:


https://github.com/MetaMask/metamask-mobile/assets/46944231/daed21fe-1bbe-432e-9c1d-71e7ccc3baa9


Add custom token/ hide token:



https://github.com/MetaMask/metamask-mobile/assets/46944231/a1911455-136f-436b-924f-01f8d5bcd5a9




### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] 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**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.
Copy link

sonarcloud bot commented Apr 12, 2024

Copy link
Contributor

@MarioAslau MarioAslau left a comment

Choose a reason for hiding this comment

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

Looks great Tomas! Thank you for giving some context

@tommasini tommasini merged commit 5440541 into main Apr 12, 2024
33 checks passed
@tommasini tommasini deleted the feat/1622-controller-utils-v5-0-2 branch April 12, 2024 14:34
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Apr 12, 2024
@metamaskbot metamaskbot added the release-7.22.0 Issue or pull request that will be included in release 7.22.0 label Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-7.22.0 Issue or pull request that will be included in release 7.22.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-mobile-platform
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants