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

Hermes resubmits already received packets #1479

Closed
5 tasks done
Tracked by #1562
mircea-c opened this issue Oct 19, 2021 · 2 comments · Fixed by #1542
Closed
5 tasks done
Tracked by #1562

Hermes resubmits already received packets #1479

mircea-c opened this issue Oct 19, 2021 · 2 comments · Fixed by #1542
Assignees
Labels
I: logic Internal: related to the relaying logic I: rpc Internal: related to (g)RPC O: usability Objective: cause to improve the user experience (UX) and ease using the product
Milestone

Comments

@mircea-c
Copy link

mircea-c commented Oct 19, 2021

Crate

ibc-rs

Summary of Bug

When gas estimation fails with packet sequence (142099) already has been received Hermes will submit the transaction using max_gas anyway.

Version

git hash: f328157ae87a1ca90b960e5b8a9004a2d720c629

Steps to Reproduce

Run two instances of hermes start submit one transaction to be relayed. The following log entries are seen:

Oct 19 10:09:17.860 DEBUG ThreadId(42) [cosmoshub-4] simulated gas: Err(
    0: #033[91mGRPC call return error status status: InvalidArgument, message: "failed to execute message; message index: 1: receive packet verification failed: packet sequence (142099) already has been received: invalid packet: invalid request", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc"} }#033[0m
 
 Location:
    #033[35m/usr/local/cargo/registry/src/git.luolix.top-1ecc6299db9ec823/flex-error-0.4.3/src/tracer_impl/eyre.rs#033[0m:#033[35m10#033[0m
 
 Backtrace omitted.
 Run with RUST_BACKTRACE=1 environment variable to display it.
 Run with RUST_BACKTRACE=full to include source snippets.)
 Oct 19 10:09:17.860 ERROR ThreadId(42) [cosmoshub-4] failed to estimate gas, falling back on max gas, error: 
    0: #033[91mGRPC call return error status status: InvalidArgument, message: "failed to execute message; message index: 1: receive packet verification failed: packet sequence (142099) already has been received: invalid packet: invalid request", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc"} }#033[0m
 
 Location:
    #033[35m/usr/local/cargo/registry/src/git.luolix.top-1ecc6299db9ec823/flex-error-0.4.3/src/tracer_impl/eyre.rs#033[0m:#033[35m10#033[0m
 
 Backtrace omitted.
 Run with RUST_BACKTRACE=1 environment variable to display it.
 Run with RUST_BACKTRACE=full to include source snippets.
 Oct 19 10:09:17.872 DEBUG ThreadId(42) [cosmoshub-4] send_tx: broadcast_tx_sync: Response { code: Ok, data: Data([]), log: Log("[]"), hash: transaction::Hash(98BA2278A493254F0DF38734E1B0CADD5253AF4FBC78B10F7C38CB3AF392EF9C) }
 Oct 19 10:09:17.872  INFO ThreadId(455) [Async~>cosmoshub-4] response(s): 1; Ok:98BA2278A493254F0DF38734E1B0CADD5253AF4FBC78B10F7C38CB3AF392EF9C

Acceptance Criteria

When gas estimation fails with this error, Hermes should not submit a redundant transaction.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@adizere adizere added I: logic Internal: related to the relaying logic I: rpc Internal: related to (g)RPC O: usability Objective: cause to improve the user experience (UX) and ease using the product labels Oct 21, 2021
@adizere adizere added this to the 11.2021 milestone Oct 21, 2021
@adizere
Copy link
Member

adizere commented Oct 21, 2021

Approach we'll take: When tx simulation fails in certain cases (such as the one reported here, already has been received: invalid packet) Hermes should trim out some messages out of the transaction before broadcast-ing it.

@ancazamfir
Copy link
Collaborator

I'm not sure the runtime is the right place to deal with this. Maybe we can just return error and the worker retry will re-evaluate the events.
This should become less costly once redundant transactions are accepted by more chains that migrate to ibc-go v1.1.0.

@romac romac self-assigned this Nov 2, 2021
@romac romac added the P-high label Nov 2, 2021
@adizere adizere changed the title Redundant transactions Hermes resubmits already received packets Nov 2, 2021
@adizere adizere assigned ancazamfir and unassigned romac Nov 3, 2021
@romac romac assigned romac and ancazamfir and unassigned ancazamfir Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: logic Internal: related to the relaying logic I: rpc Internal: related to (g)RPC O: usability Objective: cause to improve the user experience (UX) and ease using the product
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants