Skip to content

Assets detection and tokens balances controllers - #19

Merged
estebanmino merged 44 commits into
masterfrom
assets-detection-controller
Oct 19, 2018
Merged

Assets detection and tokens balances controllers#19
estebanmino merged 44 commits into
masterfrom
assets-detection-controller

Conversation

@estebanmino

@estebanminoestebanmino commented Oct 4, 2018

Copy link
Copy Markdown
Contributor

This PR adds the ability to support auto detection of assets and also add token balances.

For the second purpose a new TokenBalancesController was created to poll for balances.

Auto detection was achieved adding two new controllers.

AssetsContractController: Only responsibility to interact with contracts of assets through provider. (Probably needs a better name)
AssetsDetectionController: Responsible to auto detect new tokens and collectibles. Auto detection is being triggered when user change account or a period of 3 minutes is finished, while user is on mainnet.

Assets that are detected are the assets with their respective information in https://github.com/metamask/eth-contract-metadata.

The proposed structure for collectible to be auto detected from eth-contract-metadata,

{ name: string; address: string; symbol?: string; api?: string; collectibles_api?: string; owner_api?: string; collectibles_entry?: string; erc721?: boolean; }

name: Asset name
symbol: Asset symbol
erc721: Whether it ERC721 asset
api: API URI
collectibles_api: API specific endpoint to get collectibles information, as custom information
owner_api: API specific endpoint to get owner information, as quantity of assets owned
collectibles_entry: API parameter to get collectibles of owner from owner_api URI

The objective of this structure was to provide both compability for new assets and to support current ERC721 standard. There is support for three "types" of collectibles:

  1. Collectibles with API defined on contract-metadata, as Cryptokitties, which only interaction is through its API. In this case, collectibles are being added with custom name and image for each collectible.

  2. Collectibles with ERC721Enumerable interface support, but no support of ERC721Metadata interface. Link to EIP. In this case, if api and collectibles_api are defined, collectibles are being added with custom information as name and collectible specific image. If not, collectible is being added only with Collectible Identifier.

  3. Collectibles with ERC721Enumerable interface and no support of ERC721Metadata interface. Link to EIP. In this case, collectibles are being added with custom name and image for each collectible.

@codecov-io

codecov-io commented Oct 8, 2018

Copy link
Copy Markdown

Codecov Report

Merging #19 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #19 +/- ##
======================================
Coverage 100% 100% ======================================
Files 15 18 +3 Lines 694 956 +262 Branches 76 109 +33 ======================================
+ Hits 694 956 +262
Impacted FilesCoverage Δ
src/BaseController.ts100% <ø> (ø)⬆️
src/TokenRatesController.ts100% <100%> (ø)⬆️
src/util.ts100% <100%> (ø)⬆️
src/AssetsController.ts100% <100%> (ø)⬆️
src/TokenBalancesController.ts100% <100%> (ø)
src/AssetsContractController.ts100% <100%> (ø)
src/AssetsDetectionController.ts100% <100%> (ø)
src/NetworkController.ts100% <100%> (ø)⬆️
src/TransactionController.ts100% <0%> (ø)⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5127547...828164b. Read the comment docs.

@estebanminoestebanmino changed the title [WIP] Assets detection controllerAssets detection and tokens balances controllersOct 10, 2018

@bitpshrbitpshr 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.

Two minor suggestions, but looks great and should be ready to land.

Comment threadsrc/AssetsContractController.ts
}
try {
const contract = this.web3.eth.contract(abiERC20).at(address);
return await new Promise<typeof BN>((resolve, reject) => {

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.

Same comment here re: returning the Promise directly.

Suggested change
returnawaitnewPromise<typeofBN>((resolve,reject)=>{
returnnewPromise<typeofBN>((resolve,reject)=>{

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@bitpshr yeah this is weird, I did this as a workaround for web3/web3.js#1119 since the workaround mentioned there wasn't working. The issue is still open, let me know what you think.

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.

Ah, ok, I see. Good catch 👍

Comment threadsrc/AssetsController.ts
@estebanminoestebanmino mentioned this pull request Oct 18, 2018
@estebanmino
estebanmino merged commit 4633480 into masterOct 19, 2018
@estebanmino
estebanmino deleted the assets-detection-controller branch November 30, 2018 01:23
mcmire pushed a commit to mcmire/core that referenced this pull request Jul 17, 2023
* Improve types and type validation
* Fix JsonRpcSuccess type
* Add functions to check if a value is a JsonRpcResponse
* Fix error message
* Add test vectors
* Move test.data to fixtures folder
* Improve error messages
kanthesha pushed a commit that referenced this pull request Sep 19, 2023
Bumps [glob-parent](https://github.com/gulpjs/glob-parent) from 5.1.1 to 5.1.2.
- [Release notes](https://github.com/gulpjs/glob-parent/releases)
- [Changelog](https://github.com/gulpjs/glob-parent/blob/main/CHANGELOG.md)
- [Commits](gulpjs/glob-parent@v5.1.1...v5.1.2)
---
updated-dependencies:
- dependency-name: glob-parent
dependency-type: indirect
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
MajorLift pushed a commit that referenced this pull request Sep 22, 2023
v1.0.1 contains two changes that should have been considered
semver-major:
- Type-interface incompatability with previous version (MetaMask/json-rpc-engine#139)
- Introduced dependency `@metamask/json-rpc-engine` indicates a minimum
supported Node.js version of 16. This prevents the module from
installing on some package manager configurations, like default
yarn classic.
This will be re-released as v2.0.0.
kanthesha pushed a commit that referenced this pull request Oct 11, 2023
Mrtenz pushed a commit that referenced this pull request Oct 16, 2025
Mrtenz pushed a commit to Mrtenz/core that referenced this pull request Jun 17, 2026
…etaMask#19)
* Remove cancelled transaction when new trx with same nonce submitted
* fix list concatenation
Sign up for freeto 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

@estebanmino@codecov-io@bitpshr