-
Notifications
You must be signed in to change notification settings - Fork 14
Conversation
@@ -23,15 +23,10 @@ make install | |||
# Hermes Relayer | |||
# [Hermes](https://hermes.informal.systems/) is a Rust implementation of a relayer for the [Inter-Blockchain Communication (IBC)](https://ibcprotocol.org/) protocol. | |||
# | |||
# In order to use the hermes relayer you will need to check out a specific branch that can be used with interchain-accounts. | |||
# Currently supported by Hermes v0.9.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hermes v0.10.0 contains conditional code which attempts to call the now removed endpoint NegotiateAppVersion
if available, otherwise it fallbacks to logic to choose version based on a PortPrefix which is now incorrect for controller ports (ics27-owner) as it follows the ics-27-1. prefix.
Using hermes v0.9.0 everything works as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. We need to tell ibc-rs team.
@@ -85,7 +85,8 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ | |||
Data: data, | |||
} | |||
|
|||
_, err = k.icaControllerKeeper.TrySendTx(ctx, chanCap, portID, packetData) | |||
timeoutTimestamp := ^uint64(0) >> 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather not use this if there's an alternative. I tried testing with: uint64(ctx.BlockTime().Add(time.Hour).Unix())
but I saw no packets being relayed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can just add a comment in the short term explaining
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
I think Ideally we should be cutting an I would prefer this to point to a release rather than the latest commit off main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 🙏
Closes: #79