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

Add code space information in unknown SDK error #2268

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Conversation

soareschen
Copy link
Contributor

@soareschen soareschen commented Jun 6, 2022

Description

Follow up on #2266 to improve the error diagnostic for unknown errors coming from Cosmos SDK by including the code space for the error. This should help us identify the source of error in Cosmos SDK or ibc-go, and better format the error message in Hermes.


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@soareschen
Copy link
Contributor Author

It seems like the type tendermint_rpc::endpoint::broadcast::tx_sync::Response do not contain any code space information to help us correctly identify where the error code is coming from. The most common error we get inside integration tests looks like:

ERROR send_tx_with_account_sequence_retry{id=ibc-1-9e762a0c}: ibc_relayer::chain::cosmos::retry: broadcast_tx_sync: Response { code: Err(22), data: Data([]), log: Log("packet messages are redundant"), hash: transaction::Hash(053E3D4023EB1A90F3ADAC61FBAA58AA978804F9B5C662C080FD86270082125F) }: diagnostic: unknown TX sync response error: 22

The error seems to come from the ibc-go channel module. But without the code space information, it is not sufficient to determine if the error code 22 is really from the "channel" code space.

@adizere
Copy link
Member

adizere commented Jun 7, 2022

The most common error we get inside integration tests looks like:
ERROR send_tx_with_account_sequence_retry{id=ibc-1-9e762a0c}: ibc_relayer::chain::cosmos::retry: broadcast_tx_sync: Response { code: Err(22), data: Data([]), log: Log("packet messages are redundant"), hash: transaction::Hash(053E3D4023EB1A90F3ADAC61FBAA58AA978804F9B5C662C080FD86270082125F) }: diagnostic: unknown TX sync response error: 22

FYI this is the bug that @seanchen1991 is trying to tackle here: #2256

@adizere adizere merged commit 323a56a into master Jun 13, 2022
@adizere adizere deleted the soares/sdk-errors branch June 13, 2022 08:11
seunlanlege pushed a commit to ComposableFi/ibc-rs that referenced this pull request Jun 20, 2022
* Add codespace information in unknown SDK error (informalsystems#2268)

* ICS20 API improvements (informalsystems#2280)

* Remove `Debug` and `'static` requirements on Module trait

* Manually implement Debug for `MockRouter`

* Remove `Ics20Reader` supertrait `PortReader`

* Use primitive_types::U256 instead of uint::construct_uint!()

* Impl serde for Amount

* Add .changelog entries

* Fix relayer Dockerfile for M1 Mac (informalsystems#2286)

Otherwise library not found error will be encountered
Ref: nodejs/help#3239

* Bump uuid from 1.1.1 to 1.1.2 (informalsystems#2289)

Bumps [uuid](https://github.com/uuid-rs/uuid) from 1.1.1 to 1.1.2.
- [Release notes](https://github.com/uuid-rs/uuid/releases)
- [Commits](uuid-rs/uuid@1.1.1...1.1.2)

---
updated-dependencies:
- dependency-name: uuid
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump hdpath from 0.6.0 to 0.6.1 (informalsystems#2292)

Bumps [hdpath](https://github.com/emeraldpay/hdpath-rs) from 0.6.0 to 0.6.1.
- [Release notes](https://github.com/emeraldpay/hdpath-rs/releases)
- [Commits](https://github.com/emeraldpay/hdpath-rs/commits)

---
updated-dependencies:
- dependency-name: hdpath
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump clap from 3.1.18 to 3.2.1 (informalsystems#2291)

* Bump semver from 1.0.9 to 1.0.10 (informalsystems#2295)

* Bump http from 0.2.7 to 0.2.8 (informalsystems#2296)

* Bump tracing from 0.1.34 to 0.1.35 (informalsystems#2290)

* Hs/2210 - Fixed the variable TM to point to GAIAD_BINARY (informalsystems#2297)

* Fixed variable TM

* change log entry and lib-gm version updated to v0.1.3

Co-authored-by: Harveen Singh <[email protected]>

* Bump clap_complete from 3.1.4 to 3.2.1 (informalsystems#2288)

* Bump clap_complete from 3.1.4 to 3.2.1

Bumps [clap_complete](https://github.com/clap-rs/clap) from 3.1.4 to 3.2.1.
- [Release notes](https://github.com/clap-rs/clap/releases)
- [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md)
- [Commits](clap-rs/clap@clap_complete-v3.1.4...clap_complete-v3.2.1)

---
updated-dependencies:
- dependency-name: clap_complete
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Bump clap dependency to 3.2.4

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Mikhail Zabaluev <[email protected]>

* Fix recv packet handler incorrectly using dest port/chan to get receipt/next_seq_recv (informalsystems#2294)

* Use packet destination port/channel in recv_packet handler where appropriate

* Add .changelog entry

* Address review feedback

* Use enum for recv-packet results

* Improve RecvPacketResult

* Ignore acc seq mismath when expected < got (informalsystems#2298)

* Ignore acc seq mismath when expected < got

* Address review comments

* Changelog entry

* Add support for fetching & parsing the Tendermint version of a chain (informalsystems#2302)

* Fix for informalsystems#2301

* changelog

* changelog broken link

* KV pairs in log h/t Mikhail

* update dependencies to master branch

* update tendermint dependency

Co-authored-by: Soares Chen <[email protected]>
Co-authored-by: Shoaib Ahmed <[email protected]>
Co-authored-by: PikachuEXE <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: harveenSingh <[email protected]>
Co-authored-by: Harveen Singh <[email protected]>
Co-authored-by: Mikhail Zabaluev <[email protected]>
Co-authored-by: Anca Zamfir <[email protected]>
Co-authored-by: Adi Seredinschi <[email protected]>
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants