RavenProject / ravenwallet-ios

MIT License
25 stars 21 forks source link

Add filter (whitelist/blacklist) #141

Open TronBlack opened 4 years ago

TronBlack commented 4 years ago

Adding a filter - whitelist would allow anyone to see only the assets they want to see. It would also help provide a foundation for a white label wallet.

Adding a filter - blacklist - would hide specific assets that the user doesn't want to see.

These filters are just for the display of assets and would not affect whether the token is received and/or transferrable.

a2hill commented 4 years ago

@TronBlack Is there anything else needed for this issue or can it be closed?

TronBlack commented 4 years ago

In testing, the only thing I could find is if you clear both blacklist and whitelist and exit UI while the whitelist tab is active, then no assets show up. It seems like if there is no whitelist or blacklist, that everything should be shown.

a2hill commented 4 years ago

In this case that is the intended behavior. Whichever tab is left active is the new filter method. By leaving the "whitelist" tab active you are selecting to filter via whitelisting. Since the whitelist is empty, no assets are allowed to be shown.

I think the issue is that the UI does not effectively communicate the idea that which ever tab is left active is now the new filter method. I can play around with it and try to come up with something that makes this a little more evident. Or I can make it such that an empty whitelist allows all assets?

TronBlack commented 4 years ago

My original thought was whitelist + blacklist with either regex or wildcards. I did not express any of this in the issue.

Example: TLD/ would give you every sub-asset of TLD when you'd add this to the whitelist. An empty whitelist would act effectively like "" so that no entries at all would give the wallet the same behavior it had before. Multiple entries would be allowed, like TLD/ and T.D/

The blacklist would be applied after the whitelist to remove using the same rules, except that you'd need to add "*" to blacklist everything.

With that said, the UI that you did is much, much better than what I was imagining. It is more intuitive. An alert when closing and leaving the [Whitelist] tab active that says "Do you want an empty whitelist? No assets will be shown." Yes/No. This would solve the UI confusion. I want you to know that I appreciate your work on this, so please don't take any of this as criticism.

a2hill commented 4 years ago

Thank you! My original thoughts were along the same lines: allow the user to input their own filter strings to match against as there is likely a use case for whitelisting all sub-assets with a certain TLD. As I considered what it would take to make that sort of thing feel intuitive, I decided it would be good to get something simple working first. That way I could always come back and extend it.

I'll go ahead and add an alert for the empty whitelist case as I think that will solve the issue at hand. From there I can look at allowing the manual entry of wildcard like strings. I am still a little unclear on the use case for filtering by whitelist and then blacklist. Can you expand on that?

TronBlack commented 4 years ago

I suspect given time, I could come up with a better example for whitelist combined with the blacklist.

Let's say I'm interested in every sub-asset of NFT because there's a new service allowing the creation of NFT tokens. NFT#ArtProject33 NFT#CustomDrawing NFT#Spock_1_of_100

Now, along comes the jokester: NFT#BUTT

I don't want that last one, but I still want NFT# or NFT/ or NFT*

Note: It only matters when using wildcards or regex. With 1 to 1 whitelisting, then the absence of a token name in the whitelist is blacklisted.

Regex is flexible but may be overkill. We do use * for wildcard matches (at the end) in the RPC calls.

a2hill commented 4 years ago

I understand now, this makes sense. I'll have to think about what kind of UI would make this functionality intuitive to the user.

In the meantime I think I'll look into extending the whitelist/blacklist functionality to support wildcard matching under the hood. In addition to this I will look into how we could then specify a whitelist at build time to allow for white-labeled wallets. The last step would then be to allow the user to input whitelist/blacklist entries, with wildcards, via the UI.

TronBlack commented 4 years ago

Added by a2hill. Much appreciated.

For anyone building a white-label wallet, this could be pre-configured for your token(s)