Propagate deliver-tx error for all messages in a failed Tx

[ancazamfir]

Closes: #2333

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/).

Reviewer checklist:

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

[adizere]

I followed the instructions in the issue but I'm likely doing something wrong, because I can't reproduce the error. Instead of getting deliverTx errors, I'm catching the error at the estimate gas step:

2022-06-28T11:08:53.131609Z ERROR ThreadId(11) send_tx_with_account_sequence_retry{id=ibc-1}:estimate_gas: failed to simulate tx. propagating error to caller: gRPC call failed with status: status: Unknown, message: "failed to execute message; message index: 4: 6998375samoleans is smaller than 9000000samoleans: insufficient funds", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "x-cosmos-block-height": "35000"} }
Error: transfer error: failed while submitting the Transfer message to chain ibc-1: gRPC call failed with status: status: Unknown, message: "failed to execute message; message index: 4: 6998375samoleans is smaller than 9000000samoleans: insufficient funds", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "x-cosmos-block-height": "35000"} }

This happens while I'm running eg.

hermes tx raw ft-transfer ibc-1 ibc-0 transfer channel-0 9000000 -n 15 -o 200 -k wallet1

[ancazamfir]

I followed the instructions in the issue but I'm likely doing something wrong, because I can't reproduce the error. Instead of getting deliverTx errors, I'm catching the error at the estimate gas step:

2022-06-28T11:08:53.131609Z ERROR ThreadId(11) send_tx_with_account_sequence_retry{id=ibc-1}:estimate_gas: failed to simulate tx. propagating error to caller: gRPC call failed with status: status: Unknown, message: "failed to execute message; message index: 4: 6998375samoleans is smaller than 9000000samoleans: insufficient funds", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "x-cosmos-block-height": "35000"} }
Error: transfer error: failed while submitting the Transfer message to chain ibc-1: gRPC call failed with status: status: Unknown, message: "failed to execute message; message index: 4: 6998375samoleans is smaller than 9000000samoleans: insufficient funds", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "x-cosmos-block-height": "35000"} }

This happens while I'm running eg.

hermes tx raw ft-transfer ibc-1 ibc-0 transfer channel-0 9000000 -n 15 -o 200 -k wallet1

Sorry forgot to clarify, you need to inspect the events, e.g.

--- a/relayer/src/transfer.rs
+++ b/relayer/src/transfer.rs
@@ -182,6 +182,7 @@ pub fn build_and_send_transfer_messages<SrcChain: ChainHandle, DstChain: ChainHa
         .send_messages_and_wait_commit(TrackedMsgs::new_static(msgs, "ft-transfer"))
         .map_err(|e| TransferError::submit(packet_src_chain.id(), e))?;

+    dbg!(&events);
     // Check if the chain rejected the transaction
     let result = events
         .iter()

[soareschen]

@romac @ancazamfir I added a new TxStatus field in TxSyncResult so that pending TXs have the status TxStatus::Pending, and that wait_for_block_commits returns when all status become TxStatus::ReceivedResponse. With that the IbcEvent::Empty event can be removed.

I also added a test test_error_events to test the number of error events when the transaction failed. If you copy the same test to master branch you should see the test failed, while on this branch it is fixed.

cargo test -p ibc-integration-test -- test_error_events

I also want to note that there is still one call in Link that returns a single ChainError event. In this case the original message count is lost in translation, so it is not possible to fill in the error events with the original count. This in turn means that the RelaySummary returned would still have only one error event.

If you think that needs to be fixed, we should do it in a separate PR and have this PR merged first.

[romac]

With that the IbcEvent::Empty event can be removed.

Sweet! That's a very nice solution, thanks a lot!

I also want to note that there is still one call in Link that returns a single ChainError event. In this case the original message count is lost in translation, so it is not possible to fill in the error events with the original count. This in turn means that the RelaySummary returned would still have only one error event.

I am not fully sure I understand what's going on here, but let's get this merged already and follow-up on this at a later stage. Tracked in #2374, feel free to update the description.

[romac]

Oh and thanks for the corresponding test 🙏

Concerns have been addressed by the reviewer