Add Identity Balance section #12

jessgusclark · 2020-11-23T18:19:28Z

Defaults to pulling in RIF on Mainnet and Testnet. No defaults for ethereum or local.

Allows the user to insert the contract address of an ERC20 or ERC721 token to track. Address is saved into local storage. For testing, here is the DoC contract addresses:

Mainnet: 0xE700691Da7B9851F2F35f8b8182C69C53ccad9DB
Testnet: 0xcb46c0ddc60d18efeb0e586c17af6ea36452dae0

- Create reducer for tokens that is an array - Create token with address in reducer and update details based on the address - Display token on balance page - RIF address is saved in config file to be expanded later

…orage.

ilanolkies · 2020-11-23T23:44:27Z

src/app/state/operations/tokens.ts

@@ -0,0 +1,63 @@
+import { Dispatch } from 'react'
+import Eth from 'ethjs'


Isn’t it enough to add just contract module?

~~I looked into it and I'm not sure, from their readme it says NOTE. Module not ready for use, still in heavy development. Which is why I opted for the full version~~

Tested with the contract module, and paired with the query module it works as expected.

This module imports that module. See https://github.com/ethjs/ethjs/blob/master/src/index.js#L35

src/app/state/operations/tokens.ts

ilanolkies · 2020-11-23T23:48:14Z

src/app/state/operations/tokens.ts

+import { saveToLocalStorage, getValueFromLocalStorage } from '../../../features/localStorage'
+
+export const getTokenList = (provider: any) => (dispatch: Dispatch<any>) =>
+  getNetwork(provider)


Isn’t this recoverable from state? We are performing twice same rpc for a same value that did not change

ilanolkies · 2020-11-23T23:50:17Z

src/app/state/operations/tokens.ts

+  const eth = new Eth(provider)
+  const token = eth.contract(erc720).at(address)
+
+  eth.getCode(address)


I would check this only when adding custom token. Anyway i wouldn’t get code. Just query needed calls and if one fails => invalid token

See the comment below about name and symbol being optional. balanceOf is the only required method. I'll reorder the code to check this first and if it doesn't exist then consider it not valid.

ilanolkies · 2020-11-23T23:50:27Z

src/app/state/operations/tokens.ts

+      dispatch(addToken({ address }))
+
+      token.symbol().then((symbol: string[]) => symbol[0])
+        .then((symbol: string) => dispatch(addTokenData({ data: { address, symbol } }))).catch()


What is catch doing?

In a ERC20 and 721 token contract, name and symbol are optional functions. So, if they don't exist in the contract, it will throw an error here. I don't want it to break, I want to continue to the next call, so I add the catch() here.

I do notice that it makes the UI look a bit off, so if a token doesn't have a name function, I'll add 'Custom Token 0x123456...1213` in the catch to show something back to the user.

Should we catch ONLY the error thrown by this case? Instead of catching everything. Maybe it throws another error...

The problem is that the errors returned by ethjs are strings not objects. This a similar issue to the previous PR where we tried to catch the error. Here is the error that is returned:

[ethjs-query] while formatting outputs from RPC '{"value":{"code":-32603,"message":"Non-200 status code: '500'","data":{"jsonrpc":"2.0","id":3846821291,"error":{"code":-32015,"message":"VM execution error: transaction reverted"}}}}'

ilanolkies · 2020-11-23T23:57:28Z

src/app/state/operations/tokens.ts

+          token.balanceOf(accounts[0])
+            .then((balance: any) => {
+              dispatch(addTokenData(
+                { data: { address, balance: balance[0].toString() / Math.pow(10, decimals[0].toNumber()) } }


We should use bn.js arithmetics. I understand this is the type of any uint value returned by ethjs. See https://github.com/indutny/bn.js

ilanolkies · 2020-11-23T23:59:56Z

src/app/state/reducers/tokens.ts

+}
+
+export interface TokenState {
+  tokens: any


Storing them as a address=>data map would be more clear and efficient

I had trouble creating this with typescript and need assistance. For now, I have changed this to tokens: Token[].

ilanolkies · 2020-11-24T00:01:39Z

src/features/localStorage.ts

@@ -0,0 +1,21 @@
+const LOCAL_STORAGE_KEY = 'id-manager-settings'


I would move this to a /storage folder. Not /features

ilanolkies · 2020-11-24T00:04:36Z

src/features/localStorage.ts

@@ -0,0 +1,21 @@
+const LOCAL_STORAGE_KEY = 'id-manager-settings'


Also, we should test this module

ilanolkies · 2020-11-24T00:05:12Z

src/app/state/operations/utils.ts

@@ -0,0 +1,3 @@
+export type Callback<T> = (err?: Error, res?: T) => void
+
+export const callbackify = (promise: () => Promise<any>, cb: (arg0: undefined, arg1: undefined) => any) => cb ? promise().then((res: any) => cb(undefined, res)).catch((err: any) => cb(err, undefined)) : promise()


I didn’t find any import for this

Thanks, I tried using it and was unsuccessful, I'll remove it.

ilanolkies · 2020-11-24T00:06:48Z

src/app/state/operations/tokens.ts

+}
+
+export const addCustomToken = (provider: any, address: string) => (dispatch: Dispatch<any>) =>
+  new Promise((resolve, reject) => {


This Promise seems to be unnecessary. It could be handled with the s

the 's'?

The promise is returned to the component to let it know to stop 'loading' and either return the error or close the modal

Sorry, I want to say thens

dispatch() does not return a promise.

- Rename erc720 file - Move localStorage and add tests - Wrap identity in address/chainId to pass down to children

ilanolkies

Now that I understand it is also for ERC721 I tested with RNS token contract. It stalls :(

- Pass user address and chainId to operations - Use ethr contract and BN modules to handle opps - remove addToken() action and use addTokenData() instead. Creates new entry if missing. - Update missing props

jessgusclark · 2020-11-25T11:33:06Z

@ilanolkies Added a new commit to refactor the tokens. Fixes the issue above with erc721 non-tokens and simplifies the code quite a bit. Thanks!

jessgusclark added 5 commits November 23, 2020 16:07

Add basic iplementation for ERC20&721 coins.

af7c3e6

- Create reducer for tokens that is an array - Create token with address in reducer and update details based on the address - Display token on balance page - RIF address is saved in config file to be expanded later

Allow user to add their own cutom token.

6202429

Catch errors if address is not a contract and save address in localSt…

68730c8

…orage.

Add styles and hover state for balances that mess up the layout.

33bc89c

return original and not empty to return 0.

a914545

jessgusclark requested a review from ilanolkies November 23, 2020 18:19

ilanolkies suggested changes Nov 24, 2020

View reviewed changes

ilanolkies reviewed Nov 24, 2020

View reviewed changes

Fix smaller/easier requested changes

9a51887

- Rename erc720 file - Move localStorage and add tests - Wrap identity in address/chainId to pass down to children

ilanolkies mentioned this pull request Nov 24, 2020

Caché static token information #13

Open

ilanolkies suggested changes Nov 24, 2020

View reviewed changes

Refactor tokens

11ed14c

- Pass user address and chainId to operations - Use ethr contract and BN modules to handle opps - remove addToken() action and use addTokenData() instead. Creates new entry if missing. - Update missing props

ilanolkies approved these changes Nov 25, 2020

View reviewed changes

ilanolkies merged commit 67cd951 into main Nov 25, 2020

delete-merged-branch bot deleted the erc20 branch November 25, 2020 14:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Identity Balance section #12

Add Identity Balance section #12

jessgusclark commented Nov 23, 2020

ilanolkies Nov 23, 2020

jessgusclark Nov 24, 2020 •

edited

Loading

ilanolkies Nov 24, 2020

ilanolkies Nov 23, 2020

ilanolkies Nov 23, 2020

jessgusclark Nov 24, 2020

ilanolkies Nov 23, 2020

jessgusclark Nov 24, 2020 •

edited

Loading

ilanolkies Nov 24, 2020

jessgusclark Nov 25, 2020

ilanolkies Nov 23, 2020

ilanolkies Nov 23, 2020

jessgusclark Nov 25, 2020

ilanolkies Nov 24, 2020

ilanolkies Nov 24, 2020

ilanolkies Nov 24, 2020

jessgusclark Nov 24, 2020

ilanolkies Nov 24, 2020

jessgusclark Nov 24, 2020

ilanolkies Nov 24, 2020

jessgusclark Nov 25, 2020

ilanolkies left a comment

jessgusclark commented Nov 25, 2020

		@@ -0,0 +1,63 @@
		import { Dispatch } from 'react'
		import Eth from 'ethjs'

		@@ -0,0 +1,21 @@
		const LOCAL_STORAGE_KEY = 'id-manager-settings'

		@@ -0,0 +1,3 @@
		export type Callback<T> = (err?: Error, res?: T) => void

		export const callbackify = (promise: () => Promise<any>, cb: (arg0: undefined, arg1: undefined) => any) => cb ? promise().then((res: any) => cb(undefined, res)).catch((err: any) => cb(err, undefined)) : promise()

Add Identity Balance section #12

Add Identity Balance section #12

Conversation

jessgusclark commented Nov 23, 2020

Choose a reason for hiding this comment

jessgusclark Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessgusclark Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilanolkies left a comment

Choose a reason for hiding this comment

jessgusclark commented Nov 25, 2020

jessgusclark Nov 24, 2020 •

edited

Loading

jessgusclark Nov 24, 2020 •

edited

Loading