Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

tx which failed in deliverTx could deduct fee but can't be found #1045

Closed
yihuang opened this issue Apr 12, 2022 · 2 comments
Closed

tx which failed in deliverTx could deduct fee but can't be found #1045

yihuang opened this issue Apr 12, 2022 · 2 comments
Labels
pinned Pinned issues that won't be closed by stalebot

Comments

@yihuang
Copy link
Contributor

yihuang commented Apr 12, 2022

Proposal: Support receipt for failed transaction

Current behavior:

Currently failed transactions in block are ignored in json-rpc API due to implementation difficulties, ideally we should support them, because those tx also deduct fee and increase nonce from sender, user'll need to discover those tx in json-rpc apis.

The main difficulty was the lack of eth tx hash event for failed tx, so JSON-RPC can't find the cosmos tx by eth tx hash using the /tx_search API, but cosmos-sdk 0.45 supports emitting event even for failed tx in ante handlers. so it's possible to support this now I think.

A common cause for such failed tx is out of block gas limit.

Desired behavior: [What you would like to happen]

Use case: [Why is this important (helps with prioritizing requests)]

Requests may be closed if we're not actively planning to work on them.

@fedekunze fedekunze added the pinned Pinned issues that won't be closed by stalebot label Apr 12, 2022
@yihuang
Copy link
Contributor Author

yihuang commented Apr 20, 2022

There are two kinds of deliverTx failure:

  • failed in ante handler, fee/nonce not changed, ok to ignore.
  • failed in message handler, fee/nonce changed, should not skip. Most of failure cases are already checked before execution, remaining ones are:
    • EnableCreate/EnableCall check, we should move the check into ante handler.
    • exceed block gas limit, we should handle this failure case in RPC api to return a receipt somehow.

@yihuang yihuang changed the title Support receipt for failed transaction tx which failed in deliverTx can't be found Apr 22, 2022
@yihuang yihuang changed the title tx which failed in deliverTx can't be found tx which failed in deliverTx could deduct fee but can't be found Apr 22, 2022
yihuang added a commit to yihuang/ethermint that referenced this issue May 3, 2022
WIP: evmos#1045

Reject tx early in ante handler, avoid deduct user fee for vain.
fedekunze added a commit that referenced this issue May 3, 2022
* Check EnableCreate/EnableCall in ante handler

WIP: #1045

Reject tx early in ante handler, avoid deduct user fee for vain.

* add unit tests

* update changelog

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Federico Kunze Küllmer <federico.kunze94@gmail.com>
yihuang added a commit to yihuang/ethermint that referenced this issue May 24, 2022
WIP: evmos#1045
Solution:
- emit eth tx hash in ante handler
- modify rpc to use it

fix ante handler

support failed tx in receipt

add unit tests

need to patch cosmos-sdk to work

update cosmos-sdk to v0.45.x release branch

fix failed status

fix unit tests

add unit test cases

cleanup dead code

Apply suggestions from code review

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

fix lint

fix review suggestions

fix build

fix gas used of failed tx

add back the redundant events
fedekunze added a commit that referenced this issue May 31, 2022
…ry failed transactions (#1062)

* Emit eth tx hash in ante handler to support query failed transactions

WIP: #1045
Solution:
- emit eth tx hash in ante handler
- modify rpc to use it

fix ante handler

support failed tx in receipt

add unit tests

need to patch cosmos-sdk to work

update cosmos-sdk to v0.45.x release branch

fix failed status

fix unit tests

add unit test cases

cleanup dead code

Apply suggestions from code review

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

fix lint

fix review suggestions

fix build

fix gas used of failed tx

add back the redundant events

* fix get tx by index

* add unit tests for events

* Update rpc/types/events.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* update comments

* refactoring

* Update rpc/namespaces/ethereum/eth/api.go

* fix lint

* Apply suggestions from code review

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
yihuang added a commit to yihuang/ethermint that referenced this issue Jun 6, 2022
…ry failed transactions (evmos#1062)

* Emit eth tx hash in ante handler to support query failed transactions

WIP: evmos#1045
Solution:
- emit eth tx hash in ante handler
- modify rpc to use it

fix ante handler

support failed tx in receipt

add unit tests

need to patch cosmos-sdk to work

update cosmos-sdk to v0.45.x release branch

fix failed status

fix unit tests

add unit test cases

cleanup dead code

Apply suggestions from code review

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

fix lint

fix review suggestions

fix build

fix gas used of failed tx

add back the redundant events

* fix get tx by index

* add unit tests for events

* Update rpc/types/events.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* update comments

* refactoring

* Update rpc/namespaces/ethereum/eth/api.go

* fix lint

* Apply suggestions from code review

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
@yihuang
Copy link
Contributor Author

yihuang commented Jun 15, 2022

fixed by #1062

@yihuang yihuang closed this as completed Jun 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pinned Pinned issues that won't be closed by stalebot
Projects
None yet
Development

No branches or pull requests

2 participants