Support watching assets on a specific account #1124

Cal-L · 2023-03-08T06:58:34Z

Support watching assets on a specific account

The purpose of the changes to is allow a user to store a watched asset to the token states in the TokensController. Why is this needed? The introduction of the Permission System on mobile now allows users to interact with Dapps using accounts that are different than what is active on their wallet screen. This dynamic makes it possible for users to watch assets under the active Dapp account, which may be different than the active wallet account. As a result, we needed to enable passing of an account address to both the watchAsset and addToken method, that would allow for storing of tokens under specific account addresses. All of the listed changes below is backwards compatible and code that uses the TokensController will not need to change.

Description

CHANGED:
- TokensController.watchAsset
  - Added optional interactingAddress: string parameter with the definition of The address of the account that is requesting to watch the asset.
- TokensController.acceptWatchAsset
  - Now passes an account address (interactingAddress: string || selectedAddress: string) into TokensController.addToken under the ERC20 switch condition.
- TokensController.addToken
  - If interactingAddress is defined and is not equal to selectedAddress, this method will function the same way but will not update tokens, ignoredTokens, detectedTokens token states.
  - If interactingAddress is not defined, this method will use the selectedAddress and function the same way.
- SuggestedAssetMeta type
  - Added optional interactingAddress?: string property. This is only populated if interactingAddress is passed into TokensController.watchAsset.

Checklist

Tests are included if applicable
Any added code is fully documented

Issue

Resolves #???

Cal-L · 2023-03-15T20:40:20Z

packages/assets-controllers/src/TokensController.ts

      asset,
      id: this._generateRandomId(),
      status: SuggestedAssetStatus.pending as SuggestedAssetStatus.pending,
      time: Date.now(),
      type,
    };
+    if (interactingAddress) {


TODO - Provide selectedAddress as fallback to ensure that suggestedAssetMeta always has an interactingAddress

tommasini

LGTM!

Gudahtt · 2023-03-23T20:26:20Z

Hmm, I hadn't realized before that EIP-747 relied upon the current selected account. I guess we'll need to update https://eips.ethereum.org/EIPS/eip-747 to work more effectively with scenarios where multiple accounts are connected.

adonesky1 · 2023-03-23T20:29:27Z

Hmm, I hadn't realized before that EIP-747 relied upon the current selected account. I guess we'll need to update https://eips.ethereum.org/EIPS/eip-747 to work more effectively with scenarios where multiple accounts are connected.

we're discussing related topics in context of adding support for adding NFTs via wallet_watchAsset and architecting changes with multichain in mind. @Cal-L perhaps you should join us to discuss this work in our meeting tomorrow?

packages/assets-controllers/src/TokensController.ts

Gudahtt

LGTM! Left one suggestion for improved test coverage, and Cal your pending TODO comment looks worth pursing as well, but we don't need to block on that.

Cal-L · 2023-03-29T17:23:33Z

@metamaskbot publish-preview

github-actions · 2023-03-29T17:26:29Z

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.

{
  "@metamask/address-book-controller": "2.0.0-preview.cb2d3d9",
  "@metamask/announcement-controller": "3.0.0-preview.cb2d3d9",
  "@metamask/approval-controller": "2.0.0-preview.cb2d3d9",
  "@metamask/assets-controllers": "5.0.0-preview.cb2d3d9",
  "@metamask/base-controller": "2.0.0-preview.cb2d3d9",
  "@metamask/composable-controller": "2.0.0-preview.cb2d3d9",
  "@metamask/controller-utils": "3.0.0-preview.cb2d3d9",
  "@metamask/ens-controller": "2.0.0-preview.cb2d3d9",
  "@metamask/gas-fee-controller": "4.0.0-preview.cb2d3d9",
  "@metamask/keyring-controller": "4.0.0-preview.cb2d3d9",
  "@metamask/message-manager": "2.0.0-preview.cb2d3d9",
  "@metamask/network-controller": "5.0.0-preview.cb2d3d9",
  "@metamask/notification-controller": "2.0.0-preview.cb2d3d9",
  "@metamask/permission-controller": "3.0.0-preview.cb2d3d9",
  "@metamask/phishing-controller": "3.0.0-preview.cb2d3d9",
  "@metamask/preferences-controller": "2.1.0-preview.cb2d3d9",
  "@metamask/rate-limit-controller": "2.0.0-preview.cb2d3d9",
  "@metamask/subject-metadata-controller": "2.0.0-preview.cb2d3d9",
  "@metamask/transaction-controller": "4.0.0-preview.cb2d3d9"
}

Gudahtt

LGTM!

The `@metamask/assets-controllers` patch added as part of the permission system implementation [1] has been updated to more closely match how the feature was implemented upstream [2]. It should be functionally equivalent. This relates to MetaMask/mobile-planning#877 [1]: #5062 [2]: MetaMask/core#1124

* Add interactingAddress property to SuggestedAssetMetaBase type * Allow passing an account address to watchAsset * Pass interacting account address into addToken from acceptWatchAsset * Support adding token to an account that is not the active wallet account * Rename detection fields to interacting to support more cases * Create test for watching an asset under a specific account address * Create test for storing a watched asset under a specific account address * Use selectedAddress as fallback for interacting address * Fix TokensController tests

Cal-L changed the title ~~Patch/5773 improve watch asset~~ Support watching assets on specific accounts Mar 8, 2023

Cal-L changed the title ~~Support watching assets on specific accounts~~ Support watching assets on a specific account Mar 8, 2023

Cal-L marked this pull request as ready for review March 9, 2023 03:23

Cal-L requested a review from a team as a code owner March 9, 2023 03:23

Cal-L commented Mar 15, 2023

View reviewed changes

tommasini previously approved these changes Mar 22, 2023

View reviewed changes

Gudahtt reviewed Mar 23, 2023

View reviewed changes

packages/assets-controllers/src/TokensController.ts Show resolved Hide resolved

Gudahtt previously approved these changes Mar 23, 2023

View reviewed changes

Cal-L dismissed stale reviews from Gudahtt and tommasini via c38d0ec March 28, 2023 23:34

Cal-L force-pushed the patch/5773-improve-watch-asset branch from c38d0ec to cb2d3d9 Compare March 28, 2023 23:44

Cal-L marked this pull request as draft March 29, 2023 17:23

Cal-L marked this pull request as ready for review March 29, 2023 20:40

Cal-L added 8 commits March 29, 2023 21:56

Add interactingAddress property to SuggestedAssetMetaBase type

5a8cbdc

Allow passing an account address to watchAsset

f1a10a3

Pass interacting account address into addToken from acceptWatchAsset

07bb58d

Support adding token to an account that is not the active wallet account

c408664

Rename detection fields to interacting to support more cases

3bc110e

Create test for watching an asset under a specific account address

378056c

Create test for storing a watched asset under a specific account address

2a39df1

Use selectedAddress as fallback for interacting address

eb5cab8

Cal-L force-pushed the patch/5773-improve-watch-asset branch from cb2d3d9 to eb5cab8 Compare March 30, 2023 04:57

Fix TokensController tests

760fc14

Gudahtt approved these changes Mar 30, 2023

View reviewed changes

Cal-L merged commit 9952d5f into main Mar 30, 2023

Cal-L deleted the patch/5773-improve-watch-asset branch March 30, 2023 19:44

Gudahtt mentioned this pull request Jul 25, 2023

refactor: Update @metamask/assets-controllers patch MetaMask/metamask-mobile#6898

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support watching assets on a specific account #1124

Support watching assets on a specific account #1124

Cal-L commented Mar 8, 2023 •

edited

Loading

Cal-L Mar 15, 2023

tommasini left a comment

Gudahtt commented Mar 23, 2023

adonesky1 commented Mar 23, 2023

Gudahtt left a comment

Cal-L commented Mar 29, 2023

github-actions bot commented Mar 29, 2023

Gudahtt left a comment

Support watching assets on a specific account #1124

Support watching assets on a specific account #1124

Conversation

Cal-L commented Mar 8, 2023 • edited Loading

Cal-L Mar 15, 2023

Choose a reason for hiding this comment

tommasini left a comment

Choose a reason for hiding this comment

Gudahtt commented Mar 23, 2023

adonesky1 commented Mar 23, 2023

Gudahtt left a comment

Choose a reason for hiding this comment

Cal-L commented Mar 29, 2023

github-actions bot commented Mar 29, 2023

Gudahtt left a comment

Choose a reason for hiding this comment

Cal-L commented Mar 8, 2023 •

edited

Loading