Skip to content

Conversation

infiniteflower
Copy link
Contributor

@infiniteflower infiniteflower commented Aug 23, 2024

This PR removes Web3 in favor of just using Ethers to read values off a contract.

With Web3 No Web3 Perf improvement
App Start 4920ms 658ms ~86% decrease
Logcat 6929ms ~2500ms ~63% decrease

App start and Logcat are 2 related but different measurements. Logcat is more closely related to visually looking at the app and seeing when it starts up.

Related issues:

  1. App Performance Start-Up: Swap Optimizations metamask-mobile#10611
  2. chore: Improve startup times on Android through Swaps changes metamask-mobile#10787

Manual testing steps

Refer to MetaMask/metamask-mobile#10787 for steps on how to test

nikoferro and others added 30 commits July 18, 2024 11:40
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Copy link

socket-security bot commented Aug 23, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: npm/ws@7.4.6

View full report↗︎

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

Base automatically changed from basecontrollerv2 to rc-v10 August 26, 2024 19:26
@infiniteflower
Copy link
Contributor Author

@SocketSecurity ignore npm/ws@7.4.6

@infiniteflower infiniteflower marked this pull request as ready for review August 26, 2024 20:21
@infiniteflower infiniteflower requested a review from a team August 26, 2024 20:21
Copy link
Contributor

@martahj martahj left a comment

Choose a reason for hiding this comment

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

LGTM! Was not able to test with Android device but tested for basic regressions on iOs as instructed

@infiniteflower infiniteflower merged commit d1e72ac into rc-v10 Aug 27, 2024
@infiniteflower infiniteflower deleted the fix/android-startup-times-remove-web3 branch August 27, 2024 13:48
martahj pushed a commit that referenced this pull request Aug 27, 2024
* chore: updating dependencies to match mobile current versions

* feat: basecontrollerv2 refactor

* fix: linter fix

* fix: test fix

* chore: exposing types of actions and events

* fix: removing provider from state since its not serializable

* fix: fix for resetting state while keeping the current chain and last fetched config

* chore: export controller types

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/constants.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/swapsUtil.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* chore: pr feedback changes

* chore: make explicit which properties are already anonymous

* chore: default value for state as an optional parameter

* chore: remove web3 package

* chore: add @ethersproject/contracts

* chore: add @ethersproject/providers

* chore: remove web3 and use ethers contracts instead

* fix: broken tests

---------

Co-authored-by: nikoferro <nicolaspatricioferro@gmail.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
infiniteflower added a commit that referenced this pull request Aug 28, 2024
* chore: updating dependencies to match mobile current versions (#271)

* chore: refactor SwapsController so it extends from BaseControllerV2 (#280)

* chore: updating dependencies to match mobile current versions

* feat: basecontrollerv2 refactor

* fix: linter fix

* fix: test fix

* chore: exposing types of actions and events

* fix: removing provider from state since its not serializable

* fix: fix for resetting state while keeping the current chain and last fetched config

* chore: export controller types

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/constants.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/swapsUtil.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* chore: pr feedback changes

* chore: make explicit which properties are already anonymous

* chore: default value for state as an optional parameter

* fix: remove any types and improve util types

* fix: removes type cast that is no longer needed

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

---------

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Marta Poling <marta.hourigan.johnson@gmail.com>

* Fix/android startup times remove web3 (#293)

* chore: updating dependencies to match mobile current versions

* feat: basecontrollerv2 refactor

* fix: linter fix

* fix: test fix

* chore: exposing types of actions and events

* fix: removing provider from state since its not serializable

* fix: fix for resetting state while keeping the current chain and last fetched config

* chore: export controller types

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/constants.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/swapsUtil.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/types.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* Update src/SwapsController.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* chore: pr feedback changes

* chore: make explicit which properties are already anonymous

* chore: default value for state as an optional parameter

* chore: remove web3 package

* chore: add @ethersproject/contracts

* chore: add @ethersproject/providers

* chore: remove web3 and use ethers contracts instead

* fix: broken tests

---------

Co-authored-by: nikoferro <nicolaspatricioferro@gmail.com>
Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

* chore: update yarn lock after fixing merge conflicts

* chore: update syntax for linter

* chore: pin version of ethersproject dependency ws due to high security vulnerability status

---------

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Marta Poling <marta.hourigan.johnson@gmail.com>
Co-authored-by: infiniteflower <139582705+infiniteflower@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants