Spike on exercising ICS20 transfers to the Osmosis testnet with Hermes #3043
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hdevalence
had a problem deploying
to
smoke-test
GitHub Actions
Error
hdevalence
force-pushed
the
hermes-osmo-test-5-spike
branch
from
2c439c5
to
ea7d25e
Compare
hdevalence
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
|
Issues we discovered so far:
|
|
Current steps to reproduce the (evolving) test setup: Create a Penumbra devnet off this branchWith this branch checked out, create a new Penumbra devnet: Make note of the chain ID. It's also helpful to set an appropriate tracing filter (this is using The Start Configure HermesFirst, check out the Hermes branch in the draft PR: penumbra-zone/hermes#1 The Hermes branch has a working-copy config file, Edit it so that the ID of the Penumbra chain Next, add keys for Osmosis. Create a text file with the mnemonic in it (cf informalsystems/hermes#3625). It's probably useful to use the mnemonic for Adding keys for Penumbra is not required, because the Penumbra backend doesn't currently sign transactions. If Hermes is configured to use that account, all of the transactions will be visible on the Osmosis testnet explorer on its account page: https://testnet.mintscan.io/osmosis-testnet/account/osmo1kh0fwkdy05yp579d8vczgharkcexfw582zj488 Create a new transfer channelUse Hermes to create a new channel between your local devnet and the Osmosis testnet: This will prompt for confirmation, then build a channel. This part should only need to be done once per devnet and should work: Hermes will then exit. Running HermesTo run Hermes and relay packets between chains, use Hermes may print a supposedly fatal error, like This error is not in fact fatal, because the Penumbra backend doesn't currently need to sign transactions (it doesn't pay fees). NOTE: Because our trusting period is currently set very low (2h), if you stop running Hermes for longer than 2h, the clients will expire and you'll need to start over. We should probably adjust this parameter. Hermes is unfortunately only really usable with At some point in the tracing output, Hermes will print something like This information is useful for making transfers. Transferring from Penumbra to OsmosisTo make an ICS20 transfer from Penumbra to Osmosis, run and then a transfer command like The After this transfer is accepted by Penumbra, Hermes will report an error while submitting to Osmosis: Next StepsIt's hard to get more information about why the proof failed to verify without a running Osmosis node. Two possible directions:
Transferring from Osmosis to PenumbraTransferring from Osmosis to Penumbra requires making an Osmosis transaction. Unfortunately, the Osmosis command line tooling is not very good. The command line tool is bundled together with the node implementation, there is not any specific documentation about making IBC transfers, and the tool reports confusing errors with no context (xref #2284 (comment)). Instead, @jackzampolin pointed out that the Go relayer can do basic cross-chain transactions. To use it, we now need to configure the Go relayer, using the configs in the Penumbra repo: Note that Finally, use The resulting tx hash can be used in the explorer: https://testnet.mintscan.io/osmosis-testnet/txs/67C55AD4FC6855579C2B4B421F8F02B2B2BFE6BA0D5C1553C13BBB7DFFAD781D?height=2720869 NOTE: The Hermes will attempt to relay this packet, which will be rejected by Next Steps
Non-inclusion proof issuesIf the Osmosis->Penumbra transfer packet times out, Hermes will attempt to submit a proof of non-inclusion of the packet back to the Osmosis testnet in order to retrieve the funds. This can't work because of the proof spec issue we're ignoring for the moment, but it also doesn't work because of problems with Penumbra, giving errors like The issue here is that Hermes is requesting a nonexistent proof with key, hoping to get a noninclusion proof, but |
|
I'm suspicious of this line: https://github.com/penumbra-zone/penumbra/blob/hermes-osmo-test-5-spike/crates/core/component/ibc/src/component/proof_verification.rs#L250 Are we proto-encoding the commitment hash? Adding debug statements, and it seems that way: Note: |
|
After changing the parameters to the verification call, we accept the packet. However, we don't accept the transfer: I'm not sure this is ideal, since now we'll have accepted the packet but made the transfer unspendable. What's the correct error semantics here? (cc @avahowell) |
|
That error is because we're not json decoding the packet data here: |
|
After changing the packet decoding, I see: and !!!!!!!!! |
|
One thing I'm noticing also is that the events after getting a transfer don't include anything about a new note: Probably we should change that? We'll also probably want to demote the event-dumping events to |
|
Hmm, this should be logging the new notes? |
Sorry, what I meant was that the new notes aren't emitted by ABCI events, so they wouldn't be picked up by e.g. a block explorer. |
Then: However, it looks like there may be remaining issues. Hermes logs (prior to trying to transfer back): What's going on causing the Also, when submitting the reverse transaction:
|
|
Now we have a proof verification failure trying to verify the Ack: |
|
The proof verification failure was caused by passing wrongly encoded data. Transfers back do seem to work: https://testnet.mintscan.io/osmosis-testnet/txs/896C7E3A2525FAFBFC27BB30F597CEF56CDFFD71A56783BE595FE04CDBF08747 However, Hermes still reports errors: Presumably this indicates there's something wrong with our Acks. |
hdevalence
force-pushed
the
hermes-osmo-test-5-spike
branch
from
8c3f176
to
f5f95e2
Compare
hdevalence
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
|
the lingering issue with IBC acks is resolved now (see: https://testnet.mintscan.io/osmosis-testnet/txs/0ACF4B4EDF255770E6D9A93351B64919DBC614C22C66EF3572B25C75D914E48F?height=2756107); it was caused by us not using similar encoding/decoding logic in ibc events for the |
hdevalence
changed the title
|
Looks like we need to rebase this? |
Many of these methods used our own proto-based convenience methods for encoding typed data, but the IBC data needs to be interpreted by counterparty chains. I'm not 100% sure this is right; we should check this code carefully.
avahowell
force-pushed
the
hermes-osmo-test-5-spike
branch
from
66dd3a6
to
2961d3e
Compare
avahowell
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
avahowell
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Includes #3036
The goal of this PR, together with penumbra-zone/hermes#1 , is to successfully exercise an ICS20 transfer between a Penumbra devnet and the Osmosis testnet, making any necessary changes to Penumbra and Hermes as needed along the way.
Current status (update as needed, details in comments below):