ics4-add active client check in sendpacket #1051
Conversation
sangier
requested review from
mpoke,
AdityaSripal,
cwgoes,
colin-axner and
angbrav
as code owners
There was a problem hiding this comment.
Thank you, @sangier. I think you could add the Status function to ICS 02, together with the other functions defined for client state (e.g. verifyClientMessage, updateState, etc).
| // Returns the status of a client given its store. | ||
| function Status (client: clientState) { | ||
| if (client.FrozenHeight !== 0) { | ||
| return Frozen | ||
| } | ||
| // Get latest consensus state from clientStore to check for expiry | ||
| consState, err := client.latestClientHeight() | ||
| if err (!== nil) { | ||
| return Unknown | ||
| } | ||
| // Check if Expired | ||
| let expirationTime := consState.Timestamp + client.TrustingPeriod | ||
| if (expirationTime <== now){ | ||
| return Expired | ||
| } | ||
|
|
||
| return Active | ||
| } |
There was a problem hiding this comment.
Since this implementation of Status is specific to Tendermint, I think I would move it to ICS 07.
| connection = provableStore.get(connectionPath(channel.connectionHops[0])) | ||
| abortTransactionUnless(connection !== null) | ||
|
|
||
| client = provableStore.get(clientStatePath(connection.clientIdenfier)) |
There was a problem hiding this comment.
You could do here clientState = queryClientState(connection.clientIdenfier) (queryClientState is defined in ICS 02)
There was a problem hiding this comment.
Gotcha, thank you @crodriguezvega! Please, verify if the latest commit c1637a9 address all of your comments.
| The function is defined as: | ||
|
|
||
| ```typescript | ||
| type Status = (Client) => Void |
There was a problem hiding this comment.
SHouldn't the return value not be Void? Since it is returning a status
There was a problem hiding this comment.
Uh, definitively should not be void. @AdityaSripal would be correct to return bytes or shall we define a type Status = bytes in the Data Structure section and return Status itself?
| @@ -201,6 +201,28 @@ function verifyClientMessage(clientMsg: ClientMessage) { | |||
| } | |||
| ``` | |||
|
|
|||
| ### Retrieve client status | |||
|
|
|||
| Return the status of the wasm client. Status can be either `Active`, `Expired`, `Unknown` or `Frozen`. | |||
There was a problem hiding this comment.
Do we need the Unknown status for the wasm-client? Where it is used?
| @@ -337,6 +337,25 @@ Once misbehaviour is detected, clients SHOULD be frozen so that no future update | |||
| A permissioned entity such as a chain governance system or trusted multi-signature MAY be allowed | |||
| to intervene to unfreeze a frozen client & provide a new correct ClientMessage which updates the client to a valid state. | |||
|
|
|||
| #### Retrieve client status | |||
|
|
|||
| Status is an opaque function defined by a client type to retrieve the current clientState. Status can be either `Active`, `Expired`, `Unknown` or `Frozen`. | |||
There was a problem hiding this comment.
similarly here, If Unknown status is not needed in the wasm-client we can remove it from client-semantics too.
Closes: #555
Add:
Statusfunction (in helper function section) to retrieve the client statussendPacket