Skip to content

feat: txmgr return ErrReverted if confirmed tx reverts#2338

Merged
mslipper merged 1 commit intoethereum-optimism:developfrom
cfromknecht:bss-core-txmgr-error-revert
Mar 17, 2022
Merged

feat: txmgr return ErrReverted if confirmed tx reverts#2338
mslipper merged 1 commit intoethereum-optimism:developfrom
cfromknecht:bss-core-txmgr-error-revert

Conversation

@cfromknecht
Copy link
Contributor

Description
Currently the txmgr treats confirmation of a transaction as successful
if it achieves the proper confirmation depth, however it fails to
acknowledge the possibility that the transaction reverts. Thus to an
external caller the transaction silently succeeds.

This commit now inspects the receipt and returns an ErrReverted failure
in the event that receipt.Status is not 1. This allows higher level
applications to add logging/telemetry or take action on these events.

Metadata

  • Fixes ENG-2069

@cfromknecht cfromknecht requested a review from tynes as a code owner March 17, 2022 19:36
@changeset-bot
Copy link

changeset-bot bot commented Mar 17, 2022

🦋 Changeset detected

Latest commit: 6856b21

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@eth-optimism/batch-submitter-service Patch
@eth-optimism/teleportr Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2022

Codecov Report

Merging #2338 (16d3910) into develop (99021e2) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2338   +/-   ##
========================================
  Coverage    80.14%   80.14%           
========================================
  Files           77       77           
  Lines         2458     2458           
  Branches       450      450           
========================================
  Hits          1970     1970           
  Misses         488      488           
Flag Coverage Δ
contracts 99.29% <ø> (ø)
core-utils 86.77% <ø> (ø)
data-transport-layer 49.72% <ø> (ø)
sdk 55.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99021e2...16d3910. Read the comment docs.

Currently the txmgr treats confirmation of a transaction as successful
if it achieves the proper confirmation depth, however it fails to
acknowledge the possibility that the transaction reverts. Thus to an
external caller the transaction silently succeeds.

This commit now inspects the receipt and returns an ErrReverted failure
in the event that receipt.Status is not 1. This allows higher level
applications to add logging/telemetry or take action on these events.
@cfromknecht cfromknecht force-pushed the bss-core-txmgr-error-revert branch from 16d3910 to 6856b21 Compare March 17, 2022 19:55
sendTx := func(ctx context.Context, tx *types.Transaction) error {
if gasPricer.shouldMine(tx.GasFeeCap()) {
txHash := tx.Hash()
h.backend.mineWithStatus(&txHash, tx.GasFeeCap(), true)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see here the true is being passed through which will set the reverted status on the receipt and below its asserted that the reverted error is returned after sending the transaction

@mslipper mslipper merged commit 4bc2c01 into ethereum-optimism:develop Mar 17, 2022
@cfromknecht cfromknecht deleted the bss-core-txmgr-error-revert branch March 18, 2022 16:17
theochap pushed a commit that referenced this pull request Dec 10, 2025
Fixes #2338

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>
theochap pushed a commit that referenced this pull request Jan 14, 2026
Fixes #2338

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>
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.

4 participants