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

Read tokens.json and put tokens into the correct group in Wallet tab #3581

Closed
hboon opened this issue Dec 20, 2021 · 25 comments · Fixed by #4170 or #4304
Closed

Read tokens.json and put tokens into the correct group in Wallet tab #3581

hboon opened this issue Dec 20, 2021 · 25 comments · Fixed by #4170 or #4304

Comments

@hboon
Copy link
Member

hboon commented Dec 20, 2021

Part of #3546
Depends on: #3580

A token matches if the contract and chain ID matches an entry in each array.

This file: https://github.com/AlphaWallet/tokens/blob/main/tokens.json (check update: #3581 (comment))

"All" should contain every token that is not hidden (like it is before the PR).

If a token is not categorized, display it under "Assets".

Just ignore the tokens tagged as "Collectibles" (if there are any) in the JSON. We still show all and only ERC721 and ERC1155 in the hardcoded Collectibles tab. Otherwise it wouldn't work.

@eviltofu
Copy link
Contributor

Does this tokens.json file need to be compressed?

@hboon
Copy link
Member Author

hboon commented Jan 16, 2022

Don't work on this one yet.

@eviltofu
Copy link
Contributor

Don't work on this one yet.

@hboon Is this still on hold?

@hboon
Copy link
Member Author

hboon commented Jan 24, 2022

Yes, but should be moving soon.

@hboon
Copy link
Member Author

hboon commented Jan 24, 2022

@eviltofu ok, can you try this file instead (it's embedded in the comment) : https://github.com/AlphaWallet/tokens/issues/1#issuecomment-1014318436

It looks small enough after the filtering/verification. Let's just embed it in the app directly. You'll have to hunt down wallets (to watch) that contains those tokens (buzz me if you are not sure how).

@hboon
Copy link
Member Author

hboon commented Jan 27, 2022

@eviltofu I forgot: do remember that the "Collectibles" tab is handled differently. I think, just ignore the tokens tagged as "Collectibles" (if there are any) in the JSON. We still show all and only ERC721 and ERC1155 in the hardcoded Collectibles tab. Otherwise it wouldn't work.

(Updated the first comment to include this)

@eviltofu
Copy link
Contributor

@hboon im still trying to figure out how this screen works; where the updates are coming from etc.

@hboon
Copy link
Member Author

hboon commented Jan 28, 2022

@eviltofu Sure. I think a good place to start is how TokensViewModel exposes the data to TokensViewController's func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { etc.

Probably have to modify how this works:

enum WalletFilter {
	case all
    case type(Set<TokenType>)
	case currencyOnly
	case assetsOnly
	case collectiblesOnly
	case keyword(String)
}

Good luck :)

@hboon
Copy link
Member Author

hboon commented Mar 7, 2022

Higher prority for this. Been stuck for a long time and there's a few things hanging on this.

@eviltofu eviltofu assigned eviltofu and unassigned eviltofu Mar 21, 2022
@eviltofu
Copy link
Contributor

eviltofu commented Mar 23, 2022

@hboon

The tokens.json file has "group": "Assets", "group": "DeFi", "group": "Governance" group types. So the newly created code needs to read the tokens.json file and differentiate a token into the following types:

  • All
  • Assets
  • Defi
  • Governance
  • Collectibles

Is this correct?

An issue (#4141) has been created for this.

Can the file be named TokenGroups.json instead?

@hboon
Copy link
Member Author

hboon commented Mar 23, 2022

Is this correct?

Yes

Can the file be named TokenGroups.json instead?

No. And it's not specifically for grouping tokens. There will be other functionality tied to it.

@eviltofu
Copy link
Contributor

@hboon Part of token.json is

"contracts": [
            {"address": "0x03ab458634910aad20ef5f1c8ee96f1d6ac54919", "chainId": 1},
            {"address": "0x00e5646f60AC6Fb446f621d146B6E1886f002905", "chainId": 137},
            {
                "address": "0xaeF5bbcbFa438519a5ea80B4c7181B4E78d419f2",
                "chainId": 42161
            },
            {"address": "0xa71353bb71dda105d383b02fc2dd172c4d39ef8b", "chainId": 250}
        ],

What does the address field map to in TokenObject? The contract field?

class TokenObject: Object {
    static func generatePrimaryKey(fromContract contract: AlphaWallet.Address, server: RPCServer) -> String {
        return "\(contract.eip55String)-\(server.chainID)"
    }

    @objc dynamic var primaryKey: String = ""
    @objc dynamic var chainId: Int = 0
    @objc dynamic var contract: String = Constants.nullAddress.eip55String
    @objc dynamic var name: String = ""
    @objc dynamic var symbol: String = ""
    @objc dynamic var decimals: Int = 0
    @objc dynamic var value: String = ""
    @objc dynamic var isDisabled: Bool = false
    @objc dynamic var rawType: String = TokenType.erc20.rawValue
    @objc dynamic var shouldDisplay: Bool = true

@hboon
Copy link
Member Author

hboon commented Mar 23, 2022

Yes. And the chainId to RPCServer

@eviltofu
Copy link
Contributor

@hboon are the addresses case sensitive? I see mixed cases and some instances where both cases are used in the same address. 0x0AaB8D...

"contracts": [
            {"address": "0x595832f8fc6bf59c85c527fec3740a1b7a361269", "chainId": 1},
            {"address": "0x0AaB8DC887D34f00D50E19aee48371a941390d14", "chainId": 137},
            {
                "address": "0x4e91F2AF1ee0F84B529478f19794F5AFD423e4A6",
                "chainId": 42161
            }
        ],
        "group": "Assets"

@eviltofu
Copy link
Contributor

@hboon some entries have no group property.

"contracts": [
      {"address": "0x4AaC461C86aBfA71e9d00d9a2cde8d74E4E1aeEa", "chainId": 1},
      {
        "address": "0x14B1f37c46ECf29C9657585DF0Dd7CEe1eC7C569",
        "chainId": 43114
      }
    ]

Where does a token matching this go? Assets?

@hboon
Copy link
Member Author

hboon commented Mar 25, 2022

@hboon are the addresses case sensitive? I see mixed cases and some instances where both cases are used in the same address. 0x0AaB8D...

Check this out: https://eips.ethereum.org/EIPS/eip-55

The 1 specific mix of uppercase and lowercase for Ethereum addresses are a checksum for the address itself.

They are the same address, but when we print them out in the app, we always use the EIP55 version (look for eip55String)

@hboon
Copy link
Member Author

hboon commented Mar 25, 2022

@hboon some entries have no group property.

"contracts": [
{"address": "0x4AaC461C86aBfA71e9d00d9a2cde8d74E4E1aeEa", "chainId": 1},
{
"address": "0x14B1f37c46ECf29C9657585DF0Dd7CEe1eC7C569",
"chainId": 43114
}
]
Where does a token matching this go? Assets?

Yes, refer to #3581 (comment)

@eviltofu
Copy link
Contributor

eviltofu commented Mar 25, 2022

@hboon for the following entries, both first contracts 0x03ab458634910AaD20eF5f1C8ee96F1D6ac54919 have the same address and chainId. How should this be resolved? The file is taken from here. Is this an Asset or a Defi? Or is it both?

 {
        "contracts": [
            {"address": "0x03ab458634910AaD20eF5f1C8ee96F1D6ac54919", "chainId": 1},
            {"address": "0x7FB688CCf682d58f86D7e38e03f9D22e7705448B", "chainId": 10}
        ]
    },

and

    {
        "contracts": [
            {"address": "0x03ab458634910aad20ef5f1c8ee96f1d6ac54919", "chainId": 1},
            {"address": "0x00e5646f60AC6Fb446f621d146B6E1886f002905", "chainId": 137},
            {
                "address": "0xaeF5bbcbFa438519a5ea80B4c7181B4E78d419f2",
                "chainId": 42161
            },
            {"address": "0xa71353bb71dda105d383b02fc2dd172c4d39ef8b", "chainId": 250}
        ],
        "group": "DeFi"
    },

@hboon
Copy link
Member Author

hboon commented Mar 25, 2022

@eviltofu looks like a bug somewhere, so you can drop the group in such cases. Can you help in to https://github.com/AlphaWallet/tokens/issues/1 and @ foxgem to describe this?

@eviltofu
Copy link
Contributor

@eviltofu looks like a bug somewhere, so you can drop the group in such cases. Can you help in to AlphaWallet/tokens#1 and @ foxgem to describe this?

I will only use the first occurrence and drop any future occurrences? Or will group-less entries have lower priority over entries with groups?

@hboon
Copy link
Member Author

hboon commented Mar 28, 2022

Ignore entire groups that contains tokens that have already appeared in previous groups. Pretending they will be fixed at the source.

@eviltofu
Copy link
Contributor

eviltofu commented Mar 28, 2022

@hboon for Collectibles, it is detected via TokenType?

   var type: TokenType {
        get {
            return TokenType(rawValue: rawType)!
        }
        set {
            rawType = newValue.rawValue
        }
    }
enum TokenType: String {
    case nativeCryptocurrency = "ether"
    case erc20 = "ERC20"
    case erc875 = "ERC875"
    case erc721 = "ERC721"
    case erc721ForTickets = "ERC721ForTickets"
    case erc1155 = "ERC1155"

    init(tokenInterfaceType: TokenInterfaceType) {
        switch tokenInterfaceType {
        case .erc20:
            self = .erc20
        case .erc721:
            self = .erc721
        case .erc875:
            self = .erc875
        case .erc1155:
            self = .erc1155
        }
    }

@hboon
Copy link
Member Author

hboon commented Mar 28, 2022

Yes

@eviltofu
Copy link
Contributor

The plan now is
Refactor the code so that instead of having one UITableViewDataSource and UITableViewDelegate that handles displaying all the tabs, we have one object per tab that handles the duties of data source and delegate. So if we want to add more tabs, we just create more classes to handle the tab instead of just extending the one data source/delegate.

@hboon
Copy link
Member Author

hboon commented Mar 29, 2022

I think you might need to have just have 2 of these UITableViewDataSource/UITableViewDelegate pairs because All, Assets, Defi, Governance look and work the same way whereas Collectibles is different instead of 1 pair for each tab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment