Upgrade error handler by dimriou · Pull Request #408 · smartcontractkit/chainlink-evm

dimriou · 2026-04-02T12:53:59Z

This PR upgrades error handler for TXMv2. We no longer throw an error if the handler decides there was no transmission, which results in a better observability. Additionally, if the error handler fails to parse the error, txm will check if the pending nonce increased instead of returning an error, which follows the standard flow.

github-actions · 2026-04-02T12:56:17Z

⚠️ API Diff Results - `github.com/smartcontractkit/chainlink-evm`

⚠️ Breaking Changes (3)

`pkg/txm.(*txmMetrics)` (1)

SetRPCNonce — 🗑️ Removed

`pkg/txm.ErrorHandler` (1)

HandleError — Type changed:

func(
  context.Context, 
  + github.com/smartcontractkit/chainlink-common/pkg/logger.Logger, 
  *github.com/smartcontractkit/chainlink-evm/pkg/txm/types.Transaction, 
  error, 
  TxStore, 
  func(github.com/ethereum/go-ethereum/common.Address, 
  - uint64), 
  - bool
  + uint64)
)
- error
+ (bool, error)

`pkg/txm/clientwrappers/dualbroadcast.(*errorHandler)` (1)

HandleError — Type changed:

func(
  context.Context, 
  + github.com/smartcontractkit/chainlink-common/pkg/logger.Logger, 
  *github.com/smartcontractkit/chainlink-evm/pkg/txm/types.Transaction, 
  error, 
  github.com/smartcontractkit/chainlink-evm/pkg/txm.TxStore, 
  func(github.com/ethereum/go-ethereum/common.Address, 
  - uint64), 
  - bool
  + uint64)
)
- error
+ (bool, error)

📄 View full apidiff report

cll-gg · 2026-04-06T17:00:45Z

+	txErr error,
+	txStore txm.TxStore,
+	setNonce func(common.Address, uint64),
+	isFromBroadcastMethod bool,


Let's remove the unused isFromBroadcastMethod while we're touching this func anyway

cll-gg · 2026-04-06T17:02:04Z

+	txStore txm.TxStore,
+	setNonce func(common.Address, uint64),
+	isFromBroadcastMethod bool,
+) (noTransmission bool, err error) {


Nitpick: I somewhat dislike negations in var names, since it will lead to double negations, which can lead to confusion.

Maybe flip it around and use something like transmissionHappened?

Seems reasonable to me, but the issue with this particular case is that there is guarantee a transmission happened and we can only be sure about the noTransmission. If we flip the name to transmissionHappened or transmissionMayHappened and we check for the inversed result, the reader might assume that the error handler can deterministically returned if a transaction was broadcasted. It feels like noTransmission provides better conviction as to what happened.

Ah I see, so otherwise it would be transmissionMightHaveHappened, which is not great 😅
Let's leave it like this then. Might be worthwhile to add a comment to the code about that we can only be 100% sure a transmission has not happened

I included one on the interface level here: https://github.com/smartcontractkit/chainlink-evm/pull/408/changes#diff-b9fe8b4b005efc03661d7777aebac2846a0ff01a18ffff7d1cb715d6766736f3R60-R61

cll-gg · 2026-04-06T17:02:46Z

 		}
 		setNonce(tx.FromAddress, *tx.Nonce)
-		return fmt.Errorf("transaction with txID: %d marked as fatal", tx.ID)
+		lggr.Infof("transaction with txID: %d marked as fatal", tx.ID)


Can we log the transactionLifecycleID here?

cll-gg · 2026-04-06T17:03:17Z

+			if hErr != nil {
+				return hErr
+			}
+			if noTransmission {


Would it be helpful to add a debug log here?

For now the only ErrorHandler we have implemented is the MetaErrorHandler which already logs a message if noTransmission is true, so I think we already get the information from that.

Copilot

Pull request overview

This PR updates TXMv2’s error-handling flow to improve observability and to fall back to the “standard” pending-nonce verification path when an error cannot be classified by the error handler.

Changes:

Change the txm.ErrorHandler interface to return (noTransmission bool, err error) and accept a logger, enabling non-error fatal handling and logging.
Update Txm.sendTransactionWithError to (a) not fail the call when the handler determines “no transmission”, and (b) check PendingNonceAt when the handler cannot classify the error.
Update dualbroadcast error handler + tests to log fatal marking instead of returning a fatal error, and adjust TXM tests accordingly.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
pkg/txm/txm.go	Updates `ErrorHandler` interface and TXM send flow to use handler result + pending nonce fallback.
pkg/txm/clientwrappers/dualbroadcast/meta_error_handler.go	Implements new handler contract; logs and marks tx fatal without returning an error.
pkg/txm/clientwrappers/dualbroadcast/meta_error_handler_test.go	Updates unit tests for new handler behavior and adds log assertions.
pkg/txm/txm_test.go	Updates flow tests to assert log output and pending-nonce fallback behavior.

Comments suppressed due to low confidence (1)

pkg/txm/txm_test.go:591

Same issue here: require.NotNil(t, IDK, tx.IdempotencyKey) will always pass because IDK is non-nil. This should assert on tx.IdempotencyKey (and preferably compare it to the expected value).

	require.Equal(t, 1, count)
	require.NotNil(t, IDK, tx.IdempotencyKey)
	require.Equal(t, uint16(2), tx.AttemptCount)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-14T14:45:47Z

+		noTransmission, err := errorHandler.HandleError(t.Context(), logger.Test(t), tx, txErr, txStoreManager, setNonce)
+		require.NoError(t, err)
+		require.False(t, noTransmission)
 		_, unconfirmedCount := txStore.FetchUnconfirmedTransactionAtNonceWithCount(nonce)


This subtest no longer returns txErr (it now expects HandleError to return err == nil and noTransmission == false), but the test name still says it does. Rename the test case to match the behavior being asserted to avoid confusion when the test fails in the future.

Upgrade error handler

d34d05d

dimriou force-pushed the oev-1176_upgrade_handler branch from cab58c2 to d34d05d Compare April 2, 2026 13:01

cll-gg reviewed Apr 6, 2026

View reviewed changes

dimriou force-pushed the oev-1176_upgrade_handler branch from 5cb2762 to 156bdfc Compare April 8, 2026 14:08

Address feedback

37483bb

dimriou force-pushed the oev-1176_upgrade_handler branch from 156bdfc to 37483bb Compare April 8, 2026 15:30

dimriou marked this pull request as ready for review April 14, 2026 14:41

dimriou requested a review from a team as a code owner April 14, 2026 14:41

Copilot AI review requested due to automatic review settings April 14, 2026 14:41

dimriou requested a review from a team as a code owner April 14, 2026 14:41

product-security-plaid-production bot requested review from dhaidashenko and mchain0 April 14, 2026 14:41

Copilot started reviewing on behalf of dimriou April 14, 2026 14:41 View session

jmank88 approved these changes Apr 14, 2026

View reviewed changes

Copilot AI reviewed Apr 14, 2026

View reviewed changes

mchain0 approved these changes Apr 14, 2026

View reviewed changes

dimriou merged commit 7d4da8a into develop Apr 15, 2026
39 checks passed

dimriou deleted the oev-1176_upgrade_handler branch April 15, 2026 13:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade error handler#408

Upgrade error handler#408
dimriou merged 2 commits intodevelopfrom
oev-1176_upgrade_handler

dimriou commented Apr 2, 2026

Uh oh!

github-actions bot commented Apr 2, 2026 •

edited

Loading

Uh oh!

cll-gg Apr 6, 2026 •

edited

Loading

Uh oh!

cll-gg Apr 6, 2026

Uh oh!

dimriou Apr 8, 2026

Uh oh!

cll-gg Apr 8, 2026

Uh oh!

dimriou Apr 14, 2026

Uh oh!

cll-gg Apr 14, 2026

Uh oh!

cll-gg Apr 6, 2026

Uh oh!

cll-gg Apr 6, 2026

Uh oh!

dimriou Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

dimriou commented Apr 2, 2026

Uh oh!

github-actions bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ API Diff Results - github.com/smartcontractkit/chainlink-evm

⚠️ Breaking Changes (3)

pkg/txm.(*txmMetrics) (1)

pkg/txm.ErrorHandler (1)

pkg/txm/clientwrappers/dualbroadcast.(*errorHandler) (1)

Uh oh!

cll-gg Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Apr 2, 2026 •

edited

Loading

⚠️ API Diff Results - `github.com/smartcontractkit/chainlink-evm`

`pkg/txm.(*txmMetrics)` (1)

`pkg/txm.ErrorHandler` (1)

`pkg/txm/clientwrappers/dualbroadcast.(*errorHandler)` (1)

cll-gg Apr 6, 2026 •

edited

Loading