Various bugfixes

[coinop-logan]

Recently I ran into a bug where comparing two addresses for equality would return false even if they were the same address. It turned out that elm-ethereum had been storing some addresses with some uppercase letters, and others with all lowercase. This is my quick fix.

[cmditch]

Implemented changes in my http-events branch, but left out the address changes since I haven't heard much on this front yet.

[francescortiz]

We have created our custom implementation of EthereumAddress in order to avoid problems with casing in the address when we compare them. What we did is to have the address checksum upon creating the EtheremAddress instance from the input string. The reasoning behing computing the checksum on instantiation rather than on visualization/transfer is that since address have to be displayed and transferred checksumed, it seemed appropiate to do it once instead of on every render loop or external transfer of the same.

Basically, it is what you did with some differences:

• addressToString has been renamed to fromString:

str
    |> EthereumAddress.fromString

• addressToString has been renamed to toString:

address
    |> EthereumAddress.toString

• added equal and compare functions
• added encode and decode functions

Here is the code: https://paste.gg/p/anonymous/80299da7c7ba4e3ba2b9b93dc39ec662

Maybe it can be implemented as a independent package called elm-ethereum-address, because you might use addresses even if you don't blockchain directly (as we do).

[cmditch]

Nice @francescortiz, and thanks for the code snippet.

Interesting thought on the separate package. Worth some pondering 🤔 The necessary fixes should be no more than a minor version bump anyways, so I'll do that now at least.

[cmditch]

@francescortiz come to think of it. The reason I chose not to store the all given addresses in their checksummed form was due to computational cost. ethereum_keccak_256 is very expensive. Grabbing something which contains a ton of addresses (e.g. full block, list of logs, contract returning an array of addresses), would take significantly more time if I checksummed every incoming address.

The current implementation stores the address as you give it (uppercase, lowercase, checksummed) as long as it's a valid address - very much like web3.js.

@francescortiz @coinop-logan I'm curious to hear examples of how you've both experienced equality/comparison errors? Can you show some example code, or describe a scenario?

Thanks

[francescortiz]

In my case we had agreed that all address should be stored checksumed, but at some point addresses that are not checksumed got into a config file which, in turn, served as the source for initialization of other parts of the system. Since it is impossible for a human to konw if an address is correctly checksumed, I decided to look for address comparisons and make sure that both address are lowercase before comparing.

Following with what you say, the rendered view can use caching, but that doesn't apply to parsing tons of addresses, so I still believe that there is a need for some kind of normalization because I won't trust any source anymore, but I agree that checksuming is overkill.

[coinop-logan]

I was running into problems with the receiptDecoder hack I showed you the other day. Here is the somewhat ugly code:

You can see I'm toLower'ing in the List.filter. Without it, the function would fail to find the right event log to extract the data I need.

txReceiptToCreatedToastytradeSellId : Address -> Eth.Types.TxReceipt -> Result String BigInt
txReceiptToCreatedToastytradeSellId factoryAddress txReceipt =
    let
        maybeCreateEventData =
            txReceipt.logs
                |> List.filter
                    (\log ->
                        (Eth.Utils.addressToString >> String.toLower) log.address
                            == (Eth.Utils.addressToString >> String.toLower) factoryAddress
                    )
                |> List.head
                |> Maybe.map (\log -> log.data)
    in
    case maybeCreateEventData of
        Just createEventData ->
            createEventData
                -- Remove the '0x' at the beginning
                |> String.dropLeft 2
                -- Take the first 64-char argument
                |> String.left 64
                |> Abi.Decode.fromString Abi.Decode.uint

        Nothing ->
            Err "Can't find a log generated from the given factoryAddress. Are you looking at the wrong transaction?"

I just threw a Debug.log in there--it turns out that the TxReceipt.Log.Address is not checksummed. I guess that points to Infura for the source of the non-checksummed address sneaking in?

[cmditch]

Normalizing to lower case seems to be the best way to go then, as that's what all the JSON RPC responses I've seen are using. (would be interesting to test parity, geth, and other clients directly, as I'm also just using Infura for the tests)

https://github.com/cmditch/elm-ethereum/releases/tag/3.0.2