Upgrade and UpdateClient proposals for Tgrade

confio / poe-contracts

Tgrade PoE smart contracts

Apache License 2.0

25 stars 11 forks source link

Upgrade and UpdateClient proposals for Tgrade #191

Open hashedone opened 2 years ago

hashedone commented 2 years ago

Addresses #142, #143.

I am not perfectly sure if it is what was expected, but this is what I understood - any leads about what I missed are welcome. Besides adding messages to tgrade gov proposals I added an upgrade plan validation - the name cannot be empty, and the height has to be greater or equal to the current height. A height field in the plan is required, as time-based upgrades are deprecated and should never be sent.

maurolacy commented 2 years ago

~~Approved this in haste~~ (already dismissed), but seems some changes are required:

The underlying IBC client update (ibc-go v3.1.0) message has two extra fields.
Not sure 3.1.0 is the right ibc-go version we need to target.
Seems to me the two messages here, Upgrade and ClientUpdate should be removed, and the only needed thing is to update the IbcClientUpdate message with the proper fields, targeting the proper ibc-go version (which we still need to define / agree upon).

hashedone commented 2 years ago

@maurolacy I completely agree it may be the case. I think I would like to have confirmation about it from someone who has an exact understanding of what is needed there as I didn't find proper documentation for that.

hashedone commented 2 years ago

So now there is a question - the old IbcClientUpdate should be completely removed? That is my guess but I am not sure at this point.

maurolacy commented 2 years ago

So now there is a question - the old IbcClientUpdate should be completely removed? That is my guess but I am not sure at this point.

Either updated or removed. See this previous comment https://github.com/confio/poe-contracts/pull/191#issuecomment-1273447599 for reference.