Related to T2731. Added blacklist for token name and token ticker fields in CreateTokenForm.js, for blacklisted names with spaces, a version has been included with the space removed. Changed logic in isValidTokenName and isValidTokenTicker to set user input toLowerCase, trim leading and trailing spaces, filters excess spaces between words, joining the strings on a single space to test against the banned array. Also added logic to deal with cases where the user adds spaces between letters (ex. e c a s h). Changed test in validation.test.js that used 'doge' as an example. bannedTokenNames and bannedTokenTickers are cross checked in each input to avoid scammers swapping names / tickers to get around the ban.
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABC1eea7a1143e3: [Cashtab] Add blacklist for token name and token ticker fields
cd web/cashtab && npm start
navigate to Create Token form
input valid token names and token tickers
observe that there is no error
input various combinations of banned tickers and banned names in each input
add and remove spaces between letters
add leading and trailing spaces
observe that there is a valdation error for any of the bannedTokenTickers and bannedTokenNames
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- token-name-and-ticker-blacklist
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 20741 Build 41146: Build Diff cashtab-tests Build 41145: arc lint + arc unit
Event Timeline
Comment Actions
This is a good function. We should let the user know though why their attempted input is invalid.
So, please create a new function separate from isValidTokenName called isProbablyNotAScamTokenName; same for tokenTicker
Update the validation error msgs so that user gets a unique and specific validation error if they try to input from the blacklists, e.g. Cashtab does not support creating token names that appear to mimic existing crypto projects
Comment Actions
Responding to review feedback, created 2 new functions to handle checking against token ticker and token name blacklists. Since the functions are essentially identical, this could probably be handled by a single function. Let me know what you think. Logic around error notification under the inputs has changed to allow for more specific messaging (ie validation error vs blacklist error). Check has been added for newTokenNameIsProbablyNotAScam & newTokenTickerIsProbablyNotAScam before the Create eToken button unlocks.
Comment Actions
- Expand the blacklist to include the top 500 market cap projects on coingecko
Check out here for good way to get this info. You might want to write a simple script to convert this API result into what you need, i.e. an array of just tickers and an array of just names.
https://www.coingecko.com/en/api/documentation
https://api.coingecko.com/api/v3/coins/markets?vs_currency=usd&order=market_cap_desc&per_page=500&page=1&sparkline=false
- It makes the conditionals and error msgs more complicated, but we should provide the user with the specific reason the token cannot be created --- i.e. we should specificy in the error msg if the problem is with the ticker, the name, or with both.
| web/cashtab/src/components/Common/Ticker.js | ||
|---|---|---|
| 147 ↗ | (On Diff #35936) | since we are trimming spaces, aren't dogecoin, doge coin, and d o g e coi n all equally banned? bannedTokenNames should come from a defined list that can be easily updated in the future, for example the top 500 tickers and names on coingecko |
Comment Actions
Responding to review feedback, added top 500 coins by name, ticker and ID from coin gecko. Regarding the error messaging, it is already set up to tell the user whether the ticker or the name is banned. The newTokenTicker and newTokenName can be added to the error message for more specificity, but the message becomes fairly verbose then.
Comment Actions
- Shorten up the error msg so it's just one line. Try "Token name must not conflict with existing crypto project"
- Add national currency names and tickers to the blacklists. Not sure on the best source for this but I'm sure you can find something. e.g. right now a user could create a token with ticker "USD" and we'd like to prevent that as well.
Comment Actions
Lint error is annoying, but I think this patch will fix it anyway.
Seeing this whole list -- I think it's overkill. We should only restrict say the top 10ish largest currencies.
Probably the best way to do this is to just use currency.settingsValidation.fiatCurrency array instead of currency.bannedFiatCurrencies. It's kind of subjective what is there, but there's at least a consistent system to it. And we don't really need to be banning tickers like ALL, which might be useful and legitimate, just because it conflicts with the Albanian Lek.
So -- please remove the bannedFiatCurrencies array from the currency object, and please update the validation function to cross reference the currency.settingsValidation.fiatCurrency array instead.
Comment Actions
Responding to review feedback, bannedFiatCurrencies array has been removed, isProbablyNotScamTokenname & isProbablyNotScamTokenTicker functions now reference the settingsValidation.fiatCurrency array.