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

MyMarkets does not use replaceName for assets #1803

Open
sschiessl-bcp opened this issue Aug 27, 2018 · 24 comments
Open

MyMarkets does not use replaceName for assets #1803

sschiessl-bcp opened this issue Aug 27, 2018 · 24 comments
Labels

Comments

@sschiessl-bcp
Copy link
Contributor

In MyMarkets overview

image

symbols are directly printed (for base and quote), without using utils.replaceName or AssetName component, see

https://github.com/bitshares/bitshares-ui/blob/1ee0ba3264abb63a3c06353e35731e80c7aee56d/app/components/Exchange/MyMarkets.jsx#L1048

@startailcoon
Copy link
Contributor

startailcoon commented Aug 27, 2018

I'm not sure what the problem is, but the value on L1048

Comes from

preferredBases.map((base, index) => {

preferedBases comes from Settings and is defined on

preferredBases: SettingsStore.getState().preferredBases,

PreferedBases is just a list of token names stored in settings, like ["BTC","BTS","USD","WLS","OPEN.BTC"]. These names are then used for listing all available markets for that asset. The BTC tab lists markets like BTC/BTS, BTC/BTDIGE.BCO etc....

The List

The list of assets, that shows price and volume, is called on with <MarketGroup /> on

This component then uses <AssetName /> to view the list on

<AssetName
dataPlace="left"
name={market.quote}
/>

@startailcoon
Copy link
Contributor

startailcoon commented Aug 27, 2018

I guess your referring to the header tabs that is printed with L1048, that you refer to, isn't using using the asset replace.

I think that isn't the case because the name like BTC actually loads all BTC assets, open, bit etc.

@svk31
Copy link
Contributor

svk31 commented Aug 28, 2018

It doesn't make sense to use replaceName or AssetName here, because the content for each tab actually shows all x.BTC assets etc. Even for USD, OPEN.USD is included as well as any future USD assets by one of the supported gateways.

@sschiessl-bcp
Copy link
Contributor Author

sschiessl-bcp commented Aug 28, 2018

Yes, but only because you did a special hack to treat OPEN.BTC as BTC, it should be an abstract solution.

Following points:

  1. This needs to be refined such that you give it a wildcard symbol (*BTC) as base, which means that all similar assets are shown (bitBTC, OPEN.BTC, GDEX.BTC etc.). The implicit nature of simply putting BTC and it searches for all is confusing, some users might want to actually specify only one of those. This immediately brings up the question how to deal with the UX here. I would say currently no one understands the logic. Example is here:
    image
    No one can see why there two BTS rows. If you click on both you will of course see (if you look at the exact market) that it is in fact GDEX.BTC and OPEN.BTC. I do love this feature, but the current UX does not carry it forward

  2. The actual definition of which assets are grouped is through available Gateways, which is fine

  3. replaceName should be applied (also in the rows) due to the possibility to hide namespaces through branding.js

@svk31
Copy link
Contributor

svk31 commented Aug 28, 2018

  1. Not just OPEN.BTC, all the gateways returned by getPossibleGatewayPrefixes here: https://github.com/bitshares/bitshares-ui/blob/develop/app/lib/common/gateways.js#L82

I'm open to suggestions as to how to visually indicate what gateway the market belongs to in the case of "duplicate" markets though, and there's already an issue discussing that.

  1. replaceName/AssetName is applied in the rows already.

@sschiessl-bcp
Copy link
Contributor Author

sschiessl-bcp commented Aug 28, 2018

Ah sorry, I mixed it up with the new Dashboard feature.

We define

export function getMyMarketsBases() {
return ["BTC", "ETH", "BTS", "USD", "CNY"];
}

which correlate to BTC the bitAssets. If you would e.g. replace BTC with OPEN.BTC in branding.js, it would be displayed as OPEN.BTC, even if "OPEN." is a hidden namespace. In my opinion, BTC should lead to only displaying bitBTC. If you want grouped, it should be *BTC in the settings to distinguish the logic (also important if you want to brand your wallet to only show asset from one gateway).

@sschiessl-bcp
Copy link
Contributor Author

I'm open to suggestions as to how to visually indicate what gateway the market belongs to in the case of "duplicate" markets though, and there's already an issue discussing that.

Could you point me to it please?

@svk31
Copy link
Contributor

svk31 commented Aug 28, 2018

#1746

@svk31
Copy link
Contributor

svk31 commented Aug 28, 2018

I think it's very important to group markets this way in order to provide access to all liquid markets, and it also makes it easier for users to actually find the best, most liquid markets.

@sschiessl-bcp
Copy link
Contributor Author

Certainly. I am not saying it should not be on by default. I am saying it should not abuse the bitAsset symbols to do so, but rather be explicitly set.

@sschiessl-bcp
Copy link
Contributor Author

sschiessl-bcp commented Aug 28, 2018

Found a new issue with the implicit approach by looking into ETH.

There is actually an asset called ETH, which is neither a bitAsset or from a trusted gateway. It is even highlighted in the current reference UI by showing up in MyMarkets
https://wallet.bitshares.org/#/market/OPEN.DOGE_ETH

Having said that, the logic should be even more refined that you can either

  1. Set a wildcard to include all, but the base symbol without prefix must be a bitAsset to be shown
  2. Set a specific list of grouped symbols

Examples for:

  1. '*BTC' to cover ['BTC', 'GDEX.BTC', ...], but '*ETH' only covers [RUDEX.ETH, GDEX.ETH, ...] since ETH is not a bitAsset
  2. Set BTC = ['BTC', 'OPEN.BTC', ...] and ETH = ['RUDEX.ETH', 'GDEX.ETH', ...]

My vote would be for 1. since I don't see any other way that a non-prefixed asset should be trusted, other than being a bitAsset. I find the issue with ETH quite crucial actually.

@svk31
Copy link
Contributor

svk31 commented Aug 28, 2018

That's a good point, so some form of whitelisting should probably applied. I also like the *BTC approach, possibly with a tooltip explaining it as well.

@happyconcepts
Copy link
Contributor

want to brand your wallet to only show asset from one gateway

Why so much effort toward curating and controlling the content of the blockchain?

This attitude seems precariously exclusionary to me.

@sschiessl-bcp
Copy link
Contributor Author

@happyconcepts
See #1613

@happyconcepts
Copy link
Contributor

happyconcepts commented Aug 29, 2018

I find the issue with ETH quite crucial actually

Actually, since you are meeting in Amsterdam with George and bitSpark very shortly, and apparently he is launching Spark DEX tomorrow 30 Aug.

I am the issuer of Spark UIA on BitShares, so the same "crucial" conflict must logically exist for Spark Dex and my coin.

And I am simply not delighted at being left out of searches or lists of assets since you are suggesting to redesign the BitShares software to do so.

@sschiessl-bcp
Copy link
Contributor Author

And I am simply not delighted at being left out of searches or lists of assets since you are suggesting to redesign the BitShares oftware to do so.

On the dashboard of the Reference UI markets of bitAssets and included Gateways are highlighted. No other private asset is being left out, as no other private asset should be included. That the dashboard potetntially shows ETH markets (markets that include the symbol ETH, which is private, no gateway and no bitAsset) is merely a mistake.

@happyconcepts
Copy link
Contributor

How is it you choose to speak so, shall we say imperially, when I am trying to have a discussion. It feels very one sided.

@happyconcepts
Copy link
Contributor

No other private asset is being left out, as no other private asset should be included.

This. I believe you need to fundamentally change this attitude, for the best of BitShares.

It's 6:30am here, please unban me from telegram if you want to discuss it further.

@happyconcepts
Copy link
Contributor

since I don't see any other way that a non-prefixed asset should be trusted, other than being a bitAsset.

You will never trust my Spark coin, then, because it is a non prefixed UIA?

Or just because I came here as Spark to Bitshares before bitSpark?

@sschiessl-bcp
Copy link
Contributor Author

Actually, since you are meeting in Amsterdam with George and bitSpark very shortly, and apparently he is launching Spark DEX tomorrow 30 Aug.

Will you be in Amsterdam too @happyconcepts ?

imperially

I don't even know how to do that

This. I believe you need to fundamentally change this attitude, for the best of BitShares.

I am only talking about highlighting it on the Dashboard. The reference UI allows to search and display any asset. I suppose the UI could do some promotion for assets on the Dashboard, but that would need to be a completely new format.

It's 6:30am here, please unban me from telegram if you want to discuss it further.

What is your telegram handle and which channel are you talking?

You will never trust my Spark coin, then, because it is a non prefixed UIA?

The reference UI highlights commonly known crypto-currencies (BTC, ETH, ...) on the Dashboard. Trusted was maybe the wrong wording. Let's say "not promoting".

Or just because I came here as Spark to Bitshares before bitSpark?

That is a mere name collission. bitSpark is also not highlighted.

I don't see that this issue touches your SPARK asset at all. What is your suggestion now to do differently=

@happyconcepts
Copy link
Contributor

I don't see that this issue touches your SPARK asset at all. What is your suggestion now to do differently=

Well there is a private offering so I will be here on BitShares with Spark for at least a little longer now.

I must have somehow missed the invitation to Amsterdam. I am Spark token on telegram, banned from the BitShares group last summer within just two weeks of joining.

Maybe to make way for bitspark's arrival, or maybe coincidence, or maybe unfairly branded as an enemy-of-the-state, who knows? But there were also two attempts on my life that week, and my founding partner for Spark was shot dead Aug 5 so I had larger issues to attend to than to fight the paranoid-ban at the time. The rest is just mob mentality.

@wmbutler
Copy link
Contributor

wmbutler commented Sep 5, 2018

@sschiessl-bcp I support where you are going with this. It's important that users be able to easily identify trusted gateways and their associated tokens that represent real world crypto. @happyconcepts you are also able to use a prefix and do the same.

@clockworkgr
Copy link
Member

@sschiessl-bcp I too like the wildcard approach.

Is this being worked on?

@sschiessl-bcp
Copy link
Contributor Author

Not by me atm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants