Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Use object-oriented method to achieve coin filter #552

Open
chrisli30 opened this issue Oct 16, 2020 · 2 comments
Open

Use object-oriented method to achieve coin filter #552

chrisli30 opened this issue Oct 16, 2020 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@chrisli30
Copy link
Contributor

From Brendan,

The coin type determination should be achieved using object-oriented method as other property, instead of using a separate configuration array variable.

Example 1:
955c072#diff-91fb241661b3998d5c42b6029dd03123R45-R49
if (symbol === 'RBTC') {
rawTransaction.to = to;
rawTransaction.data = memo;
} else {
const assetContract = ASSETS_CONTRACT[symbol][type];

if (symbol === 'RBTC') should be implemented in coin.createRawTransaction(to, memo) since each coin has the property symbol.

Example 2,
in src/pages/wallet/wallet.connect/index.js line 235,

const rskCoins = _.filter(coins, (coin) => coin.symbol !== 'BTC' && coin.type === net);
    if (_.isEmpty(rskCoins)) {
      };	      return null;
    }
    return {
      address: Rsk3.utils.toChecksumAddress(rskCoins[0].address, networkId),
      privateKey: rskCoins[0].privateKey,
    };

all lines with _.filter() should be replaced by the actual logic such as coin.getPrivateKey()

@chrisli30 chrisli30 added the enhancement New feature or request label Oct 16, 2020
@chrisli30
Copy link
Contributor Author

@bguiz Please see if this issue is as described in your message.

@bguiz
Copy link

bguiz commented Oct 20, 2020

The config file should store the category for each "coin", specifically whether it is a native currency of the crypto network (e.g. R-BTC on RSK), or if it is a token (e.g. RIF on RSK`).

Once that is done, instead of deciding how to construct the transaction based on a hard coded value, can use this category value to decide instead. The intent behind this is:

  • forward looking, e.g. when we add more categories of tokens
  • less likelihood of regression, since things are configured in a single place

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants