fix: add relative timeout support for localhost clients using the transfer CLI client #3587
Conversation
| } | ||
|
|
||
| var consensusState exported.ConsensusState | ||
| if clientState.ClientType() != exported.Localhost { |
There was a problem hiding this comment.
I wonder if this command works for solo machines?
There was a problem hiding this comment.
Could we avoid checking explicitly for Localhost here? So that we don't stumble upon this problem again for future light clients that do not hold consensus state.
There was a problem hiding this comment.
I wonder if this command works for solo machines?
I think it should.
Could we avoid checking explicitly for Localhost here?
We could and just use local clock time as the standard. But I'm happy to defer that til later, I'll open an issue for it.
But I think we should just proceed with this approach and backport for 7.1. I don't see many future client implementations not having the concept of consensus states, I think localhost is an outlier
There was a problem hiding this comment.
Solo machines don't set consensus states so it shouldn't work? This appears to be true already so happy to see it fixed in #3594
| } | ||
|
|
||
| var consensusState exported.ConsensusState | ||
| if clientState.ClientType() != exported.Localhost { |
There was a problem hiding this comment.
Could we avoid checking explicitly for Localhost here? So that we don't stumble upon this problem again for future light clients that do not hold consensus state.
Co-authored-by: colin axnér <[email protected]>
…ror adaptation for another PR
| } | ||
|
|
||
| var consensusState exported.ConsensusState | ||
| if clientState.ClientType() != exported.Localhost { |
There was a problem hiding this comment.
Solo machines don't set consensus states so it shouldn't work? This appears to be true already so happy to see it fixed in #3594
…nsfer CLI client (#3587) * updating incorrect error return in 04-channel SendPacket * support localhost transfers using relative timeouts via the CLI client * Update modules/apps/transfer/client/cli/tx.go Co-authored-by: colin axnér <[email protected]> * optimise client consensus state query as per review suggestion, rm error adaptation for another PR --------- Co-authored-by: colin axnér <[email protected]> (cherry picked from commit df2841d)
…nsfer CLI client (#3587) (#3690) * updating incorrect error return in 04-channel SendPacket * support localhost transfers using relative timeouts via the CLI client * Update modules/apps/transfer/client/cli/tx.go Co-authored-by: colin axnér <[email protected]> * optimise client consensus state query as per review suggestion, rm error adaptation for another PR --------- Co-authored-by: colin axnér <[email protected]> (cherry picked from commit df2841d) Co-authored-by: Damian Nolan <[email protected]>
Description
This PR adds a little bit more code to the CLI handler in order to accommodate localhost clients.
I've implemented it as so to avoid breaking APIs which will allow it be backported for
v7.1.0Updates incorrect error return in 04-channelfix(statemachine)!: update error returned in 04-channel SendPacket #3593SendPacketcloses: #3568
Commit Message / Changelog Entry
fix: add relative timeout support for localhost clients using the transfer CLI clientsee the guidelines for commit messages. (view raw markdown for examples)
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.