Change IBCTimeout's Timestamp field to new Uint64

[chipshort]

related to #472 and CosmWasm/cosmwasm#1943 (comment)

[webmaster128]

Hmm, not sure if this is the way. But let's see.

IbcTimeout is used in both directions:

1. Rust -> Go: e.g. IbcMsg::Transfer
2. Go -> Rust: e.g. IbcPacket

On the Go side, 0 is used to represent an unset Timeout (wasmd, ibc-go). This means wasmd would have to convert 0 -> nil as a special case. Then we have two different nulls here that cannot be differentiated by ibc-go (nil and Uint64(0)), so we either need to make it impossible to every have Uint64(0) or have a step in wasmd that merged the two into 0.

Would it be possible to get Timestamp Uint64 to work instead? Not sure how flexible the combination of custom type and annotations is.

[webmaster128]

The whole concept of mapping a Go uint64 to a Rust Option is a bet flawed anyways. What you'd actually want is Option<NonNull> to keep a 1:1 mapping.

[chipshort]

Yes, the mapping is a bit flawed. Timestamp Uint64 would work with timestamp: Timestamp on the rust side. Not very rusty, but at least consistent with the go side.

[chipshort]

Or if we could make the go-gen emit Timestamp Uint64 with omitempty, then that could also work with timestamp: Option<Timestamp> on the rust side, but not sure how to best approach that.
Ofc this still has the flaw that both 0 and None are ways to represent 0 on the go side.

[webmaster128]

Yeah, I think this is pretty much what we want. Get back to the current code but just change uint64 -> Uint64.

Then we have for

Go -> Rust:

In Go, only one zero state exists, which is Uint64(0). Does it understand that to be empty and can omit it? If yes, good, then it becomes None in Rust. If no, we get Some(Timestamp::from_nanos(0)) which is not what we want in Rust.

Rust -> Go:

Option<Timestamp> can be null or "0". "0" becomes Uint64(0). But does Go understand Uint64's default? Can it translate null or unset to Uint64(0)?

[webmaster128]

Hmm, maybe we should not do this in general and instead create a IBCTimeout specific solution that deserialized into a helper type first and then turns nil into Uint64(0) for convenient of our Go users.

[chipshort]

Yes, I thought the same at first, because we probably don't always want null to become 0.
On the other hand, as you already said, that is the default behaviour for Go's uint64, so for Go users it probably makes more sense to always do that conversion.

[webmaster128]

Maybe not so relevant for Option<Timestamp>, but I can easily come up with ideas for Option<Uint64> where None means something different than Some(0). Let's look for a timeout specific solution, which can either be some sort of custom deserializer or just keep the type we currently have.

[chipshort]

If we want to make that distinction, then it could still be made with *Uint64 on the Go side: nil then serializes to null (or skipped if omitempty) and null deserializes to nil, while "0" deserializes to &Uint64(0).
But I am also fine with just keep the version we have right now.

[webmaster128]

I think we want the ability to make the distinction in other places, but not here. So I suggest keeping the current version and close this.

Glad we discussed this though!