feat(tokens): add more fields to metadata #265
Conversation
|
for network name could we just use a static mapping from network id to network name in app code? |
|
hidePriceDelta is a little weird, should it be isStableCoin instead? |
|
@cajubelt I don't think we should do a static mapping for |
|
@MuckT so if a chain forked what would you do? Seems like you would need app code changes no matter what to support both forks right? |
|
@cajubelt I am not sure we would need app code changes to support both forks with dynamic configs. Also if we did have a static mapping wouldn't it display the incorrect |
|
@MuckT to my knowledge, in past forks, one branch (I'm going to call this "the trunk", as opposed to "the fork") has kept the same name. Like Ethereum vs Ethereum Classic. Seems like it would take full-stack code changes to turn on support for the fork. Is it such a bad thing to just default to the trunk and add support for the fork in a later version if it grows popular enough to warrant that? I'm not sure we even have a choice edit: I think I mis-read your message thinking you agreed that we would need frontend code changes to support a new network. I love the ambition of getting the wallet in a state where we don't need code changes to support new networks, but I'm not sure it's realistic in the near term. For instance, currently we have an enum "NetworkId" that covers all the network id's supported by the current app version. At the very least, that would need to get removed-- but it's already used in over a hundred places |
|
For the forking use-case, as @cajubelt mentions, at a minimum we'd need to add a new network id for the forked network. So we can always add the new name along with that change. There could be some benefits of having the name remotely configurable, but this actually doesn't seem the right place to do it. The name is a 1:1 mapping to the network id so we don't need to duplicate it for all tokens. I think we can start off with just a static map in the wallet for now, as the change in names for existing network ids (at least what we have currently now) has never happened. We can always make it configurable through a dynamic config mapping network ids to names or even just an entry in the translation file in future releases |
|
@cajubelt @satish-ravi I was hopefully we could support new networks without an app release, but as you mention with networkIds We'd need an app release to update those mappings. With that in mind a static mapping sounds like it will work well. |
src/types.ts
Outdated
| @@ -41,12 +41,18 @@ export interface TokenInfoJSON { | |||
| pegTo?: string | |||
| isSupercharged?: boolean | |||
| bridge?: string | |||
| isStableCoin?: boolean | |||
| isZeroState?: boolean | |||
There was a problem hiding this comment.
Why do we need this? And would there be a more descriptive name?
| isZeroState?: boolean | |
| showZeroBalance?: boolean |
There was a problem hiding this comment.
This is for showing the token even if the balance is zero in the new Assets screen. And I've updated the name, I like it better too 😄
README.md
Outdated
| | `isStableCoin` | Whether the token is a stable coin and the price is not expected to change | | ||
| | `isZeroState` | Whether the token is displayed even if the balance is zero | | ||
| | `isCashInEligible` | Whether the token is eligible for cash-ins | | ||
| | `isCashOutEligible` | Whether the token is eligible for cash-outs | |
There was a problem hiding this comment.
Do we expect outside contributors to fill these?
The documented fields in the README were only for the required fields to keep external contributions as simple as possible.
There was a problem hiding this comment.
Nope, these are internal only. I've removed these and included some of these in types.ts where the name isn't completely obvious.
Fixes ACT-923, adds fields needed for the new token details screen.
showZeroBalance- true for tokens to be displayed when balance is 0isStableCoin- true for core stable tokensisCashInEligible- true for tokens that can be cashed-inisCashOutEligible- true for tokens that can be cashed-outinfoUrl- link to coingeckonetworkIconUrl- computed, set for all non native tokens with the native token's image url