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

Improve redundant packet logs #4254

Merged
merged 4 commits into from
Nov 18, 2024
Merged

Conversation

ljoss17
Copy link
Contributor

@ljoss17 ljoss17 commented Nov 18, 2024

Closes: #4252

Description


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/).
    • If guide has been updated, tag GitHub user mircea-c
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

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

@ljoss17 ljoss17 requested a review from romac November 18, 2024 12:38
Code::Err(code) if response.log.contains("packet messages are redundant") => {
info!(
?response,
diagnostic = ?sdk_error_from_tx_sync_error_code(code.into(), estimated_gas),
Copy link
Member

Choose a reason for hiding this comment

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

Won't this just return "unknown tx error" since this error 22 is not handled in sdk_error_from_tx_sync_error_code, and moreover it is not an SDK error but an ibc-go one?

We can maybe just remove the diagnostic flag, and change the message to "all packets in tx have been relayed already, no fees were consumed" or something similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, sorry I missed that.
Fixed it here 28d857b
I kept the diagnostic flag, but set it to the response log and added a code field

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And added the additional information about the fees here c8f7bd9

@ljoss17 ljoss17 marked this pull request as ready for review November 18, 2024 14:45
Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

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

Perfect!

@ljoss17 ljoss17 added this pull request to the merge queue Nov 18, 2024
Merged via the queue into master with commit 282b943 Nov 18, 2024
29 checks passed
@ljoss17 ljoss17 deleted the luca_joss/improve-redundant-packet-log branch November 18, 2024 18:28
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.

packet messages are redundant
2 participants