Abstract height type

[cwgoes]

Closes #439

[fedekunze]

can we rename it to Cmp instead? it's more widely used by various libraries in Go (eg big.Int)

[fedekunze]

actually, I think Cmp should be a function from Height

func (h Height) Cmp(b Height): int {
  if h.epochNumber < b.epochNumber {
    return -1
  } else if h.epochNumber == b.epochNumber {
    if a.epochHeight < b.epochHeight {
      return -1
    } else if h.epochHeight == b.epochHeight {
      return 0
    }
  }
  return 1
}

something in those lines

[fedekunze]

https://golang.org/pkg/math/big/#Int.Cmp

[cwgoes]

Hmm, I'm not a huge fan of returning an int since there are more valid values for int than valid return values for this function. It's fine if we use Cmp to implement this in the Cosmos SDK code, though.

[cwgoes]

Probably makes sense to add basic upgrade handling here as well.

[cwgoes]

We'll have to fix a key & commitment prefix for upgrade handling.

[cwgoes]

This is not yet ready to be merged, I need to check that the fields all match up to the ICS 7 implementation, but want to check that the upgrade method proposed here makes sense, cc @AdityaSripal.

[AdityaSripal]

My main concern is how we would know what the newClientState should be in advance of the upgrade so we can store it in the old chain

[AdityaSripal]

How would we know what the updated client state is going to be before the upgrade, since we don't know the first header.

This could be fixed with the comment here: cosmos/cosmos-sdk#6736 (comment)

Tho we still wouldn't know something like LatestTimestamp uint64 beforehand

[cwgoes]

See below comment.

[AdityaSripal]

The Tendermint ClientState contains things that we would only know after the upgrade happened
Currently it contains a Header
But as pointed out in cosmos/cosmos-sdk#6736 this is unnecessary, but we would still need a field like LatestTimestamp which we don't know ahead of time
So we cannot store it exactly on the old chain before upgrade
However, we could just store with that field zeroed out, and when we go to verify we set the newClientState to zero for verification purposes
and maybe just check that newClientState.Timestamp > clientstate.TimeStamp

[cwgoes]

but we would still need a field like LatestTimestamp which we don't know ahead of time

Hmm, the problem here is that latestTimestamp is required to determine whether a header is or is not within the trusting period. In the case of an upgrade, to preserve the same security model, we still want client updates to fail if they aren't submitted within the trusting period from when the new chain starts. Given that newClientState is expected to be written just when the old version halts, I think it is reasonable to expect an estimate to be put in for latestTimestamp (e.g. an hour into the future, depending on how long the restart is expected to take).

(this should be the genesis time of the new chain)

I'll add an explanatory note about this.

[cwgoes]

Just this one remaining question on client state - cosmos/cosmos-sdk#6736 (comment).