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

tx raw upgrade-chain gives 'Failed Tx: no confirmation' error #1288

Closed
5 tasks
Tracked by #1209
hu55a1n1 opened this issue Aug 16, 2021 · 8 comments · Fixed by #1979
Closed
5 tasks
Tracked by #1209

tx raw upgrade-chain gives 'Failed Tx: no confirmation' error #1288

hu55a1n1 opened this issue Aug 16, 2021 · 8 comments · Fixed by #1979
Assignees
Labels
A: bug Admin: something isn't working A: low-priority Admin: low priority / non urgent issue, expect longer wait time for PR reviews I: CLI Internal: related to the relayer's CLI O: usability Objective: cause to improve the user experience (UX) and ease using the product
Milestone

Comments

@hu55a1n1
Copy link
Member

hu55a1n1 commented Aug 16, 2021

Crate

relayer

Summary of Bug

Step 3 of the Hermes guide's testing client upgrade procedure gives a Failed Tx: no confirmation error, although the tx itself succeeds ->

$ hermes tx raw upgrade-chain ibc-0 ibc-1 07-tendermint-0 10000000 300
Aug 13 21:06:49.881 DEBUG [ibc-0] waiting for commit of block(s) 3EB2DF97AB3D2FC94918F5FC7969286E357722DECDDD65A24DDEAEE970447E43
Error: 
   0: upgrade chain error
   1: failed while submitting the Transfer message to chain ibc-0
   2: Failed Tx: no confirmation

Location:
   /home/hussaini/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/flex-error-0.4.2/src/tracer_impl/eyre.rs:10

Backtrace omitted.

Apparently, this is because the transaction doesn't result in the creation of events and Hermes interprets this as a 'no confirmation' error.

Version

v0.6.2

Steps to Reproduce

Run steps 1 to 3 of the Hermes guide's testing client upgrade.

Acceptance Criteria

Hermes is able to detect that the transaction succeeded and prints a success message.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@adizere adizere added A: bug Admin: something isn't working O: usability Objective: cause to improve the user experience (UX) and ease using the product labels Sep 6, 2021
@adizere adizere added this to the 10.2021 milestone Sep 6, 2021
@adizere adizere modified the milestones: 10.2021, 12.2021 Sep 30, 2021
@adizere adizere modified the milestones: 12.2021, v0.8.4 Nov 3, 2021
@adizere adizere added the I: CLI Internal: related to the relayer's CLI label Dec 25, 2021
@adizere adizere added the A: low-priority Admin: low priority / non urgent issue, expect longer wait time for PR reviews label Jan 21, 2022
@romac romac modified the milestones: v0.11.1, v0.12.0 Feb 2, 2022
@mzabaluev
Copy link
Contributor

It looks like one of these ABCI events from a tx_search response ought to be translated to an IBC event at https://github.com/informalsystems/ibc-rs/blob/02f2594a603b97acf25f60a13228fa038336d79b/modules/src/events.rs#L323:

        [
            Event {
                type_str: "transfer",
                attributes: [
                    Tag {
                        key: Key(
                            "recipient",
                        ),
                        value: Value(
                            "cosmos17xpfvakm2amg962yls6f84z3kell8c5lserqta",
                        ),
                    },
                    Tag {
                        key: Key(
                            "sender",
                        ),
                        value: Value(
                            "cosmos1dpfklknwtudyfea7f9fzps5g9795vxu572m2ma",
                        ),
                    },
                    Tag {
                        key: Key(
                            "amount",
                        ),
                        value: Value(
                            "293stake",
                        ),
                    },
                ],
            },
            Event {
                type_str: "message",
                attributes: [
                    Tag {
                        key: Key(
                            "sender",
                        ),
                        value: Value(
                            "cosmos1dpfklknwtudyfea7f9fzps5g9795vxu572m2ma",
                        ),
                    },
                ],
            },
            Event {
                type_str: "message",
                attributes: [
                    Tag {
                        key: Key(
                            "action",
                        ),
                        value: Value(
                            "submit_proposal",
                        ),
                    },
                ],
            },
            Event {
                type_str: "submit_proposal",
                attributes: [
                    Tag {
                        key: Key(
                            "proposal_id",
                        ),
                        value: Value(
                            "1",
                        ),
                    },
                ],
            },
            Event {
                type_str: "transfer",
                attributes: [
                    Tag {
                        key: Key(
                            "recipient",
                        ),
                        value: Value(
                            "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn",
                        ),
                    },
                    Tag {
                        key: Key(
                            "sender",
                        ),
                        value: Value(
                            "cosmos1dpfklknwtudyfea7f9fzps5g9795vxu572m2ma",
                        ),
                    },
                    Tag {
                        key: Key(
                            "amount",
                        ),
                        value: Value(
                            "10000000stake",
                        ),
                    },
                ],
            },
            Event {
                type_str: "message",
                attributes: [
                    Tag {
                        key: Key(
                            "sender",
                        ),
                        value: Value(
                            "cosmos1dpfklknwtudyfea7f9fzps5g9795vxu572m2ma",
                        ),
                    },
                ],
            },
            Event {
                type_str: "proposal_deposit",
                attributes: [
                    Tag {
                        key: Key(
                            "amount",
                        ),
                        value: Value(
                            "10000000stake",
                        ),
                    },
                    Tag {
                        key: Key(
                            "proposal_id",
                        ),
                        value: Value(
                            "1",
                        ),
                    },
                ],
            },
            Event {
                type_str: "message",
                attributes: [
                    Tag {
                        key: Key(
                            "module",
                        ),
                        value: Value(
                            "governance",
                        ),
                    },
                    Tag {
                        key: Key(
                            "sender",
                        ),
                        value: Value(
                            "cosmos1dpfklknwtudyfea7f9fzps5g9795vxu572m2ma",
                        ),
                    },
                ],
            },
            Event {
                type_str: "submit_proposal",
                attributes: [
                    Tag {
                        key: Key(
                            "proposal_type",
                        ),
                        value: Value(
                            "SoftwareUpgrade",
                        ),
                    },
                    Tag {
                        key: Key(
                            "voting_period_start",
                        ),
                        value: Value(
                            "1",
                        ),
                    },
                ],
            },
        ]

@adizere
Copy link
Member

adizere commented Feb 18, 2022

Also note that governane proposals belong to the SDK, not the IBC module, cf a remark by @hu55a1n1.

GovernanceProposals have a content field.
https://github.com/informalsystems/ibc-rs/blob/ba43f88d6813fb3fe8a60617d797de238df21955/relayer/src/upgrade_chain.rs#L131

For chain upgrades gov proposals that affect the IBC state, this content field is an IBC type from ibc.core.client.v1
https://github.com/informalsystems/ibc-rs/blob/773e50c31fb17e1b6655a1a1d5b44883c679f229/proto/src/prost/std/ibc.core.client.v1.rs#L57

This is all to say that the event we're trying to extract is probably produced by SDK (not the IBC module) unlike ChannelEvent for instance. If we want to augment the logic in from_tx_response_event we might want to add a GovProposal::try_from_tx(event) match arm. The implementation and logic for GovProposal should ideally reside in an Cosmos-SDK Rust implement, not in the ibc crate. For the moment, however, we have no SDK, so we can just add GovProposal in our ibc crate in some temporary place and add a fixme/todo note.

@mzabaluev
Copy link
Contributor

The semi-reasonable course of action seems to me is to:

  • Add a cosmos::gov::events module (to match the theme of core::ics02_client::events et al.) to define structures pertaining to Cosmos SDK events, and define a Proposal enum with a SoftwareUpgrade { voting_period_start: Height } variant there.
  • Extend IbcEvent with a SubmitProposal(cosmos::gov::events::Proposal) variant and add a Proposal::try_from_tx method call to the chain of event parsing calls in from_tx_response_event.
  • In Proposal::try_from_tx, ignore any "submit_proposal" ABCI event that does not have a "proposal_type" key, and try to translate to the only supported proposal variant if the key's value is "SoftwareUpgrade"

Note that the events captured in #1288 (comment) were produced by gaia 4 and a legacy upgrade proposal was sent. I'll check if gaia 6 has any difference.

@ancazamfir
Copy link
Collaborator

Does this issue affect anything except the user experience in testing client upgrade (i.e. the user will see an error while testing the client upgrade procedure but everything else works fine)? And is the error seen only with tx_confirmation = true?

Not clear if we should complicate the relayer code with handling the governance events as in #1909 .
If it's just for hermes tx raw upgrade-chain.. CLI which is a test command afaik, maybe we can just update the testing instructions and mention that the user should use confirmation false and manually check the tx hash.

@adizere
Copy link
Member

adizere commented Mar 14, 2022

And is the error seen only with tx_confirmation = true?

The tx raw upgrade-chain CLI ignores tx_confirmation = true because it calls directly into send_messages_and_wait_commit, so the CLI blocks waiting for events:
https://github.com/informalsystems/ibc-rs/blob/ba43f88d6813fb3fe8a60617d797de238df21955/relayer/src/upgrade_chain.rs#L144

If it's just for hermes tx raw upgrade-chain.. CLI which is a test command afaik, maybe we can just update the testing instructions

Good point! I will double check with ibc-go team to see if upgrade-chain is expected to be used in production or in extended testing scenarios.

@ancazamfir
Copy link
Collaborator

The tx raw upgrade-chain CLI ignores tx_confirmation = true because it calls directly into send_messages_and_wait_commit, so the CLI blocks waiting for events:

Good point. Could we change to send_messages_and_wait_check_tx?

I will double check with ibc-go team to see if upgrade-chain is expected to be used in production or in extended testing scenarios.

You mean our CLI? It shouldn't be used outside testing scenarios but good idea to check.

@adizere
Copy link
Member

adizere commented Mar 14, 2022

You mean our CLI? It shouldn't be used outside testing scenarios but good idea to check.

Yes, the hermes tx raw upgrade-chain CLI. I have a vague memory that they wanted it for non-dev purposes (I suspect that's why I marked this problem as tracked in #1209) but will double-check so we don't over-invest our effort.

@mzabaluev
Copy link
Contributor

manually check the tx hash.

This is still problematic with the simpler approach now implemented in #1979: the original problem is that no recognized events get retrieved for the transaction query, and the same is true for query tx events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: bug Admin: something isn't working A: low-priority Admin: low priority / non urgent issue, expect longer wait time for PR reviews I: CLI Internal: related to the relayer's CLI O: usability Objective: cause to improve the user experience (UX) and ease using the product
Projects
No open projects
Status: Closed
5 participants