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

Gaba assets controllers integration #143

Merged
merged 13 commits into from
Oct 23, 2018

Conversation

estebanmino
Copy link
Contributor

@estebanmino estebanmino commented Oct 10, 2018

Description

This PR acts together with MetaMask/core#19, providing assets auto detection and token balances.

gabaintegration

Specifically it adds three controllers from GABA:

  1. AssetsDetectionController: In charge of auto detect assets when, network changes to mainnet or current account changes, while mainnet.
  2. AssetsContractController: In charge of web3 interaction with assets controller. Important in this PR because it modifies the way GABA is used with rn-nodeify https://github.com/MetaMask/MetaMask/blob/d5ad3653a2f8bad28e19bcfdd38c3ad04140524e/scripts/postinstall.sh#L6
  3. TokenBalancesController: In charge of balances update from current token list.

And use of TokenRatesController, to get fiat value of tokens together with token balances.

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue
Related MetaMask/website#61

BLOCKER: Correct GABA version.
https://github.com/MetaMask/MetaMask/blob/d5ad3653a2f8bad28e19bcfdd38c3ad04140524e/package.json#L60

Resolves #63

Comments: Address in GIF was hard coded.

Copy link
Contributor

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

Looks good! Some minor concerns about rn-nodeify and the changes to the package.json
Other than that 💯

* @returns {number} - Number
*/
export function toNumber(value, decimals) {
return value.toNumber() / Math.pow(10, decimals);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can do value.toFormat(decimals)

app/util/number.js Show resolved Hide resolved
package.json Show resolved Hide resolved
@@ -3,6 +3,7 @@ echo "PostInstall script:"

echo "1. React Native nodeify..."
node_modules/.bin/rn-nodeify --install 'crypto,buffer,react-native-randombytes,vm,stream' --hack
Copy link
Contributor

Choose a reason for hiding this comment

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

have you tried adding gaba in this line instead?

node_modules/.bin/rn-nodeify --install 'crypto,buffer,react-native-randombytes,vm,stream,gaba' --hack

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes I tried, is super strange to be honest. I'll reach out on slack when I start making this changes.

@estebanmino estebanmino changed the title [WAITING FOR GABA] Gaba assets controllers integration Gaba assets controllers integration Oct 23, 2018
Copy link
Contributor

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

Changes LGTM!

@estebanmino estebanmino merged commit f32760e into master Oct 23, 2018
@estebanmino estebanmino deleted the gaba-assets-detection-integration branch October 23, 2018 00:59
@brunobar79 brunobar79 mentioned this pull request Nov 20, 2018
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
* GABA AssetsDetectionController integration

* Fix CollectibleImage address

* GABA with web3 integration and collectible image bugfix

* integrate AssetsContractController

* integration of GABA TokenBalancesController

* update snapshots

* fix empty collectible image URI

* add new controllers to engine tests

* update gaba to last version

* fix some gaba dependencies

* update calculate token value
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.

2 participants