Skip to content

Conversation

@tommasini
Copy link
Contributor

@tommasini tommasini commented Apr 24, 2024

Description

  • Network Controller updated to v^17.2.0

  • Gas Fee Controller updated to v^13.0.0

  • Controller utils v^8

  • Assets Controllers v^26

  • Updated the patch of Network Controller, now it adds an export a type

  • Gas Fee Controller now uses NetworkController:networkDidChange event instead of NetworkController:stateChange event.

  • Removed resolution for transaction controller and replaced it with a small change via patch

  • Assets Controllers already with Reservoir changes of this PR

  • Assets Controllers patch cleaner and smaller

  • Token Detection Controller and Token Balances Controller updated on Engine and Engine Service since now they extend BaseController v2

  • Added to the patch of assets controllers work around for TokenBalancesController and TokenRatesController listening the TokensController events wrongly since TokensController is not extending BaseControllerV2

  • Added migration to reset contractBalances property of TokenBalancesController

  • Changed on Amount and Confirm screen where contractBalances were used as BN to be converted to BN since now contractBalances property is now saving balances on hexadecimal string

Created a new core branch to follow up the assets-controllers version: patch/mobile-assets-controllers-26

Related issues

Fixes:

Manual testing steps

  1. Sendflow Transaction On linea sepolia
  2. Test dapp transaction On sepolia
  3. Switch networks in general
  4. Add custom network
  5. add networks from popular list
  6. Remove network
  7. Add network throught dapp
  8. Add tokens with auto detect feature, with token list, custom token fields and via dapp
  9. Added nfts with auto detect NFTS, play around with display nft media and ipfs gateway toggles to check privacy is still working as expected. Also added nft with custom fields
  10. Switched accounts when on dapp and check if permission systems is working (it should be able to switch accounts when switching dapp that have different accounts connected)

Screenshots/Recordings

Adding a custom network:

Screen.Recording.2024-05-08.at.16.24.54.mov

Playing around with tokens & multiple networks (auto detect, add from dapp, custom add and add from token list):

Screen.Recording.2024-05-08.at.16.14.27.mov
Screen.Recording.2024-05-08.at.16.17.05.mov

Dapp sepolia transaction:

Screen.Recording.2024-05-08.at.16.19.51.mov

Linea sepolia transcation:

Screen.Recording.2024-05-08.at.09.41.44.mov

NFT & privacy playground:

Screen.Recording.2024-05-08.at.15.57.02.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.
  • - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ] - [ ]

@github-actions
Copy link
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.

@tommasini tommasini added the team-mobile-platform Mobile Platform team label Apr 24, 2024
…ntroller patch for 17.2 and dependency of network controller to 17.2
@tommasini tommasini added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Apr 24, 2024
… initialised correctly, patched change to token detection controller to get the state of tokens controller by property instead of the messenger system since tokens controller is not extending base controller v2, patch added to token balances controller to be able to reset the state
…ngine class and added the changes via patch, TokenBalancesController also needed a reset function for the resetState function of Engine class, also added updateNftMetadata via patch to NftController
… off and then turning it on it was not updating the nft image
@tommasini tommasini changed the title feat: Network Controller v^17 & Gas Fee Controller v^13 feat: Network & Gas & Assets & Utils controllers update May 8, 2024
@tommasini tommasini marked this pull request as ready for review May 8, 2024 16:41
@tommasini tommasini requested a review from a team as a code owner May 8, 2024 16:41
@tommasini tommasini requested a review from a team May 8, 2024 16:41
@tommasini tommasini requested a review from a team as a code owner May 8, 2024 16:41
@github-actions
Copy link
Contributor

github-actions bot commented May 15, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 3291839
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d89002f5-ee62-4e2e-9cfc-c66ad9dabdd5

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

Cal-L
Cal-L previously approved these changes May 15, 2024
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@socket-security
Copy link

socket-security bot commented May 16, 2024

@github-actions
Copy link
Contributor

github-actions bot commented May 16, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 4282014
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/5f69683d-70ab-417f-90cd-f3d880014161

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

@christopherferreira9
Copy link
Contributor

Looks good for SDK 👍

Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarqubecloud
Copy link

@github-actions
Copy link
Contributor

github-actions bot commented May 16, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 586e3a3
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/e8dd589c-bc62-4a9c-b25f-9b33005e0096

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 merged commit 953086c into main May 16, 2024
@tommasini tommasini deleted the update/network-controller-v17-gas-fee-controller-v13 branch May 16, 2024 17:21
@github-actions github-actions bot locked and limited conversation to collaborators May 16, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label May 16, 2024
@metamaskbot metamaskbot added the release-7.24.0 Issue or pull request that will be included in release 7.24.0 label May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.24.0 Issue or pull request that will be included in release 7.24.0 team-mobile-platform Mobile Platform team

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants