Update Config Read

[fredo]

I noticed some bugs in the config read command

Removing tokens and chains

We don't have the concept of removing a token and a chain in the read command and state representation (For the whitelist it works). Instead when we remove a token (or a chain), we define a removal by setting all values back to the default values (i.e. updateToken(address=TOKEN_ADDRESS, transferLimit=0, ethInToken=0).

The read command will read the respective TokenUpdated events and simply update the state representation. This results in something like

        "tokens": {
            "USDC": {
                "transfer_limit": 0,
                "eth_in_token": 0
            }
        },

What we actually want is realizing that this is per definition a token removal and thus remove it from the dictionary. Otherwise we would be "spammed" by old removed tokens in the state representation.

If token.symbol() does not exist, use address instead

We recently had the discussion in a different context if we should rely on the existence of token.symbol(). In the other discussion, namely the deployment testing automization, we could rely on it as we will use our own deployed test token.
The background was that token.symbol() is not part of the ERC20 token specification.

I realized that we also rely on the existence of this function in the read process. We actually create our mapping token_addresses from the symbols instead as initially discussed, it being created by hand.
The reasoning for it is valid. When we do an initial state read call, there is no mapping, so it has to come from somewhere. However the current design is a bit flawed. When we create the desired state file we can actually choose any symbol, which will then be non existent in the state file. It's just a nice to have, but it would be great to have a clear source of symbols.

Which leads me to the original point. I accidentally uncovered the situation by a fuckup from myself. I used a wrong address as token (it was the request managers address). Obviously the contract doesnt have a symbol() function so the read command crashed. Plus it will never be fixable as all events will be replayed. In order to avoid crashing the read command by the absence of symbol() (which can happen for tokens as well), I would simply take the address as the symbol. It looks kinda stupid to resolve the address to address in token_addresses but for now it works out of the box.
If we want to optimize in the future and once the mapping origin is clear, we could make it smarter and omit adding to the token_addresses if key == value.

Removing assert

                address = config.token_addresses.get(symbol)
                if address is None:
                    config.token_addresses[symbol] = event.token_address
                else:
                    assert address == event.token_address

I believe that this check is really unnecessary. As pointed out in the commit message and in the section above, the token_addresses mapping in the state file is generated automatically from token.symbol() anyways. Additionally, it is not affected by the symbols added to the desired state file upon adding a token.
The only way this assert would be useful, would be if an existing state file would be manipulated and an address of an existing token would be changed by hand. This is anyway not intended and that's why we have the checksum in order to make sure to never work on manipulated state files.

Therefore, we can skip the check and simply overwrite the key, same as we do for the token mapping in the command above.

Still missing:

• test that adding and removing a token result in empty token_addresses and tokens
• Remove chain
• test removal of chain

[istankovic]

Apart from the first call being

updateToken(address, 0, 0)

is there any other case where these entries may not exist?

[fredo]

Sure, since the above code piece is the production code, I could simply do this call on chain and the code would break.