Skip to content

Commit

Permalink
Avoid risky early abort before attempting to send a tx in tx mgr (#11016
Browse files Browse the repository at this point in the history
)

* Warn instead of error if anything goes wrong with tx hash conflict detection

Ignore any other errors from the DELETE, since we're only looking for an edge case--usually there
will be no txhash conflict, and if there is we'll abort anyway on the
INSERT. In some cases aborting before we even try to send the tx could be hurtful (risking tx mgr
to get stuck) and in others it won't matter either way, but in no case would it ever be helpful.

* Document error handling

* Raise Warn level to Error, so a persistent error in DELETE query doesn't go unnoticed
  • Loading branch information
reductionista authored Oct 30, 2023
1 parent e3caf76 commit ef59cf4
Showing 1 changed file with 14 additions and 11 deletions.
25 changes: 14 additions & 11 deletions core/chains/evm/txmgr/evm_tx_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -1486,20 +1486,23 @@ func (o *evmTxStore) UpdateTxUnstartedToInProgress(ctx context.Context, etx *Tx,
// Note: the record of the original abandoned transaction will remain in evm.txes, only the attempt is replaced. (Any receipt
// associated with the abandoned attempt would also be lost, although this shouldn't happen since only unconfirmed transactions
// can be abandoned.)
result, err := tx.Exec(`DELETE FROM evm.tx_attempts a USING evm.txes t
res, err := tx.Exec(`DELETE FROM evm.tx_attempts a USING evm.txes t
WHERE t.id = a.eth_tx_id AND a.hash = $1 AND t.state = $2 AND t.error = 'abandoned'`,
attempt.Hash, txmgr.TxFatalError,
)
if err == nil {
count, err := result.RowsAffected()
if err != nil {
return pkgerrors.Wrap(err, "UpdateTxUnstartedToInProgress failed to get rows affected")
}
if count > 0 {
o.logger.Debugf("Replacing abandoned tx with tx hash %s with tx_id=%d with identical tx hash", attempt.Hash, attempt.TxID)
}
} else {
return pkgerrors.Wrap(err, "UpdateTxUnstartedToInProgress failed to delete abandoned transactions")

if err != nil {
// If the DELETE fails, we don't want to abort before at least attempting the INSERT. tx hash conflicts with
// abandoned transactions can only happen after a nonce reset. If the node is operating normally but there is
// some unexpected issue with the DELETE query, blocking the txmgr from sending transactions would be risky
// and could potentially get the node stuck. If the INSERT is going to succeed then we definitely want to continue.
// And even if the INSERT fails, an error message showing the txmgr is having trouble inserting tx's in the db may be
// easier to understand quickly if there is a problem with the node.
o.logger.Errorw("Ignoring unexpected db error while checking for txhash conflict", "err", err)
} else if rows, err := res.RowsAffected(); err != nil {
o.logger.Errorw("Ignoring unexpected db error reading rows affected while checking for txhash conflict", "err", err)
} else if rows > 0 {
o.logger.Debugf("Replacing abandoned tx with tx hash %s with tx_id=%d with identical tx hash", attempt.Hash, attempt.TxID)
}

var dbAttempt DbEthTxAttempt
Expand Down

0 comments on commit ef59cf4

Please sign in to comment.