Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add relative timeout support for localhost clients using the transfer CLI client #3587

Merged
merged 9 commits into from
May 29, 2023
44 changes: 37 additions & 7 deletions modules/apps/transfer/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ import (
"github.com/spf13/cobra"

"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
clientutils "github.com/cosmos/ibc-go/v7/modules/core/02-client/client/utils"
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
channelutils "github.com/cosmos/ibc-go/v7/modules/core/04-channel/client/utils"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
)

const (
Expand Down Expand Up @@ -83,26 +85,54 @@ corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
}

// if the timeouts are not absolute, retrieve latest block height and block timestamp
// for the consensus state connected to the destination port/channel
// for the consensus state connected to the destination port/channel.
// localhost clients must rely solely on local clock time in order to use relative timestamps.
if !absoluteTimeouts {
consensusState, height, _, err := channelutils.QueryLatestConsensusState(clientCtx, srcPort, srcChannel)
clientRes, err := channelutils.QueryChannelClientState(clientCtx, srcPort, srcChannel, false)
if err != nil {
return err
}

var clientState exported.ClientState
if err := clientCtx.InterfaceRegistry.UnpackAny(clientRes.IdentifiedClientState.ClientState, &clientState); err != nil {
return err
}

clientHeight, ok := clientState.GetLatestHeight().(clienttypes.Height)
if !ok {
return fmt.Errorf("invalid height type. expected type: %T, got: %T", clienttypes.Height{}, clientState.GetLatestHeight())
}

var consensusState exported.ConsensusState
if clientState.ClientType() != exported.Localhost {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this command works for solo machines?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

consensusStateRes, err := clientutils.QueryConsensusState(clientCtx, clientRes.IdentifiedClientState.ClientId, clientHeight, false, true)
if err != nil {
return err
}

if err := clientCtx.InterfaceRegistry.UnpackAny(consensusStateRes.ConsensusState, &consensusState); err != nil {
return err
}
}

if !timeoutHeight.IsZero() {
absoluteHeight := height
absoluteHeight := clientHeight
absoluteHeight.RevisionNumber += timeoutHeight.RevisionNumber
absoluteHeight.RevisionHeight += timeoutHeight.RevisionHeight
timeoutHeight = absoluteHeight
}

// use local clock time as reference time if it is later than the
// consensus state timestamp of the counterparty chain, otherwise
// still use consensus state timestamp as reference.
// for localhost clients local clock time is always used.
if timeoutTimestamp != 0 {
// use local clock time as reference time if it is later than the
// consensus state timestamp of the counter party chain, otherwise
// still use consensus state timestamp as reference
var consensusStateTimestamp uint64
if consensusState != nil {
consensusStateTimestamp = consensusState.GetTimestamp()
}

now := time.Now().UnixNano()
consensusStateTimestamp := consensusState.GetTimestamp()
if now > 0 {
now := uint64(now)
if now > consensusStateTimestamp {
Expand Down