From 7539c94753514296fed500fae13d764a75de7cf6 Mon Sep 17 00:00:00 2001 From: amit-momin Date: Wed, 12 Jun 2024 18:14:11 -0500 Subject: [PATCH 01/16] Added a finalizer component that assesses confirmed transactions for finality --- .changeset/itchy-bugs-clean.md | 5 + common/txmgr/confirmer.go | 5 +- common/txmgr/finalizer.go | 235 ++++++++++++++++++ common/txmgr/test_helpers.go | 2 +- common/txmgr/txmgr.go | 18 +- common/txmgr/types/client.go | 2 + common/txmgr/types/mocks/tx_store.go | 76 +++--- common/txmgr/types/tx.go | 12 + common/txmgr/types/tx_store.go | 3 +- core/chains/evm/txmgr/builder.go | 15 +- core/chains/evm/txmgr/client.go | 4 + core/chains/evm/txmgr/confirmer_test.go | 44 ++++ core/chains/evm/txmgr/evm_tx_store.go | 61 +++-- core/chains/evm/txmgr/evm_tx_store_test.go | 104 ++++++-- core/chains/evm/txmgr/finalizer_test.go | 160 ++++++++++++ core/chains/evm/txmgr/mocks/evm_tx_store.go | 76 +++--- core/chains/evm/txmgr/models.go | 3 +- core/chains/evm/txmgr/txmgr_test.go | 43 +++- core/services/vrf/v2/integration_v2_test.go | 2 +- core/services/vrf/v2/listener_v2_test.go | 2 +- .../0245_add_tx_finalized_column.sql | 15 ++ core/web/testdata/body/health.html | 3 + core/web/testdata/body/health.json | 9 + core/web/testdata/body/health.txt | 1 + testdata/scripts/health/multi-chain.txtar | 10 + 25 files changed, 795 insertions(+), 115 deletions(-) create mode 100644 .changeset/itchy-bugs-clean.md create mode 100644 common/txmgr/finalizer.go create mode 100644 core/chains/evm/txmgr/finalizer_test.go create mode 100644 core/store/migrate/migrations/0245_add_tx_finalized_column.sql diff --git a/.changeset/itchy-bugs-clean.md b/.changeset/itchy-bugs-clean.md new file mode 100644 index 00000000000..a09117f4ed9 --- /dev/null +++ b/.changeset/itchy-bugs-clean.md @@ -0,0 +1,5 @@ +--- +"chainlink": minor +--- + +Added a finalizer component to the TXM to mark transactions as finalized #internal diff --git a/common/txmgr/confirmer.go b/common/txmgr/confirmer.go index 294e922c1c0..4c5261d74d7 100644 --- a/common/txmgr/confirmer.go +++ b/common/txmgr/confirmer.go @@ -120,7 +120,7 @@ type Confirmer[ services.StateMachine txStore txmgrtypes.TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE] lggr logger.SugaredLogger - client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] + client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE, HEAD] txmgrtypes.TxAttemptBuilder[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] stuckTxDetector txmgrtypes.StuckTxDetector[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] resumeCallback ResumeCallback @@ -154,7 +154,7 @@ func NewConfirmer[ FEE feetypes.Fee, ]( txStore txmgrtypes.TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE], - client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE], + client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE, HEAD], chainConfig txmgrtypes.ConfirmerChainConfig, feeConfig txmgrtypes.ConfirmerFeeConfig, txConfig txmgrtypes.ConfirmerTransactionsConfig, @@ -1100,6 +1100,7 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) mar "txID", etx.ID, "attemptID", attempt.ID, "nReceipts", len(attempt.Receipts), + "finalized", etx.Finalized, "id", "confirmer", } diff --git a/common/txmgr/finalizer.go b/common/txmgr/finalizer.go new file mode 100644 index 00000000000..dc7bc66db8f --- /dev/null +++ b/common/txmgr/finalizer.go @@ -0,0 +1,235 @@ +package txmgr + +import ( + "context" + "errors" + "fmt" + "math/big" + "sync" + + "github.com/smartcontractkit/chainlink-common/pkg/logger" + "github.com/smartcontractkit/chainlink-common/pkg/services" + "github.com/smartcontractkit/chainlink-common/pkg/utils/mailbox" + feetypes "github.com/smartcontractkit/chainlink/v2/common/fee/types" + txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" + "github.com/smartcontractkit/chainlink/v2/common/types" +) + +type finalizerTxStore[CHAIN_ID types.ID, ADDR types.Hashable, TX_HASH types.Hashable, BLOCK_HASH types.Hashable, SEQ types.Sequence, FEE feetypes.Fee] interface { + FindTransactionsByState(ctx context.Context, state txmgrtypes.TxState, chainID CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) + UpdateTxesFinalized(ctx context.Context, txs []int64, chainId CHAIN_ID) error +} + +type finalizerChainClient[BLOCK_HASH types.Hashable, HEAD types.Head[BLOCK_HASH]] interface { + HeadByHash(ctx context.Context, hash BLOCK_HASH) (HEAD, error) +} + +// Finalizer handles processing new finalized blocks and marking transactions as finalized accordingly in the TXM DB +type Finalizer[CHAIN_ID types.ID, ADDR types.Hashable, TX_HASH types.Hashable, BLOCK_HASH types.Hashable, SEQ types.Sequence, FEE feetypes.Fee, HEAD types.Head[BLOCK_HASH]] struct { + services.StateMachine + lggr logger.SugaredLogger + chainId CHAIN_ID + txStore finalizerTxStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + client finalizerChainClient[BLOCK_HASH, HEAD] + mb *mailbox.Mailbox[HEAD] + stopCh services.StopChan + wg sync.WaitGroup + initSync sync.Mutex + isStarted bool +} + +func NewFinalizer[ + CHAIN_ID types.ID, + ADDR types.Hashable, + TX_HASH types.Hashable, + BLOCK_HASH types.Hashable, + SEQ types.Sequence, + FEE feetypes.Fee, + HEAD types.Head[BLOCK_HASH], +]( + lggr logger.Logger, + chainId CHAIN_ID, + txStore finalizerTxStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], + client finalizerChainClient[BLOCK_HASH, HEAD], +) *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD] { + lggr = logger.Named(lggr, "Finalizer") + return &Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]{ + txStore: txStore, + lggr: logger.Sugared(lggr), + chainId: chainId, + client: client, + mb: mailbox.NewSingle[HEAD](), + } +} + +// Start is a comment to appease the linter +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) Start(ctx context.Context) error { + return f.StartOnce("Finalizer", func() error { + return f.startInternal(ctx) + }) +} + +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) startInternal(_ context.Context) error { + f.initSync.Lock() + defer f.initSync.Unlock() + if f.isStarted { + return errors.New("Finalizer is already started") + } + + f.stopCh = make(chan struct{}) + f.wg = sync.WaitGroup{} + f.wg.Add(1) + go f.runLoop() + f.isStarted = true + return nil +} + +// Close is a comment to appease the linter +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) Close() error { + return f.StopOnce("Finalizer", func() error { + return f.closeInternal() + }) +} + +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) closeInternal() error { + f.initSync.Lock() + defer f.initSync.Unlock() + if !f.isStarted { + return fmt.Errorf("Finalizer is not started: %w", services.ErrAlreadyStopped) + } + close(f.stopCh) + f.wg.Wait() + f.isStarted = false + return nil +} + +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) Name() string { + return f.lggr.Name() +} + +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) HealthReport() map[string]error { + return map[string]error{f.Name(): f.Healthy()} +} + +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) runLoop() { + defer f.wg.Done() + ctx, cancel := f.stopCh.NewCtx() + defer cancel() + for { + select { + case <-f.mb.Notify(): + for { + if ctx.Err() != nil { + return + } + head, exists := f.mb.Retrieve() + if !exists { + break + } + if err := f.ProcessHead(ctx, head); err != nil { + f.lggr.Errorw("Error processing head", "err", err) + f.SvcErrBuffer.Append(err) + continue + } + } + case <-ctx.Done(): + return + } + } +} + +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) ProcessHead(ctx context.Context, head types.Head[BLOCK_HASH]) error { + ctx, cancel := context.WithTimeout(ctx, processHeadTimeout) + defer cancel() + return f.processHead(ctx, head) +} + +// Determines if any confirmed transactions can be marked as finalized by comparing their receipts against the latest finalized block +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) processHead(ctx context.Context, head types.Head[BLOCK_HASH]) error { + latestFinalizedHead := head.LatestFinalizedHead() + // Cannot determine finality without a finalized head for comparison + if latestFinalizedHead == nil || !latestFinalizedHead.IsValid() { + return fmt.Errorf("failed to find latest finalized head in chain") + } + earliestBlockNumInChain := latestFinalizedHead.EarliestHeadInChain().BlockNumber() + f.lggr.Debugw("processing latest finalized head", "block num", latestFinalizedHead.BlockNumber(), "block hash", latestFinalizedHead.BlockHash(), "earliest block num in chain", earliestBlockNumInChain) + + // Retrieve all confirmed transactions, loaded with attempts and receipts + confirmedTxs, err := f.txStore.FindTransactionsByState(ctx, TxConfirmed, f.chainId) + if err != nil { + return fmt.Errorf("failed to retrieve confirmed transactions: %w", err) + } + + var finalizedTxs []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + // Group by block hash transactions whose receipts cannot be validated using the cached heads + receiptBlockHashToTx := make(map[BLOCK_HASH][]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) + // Find transactions with receipt block nums older than the latest finalized block num and block hashes still in chain + for _, tx := range confirmedTxs { + // Only consider transactions not already marked as finalized + if tx.Finalized { + continue + } + receipt := tx.GetReceipt() + if receipt == nil || receipt.IsZero() || receipt.IsUnmined() { + continue + } + // Receipt newer than latest finalized head block num + if receipt.GetBlockNumber().Cmp(big.NewInt(latestFinalizedHead.BlockNumber())) > 0 { + continue + } + // Receipt block num older than earliest head in chain. Validate hash using RPC call later + if receipt.GetBlockNumber().Int64() < earliestBlockNumInChain { + receiptBlockHashToTx[receipt.GetBlockHash()] = append(receiptBlockHashToTx[receipt.GetBlockHash()], tx) + continue + } + blockHashInChain := latestFinalizedHead.HashAtHeight(receipt.GetBlockNumber().Int64()) + // Receipt block hash does not match the block hash in chain. Transaction has been re-org'd out but DB state has not been updated yet + if blockHashInChain.String() != receipt.GetBlockHash().String() { + continue + } + finalizedTxs = append(finalizedTxs, tx) + } + + // Check if block hashes exist for receipts on-chain older than the earliest cached head + // Transactions are grouped by their receipt block hash to minimize the number of RPC calls in case transactions were confirmed in the same block + // This check is only expected to be used in rare cases if there was an issue with the HeadTracker or if the node was down for significant time + var wg sync.WaitGroup + var txMu sync.RWMutex + for receiptBlockHash, txs := range receiptBlockHashToTx { + wg.Add(1) + go func(hash BLOCK_HASH, txs []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { + defer wg.Done() + if head, rpcErr := f.client.HeadByHash(ctx, hash); rpcErr == nil && head.IsValid() { + txMu.Lock() + finalizedTxs = append(finalizedTxs, txs...) + txMu.Unlock() + } + }(receiptBlockHash, txs) + } + wg.Wait() + + etxIDs := f.buildTxIdList(finalizedTxs) + + err = f.txStore.UpdateTxesFinalized(ctx, etxIDs, f.chainId) + if err != nil { + return fmt.Errorf("failed to update transactions as finalized: %w", err) + } + return nil +} + +// Build list of transaction IDs +func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) buildTxIdList(finalizedTxs []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) []int64 { + etxIDs := make([]int64, len(finalizedTxs)) + for i, tx := range finalizedTxs { + receipt := tx.GetReceipt() + f.lggr.Debugw("transaction considered finalized", + "sequence", tx.Sequence, + "fromAddress", tx.FromAddress.String(), + "txHash", receipt.GetTxHash().String(), + "receiptBlockNum", receipt.GetBlockNumber().Int64(), + "receiptBlockHash", receipt.GetBlockHash().String(), + ) + etxIDs[i] = tx.ID + } + return etxIDs +} diff --git a/common/txmgr/test_helpers.go b/common/txmgr/test_helpers.go index 3051e0985d8..ef3866815d5 100644 --- a/common/txmgr/test_helpers.go +++ b/common/txmgr/test_helpers.go @@ -10,7 +10,7 @@ import ( // TEST ONLY FUNCTIONS // these need to be exported for the txmgr tests to continue to work -func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) XXXTestSetClient(client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) { +func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) XXXTestSetClient(client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE, HEAD]) { ec.client = client } diff --git a/common/txmgr/txmgr.go b/common/txmgr/txmgr.go index 9cefa5c15ba..a9cee5fb0c7 100644 --- a/common/txmgr/txmgr.go +++ b/common/txmgr/txmgr.go @@ -110,6 +110,7 @@ type Txm[ broadcaster *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] confirmer *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] tracker *Tracker[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] + finalizer *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD] fwdMgr txmgrtypes.ForwarderManager[ADDR] txAttemptBuilder txmgrtypes.TxAttemptBuilder[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] newErrorClassifier NewErrorClassifier @@ -145,6 +146,7 @@ func NewTxm[ confirmer *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE], resender *Resender[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE], tracker *Tracker[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE], + finalizer *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD], newErrorClassifierFunc NewErrorClassifier, ) *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] { b := Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]{ @@ -167,6 +169,7 @@ func NewTxm[ resender: resender, tracker: tracker, newErrorClassifier: newErrorClassifierFunc, + finalizer: finalizer, } if txCfg.ResendAfterThreshold() <= 0 { @@ -201,6 +204,10 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Start(ctx return fmt.Errorf("Txm: Tracker failed to start: %w", err) } + if err := ms.Start(ctx, b.finalizer); err != nil { + return fmt.Errorf("Txm: Finalizer failed to start: %w", err) + } + b.logger.Info("Txm starting runLoop") b.wg.Add(1) go b.runLoop() @@ -295,6 +302,7 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) HealthRepo services.CopyHealth(report, b.broadcaster.HealthReport()) services.CopyHealth(report, b.confirmer.HealthReport()) services.CopyHealth(report, b.txAttemptBuilder.HealthReport()) + services.CopyHealth(report, b.finalizer.HealthReport()) }) if b.txConfig.ForwardersEnabled() { @@ -417,6 +425,7 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) runLoop() case head := <-b.chHeads: b.confirmer.mb.Deliver(head) b.tracker.mb.Deliver(head.BlockNumber()) + b.finalizer.mb.Deliver(head) case reset := <-b.reset: // This check prevents the weird edge-case where you can select // into this block after chStop has already been closed and the @@ -448,6 +457,10 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) runLoop() if err != nil && (!errors.Is(err, services.ErrAlreadyStopped) || !errors.Is(err, services.ErrCannotStopUnstarted)) { b.logger.Errorw(fmt.Sprintf("Failed to Close Tracker: %v", err), "err", err) } + err = b.finalizer.Close() + if err != nil && (!errors.Is(err, services.ErrAlreadyStopped) || !errors.Is(err, services.ErrCannotStopUnstarted)) { + b.logger.Errorw(fmt.Sprintf("Failed to Close Finalizer: %v", err), "err", err) + } return case <-keysChanged: // This check prevents the weird edge-case where you can select @@ -646,7 +659,10 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) GetTransac // Return unconfirmed for ConfirmedMissingReceipt since a receipt is required to determine if it is finalized return commontypes.Unconfirmed, nil case TxConfirmed: - // TODO: Check for finality and return finalized status + if tx.Finalized { + // Return finalized if tx receipt's block is equal or older than the latest finalized block + return commontypes.Finalized, nil + } // Return unconfirmed if tx receipt's block is newer than the latest finalized block return commontypes.Unconfirmed, nil case TxFatalError: diff --git a/common/txmgr/types/client.go b/common/txmgr/types/client.go index 759b15d6162..ea3ed3af29c 100644 --- a/common/txmgr/types/client.go +++ b/common/txmgr/types/client.go @@ -21,6 +21,7 @@ type TxmClient[ R ChainReceipt[TX_HASH, BLOCK_HASH], SEQ types.Sequence, FEE feetypes.Fee, + HEAD types.Head[BLOCK_HASH], ] interface { ChainClient[CHAIN_ID, ADDR, SEQ] TransactionClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] @@ -30,6 +31,7 @@ type TxmClient[ ctx context.Context, attempts []TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], ) (txReceipt []R, txErr []error, err error) + HeadByHash(ctx context.Context, hash BLOCK_HASH) (HEAD, error) } // TransactionClient contains the methods for building, simulating, broadcasting transactions diff --git a/common/txmgr/types/mocks/tx_store.go b/common/txmgr/types/mocks/tx_store.go index bb8fabf1a71..21e9169bbb1 100644 --- a/common/txmgr/types/mocks/tx_store.go +++ b/common/txmgr/types/mocks/tx_store.go @@ -310,6 +310,36 @@ func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindNextUns return r0, r1 } +// FindTransactionsByState provides a mock function with given fields: ctx, state, chainID +func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindTransactionsByState(ctx context.Context, state txmgrtypes.TxState, chainID CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) { + ret := _m.Called(ctx, state, chainID) + + if len(ret) == 0 { + panic("no return value specified for FindTransactionsByState") + } + + var r0 []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, txmgrtypes.TxState, CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error)); ok { + return rf(ctx, state, chainID) + } + if rf, ok := ret.Get(0).(func(context.Context, txmgrtypes.TxState, CHAIN_ID) []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]); ok { + r0 = rf(ctx, state, chainID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, txmgrtypes.TxState, CHAIN_ID) error); ok { + r1 = rf(ctx, state, chainID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // FindTransactionsConfirmedInBlockRange provides a mock function with given fields: ctx, highBlockNumber, lowBlockNumber, chainID func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindTransactionsConfirmedInBlockRange(ctx context.Context, highBlockNumber int64, lowBlockNumber int64, chainID CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) { ret := _m.Called(ctx, highBlockNumber, lowBlockNumber, chainID) @@ -848,34 +878,6 @@ func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) HasInProgre return r0, r1 } -// IsTxFinalized provides a mock function with given fields: ctx, blockHeight, txID, chainID -func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) IsTxFinalized(ctx context.Context, blockHeight int64, txID int64, chainID CHAIN_ID) (bool, error) { - ret := _m.Called(ctx, blockHeight, txID, chainID) - - if len(ret) == 0 { - panic("no return value specified for IsTxFinalized") - } - - var r0 bool - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, int64, int64, CHAIN_ID) (bool, error)); ok { - return rf(ctx, blockHeight, txID, chainID) - } - if rf, ok := ret.Get(0).(func(context.Context, int64, int64, CHAIN_ID) bool); ok { - r0 = rf(ctx, blockHeight, txID, chainID) - } else { - r0 = ret.Get(0).(bool) - } - - if rf, ok := ret.Get(1).(func(context.Context, int64, int64, CHAIN_ID) error); ok { - r1 = rf(ctx, blockHeight, txID, chainID) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // LoadTxAttempts provides a mock function with given fields: ctx, etx func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) LoadTxAttempts(ctx context.Context, etx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { ret := _m.Called(ctx, etx) @@ -1230,6 +1232,24 @@ func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) UpdateTxUns return r0 } +// UpdateTxesFinalized provides a mock function with given fields: ctx, etxs, chainId +func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) UpdateTxesFinalized(ctx context.Context, etxs []int64, chainId CHAIN_ID) error { + ret := _m.Called(ctx, etxs, chainId) + + if len(ret) == 0 { + panic("no return value specified for UpdateTxesFinalized") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, []int64, CHAIN_ID) error); ok { + r0 = rf(ctx, etxs, chainId) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // UpdateTxsUnconfirmed provides a mock function with given fields: ctx, ids func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) UpdateTxsUnconfirmed(ctx context.Context, ids []int64) error { ret := _m.Called(ctx, ids) diff --git a/common/txmgr/types/tx.go b/common/txmgr/types/tx.go index d2afbd209d8..72c13b450ff 100644 --- a/common/txmgr/types/tx.go +++ b/common/txmgr/types/tx.go @@ -215,6 +215,7 @@ type Tx[ InitialBroadcastAt *time.Time CreatedAt time.Time State TxState + Finalized bool TxAttempts []TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] `json:"-"` // Marshalled TxMeta // Used for additional context around transactions which you want to log @@ -340,6 +341,17 @@ func (e *Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) GetChecker() (Transm return t, nil } +// Returns the transaction's receipt if one exists, otherwise returns nil +func (e *Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) GetReceipt() ChainReceipt[TX_HASH, BLOCK_HASH] { + for _, attempt := range e.TxAttempts { + if len(attempt.Receipts) > 0 { + // Transaction will only have one receipt + return attempt.Receipts[0] + } + } + return nil +} + // Provides error classification to external components in a chain agnostic way // Only exposes the error types that could be set in the transaction error field type ErrorClassifier interface { diff --git a/common/txmgr/types/tx_store.go b/common/txmgr/types/tx_store.go index c102fb5912a..495a6d7fbae 100644 --- a/common/txmgr/types/tx_store.go +++ b/common/txmgr/types/tx_store.go @@ -84,6 +84,7 @@ type TransactionStore[ FindTransactionsConfirmedInBlockRange(ctx context.Context, highBlockNumber, lowBlockNumber int64, chainID CHAIN_ID) (etxs []*Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) FindEarliestUnconfirmedBroadcastTime(ctx context.Context, chainID CHAIN_ID) (null.Time, error) FindEarliestUnconfirmedTxAttemptBlock(ctx context.Context, chainID CHAIN_ID) (null.Int, error) + FindTransactionsByState(ctx context.Context, state TxState, chainID CHAIN_ID) (etxs []*Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) GetTxInProgress(ctx context.Context, fromAddress ADDR) (etx *Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) GetInProgressTxAttempts(ctx context.Context, address ADDR, chainID CHAIN_ID) (attempts []TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) GetAbandonedTransactionsByBatch(ctx context.Context, chainID CHAIN_ID, enabledAddrs []ADDR, offset, limit uint) (txs []*Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) @@ -106,8 +107,8 @@ type TransactionStore[ UpdateTxsUnconfirmed(ctx context.Context, ids []int64) error UpdateTxUnstartedToInProgress(ctx context.Context, etx *Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], attempt *TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error UpdateTxFatalError(ctx context.Context, etx *Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error + UpdateTxesFinalized(ctx context.Context, etxs []int64, chainId CHAIN_ID) error UpdateTxForRebroadcast(ctx context.Context, etx Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], etxAttempt TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error - IsTxFinalized(ctx context.Context, blockHeight int64, txID int64, chainID CHAIN_ID) (finalized bool, err error) } type TxHistoryReaper[CHAIN_ID types.ID] interface { diff --git a/core/chains/evm/txmgr/builder.go b/core/chains/evm/txmgr/builder.go index dcf15a4fa23..20a9d7e2555 100644 --- a/core/chains/evm/txmgr/builder.go +++ b/core/chains/evm/txmgr/builder.go @@ -53,11 +53,12 @@ func NewTxm( evmTracker := NewEvmTracker(txStore, keyStore, chainID, lggr) stuckTxDetector := NewStuckTxDetector(lggr, client.ConfiguredChainID(), chainConfig.ChainType(), fCfg.PriceMax(), txConfig.AutoPurge(), estimator, txStore, client) evmConfirmer := NewEvmConfirmer(txStore, txmClient, txmCfg, feeCfg, txConfig, dbConfig, keyStore, txAttemptBuilder, lggr, stuckTxDetector) + evmFinalizer := NewEvmFinalizer(lggr, client.ConfiguredChainID(), txStore, txmClient) var evmResender *Resender if txConfig.ResendAfterThreshold() > 0 { evmResender = NewEvmResender(lggr, txStore, txmClient, evmTracker, keyStore, txmgr.DefaultResenderPollInterval, chainConfig, txConfig) } - txm = NewEvmTxm(chainID, txmCfg, txConfig, keyStore, lggr, checker, fwdMgr, txAttemptBuilder, txStore, evmBroadcaster, evmConfirmer, evmResender, evmTracker) + txm = NewEvmTxm(chainID, txmCfg, txConfig, keyStore, lggr, checker, fwdMgr, txAttemptBuilder, txStore, evmBroadcaster, evmConfirmer, evmResender, evmTracker, evmFinalizer) return txm, nil } @@ -76,8 +77,9 @@ func NewEvmTxm( confirmer *Confirmer, resender *Resender, tracker *Tracker, + finalizer *Finalizer, ) *Txm { - return txmgr.NewTxm(chainId, cfg, txCfg, keyStore, lggr, checkerFactory, fwdMgr, txAttemptBuilder, txStore, broadcaster, confirmer, resender, tracker, client.NewTxError) + return txmgr.NewTxm(chainId, cfg, txCfg, keyStore, lggr, checkerFactory, fwdMgr, txAttemptBuilder, txStore, broadcaster, confirmer, resender, tracker, finalizer, client.NewTxError) } // NewEvmResender creates a new concrete EvmResender @@ -142,3 +144,12 @@ func NewEvmBroadcaster( nonceTracker := NewNonceTracker(logger, txStore, client) return txmgr.NewBroadcaster(txStore, client, chainConfig, feeConfig, txConfig, listenerConfig, keystore, txAttemptBuilder, nonceTracker, logger, checkerFactory, autoSyncNonce) } + +func NewEvmFinalizer( + logger logger.Logger, + chainId *big.Int, + txStore TransactionStore, + client TxmClient, +) *Finalizer { + return txmgr.NewFinalizer(logger, chainId, txStore, client) +} diff --git a/core/chains/evm/txmgr/client.go b/core/chains/evm/txmgr/client.go index 661a180af50..e995080a260 100644 --- a/core/chains/evm/txmgr/client.go +++ b/core/chains/evm/txmgr/client.go @@ -183,3 +183,7 @@ func (c *evmTxmClient) CallContract(ctx context.Context, a TxAttempt, blockNumbe }, blockNumber) return client.ExtractRPCError(errCall) } + +func (c *evmTxmClient) HeadByHash(ctx context.Context, hash common.Hash) (*evmtypes.Head, error) { + return c.client.HeadByHash(ctx, hash) +} diff --git a/core/chains/evm/txmgr/confirmer_test.go b/core/chains/evm/txmgr/confirmer_test.go index 6b107b222a6..73b4c946cb3 100644 --- a/core/chains/evm/txmgr/confirmer_test.go +++ b/core/chains/evm/txmgr/confirmer_test.go @@ -2828,6 +2828,50 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) { assert.Equal(t, txmgrtypes.TxAttemptBroadcast, attempt.State) assert.Len(t, attempt.Receipts, 1) }) + + t.Run("unconfirms, unfinalizes, and rebroadcasts finalized transactions that have receipts within head height of the chain but not included in the chain", func(t *testing.T) { + nonce := evmtypes.Nonce(8) + broadcast := time.Now() + tx := &txmgr.Tx{ + Sequence: &nonce, + FromAddress: fromAddress, + EncodedPayload: []byte{1, 2, 3}, + State: txmgrcommon.TxConfirmed, + BroadcastAt: &broadcast, + InitialBroadcastAt: &broadcast, + Finalized: true, + } + err := txStore.InsertTx(ctx, tx) + require.NoError(t, err) + etx, err := txStore.FindTxWithAttempts(ctx, tx.ID) + require.NoError(t, err) + attempt := cltest.NewLegacyEthTxAttempt(t, etx.ID) + broadcastBeforeBlockNum := int64(1) + attempt.BroadcastBeforeBlockNum = &broadcastBeforeBlockNum + attempt.State = txmgrtypes.TxAttemptBroadcast + err = txStore.InsertTxAttempt(ctx, &attempt) + require.NoError(t, err) + // Include one within head height but a different block hash + mustInsertEthReceipt(t, txStore, head.Parent.Number, testutils.NewHash(), attempt.Hash) + + ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *types.Transaction) bool { + atx, signErr := txmgr.GetGethSignedTx(attempt.SignedRawTx) + require.NoError(t, signErr) + // Keeps gas price and nonce the same + return atx.GasPrice().Cmp(tx.GasPrice()) == 0 && atx.Nonce() == tx.Nonce() + }), fromAddress).Return(commonclient.Successful, nil).Once() + + // Do the thing + require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), &head)) + + etx, err = txStore.FindTxWithAttempts(ctx, etx.ID) + require.NoError(t, err) + assert.Equal(t, txmgrcommon.TxUnconfirmed, etx.State) + require.Len(t, etx.TxAttempts, 1) + attempt = etx.TxAttempts[0] + assert.Equal(t, txmgrtypes.TxAttemptBroadcast, attempt.State) + assert.Equal(t, false, etx.Finalized) + }) } func TestEthConfirmer_ForceRebroadcast(t *testing.T) { diff --git a/core/chains/evm/txmgr/evm_tx_store.go b/core/chains/evm/txmgr/evm_tx_store.go index fd38eb7a8c9..b7a526a4742 100644 --- a/core/chains/evm/txmgr/evm_tx_store.go +++ b/core/chains/evm/txmgr/evm_tx_store.go @@ -181,6 +181,7 @@ type DbEthTx struct { // InitialBroadcastAt is recorded once, the first ever time this eth_tx is sent CreatedAt time.Time State txmgrtypes.TxState + Finalized bool // Marshalled EvmTxMeta // Used for additional context around transactions which you want to log // at send time. @@ -211,6 +212,7 @@ func (db *DbEthTx) FromTx(tx *Tx) { db.BroadcastAt = tx.BroadcastAt db.CreatedAt = tx.CreatedAt db.State = tx.State + db.Finalized = tx.Finalized db.Meta = tx.Meta db.Subject = tx.Subject db.PipelineTaskRunID = tx.PipelineTaskRunID @@ -245,6 +247,7 @@ func (db DbEthTx) ToTx(tx *Tx) { tx.BroadcastAt = db.BroadcastAt tx.CreatedAt = db.CreatedAt tx.State = db.State + tx.Finalized = db.Finalized tx.Meta = db.Meta tx.Subject = db.Subject tx.PipelineTaskRunID = db.PipelineTaskRunID @@ -529,8 +532,8 @@ func (o *evmTxStore) InsertTx(ctx context.Context, etx *Tx) error { if etx.CreatedAt == (time.Time{}) { etx.CreatedAt = time.Now() } - const insertEthTxSQL = `INSERT INTO evm.txes (nonce, from_address, to_address, encoded_payload, value, gas_limit, error, broadcast_at, initial_broadcast_at, created_at, state, meta, subject, pipeline_task_run_id, min_confirmations, evm_chain_id, transmit_checker, idempotency_key, signal_callback, callback_completed) VALUES ( -:nonce, :from_address, :to_address, :encoded_payload, :value, :gas_limit, :error, :broadcast_at, :initial_broadcast_at, :created_at, :state, :meta, :subject, :pipeline_task_run_id, :min_confirmations, :evm_chain_id, :transmit_checker, :idempotency_key, :signal_callback, :callback_completed + const insertEthTxSQL = `INSERT INTO evm.txes (nonce, from_address, to_address, encoded_payload, value, gas_limit, error, broadcast_at, initial_broadcast_at, created_at, state, meta, subject, pipeline_task_run_id, min_confirmations, evm_chain_id, transmit_checker, idempotency_key, signal_callback, callback_completed, finalized) VALUES ( +:nonce, :from_address, :to_address, :encoded_payload, :value, :gas_limit, :error, :broadcast_at, :initial_broadcast_at, :created_at, :state, :meta, :subject, :pipeline_task_run_id, :min_confirmations, :evm_chain_id, :transmit_checker, :idempotency_key, :signal_callback, :callback_completed, :finalized ) RETURNING *` var dbTx DbEthTx dbTx.FromTx(etx) @@ -1116,11 +1119,13 @@ func updateEthTxAttemptUnbroadcast(ctx context.Context, orm *evmTxStore, attempt return pkgerrors.Wrap(err, "updateEthTxAttemptUnbroadcast failed") } +// Ensure to mark the transaction as not finalized in case there is a finality violation and a "finalized" transaction +// has been considered re-org'd out func updateEthTxUnconfirm(ctx context.Context, orm *evmTxStore, etx Tx) error { if etx.State != txmgr.TxConfirmed { return errors.New("expected eth_tx state to be confirmed") } - _, err := orm.q.ExecContext(ctx, `UPDATE evm.txes SET state = 'unconfirmed' WHERE id = $1`, etx.ID) + _, err := orm.q.ExecContext(ctx, `UPDATE evm.txes SET state = 'unconfirmed', finalized = false WHERE id = $1`, etx.ID) return pkgerrors.Wrap(err, "updateEthTxUnconfirm failed") } @@ -1207,24 +1212,6 @@ AND evm_chain_id = $1`, chainID.String()).Scan(&earliestUnconfirmedTxBlock) return earliestUnconfirmedTxBlock, err } -func (o *evmTxStore) IsTxFinalized(ctx context.Context, blockHeight int64, txID int64, chainID *big.Int) (finalized bool, err error) { - var cancel context.CancelFunc - ctx, cancel = o.stopCh.Ctx(ctx) - defer cancel() - - var count int32 - err = o.q.GetContext(ctx, &count, ` - SELECT COUNT(evm.receipts.receipt) FROM evm.txes - INNER JOIN evm.tx_attempts ON evm.txes.id = evm.tx_attempts.eth_tx_id - INNER JOIN evm.receipts ON evm.tx_attempts.hash = evm.receipts.tx_hash - WHERE evm.receipts.block_number <= ($1 - evm.txes.min_confirmations) - AND evm.txes.id = $2 AND evm.txes.evm_chain_id = $3`, blockHeight, txID, chainID.String()) - if err != nil { - return false, fmt.Errorf("failed to retrieve transaction reciepts: %w", err) - } - return count > 0, nil -} - func (o *evmTxStore) saveAttemptWithNewState(ctx context.Context, attempt TxAttempt, broadcastAt time.Time) error { var dbAttempt DbEthTxAttempt dbAttempt.FromTxAttempt(&attempt) @@ -2059,3 +2046,35 @@ func (o *evmTxStore) UpdateTxAttemptBroadcastBeforeBlockNum(ctx context.Context, _, err := o.q.ExecContext(ctx, sql, blockNum, id) return err } + +// Returns all transaction in a specified state +func (o *evmTxStore) FindTransactionsByState(ctx context.Context, state txmgrtypes.TxState, chainID *big.Int) (txes []*Tx, err error) { + var cancel context.CancelFunc + ctx, cancel = o.stopCh.Ctx(ctx) + defer cancel() + err = o.Transact(ctx, true, func(orm *evmTxStore) error { + sql := "SELECT * FROM evm.txes WHERE state = $1 AND evm_chain_id = $2" + var dbEtxs []DbEthTx + err = o.q.SelectContext(ctx, &dbEtxs, sql, state, chainID.String()) + txes = make([]*Tx, len(dbEtxs)) + dbEthTxsToEvmEthTxPtrs(dbEtxs, txes) + if err = orm.LoadTxesAttempts(ctx, txes); err != nil { + return pkgerrors.Wrapf(err, "failed to load evm.tx_attempts for evm.tx") + } + if err = orm.loadEthTxesAttemptsReceipts(ctx, txes); err != nil { + return pkgerrors.Wrapf(err, "failed to load evm.receipts for evm.tx") + } + return nil + }) + return txes, err +} + +// Mark transactions provided as finalized +func (o *evmTxStore) UpdateTxesFinalized(ctx context.Context, etxIDs []int64, chainId *big.Int) error { + var cancel context.CancelFunc + ctx, cancel = o.stopCh.Ctx(ctx) + defer cancel() + sql := "UPDATE evm.txes SET finalized = true WHERE id = ANY($1) AND evm_chain_id = $2" + _, err := o.q.ExecContext(ctx, sql, pq.Array(etxIDs), chainId.String()) + return err +} diff --git a/core/chains/evm/txmgr/evm_tx_store_test.go b/core/chains/evm/txmgr/evm_tx_store_test.go index afb8de4ca52..09ec271770f 100644 --- a/core/chains/evm/txmgr/evm_tx_store_test.go +++ b/core/chains/evm/txmgr/evm_tx_store_test.go @@ -783,30 +783,6 @@ func TestORM_UpdateTxForRebroadcast(t *testing.T) { }) } -func TestORM_IsTxFinalized(t *testing.T) { - t.Parallel() - - db := pgtest.NewSqlxDB(t) - txStore := cltest.NewTestTxStore(t, db) - ethClient := evmtest.NewEthClientMockWithDefaultChain(t) - - t.Run("confirmed tx not past finality_depth", func(t *testing.T) { - confirmedAddr := cltest.MustGenerateRandomKey(t).Address - tx := mustInsertConfirmedEthTxWithReceipt(t, txStore, confirmedAddr, 123, 1) - finalized, err := txStore.IsTxFinalized(tests.Context(t), 2, tx.ID, ethClient.ConfiguredChainID()) - require.NoError(t, err) - require.False(t, finalized) - }) - - t.Run("confirmed tx past finality_depth", func(t *testing.T) { - confirmedAddr := cltest.MustGenerateRandomKey(t).Address - tx := mustInsertConfirmedEthTxWithReceipt(t, txStore, confirmedAddr, 123, 1) - finalized, err := txStore.IsTxFinalized(tests.Context(t), 10, tx.ID, ethClient.ConfiguredChainID()) - require.NoError(t, err) - require.True(t, finalized) - }) -} - func TestORM_FindTransactionsConfirmedInBlockRange(t *testing.T) { t.Parallel() @@ -1382,7 +1358,7 @@ func TestORM_UpdateTxUnstartedToInProgress(t *testing.T) { evmTxmCfg := txmgr.NewEvmTxmConfig(ccfg.EVM()) ec := evmtest.NewEthClientMockWithDefaultChain(t) txMgr := txmgr.NewEvmTxm(ec.ConfiguredChainID(), evmTxmCfg, ccfg.EVM().Transactions(), nil, logger.Test(t), nil, nil, - nil, txStore, nil, nil, nil, nil) + nil, txStore, nil, nil, nil, nil, nil) err := txMgr.XXXTestAbandon(fromAddress) // mark transaction as abandoned require.NoError(t, err) @@ -1871,3 +1847,81 @@ func AssertCountPerSubject(t *testing.T, txStore txmgr.TestEvmTxStore, expected require.NoError(t, err) require.Equal(t, int(expected), count) } + +func TestORM_FindTransactionsByState(t *testing.T) { + t.Parallel() + + ctx := tests.Context(t) + db := pgtest.NewSqlxDB(t) + txStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + mustInsertUnstartedTx(t, txStore, fromAddress) + mustInsertInProgressEthTxWithAttempt(t, txStore, 0, fromAddress) + mustInsertUnconfirmedEthTxWithAttemptState(t, txStore, 1, fromAddress, txmgrtypes.TxAttemptBroadcast) + mustInsertConfirmedMissingReceiptEthTxWithLegacyAttempt(t, txStore, 2, 100, time.Now(), fromAddress) + mustInsertConfirmedEthTxWithReceipt(t, txStore, fromAddress, 3, 100) + mustInsertFatalErrorEthTx(t, txStore, fromAddress) + + var txStates []txmgrtypes.TxState + txStates = append(txStates, txmgrcommon.TxUnstarted) + txStates = append(txStates, txmgrcommon.TxInProgress) + txStates = append(txStates, txmgrcommon.TxUnconfirmed) + txStates = append(txStates, txmgrcommon.TxConfirmed) + txStates = append(txStates, txmgrcommon.TxConfirmedMissingReceipt) + txStates = append(txStates, txmgrcommon.TxConfirmed) + + for _, state := range txStates { + txs, err := txStore.FindTransactionsByState(ctx, state, testutils.FixtureChainID) + require.NoError(t, err) + require.Len(t, txs, 1) + } +} + +func TestORM_UpdateTxesFinalized(t *testing.T) { + t.Parallel() + + ctx := tests.Context(t) + db := pgtest.NewSqlxDB(t) + txStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db) + broadcast := time.Now() + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + t.Run("successfully finalizes a confirmed transaction", func(t *testing.T) { + nonce := evmtypes.Nonce(0) + tx := &txmgr.Tx{ + Sequence: &nonce, + FromAddress: fromAddress, + EncodedPayload: []byte{1, 2, 3}, + State: txmgrcommon.TxConfirmed, + BroadcastAt: &broadcast, + InitialBroadcastAt: &broadcast, + } + err := txStore.InsertTx(ctx, tx) + require.NoError(t, err) + err = txStore.UpdateTxesFinalized(ctx, []int64{tx.ID}, testutils.FixtureChainID) + require.NoError(t, err) + etx, err := txStore.FindTxWithAttempts(ctx, tx.ID) + require.NoError(t, err) + require.True(t, etx.Finalized) + }) + t.Run("fails to finalize an unconfirmed transaction", func(t *testing.T) { + nonce := evmtypes.Nonce(1) + tx := &txmgr.Tx{ + Sequence: &nonce, + FromAddress: fromAddress, + EncodedPayload: []byte{1, 2, 3}, + State: txmgrcommon.TxUnconfirmed, + BroadcastAt: &broadcast, + InitialBroadcastAt: &broadcast, + } + err := txStore.InsertTx(ctx, tx) + require.NoError(t, err) + err = txStore.UpdateTxesFinalized(ctx, []int64{tx.ID}, testutils.FixtureChainID) + // Fails due to chk_eth_txes_state_finalized constraint + // Tx Store is poisoned after this + require.ErrorContains(t, err, "chk_eth_txes_state_finalized") + }) +} diff --git a/core/chains/evm/txmgr/finalizer_test.go b/core/chains/evm/txmgr/finalizer_test.go new file mode 100644 index 00000000000..263f77fafc0 --- /dev/null +++ b/core/chains/evm/txmgr/finalizer_test.go @@ -0,0 +1,160 @@ +package txmgr_test + +import ( + "testing" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/google/uuid" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink-common/pkg/logger" + "github.com/smartcontractkit/chainlink-common/pkg/utils/tests" + txmgrcommon "github.com/smartcontractkit/chainlink/v2/common/txmgr" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/testutils" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" + evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/utils" + "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" +) + +func TestFinalizer_MarkTxFinalized(t *testing.T) { + t.Parallel() + ctx := tests.Context(t) + db := pgtest.NewSqlxDB(t) + txStore := cltest.NewTestTxStore(t, db) + ethKeyStore := cltest.NewKeyStore(t, db).Eth() + feeLimit := uint64(10_000) + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + + finalizer := txmgr.NewEvmFinalizer(logger.Test(t), testutils.FixtureChainID, txStore, txmgr.NewEvmTxmClient(ethClient, nil)) + err := finalizer.Start(ctx) + require.NoError(t, err) + + head := &evmtypes.Head{ + Hash: utils.NewHash(), + Number: 100, + Parent: &evmtypes.Head{ + Hash: utils.NewHash(), + Number: 99, + IsFinalized: true, + }, + } + + t.Run("returns not finalized for tx with receipt newer than finalized block", func(t *testing.T) { + idempotencyKey := uuid.New().String() + _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + nonce := evmtypes.Nonce(0) + broadcast := time.Now() + tx := &txmgr.Tx{ + Sequence: &nonce, + IdempotencyKey: &idempotencyKey, + FromAddress: fromAddress, + EncodedPayload: []byte{1, 2, 3}, + FeeLimit: feeLimit, + State: txmgrcommon.TxConfirmed, + BroadcastAt: &broadcast, + InitialBroadcastAt: &broadcast, + } + attemptHash := insertTxAndAttemptWithIdempotencyKey(t, txStore, tx, idempotencyKey) + // Insert receipt for unfinalized block num + mustInsertEthReceipt(t, txStore, head.Number, head.Hash, attemptHash) + err = finalizer.ProcessHead(ctx, head) + require.NoError(t, err) + tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) + require.NoError(t, err) + require.Equal(t, false, tx.Finalized) + }) + + t.Run("returns not finalized for tx with receipt re-org'd out", func(t *testing.T) { + idempotencyKey := uuid.New().String() + _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + nonce := evmtypes.Nonce(0) + broadcast := time.Now() + tx := &txmgr.Tx{ + Sequence: &nonce, + IdempotencyKey: &idempotencyKey, + FromAddress: fromAddress, + EncodedPayload: []byte{1, 2, 3}, + FeeLimit: feeLimit, + State: txmgrcommon.TxConfirmed, + BroadcastAt: &broadcast, + InitialBroadcastAt: &broadcast, + } + attemptHash := insertTxAndAttemptWithIdempotencyKey(t, txStore, tx, idempotencyKey) + // Insert receipt for finalized block num + mustInsertEthReceipt(t, txStore, head.Parent.Number, utils.NewHash(), attemptHash) + err = finalizer.ProcessHead(ctx, head) + require.NoError(t, err) + tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) + require.NoError(t, err) + require.Equal(t, false, tx.Finalized) + }) + + t.Run("returns finalized for tx with receipt in a finalized block", func(t *testing.T) { + idempotencyKey := uuid.New().String() + _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + nonce := evmtypes.Nonce(0) + broadcast := time.Now() + tx := &txmgr.Tx{ + Sequence: &nonce, + IdempotencyKey: &idempotencyKey, + FromAddress: fromAddress, + EncodedPayload: []byte{1, 2, 3}, + FeeLimit: feeLimit, + State: txmgrcommon.TxConfirmed, + BroadcastAt: &broadcast, + InitialBroadcastAt: &broadcast, + Finalized: true, + } + attemptHash := insertTxAndAttemptWithIdempotencyKey(t, txStore, tx, idempotencyKey) + // Insert receipt for finalized block num + mustInsertEthReceipt(t, txStore, head.Parent.Number, head.Parent.Hash, attemptHash) + err = finalizer.ProcessHead(ctx, head) + require.NoError(t, err) + tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) + require.NoError(t, err) + require.Equal(t, true, tx.Finalized) + }) + + t.Run("returns finalized for tx with receipt older than block history depth", func(t *testing.T) { + idempotencyKey := uuid.New().String() + _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + nonce := evmtypes.Nonce(0) + broadcast := time.Now() + tx := &txmgr.Tx{ + Sequence: &nonce, + IdempotencyKey: &idempotencyKey, + FromAddress: fromAddress, + EncodedPayload: []byte{1, 2, 3}, + FeeLimit: feeLimit, + State: txmgrcommon.TxConfirmed, + BroadcastAt: &broadcast, + InitialBroadcastAt: &broadcast, + } + attemptHash := insertTxAndAttemptWithIdempotencyKey(t, txStore, tx, idempotencyKey) + // Insert receipt for finalized block num + receiptHash := utils.NewHash() + mustInsertEthReceipt(t, txStore, head.Parent.Number-1, receiptHash, attemptHash) + ethClient.On("HeadByHash", mock.Anything, receiptHash).Return(&evmtypes.Head{Number: head.Parent.Number - 1, Hash: receiptHash}, nil) + err = finalizer.ProcessHead(ctx, head) + require.NoError(t, err) + tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) + require.NoError(t, err) + require.Equal(t, true, tx.Finalized) + }) +} + +func insertTxAndAttemptWithIdempotencyKey(t *testing.T, txStore txmgr.TestEvmTxStore, tx *txmgr.Tx, idempotencyKey string) common.Hash { + ctx := tests.Context(t) + err := txStore.InsertTx(ctx, tx) + require.NoError(t, err) + tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) + require.NoError(t, err) + attempt := cltest.NewLegacyEthTxAttempt(t, tx.ID) + err = txStore.InsertTxAttempt(ctx, &attempt) + require.NoError(t, err) + return attempt.Hash +} diff --git a/core/chains/evm/txmgr/mocks/evm_tx_store.go b/core/chains/evm/txmgr/mocks/evm_tx_store.go index 465681247e2..ab695ec2045 100644 --- a/core/chains/evm/txmgr/mocks/evm_tx_store.go +++ b/core/chains/evm/txmgr/mocks/evm_tx_store.go @@ -313,6 +313,36 @@ func (_m *EvmTxStore) FindNextUnstartedTransactionFromAddress(ctx context.Contex return r0, r1 } +// FindTransactionsByState provides a mock function with given fields: ctx, state, chainID +func (_m *EvmTxStore) FindTransactionsByState(ctx context.Context, state types.TxState, chainID *big.Int) ([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], error) { + ret := _m.Called(ctx, state, chainID) + + if len(ret) == 0 { + panic("no return value specified for FindTransactionsByState") + } + + var r0 []*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee] + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, types.TxState, *big.Int) ([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], error)); ok { + return rf(ctx, state, chainID) + } + if rf, ok := ret.Get(0).(func(context.Context, types.TxState, *big.Int) []*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]); ok { + r0 = rf(ctx, state, chainID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, types.TxState, *big.Int) error); ok { + r1 = rf(ctx, state, chainID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // FindTransactionsConfirmedInBlockRange provides a mock function with given fields: ctx, highBlockNumber, lowBlockNumber, chainID func (_m *EvmTxStore) FindTransactionsConfirmedInBlockRange(ctx context.Context, highBlockNumber int64, lowBlockNumber int64, chainID *big.Int) ([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], error) { ret := _m.Called(ctx, highBlockNumber, lowBlockNumber, chainID) @@ -999,34 +1029,6 @@ func (_m *EvmTxStore) HasInProgressTransaction(ctx context.Context, account comm return r0, r1 } -// IsTxFinalized provides a mock function with given fields: ctx, blockHeight, txID, chainID -func (_m *EvmTxStore) IsTxFinalized(ctx context.Context, blockHeight int64, txID int64, chainID *big.Int) (bool, error) { - ret := _m.Called(ctx, blockHeight, txID, chainID) - - if len(ret) == 0 { - panic("no return value specified for IsTxFinalized") - } - - var r0 bool - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, int64, int64, *big.Int) (bool, error)); ok { - return rf(ctx, blockHeight, txID, chainID) - } - if rf, ok := ret.Get(0).(func(context.Context, int64, int64, *big.Int) bool); ok { - r0 = rf(ctx, blockHeight, txID, chainID) - } else { - r0 = ret.Get(0).(bool) - } - - if rf, ok := ret.Get(1).(func(context.Context, int64, int64, *big.Int) error); ok { - r1 = rf(ctx, blockHeight, txID, chainID) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // LoadTxAttempts provides a mock function with given fields: ctx, etx func (_m *EvmTxStore) LoadTxAttempts(ctx context.Context, etx *types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]) error { ret := _m.Called(ctx, etx) @@ -1492,6 +1494,24 @@ func (_m *EvmTxStore) UpdateTxUnstartedToInProgress(ctx context.Context, etx *ty return r0 } +// UpdateTxesFinalized provides a mock function with given fields: ctx, etxs, chainId +func (_m *EvmTxStore) UpdateTxesFinalized(ctx context.Context, etxs []int64, chainId *big.Int) error { + ret := _m.Called(ctx, etxs, chainId) + + if len(ret) == 0 { + panic("no return value specified for UpdateTxesFinalized") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, []int64, *big.Int) error); ok { + r0 = rf(ctx, etxs, chainId) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // UpdateTxsUnconfirmed provides a mock function with given fields: ctx, ids func (_m *EvmTxStore) UpdateTxsUnconfirmed(ctx context.Context, ids []int64) error { ret := _m.Called(ctx, ids) diff --git a/core/chains/evm/txmgr/models.go b/core/chains/evm/txmgr/models.go index f8682ffd500..92e18b820bf 100644 --- a/core/chains/evm/txmgr/models.go +++ b/core/chains/evm/txmgr/models.go @@ -39,9 +39,10 @@ type ( Receipt = dbReceipt // EvmReceipt is the exported DB table model for receipts ReceiptPlus = txmgrtypes.ReceiptPlus[*evmtypes.Receipt] StuckTxDetector = txmgrtypes.StuckTxDetector[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee] - TxmClient = txmgrtypes.TxmClient[*big.Int, common.Address, common.Hash, common.Hash, *evmtypes.Receipt, evmtypes.Nonce, gas.EvmFee] + TxmClient = txmgrtypes.TxmClient[*big.Int, common.Address, common.Hash, common.Hash, *evmtypes.Receipt, evmtypes.Nonce, gas.EvmFee, *evmtypes.Head] TransactionClient = txmgrtypes.TransactionClient[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee] ChainReceipt = txmgrtypes.ChainReceipt[common.Hash, common.Hash] + Finalizer = txmgr.Finalizer[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee, *evmtypes.Head] ) var _ KeyStore = (keystore.Eth)(nil) // check interface in txmgr to avoid circular import diff --git a/core/chains/evm/txmgr/txmgr_test.go b/core/chains/evm/txmgr/txmgr_test.go index dac0ef29cd8..3945d6f32a9 100644 --- a/core/chains/evm/txmgr/txmgr_test.go +++ b/core/chains/evm/txmgr/txmgr_test.go @@ -685,7 +685,7 @@ func TestTxm_GetTransactionStatus(t *testing.T) { require.Equal(t, commontypes.Unconfirmed, state) }) - t.Run("returns unconfirmed for confirmed state", func(t *testing.T) { + t.Run("returns unconfirmed for confirmed state not marked as finalized", func(t *testing.T) { idempotencyKey := uuid.New().String() _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) nonce := evmtypes.Nonce(0) @@ -699,6 +699,7 @@ func TestTxm_GetTransactionStatus(t *testing.T) { State: txmgrcommon.TxConfirmed, BroadcastAt: &broadcast, InitialBroadcastAt: &broadcast, + Finalized: false, // Set to false by default in DB but here for explicitness } err := txStore.InsertTx(ctx, tx) require.NoError(t, err) @@ -707,13 +708,43 @@ func TestTxm_GetTransactionStatus(t *testing.T) { attempt := cltest.NewLegacyEthTxAttempt(t, tx.ID) err = txStore.InsertTxAttempt(ctx, &attempt) require.NoError(t, err) - // Insert receipt for finalized block num - mustInsertEthReceipt(t, txStore, head.Parent.Number, head.ParentHash, attempt.Hash) + // Insert receipt for unfinalized block num + mustInsertEthReceipt(t, txStore, head.Number, head.Hash, attempt.Hash) state, err := txm.GetTransactionStatus(ctx, idempotencyKey) require.NoError(t, err) require.Equal(t, commontypes.Unconfirmed, state) }) + t.Run("returns finalized for confirmed state marked as finalized", func(t *testing.T) { + idempotencyKey := uuid.New().String() + _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + nonce := evmtypes.Nonce(0) + broadcast := time.Now() + tx := &txmgr.Tx{ + Sequence: &nonce, + IdempotencyKey: &idempotencyKey, + FromAddress: fromAddress, + EncodedPayload: []byte{1, 2, 3}, + FeeLimit: feeLimit, + State: txmgrcommon.TxConfirmed, + BroadcastAt: &broadcast, + InitialBroadcastAt: &broadcast, + Finalized: true, + } + err := txStore.InsertTx(ctx, tx) + require.NoError(t, err) + tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) + require.NoError(t, err) + attempt := cltest.NewLegacyEthTxAttempt(t, tx.ID) + err = txStore.InsertTxAttempt(ctx, &attempt) + require.NoError(t, err) + // Insert receipt for finalized block num + mustInsertEthReceipt(t, txStore, head.Parent.Number, head.Parent.Hash, attempt.Hash) + state, err := txm.GetTransactionStatus(ctx, idempotencyKey) + require.NoError(t, err) + require.Equal(t, commontypes.Finalized, state) + }) + t.Run("returns unconfirmed for confirmed missing receipt state", func(t *testing.T) { idempotencyKey := uuid.New().String() _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) @@ -1010,6 +1041,12 @@ func mustCreateUnstartedTxFromEvmTxRequest(t testing.TB, txStore txmgr.EvmTxStor return tx } +func mustInsertUnstartedTx(t testing.TB, txStore txmgr.TestEvmTxStore, fromAddress common.Address) { + etx := cltest.NewEthTx(fromAddress) + ctx := tests.Context(t) + require.NoError(t, txStore.InsertTx(ctx, &etx)) +} + func txRequestWithStrategy(strategy txmgrtypes.TxStrategy) func(*txmgr.TxRequest) { return func(tx *txmgr.TxRequest) { tx.Strategy = strategy diff --git a/core/services/vrf/v2/integration_v2_test.go b/core/services/vrf/v2/integration_v2_test.go index e8d4fd255f7..9c5639540af 100644 --- a/core/services/vrf/v2/integration_v2_test.go +++ b/core/services/vrf/v2/integration_v2_test.go @@ -142,7 +142,7 @@ func makeTestTxm(t *testing.T, txStore txmgr.TestEvmTxStore, keyStore keystore.M _, _, evmConfig := txmgr.MakeTestConfigs(t) txmConfig := txmgr.NewEvmTxmConfig(evmConfig) txm := txmgr.NewEvmTxm(ec.ConfiguredChainID(), txmConfig, evmConfig.Transactions(), keyStore.Eth(), logger.TestLogger(t), nil, nil, - nil, txStore, nil, nil, nil, nil) + nil, txStore, nil, nil, nil, nil, nil) return txm } diff --git a/core/services/vrf/v2/listener_v2_test.go b/core/services/vrf/v2/listener_v2_test.go index ac59f1fdb69..b7a8710c4f8 100644 --- a/core/services/vrf/v2/listener_v2_test.go +++ b/core/services/vrf/v2/listener_v2_test.go @@ -40,7 +40,7 @@ func makeTestTxm(t *testing.T, txStore txmgr.TestEvmTxStore, keyStore keystore.M ec := evmtest.NewEthClientMockWithDefaultChain(t) txmConfig := txmgr.NewEvmTxmConfig(evmConfig) txm := txmgr.NewEvmTxm(ec.ConfiguredChainID(), txmConfig, evmConfig.Transactions(), keyStore.Eth(), logger.TestLogger(t), nil, nil, - nil, txStore, nil, nil, nil, nil) + nil, txStore, nil, nil, nil, nil, nil) return txm } diff --git a/core/store/migrate/migrations/0245_add_tx_finalized_column.sql b/core/store/migrate/migrations/0245_add_tx_finalized_column.sql new file mode 100644 index 00000000000..cc0b09fd3dd --- /dev/null +++ b/core/store/migrate/migrations/0245_add_tx_finalized_column.sql @@ -0,0 +1,15 @@ +-- +goose Up +-- +goose StatementBegin +ALTER TABLE evm.txes ADD COLUMN finalized boolean NOT NULL DEFAULT false; +ALTER TABLE evm.txes ADD CONSTRAINT chk_eth_txes_state_finalized CHECK ( + state <> 'confirmed'::eth_txes_state AND finalized = false + OR + state = 'confirmed'::eth_txes_state +) NOT VALID; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +ALTER TABLE evm.txes DROP CONSTRAINT chk_eth_txes_state_finalized; +ALTER TABLE evm.txes DROP COLUMN finalized; +-- +goose StatementEnd diff --git a/core/web/testdata/body/health.html b/core/web/testdata/body/health.html index 2a1b2227530..90d301bc8b8 100644 --- a/core/web/testdata/body/health.html +++ b/core/web/testdata/body/health.html @@ -63,6 +63,9 @@
Confirmer
+
+ Finalizer +
WrappedEvmEstimator
diff --git a/core/web/testdata/body/health.json b/core/web/testdata/body/health.json index 10415c0abdc..839428a5103 100644 --- a/core/web/testdata/body/health.json +++ b/core/web/testdata/body/health.json @@ -90,6 +90,15 @@ "output": "" } }, + { + "type": "checks", + "id": "EVM.0.Txm.Finalizer", + "attributes": { + "name": "EVM.0.Txm.Finalizer", + "status": "passing", + "output": "" + } + }, { "type": "checks", "id": "EVM.0.Txm.WrappedEvmEstimator", diff --git a/core/web/testdata/body/health.txt b/core/web/testdata/body/health.txt index 09c8cff6c2d..3709b4e15f0 100644 --- a/core/web/testdata/body/health.txt +++ b/core/web/testdata/body/health.txt @@ -9,6 +9,7 @@ ok EVM.0.Txm ok EVM.0.Txm.BlockHistoryEstimator ok EVM.0.Txm.Broadcaster ok EVM.0.Txm.Confirmer +ok EVM.0.Txm.Finalizer ok EVM.0.Txm.WrappedEvmEstimator ok JobSpawner ok Mailbox.Monitor diff --git a/testdata/scripts/health/multi-chain.txtar b/testdata/scripts/health/multi-chain.txtar index 8178f8e8213..825e788586c 100644 --- a/testdata/scripts/health/multi-chain.txtar +++ b/testdata/scripts/health/multi-chain.txtar @@ -74,6 +74,7 @@ ok EVM.1.Txm ok EVM.1.Txm.BlockHistoryEstimator ok EVM.1.Txm.Broadcaster ok EVM.1.Txm.Confirmer +ok EVM.1.Txm.Finalizer ok EVM.1.Txm.WrappedEvmEstimator ok JobSpawner ok Mailbox.Monitor @@ -207,6 +208,15 @@ ok TelemetryManager "output": "" } }, + { + "type": "checks", + "id": "EVM.1.Txm.Finalizer", + "attributes": { + "name": "EVM.1.Txm.Finalizer", + "status": "passing", + "output": "" + } + }, { "type": "checks", "id": "EVM.1.Txm.WrappedEvmEstimator", From d91d2f215f6a24f8eef95c6404e23bab9d96dce7 Mon Sep 17 00:00:00 2001 From: amit-momin Date: Mon, 24 Jun 2024 17:12:43 -0500 Subject: [PATCH 02/16] Moved Finalizer component into EVM code and addressed feedback --- common/txmgr/reaper.go | 9 +- common/txmgr/txmgr.go | 8 +- common/txmgr/types/config.go | 8 - common/txmgr/types/finalizer.go | 12 ++ common/txmgr/types/mocks/tx_store.go | 70 +++---- common/txmgr/types/tx_store.go | 4 +- core/chains/evm/txmgr/builder.go | 17 +- core/chains/evm/txmgr/config.go | 1 - core/chains/evm/txmgr/evm_tx_store.go | 21 +- core/chains/evm/txmgr/evm_tx_store_test.go | 17 +- .../chains/evm}/txmgr/finalizer.go | 180 ++++++++++-------- core/chains/evm/txmgr/finalizer_test.go | 15 +- core/chains/evm/txmgr/mocks/evm_tx_store.go | 70 +++---- core/chains/evm/txmgr/models.go | 2 +- core/chains/evm/txmgr/reaper_test.go | 38 ++-- 15 files changed, 240 insertions(+), 232 deletions(-) create mode 100644 common/txmgr/types/finalizer.go rename {common => core/chains/evm}/txmgr/finalizer.go (50%) diff --git a/common/txmgr/reaper.go b/common/txmgr/reaper.go index 3ed05b2caee..83c41c39947 100644 --- a/common/txmgr/reaper.go +++ b/common/txmgr/reaper.go @@ -16,7 +16,6 @@ import ( // Reaper handles periodic database cleanup for Txm type Reaper[CHAIN_ID types.ID] struct { store txmgrtypes.TxHistoryReaper[CHAIN_ID] - config txmgrtypes.ReaperChainConfig txConfig txmgrtypes.ReaperTransactionsConfig chainID CHAIN_ID log logger.Logger @@ -27,10 +26,9 @@ type Reaper[CHAIN_ID types.ID] struct { } // NewReaper instantiates a new reaper object -func NewReaper[CHAIN_ID types.ID](lggr logger.Logger, store txmgrtypes.TxHistoryReaper[CHAIN_ID], config txmgrtypes.ReaperChainConfig, txConfig txmgrtypes.ReaperTransactionsConfig, chainID CHAIN_ID) *Reaper[CHAIN_ID] { +func NewReaper[CHAIN_ID types.ID](lggr logger.Logger, store txmgrtypes.TxHistoryReaper[CHAIN_ID], txConfig txmgrtypes.ReaperTransactionsConfig, chainID CHAIN_ID) *Reaper[CHAIN_ID] { r := &Reaper[CHAIN_ID]{ store, - config, txConfig, chainID, logger.Named(lggr, "Reaper"), @@ -106,13 +104,12 @@ func (r *Reaper[CHAIN_ID]) ReapTxes(headNum int64) error { r.log.Debug("Transactions.ReaperThreshold set to 0; skipping ReapTxes") return nil } - minBlockNumberToKeep := headNum - int64(r.config.FinalityDepth()) mark := time.Now() timeThreshold := mark.Add(-threshold) - r.log.Debugw(fmt.Sprintf("reaping old txes created before %s", timeThreshold.Format(time.RFC3339)), "ageThreshold", threshold, "timeThreshold", timeThreshold, "minBlockNumberToKeep", minBlockNumberToKeep) + r.log.Debugw(fmt.Sprintf("reaping old txes created before %s", timeThreshold.Format(time.RFC3339)), "ageThreshold", threshold, "timeThreshold", timeThreshold) - if err := r.store.ReapTxHistory(ctx, minBlockNumberToKeep, timeThreshold, r.chainID); err != nil { + if err := r.store.ReapTxHistory(ctx, timeThreshold, r.chainID); err != nil { return err } diff --git a/common/txmgr/txmgr.go b/common/txmgr/txmgr.go index a9cee5fb0c7..671ec012243 100644 --- a/common/txmgr/txmgr.go +++ b/common/txmgr/txmgr.go @@ -110,7 +110,7 @@ type Txm[ broadcaster *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] confirmer *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] tracker *Tracker[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] - finalizer *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD] + finalizer txmgrtypes.Finalizer[BLOCK_HASH, HEAD] fwdMgr txmgrtypes.ForwarderManager[ADDR] txAttemptBuilder txmgrtypes.TxAttemptBuilder[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] newErrorClassifier NewErrorClassifier @@ -146,7 +146,7 @@ func NewTxm[ confirmer *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE], resender *Resender[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE], tracker *Tracker[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE], - finalizer *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD], + finalizer txmgrtypes.Finalizer[BLOCK_HASH, HEAD], newErrorClassifierFunc NewErrorClassifier, ) *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] { b := Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]{ @@ -176,7 +176,7 @@ func NewTxm[ b.logger.Info("Resender: Disabled") } if txCfg.ReaperThreshold() > 0 && txCfg.ReaperInterval() > 0 { - b.reaper = NewReaper[CHAIN_ID](lggr, b.txStore, cfg, txCfg, chainId) + b.reaper = NewReaper[CHAIN_ID](lggr, b.txStore, txCfg, chainId) } else { b.logger.Info("TxReaper: Disabled") } @@ -425,7 +425,7 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) runLoop() case head := <-b.chHeads: b.confirmer.mb.Deliver(head) b.tracker.mb.Deliver(head.BlockNumber()) - b.finalizer.mb.Deliver(head) + b.finalizer.DeliverHead(head) case reset := <-b.reset: // This check prevents the weird edge-case where you can select // into this block after chStop has already been closed and the diff --git a/common/txmgr/types/config.go b/common/txmgr/types/config.go index 53e35cd4b6e..8b11a45d11d 100644 --- a/common/txmgr/types/config.go +++ b/common/txmgr/types/config.go @@ -5,7 +5,6 @@ import "time" type TransactionManagerChainConfig interface { BroadcasterChainConfig ConfirmerChainConfig - ReaperChainConfig } type TransactionManagerFeeConfig interface { @@ -74,13 +73,6 @@ type ResenderTransactionsConfig interface { MaxInFlight() uint32 } -// ReaperConfig is the config subset used by the reaper -// -//go:generate mockery --quiet --name ReaperChainConfig --structname ReaperConfig --output ./mocks/ --case=underscore -type ReaperChainConfig interface { - FinalityDepth() uint32 -} - type ReaperTransactionsConfig interface { ReaperInterval() time.Duration ReaperThreshold() time.Duration diff --git a/common/txmgr/types/finalizer.go b/common/txmgr/types/finalizer.go new file mode 100644 index 00000000000..d7fd6b7f603 --- /dev/null +++ b/common/txmgr/types/finalizer.go @@ -0,0 +1,12 @@ +package types + +import ( + "github.com/smartcontractkit/chainlink-common/pkg/services" + "github.com/smartcontractkit/chainlink/v2/common/types" +) + +type Finalizer[BLOCK_HASH types.Hashable, HEAD types.Head[BLOCK_HASH]] interface { + // interfaces for running the underlying estimator + services.Service + DeliverHead(head HEAD) bool +} diff --git a/common/txmgr/types/mocks/tx_store.go b/common/txmgr/types/mocks/tx_store.go index 21e9169bbb1..709d5a6d45a 100644 --- a/common/txmgr/types/mocks/tx_store.go +++ b/common/txmgr/types/mocks/tx_store.go @@ -196,6 +196,36 @@ func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) DeleteInPro return r0 } +// FindConfirmedTxesAwaitingFinalization provides a mock function with given fields: ctx, chainID +func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindConfirmedTxesAwaitingFinalization(ctx context.Context, chainID CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) { + ret := _m.Called(ctx, chainID) + + if len(ret) == 0 { + panic("no return value specified for FindConfirmedTxesAwaitingFinalization") + } + + var r0 []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error)); ok { + return rf(ctx, chainID) + } + if rf, ok := ret.Get(0).(func(context.Context, CHAIN_ID) []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]); ok { + r0 = rf(ctx, chainID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, CHAIN_ID) error); ok { + r1 = rf(ctx, chainID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // FindEarliestUnconfirmedBroadcastTime provides a mock function with given fields: ctx, chainID func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindEarliestUnconfirmedBroadcastTime(ctx context.Context, chainID CHAIN_ID) (null.Time, error) { ret := _m.Called(ctx, chainID) @@ -310,36 +340,6 @@ func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindNextUns return r0, r1 } -// FindTransactionsByState provides a mock function with given fields: ctx, state, chainID -func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindTransactionsByState(ctx context.Context, state txmgrtypes.TxState, chainID CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) { - ret := _m.Called(ctx, state, chainID) - - if len(ret) == 0 { - panic("no return value specified for FindTransactionsByState") - } - - var r0 []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, txmgrtypes.TxState, CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error)); ok { - return rf(ctx, state, chainID) - } - if rf, ok := ret.Get(0).(func(context.Context, txmgrtypes.TxState, CHAIN_ID) []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]); ok { - r0 = rf(ctx, state, chainID) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, txmgrtypes.TxState, CHAIN_ID) error); ok { - r1 = rf(ctx, state, chainID) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // FindTransactionsConfirmedInBlockRange provides a mock function with given fields: ctx, highBlockNumber, lowBlockNumber, chainID func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindTransactionsConfirmedInBlockRange(ctx context.Context, highBlockNumber int64, lowBlockNumber int64, chainID CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) { ret := _m.Called(ctx, highBlockNumber, lowBlockNumber, chainID) @@ -980,17 +980,17 @@ func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) PruneUnstar return r0, r1 } -// ReapTxHistory provides a mock function with given fields: ctx, minBlockNumberToKeep, timeThreshold, chainID -func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) ReapTxHistory(ctx context.Context, minBlockNumberToKeep int64, timeThreshold time.Time, chainID CHAIN_ID) error { - ret := _m.Called(ctx, minBlockNumberToKeep, timeThreshold, chainID) +// ReapTxHistory provides a mock function with given fields: ctx, timeThreshold, chainID +func (_m *TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) ReapTxHistory(ctx context.Context, timeThreshold time.Time, chainID CHAIN_ID) error { + ret := _m.Called(ctx, timeThreshold, chainID) if len(ret) == 0 { panic("no return value specified for ReapTxHistory") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, int64, time.Time, CHAIN_ID) error); ok { - r0 = rf(ctx, minBlockNumberToKeep, timeThreshold, chainID) + if rf, ok := ret.Get(0).(func(context.Context, time.Time, CHAIN_ID) error); ok { + r0 = rf(ctx, timeThreshold, chainID) } else { r0 = ret.Error(0) } diff --git a/common/txmgr/types/tx_store.go b/common/txmgr/types/tx_store.go index 495a6d7fbae..487b1b0ba82 100644 --- a/common/txmgr/types/tx_store.go +++ b/common/txmgr/types/tx_store.go @@ -84,7 +84,7 @@ type TransactionStore[ FindTransactionsConfirmedInBlockRange(ctx context.Context, highBlockNumber, lowBlockNumber int64, chainID CHAIN_ID) (etxs []*Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) FindEarliestUnconfirmedBroadcastTime(ctx context.Context, chainID CHAIN_ID) (null.Time, error) FindEarliestUnconfirmedTxAttemptBlock(ctx context.Context, chainID CHAIN_ID) (null.Int, error) - FindTransactionsByState(ctx context.Context, state TxState, chainID CHAIN_ID) (etxs []*Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) + FindConfirmedTxesAwaitingFinalization(ctx context.Context, chainID CHAIN_ID) (etxs []*Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) GetTxInProgress(ctx context.Context, fromAddress ADDR) (etx *Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) GetInProgressTxAttempts(ctx context.Context, address ADDR, chainID CHAIN_ID) (attempts []TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) GetAbandonedTransactionsByBatch(ctx context.Context, chainID CHAIN_ID, enabledAddrs []ADDR, offset, limit uint) (txs []*Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], err error) @@ -112,7 +112,7 @@ type TransactionStore[ } type TxHistoryReaper[CHAIN_ID types.ID] interface { - ReapTxHistory(ctx context.Context, minBlockNumberToKeep int64, timeThreshold time.Time, chainID CHAIN_ID) error + ReapTxHistory(ctx context.Context, timeThreshold time.Time, chainID CHAIN_ID) error } type UnstartedTxQueuePruner interface { diff --git a/core/chains/evm/txmgr/builder.go b/core/chains/evm/txmgr/builder.go index 20a9d7e2555..23d802c521b 100644 --- a/core/chains/evm/txmgr/builder.go +++ b/core/chains/evm/txmgr/builder.go @@ -53,7 +53,7 @@ func NewTxm( evmTracker := NewEvmTracker(txStore, keyStore, chainID, lggr) stuckTxDetector := NewStuckTxDetector(lggr, client.ConfiguredChainID(), chainConfig.ChainType(), fCfg.PriceMax(), txConfig.AutoPurge(), estimator, txStore, client) evmConfirmer := NewEvmConfirmer(txStore, txmClient, txmCfg, feeCfg, txConfig, dbConfig, keyStore, txAttemptBuilder, lggr, stuckTxDetector) - evmFinalizer := NewEvmFinalizer(lggr, client.ConfiguredChainID(), txStore, txmClient) + evmFinalizer := NewEvmFinalizer(lggr, client.ConfiguredChainID(), txStore, client) var evmResender *Resender if txConfig.ResendAfterThreshold() > 0 { evmResender = NewEvmResender(lggr, txStore, txmClient, evmTracker, keyStore, txmgr.DefaultResenderPollInterval, chainConfig, txConfig) @@ -77,7 +77,7 @@ func NewEvmTxm( confirmer *Confirmer, resender *Resender, tracker *Tracker, - finalizer *Finalizer, + finalizer Finalizer, ) *Txm { return txmgr.NewTxm(chainId, cfg, txCfg, keyStore, lggr, checkerFactory, fwdMgr, txAttemptBuilder, txStore, broadcaster, confirmer, resender, tracker, finalizer, client.NewTxError) } @@ -97,8 +97,8 @@ func NewEvmResender( } // NewEvmReaper instantiates a new EVM-specific reaper object -func NewEvmReaper(lggr logger.Logger, store txmgrtypes.TxHistoryReaper[*big.Int], config EvmReaperConfig, txConfig txmgrtypes.ReaperTransactionsConfig, chainID *big.Int) *Reaper { - return txmgr.NewReaper(lggr, store, config, txConfig, chainID) +func NewEvmReaper(lggr logger.Logger, store txmgrtypes.TxHistoryReaper[*big.Int], txConfig txmgrtypes.ReaperTransactionsConfig, chainID *big.Int) *Reaper { + return txmgr.NewReaper(lggr, store, txConfig, chainID) } // NewEvmConfirmer instantiates a new EVM confirmer @@ -144,12 +144,3 @@ func NewEvmBroadcaster( nonceTracker := NewNonceTracker(logger, txStore, client) return txmgr.NewBroadcaster(txStore, client, chainConfig, feeConfig, txConfig, listenerConfig, keystore, txAttemptBuilder, nonceTracker, logger, checkerFactory, autoSyncNonce) } - -func NewEvmFinalizer( - logger logger.Logger, - chainId *big.Int, - txStore TransactionStore, - client TxmClient, -) *Finalizer { - return txmgr.NewFinalizer(logger, chainId, txStore, client) -} diff --git a/core/chains/evm/txmgr/config.go b/core/chains/evm/txmgr/config.go index c34de17369e..57088ddff23 100644 --- a/core/chains/evm/txmgr/config.go +++ b/core/chains/evm/txmgr/config.go @@ -50,7 +50,6 @@ type ( EvmBroadcasterConfig txmgrtypes.BroadcasterChainConfig EvmConfirmerConfig txmgrtypes.ConfirmerChainConfig EvmResenderConfig txmgrtypes.ResenderChainConfig - EvmReaperConfig txmgrtypes.ReaperChainConfig ) var _ EvmTxmConfig = (*evmTxmConfig)(nil) diff --git a/core/chains/evm/txmgr/evm_tx_store.go b/core/chains/evm/txmgr/evm_tx_store.go index b7a526a4742..0b97bf78770 100644 --- a/core/chains/evm/txmgr/evm_tx_store.go +++ b/core/chains/evm/txmgr/evm_tx_store.go @@ -1863,7 +1863,7 @@ id < ( return } -func (o *evmTxStore) ReapTxHistory(ctx context.Context, minBlockNumberToKeep int64, timeThreshold time.Time, chainID *big.Int) error { +func (o *evmTxStore) ReapTxHistory(ctx context.Context, timeThreshold time.Time, chainID *big.Int) error { var cancel context.CancelFunc ctx, cancel = o.stopCh.Ctx(ctx) defer cancel() @@ -1876,17 +1876,17 @@ func (o *evmTxStore) ReapTxHistory(ctx context.Context, minBlockNumberToKeep int res, err := o.q.ExecContext(ctx, ` WITH old_enough_receipts AS ( SELECT tx_hash FROM evm.receipts - WHERE block_number < $1 ORDER BY block_number ASC, id ASC - LIMIT $2 + LIMIT $1 ) DELETE FROM evm.txes USING old_enough_receipts, evm.tx_attempts WHERE evm.tx_attempts.eth_tx_id = evm.txes.id AND evm.tx_attempts.hash = old_enough_receipts.tx_hash -AND evm.txes.created_at < $3 +AND evm.txes.created_at < $2 AND evm.txes.state = 'confirmed' -AND evm_chain_id = $4`, minBlockNumberToKeep, limit, timeThreshold, chainID.String()) +AND evm.txes.finalized = true +AND evm_chain_id = $3`, limit, timeThreshold, chainID.String()) if err != nil { return count, pkgerrors.Wrap(err, "ReapTxes failed to delete old confirmed evm.txes") } @@ -2047,15 +2047,18 @@ func (o *evmTxStore) UpdateTxAttemptBroadcastBeforeBlockNum(ctx context.Context, return err } -// Returns all transaction in a specified state -func (o *evmTxStore) FindTransactionsByState(ctx context.Context, state txmgrtypes.TxState, chainID *big.Int) (txes []*Tx, err error) { +// Returns all confirmed transactions not yet marked as finalized +func (o *evmTxStore) FindConfirmedTxesAwaitingFinalization(ctx context.Context, chainID *big.Int) (txes []*Tx, err error) { var cancel context.CancelFunc ctx, cancel = o.stopCh.Ctx(ctx) defer cancel() err = o.Transact(ctx, true, func(orm *evmTxStore) error { - sql := "SELECT * FROM evm.txes WHERE state = $1 AND evm_chain_id = $2" + sql := "SELECT * FROM evm.txes WHERE state = 'confirmed' AND finalized = false AND evm_chain_id = $1" var dbEtxs []DbEthTx - err = o.q.SelectContext(ctx, &dbEtxs, sql, state, chainID.String()) + err = o.q.SelectContext(ctx, &dbEtxs, sql, chainID.String()) + if len(dbEtxs) == 0 { + return nil + } txes = make([]*Tx, len(dbEtxs)) dbEthTxsToEvmEthTxPtrs(dbEtxs, txes) if err = orm.LoadTxesAttempts(ctx, txes); err != nil { diff --git a/core/chains/evm/txmgr/evm_tx_store_test.go b/core/chains/evm/txmgr/evm_tx_store_test.go index 09ec271770f..b0a0a0379f1 100644 --- a/core/chains/evm/txmgr/evm_tx_store_test.go +++ b/core/chains/evm/txmgr/evm_tx_store_test.go @@ -1864,19 +1864,10 @@ func TestORM_FindTransactionsByState(t *testing.T) { mustInsertConfirmedEthTxWithReceipt(t, txStore, fromAddress, 3, 100) mustInsertFatalErrorEthTx(t, txStore, fromAddress) - var txStates []txmgrtypes.TxState - txStates = append(txStates, txmgrcommon.TxUnstarted) - txStates = append(txStates, txmgrcommon.TxInProgress) - txStates = append(txStates, txmgrcommon.TxUnconfirmed) - txStates = append(txStates, txmgrcommon.TxConfirmed) - txStates = append(txStates, txmgrcommon.TxConfirmedMissingReceipt) - txStates = append(txStates, txmgrcommon.TxConfirmed) - - for _, state := range txStates { - txs, err := txStore.FindTransactionsByState(ctx, state, testutils.FixtureChainID) - require.NoError(t, err) - require.Len(t, txs, 1) - } + txs, err := txStore.FindConfirmedTxesAwaitingFinalization(ctx, testutils.FixtureChainID) + require.NoError(t, err) + require.Len(t, txs, 1) + } func TestORM_UpdateTxesFinalized(t *testing.T) { diff --git a/common/txmgr/finalizer.go b/core/chains/evm/txmgr/finalizer.go similarity index 50% rename from common/txmgr/finalizer.go rename to core/chains/evm/txmgr/finalizer.go index dc7bc66db8f..477607b0ecf 100644 --- a/common/txmgr/finalizer.go +++ b/core/chains/evm/txmgr/finalizer.go @@ -2,116 +2,101 @@ package txmgr import ( "context" - "errors" "fmt" "math/big" "sync" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/rpc" "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/services" "github.com/smartcontractkit/chainlink-common/pkg/utils/mailbox" - feetypes "github.com/smartcontractkit/chainlink/v2/common/fee/types" - txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" - "github.com/smartcontractkit/chainlink/v2/common/types" + + evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" ) -type finalizerTxStore[CHAIN_ID types.ID, ADDR types.Hashable, TX_HASH types.Hashable, BLOCK_HASH types.Hashable, SEQ types.Sequence, FEE feetypes.Fee] interface { - FindTransactionsByState(ctx context.Context, state txmgrtypes.TxState, chainID CHAIN_ID) ([]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) - UpdateTxesFinalized(ctx context.Context, txs []int64, chainId CHAIN_ID) error +var _ Finalizer = (*evmFinalizer)(nil) + +// processHeadTimeout represents a sanity limit on how long ProcessHead +// should take to complete +const processHeadTimeout = 10 * time.Minute + +type finalizerTxStore interface { + FindConfirmedTxesAwaitingFinalization(ctx context.Context, chainID *big.Int) ([]*Tx, error) + UpdateTxesFinalized(ctx context.Context, txs []int64, chainId *big.Int) error } -type finalizerChainClient[BLOCK_HASH types.Hashable, HEAD types.Head[BLOCK_HASH]] interface { - HeadByHash(ctx context.Context, hash BLOCK_HASH) (HEAD, error) +type finalizerChainClient interface { + BatchCallContext(ctx context.Context, elems []rpc.BatchElem) error } // Finalizer handles processing new finalized blocks and marking transactions as finalized accordingly in the TXM DB -type Finalizer[CHAIN_ID types.ID, ADDR types.Hashable, TX_HASH types.Hashable, BLOCK_HASH types.Hashable, SEQ types.Sequence, FEE feetypes.Fee, HEAD types.Head[BLOCK_HASH]] struct { +type evmFinalizer struct { services.StateMachine - lggr logger.SugaredLogger - chainId CHAIN_ID - txStore finalizerTxStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] - client finalizerChainClient[BLOCK_HASH, HEAD] - mb *mailbox.Mailbox[HEAD] - stopCh services.StopChan - wg sync.WaitGroup - initSync sync.Mutex - isStarted bool -} - -func NewFinalizer[ - CHAIN_ID types.ID, - ADDR types.Hashable, - TX_HASH types.Hashable, - BLOCK_HASH types.Hashable, - SEQ types.Sequence, - FEE feetypes.Fee, - HEAD types.Head[BLOCK_HASH], -]( + lggr logger.SugaredLogger + chainId *big.Int + txStore finalizerTxStore + client finalizerChainClient + mb *mailbox.Mailbox[*evmtypes.Head] + stopCh services.StopChan + wg sync.WaitGroup +} + +func NewEvmFinalizer( lggr logger.Logger, - chainId CHAIN_ID, - txStore finalizerTxStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], - client finalizerChainClient[BLOCK_HASH, HEAD], -) *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD] { + chainId *big.Int, + txStore finalizerTxStore, + client finalizerChainClient, +) *evmFinalizer { lggr = logger.Named(lggr, "Finalizer") - return &Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]{ + return &evmFinalizer{ txStore: txStore, lggr: logger.Sugared(lggr), chainId: chainId, client: client, - mb: mailbox.NewSingle[HEAD](), + mb: mailbox.NewSingle[*evmtypes.Head](), } } // Start is a comment to appease the linter -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) Start(ctx context.Context) error { +func (f *evmFinalizer) Start(ctx context.Context) error { return f.StartOnce("Finalizer", func() error { return f.startInternal(ctx) }) } -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) startInternal(_ context.Context) error { - f.initSync.Lock() - defer f.initSync.Unlock() - if f.isStarted { - return errors.New("Finalizer is already started") - } - +func (f *evmFinalizer) startInternal(_ context.Context) error { f.stopCh = make(chan struct{}) f.wg = sync.WaitGroup{} f.wg.Add(1) go f.runLoop() - f.isStarted = true return nil } // Close is a comment to appease the linter -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) Close() error { +func (f *evmFinalizer) Close() error { return f.StopOnce("Finalizer", func() error { return f.closeInternal() }) } -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) closeInternal() error { - f.initSync.Lock() - defer f.initSync.Unlock() - if !f.isStarted { - return fmt.Errorf("Finalizer is not started: %w", services.ErrAlreadyStopped) - } +func (f *evmFinalizer) closeInternal() error { close(f.stopCh) f.wg.Wait() - f.isStarted = false return nil } -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) Name() string { +func (f *evmFinalizer) Name() string { return f.lggr.Name() } -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) HealthReport() map[string]error { +func (f *evmFinalizer) HealthReport() map[string]error { return map[string]error{f.Name(): f.Healthy()} } -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) runLoop() { +func (f *evmFinalizer) runLoop() { defer f.wg.Done() ctx, cancel := f.stopCh.NewCtx() defer cancel() @@ -138,14 +123,18 @@ func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) runLoop } } -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) ProcessHead(ctx context.Context, head types.Head[BLOCK_HASH]) error { +func (f *evmFinalizer) DeliverHead(head *evmtypes.Head) bool { + return f.mb.Deliver(head) +} + +func (f *evmFinalizer) ProcessHead(ctx context.Context, head *evmtypes.Head) error { ctx, cancel := context.WithTimeout(ctx, processHeadTimeout) defer cancel() return f.processHead(ctx, head) } // Determines if any confirmed transactions can be marked as finalized by comparing their receipts against the latest finalized block -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) processHead(ctx context.Context, head types.Head[BLOCK_HASH]) error { +func (f *evmFinalizer) processHead(ctx context.Context, head *evmtypes.Head) error { latestFinalizedHead := head.LatestFinalizedHead() // Cannot determine finality without a finalized head for comparison if latestFinalizedHead == nil || !latestFinalizedHead.IsValid() { @@ -155,22 +144,23 @@ func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) process f.lggr.Debugw("processing latest finalized head", "block num", latestFinalizedHead.BlockNumber(), "block hash", latestFinalizedHead.BlockHash(), "earliest block num in chain", earliestBlockNumInChain) // Retrieve all confirmed transactions, loaded with attempts and receipts - confirmedTxs, err := f.txStore.FindTransactionsByState(ctx, TxConfirmed, f.chainId) + unfinalizedTxs, err := f.txStore.FindConfirmedTxesAwaitingFinalization(ctx, f.chainId) if err != nil { return fmt.Errorf("failed to retrieve confirmed transactions: %w", err) } - var finalizedTxs []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + var finalizedTxs []*Tx // Group by block hash transactions whose receipts cannot be validated using the cached heads - receiptBlockHashToTx := make(map[BLOCK_HASH][]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) + receiptBlockHashToTx := make(map[common.Hash][]*Tx) // Find transactions with receipt block nums older than the latest finalized block num and block hashes still in chain - for _, tx := range confirmedTxs { + for _, tx := range unfinalizedTxs { // Only consider transactions not already marked as finalized if tx.Finalized { continue } receipt := tx.GetReceipt() if receipt == nil || receipt.IsZero() || receipt.IsUnmined() { + f.lggr.AssumptionViolationw("invalid receipt found for confirmed transaction", "tx", tx, "receipt", receipt) continue } // Receipt newer than latest finalized head block num @@ -191,22 +181,14 @@ func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) process } // Check if block hashes exist for receipts on-chain older than the earliest cached head - // Transactions are grouped by their receipt block hash to minimize the number of RPC calls in case transactions were confirmed in the same block - // This check is only expected to be used in rare cases if there was an issue with the HeadTracker or if the node was down for significant time - var wg sync.WaitGroup - var txMu sync.RWMutex - for receiptBlockHash, txs := range receiptBlockHashToTx { - wg.Add(1) - go func(hash BLOCK_HASH, txs []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { - defer wg.Done() - if head, rpcErr := f.client.HeadByHash(ctx, hash); rpcErr == nil && head.IsValid() { - txMu.Lock() - finalizedTxs = append(finalizedTxs, txs...) - txMu.Unlock() - } - }(receiptBlockHash, txs) + // Transactions are grouped by their receipt block hash to avoid repeat requests on the same hash in case transactions were confirmed in the same block + validatedReceiptTxs, err := f.batchCheckReceiptHashes(ctx, receiptBlockHashToTx, latestFinalizedHead.BlockNumber()) + if err != nil { + // Do not error out to allow transactions that did not need RPC validation to still be marked as finalized + // The transcations failed to be validated will be checked again in the next round + f.lggr.Errorf("failed to validate receipt block hashes over RPC: %v", err) } - wg.Wait() + finalizedTxs = append(finalizedTxs, validatedReceiptTxs...) etxIDs := f.buildTxIdList(finalizedTxs) @@ -217,8 +199,50 @@ func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) process return nil } +func (f *evmFinalizer) batchCheckReceiptHashes(ctx context.Context, receiptMap map[common.Hash][]*Tx, latestFinalizedBlockNum int64) ([]*Tx, error) { + if len(receiptMap) == 0 { + return nil, nil + } + var rpcBatchCalls []rpc.BatchElem + for hash := range receiptMap { + elem := rpc.BatchElem{ + Method: "eth_getBlockByHash", + Args: []any{ + hash, + false, + }, + Result: new(evmtypes.Head), + } + rpcBatchCalls = append(rpcBatchCalls, elem) + } + + err := f.client.BatchCallContext(ctx, rpcBatchCalls) + if err != nil { + return nil, fmt.Errorf("get block hash batch call failed: %w", err) + } + var finalizedTxs []*Tx + for _, req := range rpcBatchCalls { + if req.Error != nil { + f.lggr.Debugw("failed to find block by hash", "hash", req.Args[0]) + continue + } + head := req.Result.(*evmtypes.Head) + if head == nil { + f.lggr.Debugw("failed to find block by hash", "hash", req.Args[0]) + continue + } + // Confirmed receipt's block hash exists on-chain still + // Add to finalized list if block num less than or equal to the latest finalized head block num + if head.BlockNumber() <= latestFinalizedBlockNum { + txs := receiptMap[head.BlockHash()] + finalizedTxs = append(finalizedTxs, txs...) + } + } + return finalizedTxs, nil +} + // Build list of transaction IDs -func (f *Finalizer[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE, HEAD]) buildTxIdList(finalizedTxs []*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) []int64 { +func (f *evmFinalizer) buildTxIdList(finalizedTxs []*Tx) []int64 { etxIDs := make([]int64, len(finalizedTxs)) for i, tx := range finalizedTxs { receipt := tx.GetReceipt() diff --git a/core/chains/evm/txmgr/finalizer_test.go b/core/chains/evm/txmgr/finalizer_test.go index 263f77fafc0..a9e7b33dc32 100644 --- a/core/chains/evm/txmgr/finalizer_test.go +++ b/core/chains/evm/txmgr/finalizer_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/rpc" "github.com/google/uuid" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -29,7 +30,7 @@ func TestFinalizer_MarkTxFinalized(t *testing.T) { feeLimit := uint64(10_000) ethClient := testutils.NewEthClientMockWithDefaultChain(t) - finalizer := txmgr.NewEvmFinalizer(logger.Test(t), testutils.FixtureChainID, txStore, txmgr.NewEvmTxmClient(ethClient, nil)) + finalizer := txmgr.NewEvmFinalizer(logger.Test(t), testutils.FixtureChainID, txStore, ethClient) err := finalizer.Start(ctx) require.NoError(t, err) @@ -138,7 +139,17 @@ func TestFinalizer_MarkTxFinalized(t *testing.T) { // Insert receipt for finalized block num receiptHash := utils.NewHash() mustInsertEthReceipt(t, txStore, head.Parent.Number-1, receiptHash, attemptHash) - ethClient.On("HeadByHash", mock.Anything, receiptHash).Return(&evmtypes.Head{Number: head.Parent.Number - 1, Hash: receiptHash}, nil) + ethClient.On("BatchCallContext", mock.Anything, mock.IsType([]rpc.BatchElem{})).Run(func(args mock.Arguments) { + rpcElements := args.Get(1).([]rpc.BatchElem) + require.Equal(t, 1, len(rpcElements)) + + require.Equal(t, "eth_getBlockByHash", rpcElements[0].Method) + require.Equal(t, receiptHash.String(), rpcElements[0].Args[0].(common.Hash).String()) + require.Equal(t, false, rpcElements[0].Args[1]) + + head := evmtypes.Head{Number: head.Parent.Number - 1, Hash: receiptHash} + rpcElements[0].Result = &head + }).Return(nil).Once() err = finalizer.ProcessHead(ctx, head) require.NoError(t, err) tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) diff --git a/core/chains/evm/txmgr/mocks/evm_tx_store.go b/core/chains/evm/txmgr/mocks/evm_tx_store.go index ab695ec2045..b1cc3155557 100644 --- a/core/chains/evm/txmgr/mocks/evm_tx_store.go +++ b/core/chains/evm/txmgr/mocks/evm_tx_store.go @@ -199,6 +199,36 @@ func (_m *EvmTxStore) DeleteInProgressAttempt(ctx context.Context, attempt types return r0 } +// FindConfirmedTxesAwaitingFinalization provides a mock function with given fields: ctx, chainID +func (_m *EvmTxStore) FindConfirmedTxesAwaitingFinalization(ctx context.Context, chainID *big.Int) ([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], error) { + ret := _m.Called(ctx, chainID) + + if len(ret) == 0 { + panic("no return value specified for FindConfirmedTxesAwaitingFinalization") + } + + var r0 []*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee] + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *big.Int) ([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], error)); ok { + return rf(ctx, chainID) + } + if rf, ok := ret.Get(0).(func(context.Context, *big.Int) []*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]); ok { + r0 = rf(ctx, chainID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *big.Int) error); ok { + r1 = rf(ctx, chainID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // FindEarliestUnconfirmedBroadcastTime provides a mock function with given fields: ctx, chainID func (_m *EvmTxStore) FindEarliestUnconfirmedBroadcastTime(ctx context.Context, chainID *big.Int) (null.Time, error) { ret := _m.Called(ctx, chainID) @@ -313,36 +343,6 @@ func (_m *EvmTxStore) FindNextUnstartedTransactionFromAddress(ctx context.Contex return r0, r1 } -// FindTransactionsByState provides a mock function with given fields: ctx, state, chainID -func (_m *EvmTxStore) FindTransactionsByState(ctx context.Context, state types.TxState, chainID *big.Int) ([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], error) { - ret := _m.Called(ctx, state, chainID) - - if len(ret) == 0 { - panic("no return value specified for FindTransactionsByState") - } - - var r0 []*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee] - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, types.TxState, *big.Int) ([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], error)); ok { - return rf(ctx, state, chainID) - } - if rf, ok := ret.Get(0).(func(context.Context, types.TxState, *big.Int) []*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]); ok { - r0 = rf(ctx, state, chainID) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, types.TxState, *big.Int) error); ok { - r1 = rf(ctx, state, chainID) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // FindTransactionsConfirmedInBlockRange provides a mock function with given fields: ctx, highBlockNumber, lowBlockNumber, chainID func (_m *EvmTxStore) FindTransactionsConfirmedInBlockRange(ctx context.Context, highBlockNumber int64, lowBlockNumber int64, chainID *big.Int) ([]*types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], error) { ret := _m.Called(ctx, highBlockNumber, lowBlockNumber, chainID) @@ -1131,17 +1131,17 @@ func (_m *EvmTxStore) PruneUnstartedTxQueue(ctx context.Context, queueSize uint3 return r0, r1 } -// ReapTxHistory provides a mock function with given fields: ctx, minBlockNumberToKeep, timeThreshold, chainID -func (_m *EvmTxStore) ReapTxHistory(ctx context.Context, minBlockNumberToKeep int64, timeThreshold time.Time, chainID *big.Int) error { - ret := _m.Called(ctx, minBlockNumberToKeep, timeThreshold, chainID) +// ReapTxHistory provides a mock function with given fields: ctx, timeThreshold, chainID +func (_m *EvmTxStore) ReapTxHistory(ctx context.Context, timeThreshold time.Time, chainID *big.Int) error { + ret := _m.Called(ctx, timeThreshold, chainID) if len(ret) == 0 { panic("no return value specified for ReapTxHistory") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, int64, time.Time, *big.Int) error); ok { - r0 = rf(ctx, minBlockNumberToKeep, timeThreshold, chainID) + if rf, ok := ret.Get(0).(func(context.Context, time.Time, *big.Int) error); ok { + r0 = rf(ctx, timeThreshold, chainID) } else { r0 = ret.Error(0) } diff --git a/core/chains/evm/txmgr/models.go b/core/chains/evm/txmgr/models.go index 92e18b820bf..5361402c469 100644 --- a/core/chains/evm/txmgr/models.go +++ b/core/chains/evm/txmgr/models.go @@ -42,7 +42,7 @@ type ( TxmClient = txmgrtypes.TxmClient[*big.Int, common.Address, common.Hash, common.Hash, *evmtypes.Receipt, evmtypes.Nonce, gas.EvmFee, *evmtypes.Head] TransactionClient = txmgrtypes.TransactionClient[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee] ChainReceipt = txmgrtypes.ChainReceipt[common.Hash, common.Hash] - Finalizer = txmgr.Finalizer[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee, *evmtypes.Head] + Finalizer = txmgrtypes.Finalizer[common.Hash, *evmtypes.Head] ) var _ KeyStore = (keystore.Eth)(nil) // check interface in txmgr to avoid circular import diff --git a/core/chains/evm/txmgr/reaper_test.go b/core/chains/evm/txmgr/reaper_test.go index b3ce48b702c..d1d84939127 100644 --- a/core/chains/evm/txmgr/reaper_test.go +++ b/core/chains/evm/txmgr/reaper_test.go @@ -12,18 +12,17 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/utils" txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" - txmgrmocks "github.com/smartcontractkit/chainlink/v2/common/txmgr/types/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" ) -func newReaperWithChainID(t *testing.T, db txmgrtypes.TxHistoryReaper[*big.Int], cfg txmgrtypes.ReaperChainConfig, txConfig txmgrtypes.ReaperTransactionsConfig, cid *big.Int) *txmgr.Reaper { - return txmgr.NewEvmReaper(logger.Test(t), db, cfg, txConfig, cid) +func newReaperWithChainID(t *testing.T, db txmgrtypes.TxHistoryReaper[*big.Int], txConfig txmgrtypes.ReaperTransactionsConfig, cid *big.Int) *txmgr.Reaper { + return txmgr.NewEvmReaper(logger.Test(t), db, txConfig, cid) } -func newReaper(t *testing.T, db txmgrtypes.TxHistoryReaper[*big.Int], cfg txmgrtypes.ReaperChainConfig, txConfig txmgrtypes.ReaperTransactionsConfig) *txmgr.Reaper { - return newReaperWithChainID(t, db, cfg, txConfig, &cltest.FixtureChainID) +func newReaper(t *testing.T, db txmgrtypes.TxHistoryReaper[*big.Int], txConfig txmgrtypes.ReaperTransactionsConfig) *txmgr.Reaper { + return newReaperWithChainID(t, db, txConfig, &cltest.FixtureChainID) } type reaperConfig struct { @@ -51,12 +50,9 @@ func TestReaper_ReapTxes(t *testing.T) { oneDayAgo := time.Now().Add(-24 * time.Hour) t.Run("with nothing in the database, doesn't error", func(t *testing.T) { - config := txmgrmocks.NewReaperConfig(t) - config.On("FinalityDepth").Return(uint32(10)) - tc := &reaperConfig{reaperThreshold: 1 * time.Hour} - r := newReaper(t, txStore, config, tc) + r := newReaper(t, txStore, tc) err := r.ReapTxes(42) assert.NoError(t, err) @@ -66,11 +62,10 @@ func TestReaper_ReapTxes(t *testing.T) { mustInsertConfirmedEthTxWithReceipt(t, txStore, from, nonce, 5) t.Run("skips if threshold=0", func(t *testing.T) { - config := txmgrmocks.NewReaperConfig(t) tc := &reaperConfig{reaperThreshold: 0 * time.Second} - r := newReaper(t, txStore, config, tc) + r := newReaper(t, txStore, tc) err := r.ReapTxes(42) assert.NoError(t, err) @@ -79,12 +74,9 @@ func TestReaper_ReapTxes(t *testing.T) { }) t.Run("doesn't touch ethtxes with different chain ID", func(t *testing.T) { - config := txmgrmocks.NewReaperConfig(t) - config.On("FinalityDepth").Return(uint32(10)) - tc := &reaperConfig{reaperThreshold: 1 * time.Hour} - r := newReaperWithChainID(t, txStore, config, tc, big.NewInt(42)) + r := newReaperWithChainID(t, txStore, tc, big.NewInt(42)) err := r.ReapTxes(42) assert.NoError(t, err) @@ -92,13 +84,10 @@ func TestReaper_ReapTxes(t *testing.T) { cltest.AssertCount(t, db, "evm.txes", 1) }) - t.Run("deletes confirmed evm.txes that exceed the age threshold with at least EVM.FinalityDepth blocks above their receipt", func(t *testing.T) { - config := txmgrmocks.NewReaperConfig(t) - config.On("FinalityDepth").Return(uint32(10)) - + t.Run("deletes confirmed evm.txes marked as finalized that exceed the age threshold", func(t *testing.T) { tc := &reaperConfig{reaperThreshold: 1 * time.Hour} - r := newReaper(t, txStore, config, tc) + r := newReaper(t, txStore, tc) err := r.ReapTxes(42) assert.NoError(t, err) @@ -109,9 +98,11 @@ func TestReaper_ReapTxes(t *testing.T) { err = r.ReapTxes(12) assert.NoError(t, err) - // Didn't delete because eth_tx although old enough, was still within EVM.FinalityDepth of the current head + // Didn't delete because eth_tx although old enough, was not marked as finalized cltest.AssertCount(t, db, "evm.txes", 1) + pgtest.MustExec(t, db, `UPDATE evm.txes SET finalized=true`) + err = r.ReapTxes(42) assert.NoError(t, err) // Now it deleted because the eth_tx was past EVM.FinalityDepth @@ -121,12 +112,9 @@ func TestReaper_ReapTxes(t *testing.T) { mustInsertFatalErrorEthTx(t, txStore, from) t.Run("deletes errored evm.txes that exceed the age threshold", func(t *testing.T) { - config := txmgrmocks.NewReaperConfig(t) - config.On("FinalityDepth").Return(uint32(10)) - tc := &reaperConfig{reaperThreshold: 1 * time.Hour} - r := newReaper(t, txStore, config, tc) + r := newReaper(t, txStore, tc) err := r.ReapTxes(42) assert.NoError(t, err) From 8c881e0e9c789cc0172cd2c6a32faa762da0c40d Mon Sep 17 00:00:00 2001 From: amit-momin Date: Mon, 24 Jun 2024 17:32:37 -0500 Subject: [PATCH 03/16] Fixed linting and renumbered sql migration --- core/chains/evm/txmgr/evm_tx_store_test.go | 1 - core/chains/evm/txmgr/finalizer.go | 2 +- core/chains/evm/txmgr/reaper_test.go | 1 - ..._finalized_column.sql => 0246_add_tx_finalized_column.sql} | 4 ++-- 4 files changed, 3 insertions(+), 5 deletions(-) rename core/store/migrate/migrations/{0245_add_tx_finalized_column.sql => 0246_add_tx_finalized_column.sql} (79%) diff --git a/core/chains/evm/txmgr/evm_tx_store_test.go b/core/chains/evm/txmgr/evm_tx_store_test.go index b0a0a0379f1..dde8e047bfd 100644 --- a/core/chains/evm/txmgr/evm_tx_store_test.go +++ b/core/chains/evm/txmgr/evm_tx_store_test.go @@ -1867,7 +1867,6 @@ func TestORM_FindTransactionsByState(t *testing.T) { txs, err := txStore.FindConfirmedTxesAwaitingFinalization(ctx, testutils.FixtureChainID) require.NoError(t, err) require.Len(t, txs, 1) - } func TestORM_UpdateTxesFinalized(t *testing.T) { diff --git a/core/chains/evm/txmgr/finalizer.go b/core/chains/evm/txmgr/finalizer.go index 477607b0ecf..87a0726ecaa 100644 --- a/core/chains/evm/txmgr/finalizer.go +++ b/core/chains/evm/txmgr/finalizer.go @@ -185,7 +185,7 @@ func (f *evmFinalizer) processHead(ctx context.Context, head *evmtypes.Head) err validatedReceiptTxs, err := f.batchCheckReceiptHashes(ctx, receiptBlockHashToTx, latestFinalizedHead.BlockNumber()) if err != nil { // Do not error out to allow transactions that did not need RPC validation to still be marked as finalized - // The transcations failed to be validated will be checked again in the next round + // The transactions failed to be validated will be checked again in the next round f.lggr.Errorf("failed to validate receipt block hashes over RPC: %v", err) } finalizedTxs = append(finalizedTxs, validatedReceiptTxs...) diff --git a/core/chains/evm/txmgr/reaper_test.go b/core/chains/evm/txmgr/reaper_test.go index d1d84939127..eaa7eecb252 100644 --- a/core/chains/evm/txmgr/reaper_test.go +++ b/core/chains/evm/txmgr/reaper_test.go @@ -62,7 +62,6 @@ func TestReaper_ReapTxes(t *testing.T) { mustInsertConfirmedEthTxWithReceipt(t, txStore, from, nonce, 5) t.Run("skips if threshold=0", func(t *testing.T) { - tc := &reaperConfig{reaperThreshold: 0 * time.Second} r := newReaper(t, txStore, tc) diff --git a/core/store/migrate/migrations/0245_add_tx_finalized_column.sql b/core/store/migrate/migrations/0246_add_tx_finalized_column.sql similarity index 79% rename from core/store/migrate/migrations/0245_add_tx_finalized_column.sql rename to core/store/migrate/migrations/0246_add_tx_finalized_column.sql index cc0b09fd3dd..5f2c5c5ffb2 100644 --- a/core/store/migrate/migrations/0245_add_tx_finalized_column.sql +++ b/core/store/migrate/migrations/0246_add_tx_finalized_column.sql @@ -2,9 +2,9 @@ -- +goose StatementBegin ALTER TABLE evm.txes ADD COLUMN finalized boolean NOT NULL DEFAULT false; ALTER TABLE evm.txes ADD CONSTRAINT chk_eth_txes_state_finalized CHECK ( - state <> 'confirmed'::eth_txes_state AND finalized = false + state <> 'confirmed'::evm.txes_state AND finalized = false OR - state = 'confirmed'::eth_txes_state + state = 'confirmed'::evm.txes_state ) NOT VALID; -- +goose StatementEnd From 36d8c70dabb9ba219d2a7d7c8e08b1904f798d98 Mon Sep 17 00:00:00 2001 From: amit-momin Date: Tue, 25 Jun 2024 16:22:58 -0500 Subject: [PATCH 04/16] Added limit to Finalizer RPC batch calls --- core/chains/evm/txmgr/builder.go | 2 +- core/chains/evm/txmgr/finalizer.go | 81 +++++++++++++++---------- core/chains/evm/txmgr/finalizer_test.go | 39 +++++++++--- 3 files changed, 81 insertions(+), 41 deletions(-) diff --git a/core/chains/evm/txmgr/builder.go b/core/chains/evm/txmgr/builder.go index 23d802c521b..4e6949dcca7 100644 --- a/core/chains/evm/txmgr/builder.go +++ b/core/chains/evm/txmgr/builder.go @@ -53,7 +53,7 @@ func NewTxm( evmTracker := NewEvmTracker(txStore, keyStore, chainID, lggr) stuckTxDetector := NewStuckTxDetector(lggr, client.ConfiguredChainID(), chainConfig.ChainType(), fCfg.PriceMax(), txConfig.AutoPurge(), estimator, txStore, client) evmConfirmer := NewEvmConfirmer(txStore, txmClient, txmCfg, feeCfg, txConfig, dbConfig, keyStore, txAttemptBuilder, lggr, stuckTxDetector) - evmFinalizer := NewEvmFinalizer(lggr, client.ConfiguredChainID(), txStore, client) + evmFinalizer := NewEvmFinalizer(lggr, client.ConfiguredChainID(), chainConfig.RPCDefaultBatchSize(), txStore, client) var evmResender *Resender if txConfig.ResendAfterThreshold() > 0 { evmResender = NewEvmResender(lggr, txStore, txmClient, evmTracker, keyStore, txmgr.DefaultResenderPollInterval, chainConfig, txConfig) diff --git a/core/chains/evm/txmgr/finalizer.go b/core/chains/evm/txmgr/finalizer.go index 87a0726ecaa..03c160726ac 100644 --- a/core/chains/evm/txmgr/finalizer.go +++ b/core/chains/evm/txmgr/finalizer.go @@ -35,28 +35,31 @@ type finalizerChainClient interface { // Finalizer handles processing new finalized blocks and marking transactions as finalized accordingly in the TXM DB type evmFinalizer struct { services.StateMachine - lggr logger.SugaredLogger - chainId *big.Int - txStore finalizerTxStore - client finalizerChainClient - mb *mailbox.Mailbox[*evmtypes.Head] - stopCh services.StopChan - wg sync.WaitGroup + lggr logger.SugaredLogger + chainId *big.Int + rpcBatchSize int + txStore finalizerTxStore + client finalizerChainClient + mb *mailbox.Mailbox[*evmtypes.Head] + stopCh services.StopChan + wg sync.WaitGroup } func NewEvmFinalizer( lggr logger.Logger, chainId *big.Int, + rpcBatchSize uint32, txStore finalizerTxStore, client finalizerChainClient, ) *evmFinalizer { lggr = logger.Named(lggr, "Finalizer") return &evmFinalizer{ - txStore: txStore, - lggr: logger.Sugared(lggr), - chainId: chainId, - client: client, - mb: mailbox.NewSingle[*evmtypes.Head](), + lggr: logger.Sugared(lggr), + chainId: chainId, + rpcBatchSize: int(rpcBatchSize), + txStore: txStore, + client: client, + mb: mailbox.NewSingle[*evmtypes.Head](), } } @@ -182,7 +185,7 @@ func (f *evmFinalizer) processHead(ctx context.Context, head *evmtypes.Head) err // Check if block hashes exist for receipts on-chain older than the earliest cached head // Transactions are grouped by their receipt block hash to avoid repeat requests on the same hash in case transactions were confirmed in the same block - validatedReceiptTxs, err := f.batchCheckReceiptHashes(ctx, receiptBlockHashToTx, latestFinalizedHead.BlockNumber()) + validatedReceiptTxs, err := f.batchCheckReceiptHashesOnchain(ctx, receiptBlockHashToTx, latestFinalizedHead.BlockNumber()) if err != nil { // Do not error out to allow transactions that did not need RPC validation to still be marked as finalized // The transactions failed to be validated will be checked again in the next round @@ -199,11 +202,13 @@ func (f *evmFinalizer) processHead(ctx context.Context, head *evmtypes.Head) err return nil } -func (f *evmFinalizer) batchCheckReceiptHashes(ctx context.Context, receiptMap map[common.Hash][]*Tx, latestFinalizedBlockNum int64) ([]*Tx, error) { +func (f *evmFinalizer) batchCheckReceiptHashesOnchain(ctx context.Context, receiptMap map[common.Hash][]*Tx, latestFinalizedBlockNum int64) ([]*Tx, error) { if len(receiptMap) == 0 { return nil, nil } - var rpcBatchCalls []rpc.BatchElem + // Group the RPC batch calls in groups of rpcBatchSize + var rpcBatchGroups [][]rpc.BatchElem + var rpcBatch []rpc.BatchElem for hash := range receiptMap { elem := rpc.BatchElem{ Method: "eth_getBlockByHash", @@ -213,29 +218,39 @@ func (f *evmFinalizer) batchCheckReceiptHashes(ctx context.Context, receiptMap m }, Result: new(evmtypes.Head), } - rpcBatchCalls = append(rpcBatchCalls, elem) + rpcBatch = append(rpcBatch, elem) + if len(rpcBatch) >= f.rpcBatchSize { + rpcBatchGroups = append(rpcBatchGroups, rpcBatch) + rpcBatch = []rpc.BatchElem{} + } } - err := f.client.BatchCallContext(ctx, rpcBatchCalls) - if err != nil { - return nil, fmt.Errorf("get block hash batch call failed: %w", err) - } var finalizedTxs []*Tx - for _, req := range rpcBatchCalls { - if req.Error != nil { - f.lggr.Debugw("failed to find block by hash", "hash", req.Args[0]) - continue - } - head := req.Result.(*evmtypes.Head) - if head == nil { - f.lggr.Debugw("failed to find block by hash", "hash", req.Args[0]) + for _, rpcBatch := range rpcBatchGroups { + err := f.client.BatchCallContext(ctx, rpcBatch) + if err != nil { + // Continue if batch RPC call failed so other batches can still be considered for finalization + f.lggr.Debugw("failed to find blocks due to batch call failure") continue } - // Confirmed receipt's block hash exists on-chain still - // Add to finalized list if block num less than or equal to the latest finalized head block num - if head.BlockNumber() <= latestFinalizedBlockNum { - txs := receiptMap[head.BlockHash()] - finalizedTxs = append(finalizedTxs, txs...) + for _, req := range rpcBatch { + if req.Error != nil { + // Continue if particular RPC call failed so other txs can still be considered for finalization + f.lggr.Debugw("failed to find block by hash", "hash", req.Args[0]) + continue + } + head := req.Result.(*evmtypes.Head) + if head == nil { + // Continue if particular RPC call yielded a nil head so other txs can still be considered for finalization + f.lggr.Debugw("failed to find block by hash", "hash", req.Args[0]) + continue + } + // Confirmed receipt's block hash exists on-chain + // Add to finalized list if block num less than or equal to the latest finalized head block num + if head.BlockNumber() <= latestFinalizedBlockNum { + txs := receiptMap[head.BlockHash()] + finalizedTxs = append(finalizedTxs, txs...) + } } } return finalizedTxs, nil diff --git a/core/chains/evm/txmgr/finalizer_test.go b/core/chains/evm/txmgr/finalizer_test.go index a9e7b33dc32..559eaa42485 100644 --- a/core/chains/evm/txmgr/finalizer_test.go +++ b/core/chains/evm/txmgr/finalizer_test.go @@ -29,8 +29,9 @@ func TestFinalizer_MarkTxFinalized(t *testing.T) { ethKeyStore := cltest.NewKeyStore(t, db).Eth() feeLimit := uint64(10_000) ethClient := testutils.NewEthClientMockWithDefaultChain(t) + rpcBatchSize := uint32(1) - finalizer := txmgr.NewEvmFinalizer(logger.Test(t), testutils.FixtureChainID, txStore, ethClient) + finalizer := txmgr.NewEvmFinalizer(logger.Test(t), testutils.FixtureChainID, rpcBatchSize, txStore, ethClient) err := finalizer.Start(ctx) require.NoError(t, err) @@ -137,19 +138,43 @@ func TestFinalizer_MarkTxFinalized(t *testing.T) { } attemptHash := insertTxAndAttemptWithIdempotencyKey(t, txStore, tx, idempotencyKey) // Insert receipt for finalized block num - receiptHash := utils.NewHash() - mustInsertEthReceipt(t, txStore, head.Parent.Number-1, receiptHash, attemptHash) + receiptHash1 := utils.NewHash() + mustInsertEthReceipt(t, txStore, head.Parent.Number-2, receiptHash1, attemptHash) + idempotencyKey = uuid.New().String() + nonce = evmtypes.Nonce(1) + tx = &txmgr.Tx{ + Sequence: &nonce, + IdempotencyKey: &idempotencyKey, + FromAddress: fromAddress, + EncodedPayload: []byte{1, 2, 3}, + FeeLimit: feeLimit, + State: txmgrcommon.TxConfirmed, + BroadcastAt: &broadcast, + InitialBroadcastAt: &broadcast, + } + attemptHash = insertTxAndAttemptWithIdempotencyKey(t, txStore, tx, idempotencyKey) + // Insert receipt for finalized block num + receiptHash2 := utils.NewHash() + mustInsertEthReceipt(t, txStore, head.Parent.Number-1, receiptHash2, attemptHash) + // Separate batch calls will be made for each tx due to RPC batch size set to 1 when finalizer initialized above ethClient.On("BatchCallContext", mock.Anything, mock.IsType([]rpc.BatchElem{})).Run(func(args mock.Arguments) { rpcElements := args.Get(1).([]rpc.BatchElem) require.Equal(t, 1, len(rpcElements)) require.Equal(t, "eth_getBlockByHash", rpcElements[0].Method) - require.Equal(t, receiptHash.String(), rpcElements[0].Args[0].(common.Hash).String()) require.Equal(t, false, rpcElements[0].Args[1]) - head := evmtypes.Head{Number: head.Parent.Number - 1, Hash: receiptHash} - rpcElements[0].Result = &head - }).Return(nil).Once() + reqHash := rpcElements[0].Args[0].(common.Hash).String() + var headResult evmtypes.Head + if receiptHash1.String() == reqHash { + headResult = evmtypes.Head{Number: head.Parent.Number - 2, Hash: receiptHash1} + } else if receiptHash2.String() == reqHash { + headResult = evmtypes.Head{Number: head.Parent.Number - 1, Hash: receiptHash2} + } else { + require.Fail(t, "unrecognized block hash") + } + rpcElements[0].Result = &headResult + }).Return(nil).Twice() err = finalizer.ProcessHead(ctx, head) require.NoError(t, err) tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) From d9daeb2005944440f665002cb8f4a0b44fe5432a Mon Sep 17 00:00:00 2001 From: amit-momin Date: Wed, 26 Jun 2024 12:35:05 -0500 Subject: [PATCH 05/16] Cleaned up unneeded code --- common/txmgr/confirmer.go | 4 +- common/txmgr/test_helpers.go | 2 +- common/txmgr/types/client.go | 2 - .../txmgr/types/mocks/reaper_chain_config.go | 42 ------------------- core/chains/evm/txmgr/finalizer.go | 19 ++------- core/chains/evm/txmgr/models.go | 2 +- 6 files changed, 8 insertions(+), 63 deletions(-) delete mode 100644 common/txmgr/types/mocks/reaper_chain_config.go diff --git a/common/txmgr/confirmer.go b/common/txmgr/confirmer.go index 2cdc48bea19..a5c2af0d4d7 100644 --- a/common/txmgr/confirmer.go +++ b/common/txmgr/confirmer.go @@ -120,7 +120,7 @@ type Confirmer[ services.StateMachine txStore txmgrtypes.TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE] lggr logger.SugaredLogger - client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE, HEAD] + client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] txmgrtypes.TxAttemptBuilder[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] stuckTxDetector txmgrtypes.StuckTxDetector[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] resumeCallback ResumeCallback @@ -154,7 +154,7 @@ func NewConfirmer[ FEE feetypes.Fee, ]( txStore txmgrtypes.TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE], - client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE, HEAD], + client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE], chainConfig txmgrtypes.ConfirmerChainConfig, feeConfig txmgrtypes.ConfirmerFeeConfig, txConfig txmgrtypes.ConfirmerTransactionsConfig, diff --git a/common/txmgr/test_helpers.go b/common/txmgr/test_helpers.go index ef3866815d5..3051e0985d8 100644 --- a/common/txmgr/test_helpers.go +++ b/common/txmgr/test_helpers.go @@ -10,7 +10,7 @@ import ( // TEST ONLY FUNCTIONS // these need to be exported for the txmgr tests to continue to work -func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) XXXTestSetClient(client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE, HEAD]) { +func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) XXXTestSetClient(client txmgrtypes.TxmClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) { ec.client = client } diff --git a/common/txmgr/types/client.go b/common/txmgr/types/client.go index ea3ed3af29c..759b15d6162 100644 --- a/common/txmgr/types/client.go +++ b/common/txmgr/types/client.go @@ -21,7 +21,6 @@ type TxmClient[ R ChainReceipt[TX_HASH, BLOCK_HASH], SEQ types.Sequence, FEE feetypes.Fee, - HEAD types.Head[BLOCK_HASH], ] interface { ChainClient[CHAIN_ID, ADDR, SEQ] TransactionClient[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] @@ -31,7 +30,6 @@ type TxmClient[ ctx context.Context, attempts []TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], ) (txReceipt []R, txErr []error, err error) - HeadByHash(ctx context.Context, hash BLOCK_HASH) (HEAD, error) } // TransactionClient contains the methods for building, simulating, broadcasting transactions diff --git a/common/txmgr/types/mocks/reaper_chain_config.go b/common/txmgr/types/mocks/reaper_chain_config.go deleted file mode 100644 index 30f58c10003..00000000000 --- a/common/txmgr/types/mocks/reaper_chain_config.go +++ /dev/null @@ -1,42 +0,0 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. - -package mocks - -import mock "github.com/stretchr/testify/mock" - -// ReaperConfig is an autogenerated mock type for the ReaperChainConfig type -type ReaperConfig struct { - mock.Mock -} - -// FinalityDepth provides a mock function with given fields: -func (_m *ReaperConfig) FinalityDepth() uint32 { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for FinalityDepth") - } - - var r0 uint32 - if rf, ok := ret.Get(0).(func() uint32); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(uint32) - } - - return r0 -} - -// NewReaperConfig creates a new instance of ReaperConfig. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewReaperConfig(t interface { - mock.TestingT - Cleanup(func()) -}) *ReaperConfig { - mock := &ReaperConfig{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/core/chains/evm/txmgr/finalizer.go b/core/chains/evm/txmgr/finalizer.go index 03c160726ac..eba6d5a006b 100644 --- a/core/chains/evm/txmgr/finalizer.go +++ b/core/chains/evm/txmgr/finalizer.go @@ -63,29 +63,18 @@ func NewEvmFinalizer( } } -// Start is a comment to appease the linter +// Start the finalizer func (f *evmFinalizer) Start(ctx context.Context) error { - return f.StartOnce("Finalizer", func() error { - return f.startInternal(ctx) - }) -} - -func (f *evmFinalizer) startInternal(_ context.Context) error { + f.lggr.Debugf("started Finalizer with RPC batch size limit: %d", f.rpcBatchSize) f.stopCh = make(chan struct{}) - f.wg = sync.WaitGroup{} f.wg.Add(1) go f.runLoop() return nil } -// Close is a comment to appease the linter +// Close the finalizer func (f *evmFinalizer) Close() error { - return f.StopOnce("Finalizer", func() error { - return f.closeInternal() - }) -} - -func (f *evmFinalizer) closeInternal() error { + f.lggr.Debug("closing Finalizer") close(f.stopCh) f.wg.Wait() return nil diff --git a/core/chains/evm/txmgr/models.go b/core/chains/evm/txmgr/models.go index 5361402c469..557e3957445 100644 --- a/core/chains/evm/txmgr/models.go +++ b/core/chains/evm/txmgr/models.go @@ -39,7 +39,7 @@ type ( Receipt = dbReceipt // EvmReceipt is the exported DB table model for receipts ReceiptPlus = txmgrtypes.ReceiptPlus[*evmtypes.Receipt] StuckTxDetector = txmgrtypes.StuckTxDetector[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee] - TxmClient = txmgrtypes.TxmClient[*big.Int, common.Address, common.Hash, common.Hash, *evmtypes.Receipt, evmtypes.Nonce, gas.EvmFee, *evmtypes.Head] + TxmClient = txmgrtypes.TxmClient[*big.Int, common.Address, common.Hash, common.Hash, *evmtypes.Receipt, evmtypes.Nonce, gas.EvmFee] TransactionClient = txmgrtypes.TransactionClient[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee] ChainReceipt = txmgrtypes.ChainReceipt[common.Hash, common.Hash] Finalizer = txmgrtypes.Finalizer[common.Hash, *evmtypes.Head] From 7f145bcf38811fb3bab8338980c1a67e80012c7c Mon Sep 17 00:00:00 2001 From: amit-momin Date: Fri, 28 Jun 2024 12:48:28 -0500 Subject: [PATCH 06/16] Renumbered sql migration --- ...d_tx_finalized_column.sql => 0247_add_tx_finalized_column.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename core/store/migrate/migrations/{0246_add_tx_finalized_column.sql => 0247_add_tx_finalized_column.sql} (100%) diff --git a/core/store/migrate/migrations/0246_add_tx_finalized_column.sql b/core/store/migrate/migrations/0247_add_tx_finalized_column.sql similarity index 100% rename from core/store/migrate/migrations/0246_add_tx_finalized_column.sql rename to core/store/migrate/migrations/0247_add_tx_finalized_column.sql From c90a0006a7f2e64868f7695d60805359c17fc9c8 Mon Sep 17 00:00:00 2001 From: amit-momin Date: Mon, 1 Jul 2024 13:28:23 -0500 Subject: [PATCH 07/16] Updated Finalizer to use LatestAndFinalizedBlock method from HeadTracker --- common/txmgr/txmgr.go | 2 +- common/txmgr/types/finalizer.go | 2 +- .../evm/headtracker/simulated_head_tracker.go | 29 ++++++++++++++++ core/chains/evm/txmgr/builder.go | 4 ++- core/chains/evm/txmgr/finalizer.go | 22 ++++++++++--- core/chains/evm/txmgr/finalizer_test.go | 28 +++++++++++++++- core/chains/evm/txmgr/test_helpers.go | 7 ++-- core/chains/evm/txmgr/txmgr_test.go | 33 ++++++++++++------- core/chains/legacyevm/chain.go | 2 +- core/chains/legacyevm/evm_txm.go | 5 ++- .../promreporter/prom_reporter_test.go | 3 +- core/services/vrf/delegate_test.go | 2 +- 12 files changed, 111 insertions(+), 28 deletions(-) diff --git a/common/txmgr/txmgr.go b/common/txmgr/txmgr.go index ff2c9c2c605..1ceb4df3424 100644 --- a/common/txmgr/txmgr.go +++ b/common/txmgr/txmgr.go @@ -425,7 +425,7 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) runLoop() case head := <-b.chHeads: b.confirmer.mb.Deliver(head) b.tracker.mb.Deliver(head.BlockNumber()) - b.finalizer.DeliverHead(head) + b.finalizer.DeliverLatestHead(head) case reset := <-b.reset: // This check prevents the weird edge-case where you can select // into this block after chStop has already been closed and the diff --git a/common/txmgr/types/finalizer.go b/common/txmgr/types/finalizer.go index d7fd6b7f603..be3c897d0e2 100644 --- a/common/txmgr/types/finalizer.go +++ b/common/txmgr/types/finalizer.go @@ -8,5 +8,5 @@ import ( type Finalizer[BLOCK_HASH types.Hashable, HEAD types.Head[BLOCK_HASH]] interface { // interfaces for running the underlying estimator services.Service - DeliverHead(head HEAD) bool + DeliverLatestHead(head HEAD) bool } diff --git a/core/chains/evm/headtracker/simulated_head_tracker.go b/core/chains/evm/headtracker/simulated_head_tracker.go index e1e550de992..d4e09690989 100644 --- a/core/chains/evm/headtracker/simulated_head_tracker.go +++ b/core/chains/evm/headtracker/simulated_head_tracker.go @@ -2,6 +2,7 @@ package headtracker import ( "context" + "errors" "fmt" "math/big" @@ -51,3 +52,31 @@ func (ht *simulatedHeadTracker) LatestAndFinalizedBlock(ctx context.Context) (*e return latest, finalizedBlock, nil } + +func (ht *simulatedHeadTracker) LatestChain() *evmtypes.Head { + return nil +} + +func (ht *simulatedHeadTracker) HealthReport() map[string]error { + return nil +} + +func (ht *simulatedHeadTracker) Start(_ context.Context) error { + return nil +} + +func (ht *simulatedHeadTracker) Close() error { + return nil +} + +func (ht *simulatedHeadTracker) Backfill(_ context.Context, _ *evmtypes.Head) error { + return errors.New("unimplemented") +} + +func (ht *simulatedHeadTracker) Name() string { + return "SimulatedHeadTracker" +} + +func (ht *simulatedHeadTracker) Ready() error { + return nil +} \ No newline at end of file diff --git a/core/chains/evm/txmgr/builder.go b/core/chains/evm/txmgr/builder.go index 4e6949dcca7..9614f494482 100644 --- a/core/chains/evm/txmgr/builder.go +++ b/core/chains/evm/txmgr/builder.go @@ -12,6 +12,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/forwarders" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" + httypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/headtracker/types" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/keystore" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" @@ -31,6 +32,7 @@ func NewTxm( logPoller logpoller.LogPoller, keyStore keystore.Eth, estimator gas.EvmFeeEstimator, + headTracker httypes.HeadTracker, ) (txm TxManager, err error, ) { @@ -53,7 +55,7 @@ func NewTxm( evmTracker := NewEvmTracker(txStore, keyStore, chainID, lggr) stuckTxDetector := NewStuckTxDetector(lggr, client.ConfiguredChainID(), chainConfig.ChainType(), fCfg.PriceMax(), txConfig.AutoPurge(), estimator, txStore, client) evmConfirmer := NewEvmConfirmer(txStore, txmClient, txmCfg, feeCfg, txConfig, dbConfig, keyStore, txAttemptBuilder, lggr, stuckTxDetector) - evmFinalizer := NewEvmFinalizer(lggr, client.ConfiguredChainID(), chainConfig.RPCDefaultBatchSize(), txStore, client) + evmFinalizer := NewEvmFinalizer(lggr, client.ConfiguredChainID(), chainConfig.RPCDefaultBatchSize(), txStore, client, headTracker) var evmResender *Resender if txConfig.ResendAfterThreshold() > 0 { evmResender = NewEvmResender(lggr, txStore, txmClient, evmTracker, keyStore, txmgr.DefaultResenderPollInterval, chainConfig, txConfig) diff --git a/core/chains/evm/txmgr/finalizer.go b/core/chains/evm/txmgr/finalizer.go index eba6d5a006b..666a54e7af0 100644 --- a/core/chains/evm/txmgr/finalizer.go +++ b/core/chains/evm/txmgr/finalizer.go @@ -32,14 +32,21 @@ type finalizerChainClient interface { BatchCallContext(ctx context.Context, elems []rpc.BatchElem) error } +type finalizerHeadTracker interface { + LatestAndFinalizedBlock(ctx context.Context) (latest, finalized *evmtypes.Head, err error) +} + // Finalizer handles processing new finalized blocks and marking transactions as finalized accordingly in the TXM DB type evmFinalizer struct { services.StateMachine lggr logger.SugaredLogger chainId *big.Int rpcBatchSize int + txStore finalizerTxStore client finalizerChainClient + headTracker finalizerHeadTracker + mb *mailbox.Mailbox[*evmtypes.Head] stopCh services.StopChan wg sync.WaitGroup @@ -51,6 +58,7 @@ func NewEvmFinalizer( rpcBatchSize uint32, txStore finalizerTxStore, client finalizerChainClient, + headTracker finalizerHeadTracker, ) *evmFinalizer { lggr = logger.Named(lggr, "Finalizer") return &evmFinalizer{ @@ -59,6 +67,7 @@ func NewEvmFinalizer( rpcBatchSize: int(rpcBatchSize), txStore: txStore, client: client, + headTracker: headTracker, mb: mailbox.NewSingle[*evmtypes.Head](), } } @@ -115,22 +124,25 @@ func (f *evmFinalizer) runLoop() { } } -func (f *evmFinalizer) DeliverHead(head *evmtypes.Head) bool { +func (f *evmFinalizer) DeliverLatestHead(head *evmtypes.Head) bool { return f.mb.Deliver(head) } func (f *evmFinalizer) ProcessHead(ctx context.Context, head *evmtypes.Head) error { ctx, cancel := context.WithTimeout(ctx, processHeadTimeout) defer cancel() - return f.processHead(ctx, head) + _, latestFinalizedHead, err := f.headTracker.LatestAndFinalizedBlock(ctx) + if err != nil { + return fmt.Errorf("failed to retrieve latest finalized head: %w", err) + } + return f.processFinalizedHead(ctx, latestFinalizedHead) } // Determines if any confirmed transactions can be marked as finalized by comparing their receipts against the latest finalized block -func (f *evmFinalizer) processHead(ctx context.Context, head *evmtypes.Head) error { - latestFinalizedHead := head.LatestFinalizedHead() +func (f *evmFinalizer) processFinalizedHead(ctx context.Context, latestFinalizedHead *evmtypes.Head) error { // Cannot determine finality without a finalized head for comparison if latestFinalizedHead == nil || !latestFinalizedHead.IsValid() { - return fmt.Errorf("failed to find latest finalized head in chain") + return fmt.Errorf("invalid latestFinalizedHead") } earliestBlockNumInChain := latestFinalizedHead.EarliestHeadInChain().BlockNumber() f.lggr.Debugw("processing latest finalized head", "block num", latestFinalizedHead.BlockNumber(), "block hash", latestFinalizedHead.BlockHash(), "earliest block num in chain", earliestBlockNumInChain) diff --git a/core/chains/evm/txmgr/finalizer_test.go b/core/chains/evm/txmgr/finalizer_test.go index 559eaa42485..24dc97b4cbd 100644 --- a/core/chains/evm/txmgr/finalizer_test.go +++ b/core/chains/evm/txmgr/finalizer_test.go @@ -1,6 +1,7 @@ package txmgr_test import ( + "errors" "testing" "time" @@ -12,7 +13,10 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/utils/tests" + txmgrcommon "github.com/smartcontractkit/chainlink/v2/common/txmgr" + + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/headtracker" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/testutils" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" @@ -30,8 +34,9 @@ func TestFinalizer_MarkTxFinalized(t *testing.T) { feeLimit := uint64(10_000) ethClient := testutils.NewEthClientMockWithDefaultChain(t) rpcBatchSize := uint32(1) + ht := headtracker.NewSimulatedHeadTracker(ethClient, true, 0) - finalizer := txmgr.NewEvmFinalizer(logger.Test(t), testutils.FixtureChainID, rpcBatchSize, txStore, ethClient) + finalizer := txmgr.NewEvmFinalizer(logger.Test(t), testutils.FixtureChainID, rpcBatchSize, txStore, ethClient, ht) err := finalizer.Start(ctx) require.NoError(t, err) @@ -63,6 +68,8 @@ func TestFinalizer_MarkTxFinalized(t *testing.T) { attemptHash := insertTxAndAttemptWithIdempotencyKey(t, txStore, tx, idempotencyKey) // Insert receipt for unfinalized block num mustInsertEthReceipt(t, txStore, head.Number, head.Hash, attemptHash) + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(head, nil).Once() + ethClient.On("LatestFinalizedBlock", mock.Anything).Return(head.Parent, nil).Once() err = finalizer.ProcessHead(ctx, head) require.NoError(t, err) tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) @@ -88,6 +95,8 @@ func TestFinalizer_MarkTxFinalized(t *testing.T) { attemptHash := insertTxAndAttemptWithIdempotencyKey(t, txStore, tx, idempotencyKey) // Insert receipt for finalized block num mustInsertEthReceipt(t, txStore, head.Parent.Number, utils.NewHash(), attemptHash) + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(head, nil).Once() + ethClient.On("LatestFinalizedBlock", mock.Anything).Return(head.Parent, nil).Once() err = finalizer.ProcessHead(ctx, head) require.NoError(t, err) tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) @@ -114,6 +123,8 @@ func TestFinalizer_MarkTxFinalized(t *testing.T) { attemptHash := insertTxAndAttemptWithIdempotencyKey(t, txStore, tx, idempotencyKey) // Insert receipt for finalized block num mustInsertEthReceipt(t, txStore, head.Parent.Number, head.Parent.Hash, attemptHash) + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(head, nil).Once() + ethClient.On("LatestFinalizedBlock", mock.Anything).Return(head.Parent, nil).Once() err = finalizer.ProcessHead(ctx, head) require.NoError(t, err) tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) @@ -175,12 +186,27 @@ func TestFinalizer_MarkTxFinalized(t *testing.T) { } rpcElements[0].Result = &headResult }).Return(nil).Twice() + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(head, nil).Once() + ethClient.On("LatestFinalizedBlock", mock.Anything).Return(head.Parent, nil).Once() err = finalizer.ProcessHead(ctx, head) require.NoError(t, err) tx, err = txStore.FindTxWithIdempotencyKey(ctx, idempotencyKey, testutils.FixtureChainID) require.NoError(t, err) require.Equal(t, true, tx.Finalized) }) + + t.Run("returns error if failed to retrieve latest head in headtracker", func(t *testing.T) { + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(nil, errors.New("failed to get latest head")).Once() + err = finalizer.ProcessHead(ctx, head) + require.Error(t, err) + }) + + t.Run("returns error if failed to calculate latest finalized head in headtracker", func(t *testing.T) { + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(head, nil).Once() + ethClient.On("LatestFinalizedBlock", mock.Anything).Return(nil, errors.New("failed to calculate latest finalized head")).Once() + err = finalizer.ProcessHead(ctx, head) + require.Error(t, err) + }) } func insertTxAndAttemptWithIdempotencyKey(t *testing.T, txStore txmgr.TestEvmTxStore, tx *txmgr.Tx, idempotencyKey string) common.Hash { diff --git a/core/chains/evm/txmgr/test_helpers.go b/core/chains/evm/txmgr/test_helpers.go index 3b3584a988b..cbcb8773840 100644 --- a/core/chains/evm/txmgr/test_helpers.go +++ b/core/chains/evm/txmgr/test_helpers.go @@ -53,6 +53,7 @@ type TestEvmConfig struct { Threshold uint32 MinAttempts uint32 DetectionApiUrl *url.URL + RpcDefaultBatchSize uint32 } func (e *TestEvmConfig) Transactions() evmconfig.Transactions { @@ -65,6 +66,8 @@ func (e *TestEvmConfig) FinalityDepth() uint32 { return 42 } func (e *TestEvmConfig) ChainType() chaintype.ChainType { return "" } +func (c *TestEvmConfig) RPCDefaultBatchSize() uint32 { return c.RpcDefaultBatchSize } + type TestGasEstimatorConfig struct { bumpThreshold uint64 } @@ -142,7 +145,6 @@ func (a *autoPurgeConfig) Enabled() bool { return false } type MockConfig struct { EvmConfig *TestEvmConfig - RpcDefaultBatchSize uint32 finalityDepth uint32 finalityTagEnabled bool } @@ -156,11 +158,10 @@ func (c *MockConfig) ChainType() chaintype.ChainType { return "" } func (c *MockConfig) FinalityDepth() uint32 { return c.finalityDepth } func (c *MockConfig) SetFinalityDepth(fd uint32) { c.finalityDepth = fd } func (c *MockConfig) FinalityTagEnabled() bool { return c.finalityTagEnabled } -func (c *MockConfig) RPCDefaultBatchSize() uint32 { return c.RpcDefaultBatchSize } func MakeTestConfigs(t *testing.T) (*MockConfig, *TestDatabaseConfig, *TestEvmConfig) { db := &TestDatabaseConfig{defaultQueryTimeout: utils.DefaultQueryTimeout} - ec := &TestEvmConfig{BumpThreshold: 42, MaxInFlight: uint32(42), MaxQueued: uint64(0), ReaperInterval: time.Duration(0), ReaperThreshold: time.Duration(0)} + ec := &TestEvmConfig{BumpThreshold: 42, MaxInFlight: uint32(42), MaxQueued: uint64(0), ReaperInterval: time.Duration(0), ReaperThreshold: time.Duration(0), RpcDefaultBatchSize: uint32(250)} config := &MockConfig{EvmConfig: ec} return config, db, ec } diff --git a/core/chains/evm/txmgr/txmgr_test.go b/core/chains/evm/txmgr/txmgr_test.go index c4ccc85a6a4..b0e80b396bf 100644 --- a/core/chains/evm/txmgr/txmgr_test.go +++ b/core/chains/evm/txmgr/txmgr_test.go @@ -85,7 +85,8 @@ func makeTestEvmTxm( lggr, lp, keyStore, - estimator) + estimator, + ht) } func TestTxm_SendNativeToken_DoesNotSendToZero(t *testing.T) { @@ -489,14 +490,20 @@ func TestTxm_Lifecycle(t *testing.T) { config, dbConfig, evmConfig := txmgr.MakeTestConfigs(t) config.SetFinalityDepth(uint32(42)) - config.RpcDefaultBatchSize = uint32(4) + evmConfig.RpcDefaultBatchSize = uint32(4) evmConfig.ResendAfterThreshold = 1 * time.Hour evmConfig.ReaperThreshold = 1 * time.Hour evmConfig.ReaperInterval = 1 * time.Hour kst.On("EnabledAddressesForChain", mock.Anything, &cltest.FixtureChainID).Return([]common.Address{}, nil) + head := cltest.Head(42) + finalizedHead := cltest.Head(0) + + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(head, nil).Once() + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(finalizedHead, nil).Once() + keyChangeCh := make(chan struct{}) unsub := cltest.NewAwaiter() kst.On("SubscribeToKeyChanges", mock.Anything).Return(keyChangeCh, unsub.ItHappened) @@ -505,7 +512,6 @@ func TestTxm_Lifecycle(t *testing.T) { txm, err := makeTestEvmTxm(t, db, ethClient, estimator, evmConfig, evmConfig.GasEstimator(), evmConfig.Transactions(), dbConfig, dbConfig.Listener(), kst) require.NoError(t, err) - head := cltest.Head(42) // It should not hang or panic txm.OnNewLongestChain(tests.Context(t), head) @@ -607,8 +613,20 @@ func TestTxm_GetTransactionStatus(t *testing.T) { gcfg := configtest.NewTestGeneralConfig(t) cfg := evmtest.NewChainScopedConfig(t, gcfg) + head := &evmtypes.Head{ + Hash: utils.NewHash(), + Number: 100, + Parent: &evmtypes.Head{ + Hash: utils.NewHash(), + Number: 99, + IsFinalized: true, + }, + } + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) ethClient.On("PendingNonceAt", mock.Anything, mock.Anything).Return(uint64(0), nil).Maybe() + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(head, nil).Once() + ethClient.On("HeadByNumber", mock.Anything, mock.Anything).Return(head.Parent, nil).Once() feeEstimator := gasmocks.NewEvmFeeEstimator(t) feeEstimator.On("Start", mock.Anything).Return(nil).Once() feeEstimator.On("Close", mock.Anything).Return(nil).Once() @@ -617,15 +635,6 @@ func TestTxm_GetTransactionStatus(t *testing.T) { require.NoError(t, err) servicetest.Run(t, txm) - head := &evmtypes.Head{ - Hash: utils.NewHash(), - Number: 100, - Parent: &evmtypes.Head{ - Hash: utils.NewHash(), - Number: 99, - IsFinalized: true, - }, - } txm.OnNewLongestChain(ctx, head) t.Run("returns error if transaction not found", func(t *testing.T) { diff --git a/core/chains/legacyevm/chain.go b/core/chains/legacyevm/chain.go index b38cd2c4508..b0f174dcf42 100644 --- a/core/chains/legacyevm/chain.go +++ b/core/chains/legacyevm/chain.go @@ -250,7 +250,7 @@ func newChain(ctx context.Context, cfg *evmconfig.ChainScoped, nodes []*toml.Nod } // note: gas estimator is started as a part of the txm - txm, gasEstimator, err := newEvmTxm(opts.DS, cfg.EVM(), opts.AppConfig.EVMRPCEnabled(), opts.AppConfig.Database(), opts.AppConfig.Database().Listener(), client, l, logPoller, opts) + txm, gasEstimator, err := newEvmTxm(opts.DS, cfg.EVM(), opts.AppConfig.EVMRPCEnabled(), opts.AppConfig.Database(), opts.AppConfig.Database().Listener(), client, l, logPoller, opts, headTracker) if err != nil { return nil, fmt.Errorf("failed to instantiate EvmTxm for chain with ID %s: %w", chainID.String(), err) } diff --git a/core/chains/legacyevm/evm_txm.go b/core/chains/legacyevm/evm_txm.go index cecfd4ffafe..ab116749665 100644 --- a/core/chains/legacyevm/evm_txm.go +++ b/core/chains/legacyevm/evm_txm.go @@ -7,6 +7,7 @@ import ( evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" evmconfig "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" + httypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/headtracker/types" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" "github.com/smartcontractkit/chainlink/v2/core/logger" @@ -22,6 +23,7 @@ func newEvmTxm( lggr logger.Logger, logPoller logpoller.LogPoller, opts ChainRelayExtenderConfig, + headTracker httypes.HeadTracker, ) (txm txmgr.TxManager, estimator gas.EvmFeeEstimator, err error, @@ -63,7 +65,8 @@ func newEvmTxm( lggr, logPoller, opts.KeyStore, - estimator) + estimator, + headTracker) } else { txm = opts.GenTxManager(chainID) } diff --git a/core/services/promreporter/prom_reporter_test.go b/core/services/promreporter/prom_reporter_test.go index b61fa25bdc4..a0a4a247c21 100644 --- a/core/services/promreporter/prom_reporter_test.go +++ b/core/services/promreporter/prom_reporter_test.go @@ -62,7 +62,8 @@ func newLegacyChainContainer(t *testing.T, db *sqlx.DB) legacyevm.LegacyChainCon lggr, lp, keyStore, - estimator) + estimator, + ht) require.NoError(t, err) cfg := configtest.NewGeneralConfig(t, nil) diff --git a/core/services/vrf/delegate_test.go b/core/services/vrf/delegate_test.go index 889b19d0e04..9718dc376a7 100644 --- a/core/services/vrf/delegate_test.go +++ b/core/services/vrf/delegate_test.go @@ -83,7 +83,7 @@ func buildVrfUni(t *testing.T, db *sqlx.DB, cfg chainlink.GeneralConfig) vrfUniv btORM := bridges.NewORM(db) ks := keystore.NewInMemory(db, utils.FastScryptParams, lggr) _, dbConfig, evmConfig := txmgr.MakeTestConfigs(t) - txm, err := txmgr.NewTxm(db, evmConfig, evmConfig.GasEstimator(), evmConfig.Transactions(), nil, dbConfig, dbConfig.Listener(), ec, logger.TestLogger(t), nil, ks.Eth(), nil) + txm, err := txmgr.NewTxm(db, evmConfig, evmConfig.GasEstimator(), evmConfig.Transactions(), nil, dbConfig, dbConfig.Listener(), ec, logger.TestLogger(t), nil, ks.Eth(), nil, nil) orm := headtracker.NewORM(*testutils.FixtureChainID, db) require.NoError(t, orm.IdempotentInsertHead(testutils.Context(t), cltest.Head(51))) jrm := job.NewORM(db, prm, btORM, ks, lggr) From 9f56b4ee90cfb7384a5b51f66ffa81085526498c Mon Sep 17 00:00:00 2001 From: amit-momin Date: Mon, 1 Jul 2024 14:40:58 -0500 Subject: [PATCH 08/16] Fixed health check tests and fixed linting --- core/chains/evm/txmgr/finalizer.go | 34 +++++++++++++++------------ core/chains/evm/txmgr/test_helpers.go | 8 +++---- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/core/chains/evm/txmgr/finalizer.go b/core/chains/evm/txmgr/finalizer.go index 666a54e7af0..08e3c8d8f42 100644 --- a/core/chains/evm/txmgr/finalizer.go +++ b/core/chains/evm/txmgr/finalizer.go @@ -43,13 +43,13 @@ type evmFinalizer struct { chainId *big.Int rpcBatchSize int - txStore finalizerTxStore - client finalizerChainClient + txStore finalizerTxStore + client finalizerChainClient headTracker finalizerHeadTracker - mb *mailbox.Mailbox[*evmtypes.Head] - stopCh services.StopChan - wg sync.WaitGroup + mb *mailbox.Mailbox[*evmtypes.Head] + stopCh services.StopChan + wg sync.WaitGroup } func NewEvmFinalizer( @@ -67,26 +67,30 @@ func NewEvmFinalizer( rpcBatchSize: int(rpcBatchSize), txStore: txStore, client: client, - headTracker: headTracker, + headTracker: headTracker, mb: mailbox.NewSingle[*evmtypes.Head](), } } // Start the finalizer func (f *evmFinalizer) Start(ctx context.Context) error { - f.lggr.Debugf("started Finalizer with RPC batch size limit: %d", f.rpcBatchSize) - f.stopCh = make(chan struct{}) - f.wg.Add(1) - go f.runLoop() - return nil + return f.StartOnce("Finalizer", func() error { + f.lggr.Debugf("started Finalizer with RPC batch size limit: %d", f.rpcBatchSize) + f.stopCh = make(chan struct{}) + f.wg.Add(1) + go f.runLoop() + return nil + }) } // Close the finalizer func (f *evmFinalizer) Close() error { - f.lggr.Debug("closing Finalizer") - close(f.stopCh) - f.wg.Wait() - return nil + return f.StopOnce("Finalizer", func() error { + f.lggr.Debug("closing Finalizer") + close(f.stopCh) + f.wg.Wait() + return nil + }) } func (f *evmFinalizer) Name() string { diff --git a/core/chains/evm/txmgr/test_helpers.go b/core/chains/evm/txmgr/test_helpers.go index cbcb8773840..eca0640d662 100644 --- a/core/chains/evm/txmgr/test_helpers.go +++ b/core/chains/evm/txmgr/test_helpers.go @@ -66,7 +66,7 @@ func (e *TestEvmConfig) FinalityDepth() uint32 { return 42 } func (e *TestEvmConfig) ChainType() chaintype.ChainType { return "" } -func (c *TestEvmConfig) RPCDefaultBatchSize() uint32 { return c.RpcDefaultBatchSize } +func (c *TestEvmConfig) RPCDefaultBatchSize() uint32 { return c.RpcDefaultBatchSize } type TestGasEstimatorConfig struct { bumpThreshold uint64 @@ -144,9 +144,9 @@ type autoPurgeConfig struct { func (a *autoPurgeConfig) Enabled() bool { return false } type MockConfig struct { - EvmConfig *TestEvmConfig - finalityDepth uint32 - finalityTagEnabled bool + EvmConfig *TestEvmConfig + finalityDepth uint32 + finalityTagEnabled bool } func (c *MockConfig) EVM() evmconfig.EVM { From aa8f0dc29791f8b8b5dcfe157cb0f1231d1aafc2 Mon Sep 17 00:00:00 2001 From: amit-momin Date: Mon, 1 Jul 2024 14:59:14 -0500 Subject: [PATCH 09/16] Fixed lint error --- core/chains/evm/txmgr/test_helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/chains/evm/txmgr/test_helpers.go b/core/chains/evm/txmgr/test_helpers.go index eca0640d662..8d208744329 100644 --- a/core/chains/evm/txmgr/test_helpers.go +++ b/core/chains/evm/txmgr/test_helpers.go @@ -66,7 +66,7 @@ func (e *TestEvmConfig) FinalityDepth() uint32 { return 42 } func (e *TestEvmConfig) ChainType() chaintype.ChainType { return "" } -func (c *TestEvmConfig) RPCDefaultBatchSize() uint32 { return c.RpcDefaultBatchSize } +func (e *TestEvmConfig) RPCDefaultBatchSize() uint32 { return e.RpcDefaultBatchSize } type TestGasEstimatorConfig struct { bumpThreshold uint64 From 745a0e071ece7b3231cfaee35f9e0cd9d46e384a Mon Sep 17 00:00:00 2001 From: amit-momin Date: Mon, 1 Jul 2024 15:08:43 -0500 Subject: [PATCH 10/16] Fixed lint error --- core/chains/evm/headtracker/simulated_head_tracker.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/chains/evm/headtracker/simulated_head_tracker.go b/core/chains/evm/headtracker/simulated_head_tracker.go index d4e09690989..62bb4968c2f 100644 --- a/core/chains/evm/headtracker/simulated_head_tracker.go +++ b/core/chains/evm/headtracker/simulated_head_tracker.go @@ -59,15 +59,15 @@ func (ht *simulatedHeadTracker) LatestChain() *evmtypes.Head { func (ht *simulatedHeadTracker) HealthReport() map[string]error { return nil -} +} func (ht *simulatedHeadTracker) Start(_ context.Context) error { return nil -} +} func (ht *simulatedHeadTracker) Close() error { return nil -} +} func (ht *simulatedHeadTracker) Backfill(_ context.Context, _ *evmtypes.Head) error { return errors.New("unimplemented") @@ -79,4 +79,4 @@ func (ht *simulatedHeadTracker) Name() string { func (ht *simulatedHeadTracker) Ready() error { return nil -} \ No newline at end of file +} From 0ea4733cee9a22827fe9b44a107b6c266a69766c Mon Sep 17 00:00:00 2001 From: Farber98 Date: Wed, 3 Jul 2024 14:10:52 -0300 Subject: [PATCH 11/16] removing finality within common/txmgr --- common/txmgr/confirmer.go | 16 ++++++++-------- common/txmgr/types/tx_store.go | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/common/txmgr/confirmer.go b/common/txmgr/confirmer.go index a5c2af0d4d7..90f53df4e84 100644 --- a/common/txmgr/confirmer.go +++ b/common/txmgr/confirmer.go @@ -297,7 +297,7 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) pro return fmt.Errorf("CheckConfirmedMissingReceipt failed: %w", err) } - if err := ec.CheckForReceipts(ctx, head.BlockNumber()); err != nil { + if err := ec.CheckForReceipts(ctx, head.BlockNumber(), head.LatestFinalizedHead().BlockNumber()); err != nil { return fmt.Errorf("CheckForReceipts failed: %w", err) } @@ -395,8 +395,8 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Che return } -// CheckForReceipts finds attempts that are still pending and checks to see if a receipt is present for the given block number -func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) CheckForReceipts(ctx context.Context, blockNum int64) error { +// CheckForReceipts finds attempts that are still pending and checks to see if a receipt is present for the given block number. +func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) CheckForReceipts(ctx context.Context, blockNum int64, latestFinalizedBlockNum int64) error { attempts, err := ec.txStore.FindTxAttemptsRequiringReceiptFetch(ctx, ec.chainID) if err != nil { return fmt.Errorf("FindTxAttemptsRequiringReceiptFetch failed: %w", err) @@ -443,7 +443,7 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Che return fmt.Errorf("unable to mark txes as 'confirmed_missing_receipt': %w", err) } - if err := ec.txStore.MarkOldTxesMissingReceiptAsErrored(ctx, blockNum, ec.chainConfig.FinalityDepth(), ec.chainID); err != nil { + if err := ec.txStore.MarkOldTxesMissingReceiptAsErrored(ctx, blockNum, latestFinalizedBlockNum, ec.chainID); err != nil { return fmt.Errorf("unable to confirm buried unconfirmed txes': %w", err) } return nil @@ -1007,15 +1007,15 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) han // If any of the confirmed transactions does not have a receipt in the chain, it has been // re-org'd out and will be rebroadcast. func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) EnsureConfirmedTransactionsInLongestChain(ctx context.Context, head types.Head[BLOCK_HASH]) error { - if head.ChainLength() < ec.chainConfig.FinalityDepth() { + if head.ChainLength() < uint32(head.LatestFinalizedHead().BlockNumber()) { logArgs := []interface{}{ - "chainLength", head.ChainLength(), "finalityDepth", ec.chainConfig.FinalityDepth(), + "chainLength", head.ChainLength(), "latestFinalizedHead", head.LatestFinalizedHead().BlockNumber(), } if ec.nConsecutiveBlocksChainTooShort > logAfterNConsecutiveBlocksChainTooShort { - warnMsg := "Chain length supplied for re-org detection was shorter than FinalityDepth. Re-org protection is not working properly. This could indicate a problem with the remote RPC endpoint, a compatibility issue with a particular blockchain, a bug with this particular blockchain, heads table being truncated too early, remote node out of sync, or something else. If this happens a lot please raise a bug with the Chainlink team including a log output sample and details of the chain and RPC endpoint you are using." + warnMsg := "Chain length supplied for re-org detection was shorter than LatestFinalizedHead. Re-org protection is not working properly. This could indicate a problem with the remote RPC endpoint, a compatibility issue with a particular blockchain, a bug with this particular blockchain, heads table being truncated too early, remote node out of sync, or something else. If this happens a lot please raise a bug with the Chainlink team including a log output sample and details of the chain and RPC endpoint you are using." ec.lggr.Warnw(warnMsg, append(logArgs, "nConsecutiveBlocksChainTooShort", ec.nConsecutiveBlocksChainTooShort)...) } else { - logMsg := "Chain length supplied for re-org detection was shorter than FinalityDepth" + logMsg := "Chain length supplied for re-org detection was shorter than LatestFinalizedHead" ec.lggr.Debugw(logMsg, append(logArgs, "nConsecutiveBlocksChainTooShort", ec.nConsecutiveBlocksChainTooShort)...) } ec.nConsecutiveBlocksChainTooShort++ diff --git a/common/txmgr/types/tx_store.go b/common/txmgr/types/tx_store.go index 487b1b0ba82..398fa6219a1 100644 --- a/common/txmgr/types/tx_store.go +++ b/common/txmgr/types/tx_store.go @@ -92,7 +92,7 @@ type TransactionStore[ HasInProgressTransaction(ctx context.Context, account ADDR, chainID CHAIN_ID) (exists bool, err error) LoadTxAttempts(ctx context.Context, etx *Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error MarkAllConfirmedMissingReceipt(ctx context.Context, chainID CHAIN_ID) (err error) - MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, finalityDepth uint32, chainID CHAIN_ID) error + MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, latestFinalizedBlockNum int64, chainID CHAIN_ID) error PreloadTxes(ctx context.Context, attempts []TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error SaveConfirmedMissingReceiptAttempt(ctx context.Context, timeout time.Duration, attempt *TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error SaveInProgressAttempt(ctx context.Context, attempt *TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error From fcac24a042461477479030d3311b5c32346dd138 Mon Sep 17 00:00:00 2001 From: Farber98 Date: Wed, 3 Jul 2024 14:12:24 -0300 Subject: [PATCH 12/16] remove finality from common/txmgr comment --- common/txmgr/confirmer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/txmgr/confirmer.go b/common/txmgr/confirmer.go index 90f53df4e84..40031630f06 100644 --- a/common/txmgr/confirmer.go +++ b/common/txmgr/confirmer.go @@ -1000,7 +1000,7 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) han } } -// EnsureConfirmedTransactionsInLongestChain finds all confirmed txes up to the depth +// EnsureConfirmedTransactionsInLongestChain finds all confirmed txes up to the LatestFinalizedHead // of the given chain and ensures that every one has a receipt with a block hash that is // in the given chain. // From 732bfd7616093c03adfd29db09cecbbc1ed58949 Mon Sep 17 00:00:00 2001 From: Farber98 Date: Wed, 3 Jul 2024 14:27:21 -0300 Subject: [PATCH 13/16] latestFinalizedBlockNum support within evm/txmgr txstore --- core/chains/evm/txmgr/evm_tx_store.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/core/chains/evm/txmgr/evm_tx_store.go b/core/chains/evm/txmgr/evm_tx_store.go index 0b97bf78770..3b3437af88e 100644 --- a/core/chains/evm/txmgr/evm_tx_store.go +++ b/core/chains/evm/txmgr/evm_tx_store.go @@ -1433,23 +1433,18 @@ ORDER BY nonce ASC // markOldTxesMissingReceiptAsErrored // -// Once eth_tx has all of its attempts broadcast before some cutoff threshold +// Once eth_tx has all of its attempts broadcast before latestFinalizedBlockNum // without receiving any receipts, we mark it as fatally errored (never sent). // // The job run will also be marked as errored in this case since we never got a // receipt and thus cannot pass on any transaction hash -func (o *evmTxStore) MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, finalityDepth uint32, chainID *big.Int) error { +func (o *evmTxStore) MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, latestFinalizedBlockNum int64, chainID *big.Int) error { var cancel context.CancelFunc ctx, cancel = o.stopCh.Ctx(ctx) defer cancel() - // cutoffBlockNum is a block height - // Any 'confirmed_missing_receipt' eth_tx with all attempts older than this block height will be marked as errored + // Any 'confirmed_missing_receipt' eth_tx with all attempts older than latestFinalizedBlockNum will be marked as errored // We will not try to query for receipts for this transaction any more - cutoff := blockNum - int64(finalityDepth) - if cutoff <= 0 { - return nil - } - if cutoff <= 0 { + if latestFinalizedBlockNum <= 0 { return nil } // note: if QOpt passes in a sql.Tx this will reuse it @@ -1474,7 +1469,7 @@ FROM ( FOR UPDATE OF e1 ) e0 WHERE e0.id = evm.txes.id -RETURNING e0.id, e0.nonce`, ErrCouldNotGetReceipt, cutoff, chainID.String()) +RETURNING e0.id, e0.nonce`, ErrCouldNotGetReceipt, latestFinalizedBlockNum, chainID.String()) if err != nil { return pkgerrors.Wrap(err, "markOldTxesMissingReceiptAsErrored failed to query") From 3a97140bc645851a4913d81e1b95983560a64244 Mon Sep 17 00:00:00 2001 From: Farber98 Date: Wed, 3 Jul 2024 14:28:51 -0300 Subject: [PATCH 14/16] refactor tests after evm/txmgr txstore changes --- core/chains/evm/txmgr/confirmer_test.go | 95 ++++++++++++---------- core/chains/evm/txmgr/evm_tx_store_test.go | 7 +- 2 files changed, 56 insertions(+), 46 deletions(-) diff --git a/core/chains/evm/txmgr/confirmer_test.go b/core/chains/evm/txmgr/confirmer_test.go index 73b4c946cb3..bb6f2d9f47a 100644 --- a/core/chains/evm/txmgr/confirmer_test.go +++ b/core/chains/evm/txmgr/confirmer_test.go @@ -152,9 +152,10 @@ func TestEthConfirmer_Lifecycle(t *testing.T) { Hash: testutils.NewHash(), Number: 9, Parent: &evmtypes.Head{ - Number: 8, - Hash: testutils.NewHash(), - Parent: nil, + Number: 8, + Hash: testutils.NewHash(), + IsFinalized: true, + Parent: nil, }, }, } @@ -199,6 +200,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { nonce := int64(0) ctx := tests.Context(t) blockNum := int64(0) + latestFinalizedBlockNum := int64(0) t.Run("only finds eth_txes in unconfirmed state with at least one broadcast attempt", func(t *testing.T) { mustInsertFatalErrorEthTx(t, txStore, fromAddress) @@ -211,7 +213,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { mustCreateUnstartedGeneratedTx(t, txStore, fromAddress, config.EVM().ChainID()) // Do the thing - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) }) etx1 := cltest.MustInsertUnconfirmedEthTxWithBroadcastLegacyAttempt(t, txStore, nonce, fromAddress) @@ -232,7 +234,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // Do the thing - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) var err error etx1, err = txStore.FindTxWithAttempts(ctx, etx1.ID) @@ -261,7 +263,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // No error because it is merely logged - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) etx, err := txStore.FindTxWithAttempts(ctx, etx1.ID) require.NoError(t, err) @@ -289,7 +291,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // No error because it is merely logged - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) etx, err := txStore.FindTxWithAttempts(ctx, etx1.ID) require.NoError(t, err) @@ -326,7 +328,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // Do the thing - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) // Check that the receipt was saved etx, err := txStore.FindTxWithAttempts(ctx, etx1.ID) @@ -388,7 +390,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // Do the thing - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) // Check that the state was updated etx, err := txStore.FindTxWithAttempts(ctx, etx2.ID) @@ -416,7 +418,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // Do the thing - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) // No receipt, but no error either etx, err := txStore.FindTxWithAttempts(ctx, etx3.ID) @@ -443,7 +445,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // Do the thing - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) // No receipt, but no error either etx, err := txStore.FindTxWithAttempts(ctx, etx3.ID) @@ -472,7 +474,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // Do the thing - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) // Check that the receipt was unchanged etx, err := txStore.FindTxWithAttempts(ctx, etx3.ID) @@ -523,7 +525,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // Do the thing - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) // Check that the state was updated var err error @@ -576,7 +578,7 @@ func TestEthConfirmer_CheckForReceipts(t *testing.T) { }).Once() // Do the thing - require.NoError(t, ec.CheckForReceipts(ctx, blockNum)) + require.NoError(t, ec.CheckForReceipts(ctx, blockNum, latestFinalizedBlockNum)) // Check that the state was updated etx5, err = txStore.FindTxWithAttempts(ctx, etx5.ID) @@ -614,6 +616,7 @@ func TestEthConfirmer_CheckForReceipts_batching(t *testing.T) { etx := cltest.MustInsertUnconfirmedEthTx(t, txStore, 0, fromAddress) var attempts []txmgr.TxAttempt + latestFinalizedBlockNum := int64(0) // Total of 5 attempts should lead to 3 batched fetches (2, 2, 1) for i := 0; i < 5; i++ { @@ -650,7 +653,7 @@ func TestEthConfirmer_CheckForReceipts_batching(t *testing.T) { elems[0].Result = &evmtypes.Receipt{} }).Once() - require.NoError(t, ec.CheckForReceipts(ctx, 42)) + require.NoError(t, ec.CheckForReceipts(ctx, 42, latestFinalizedBlockNum)) } func TestEthConfirmer_CheckForReceipts_HandlesNonFwdTxsWithForwardingEnabled(t *testing.T) { @@ -671,6 +674,8 @@ func TestEthConfirmer_CheckForReceipts_HandlesNonFwdTxsWithForwardingEnabled(t * _, fromAddress := cltest.MustInsertRandomKeyReturningState(t, ethKeyStore) ec := newEthConfirmer(t, txStore, ethClient, cfg, evmcfg, ethKeyStore, nil) ctx := tests.Context(t) + latestFinalizedBlockNum := int64(0) + // tx is not forwarded and doesn't have meta set. EthConfirmer should handle nil meta values etx := cltest.MustInsertUnconfirmedEthTx(t, txStore, 0, fromAddress) attempt := newBroadcastLegacyEthTxAttempt(t, etx.ID, 2) @@ -697,7 +702,7 @@ func TestEthConfirmer_CheckForReceipts_HandlesNonFwdTxsWithForwardingEnabled(t * *(elems[0].Result.(*evmtypes.Receipt)) = txmReceipt // confirmed }).Once() - require.NoError(t, ec.CheckForReceipts(ctx, 42)) + require.NoError(t, ec.CheckForReceipts(ctx, 42, latestFinalizedBlockNum)) // Check receipt is inserted correctly. dbtx, err = txStore.FindTxWithAttempts(ctx, etx.ID) @@ -724,6 +729,7 @@ func TestEthConfirmer_CheckForReceipts_only_likely_confirmed(t *testing.T) { ec := newEthConfirmer(t, txStore, ethClient, cfg, evmcfg, ethKeyStore, nil) ctx := tests.Context(t) + latestFinalizedBlockNum := int64(0) var attempts []txmgr.TxAttempt // inserting in DESC nonce order to test DB ASC ordering @@ -755,7 +761,7 @@ func TestEthConfirmer_CheckForReceipts_only_likely_confirmed(t *testing.T) { elems[3].Result = &evmtypes.Receipt{} }).Once() - require.NoError(t, ec.CheckForReceipts(ctx, 42)) + require.NoError(t, ec.CheckForReceipts(ctx, 42, latestFinalizedBlockNum)) cltest.BatchElemMustMatchParams(t, captured[0], attempts[0].Hash, "eth_getTransactionReceipt") cltest.BatchElemMustMatchParams(t, captured[1], attempts[1].Hash, "eth_getTransactionReceipt") @@ -778,6 +784,7 @@ func TestEthConfirmer_CheckForReceipts_should_not_check_for_likely_unconfirmed(t ec := newEthConfirmer(t, txStore, ethClient, gconfig, config, ethKeyStore, nil) ctx := tests.Context(t) + latestFinalizedBlockNum := int64(0) etx := cltest.MustInsertUnconfirmedEthTx(t, txStore, 1, fromAddress) for i := 0; i < 4; i++ { @@ -788,7 +795,7 @@ func TestEthConfirmer_CheckForReceipts_should_not_check_for_likely_unconfirmed(t // latest nonce is lower that all attempts' nonces ethClient.On("SequenceAt", mock.Anything, mock.Anything, mock.Anything).Return(evmtypes.Nonce(0), nil) - require.NoError(t, ec.CheckForReceipts(ctx, 42)) + require.NoError(t, ec.CheckForReceipts(ctx, 42, latestFinalizedBlockNum)) } func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt_scoped_to_key(t *testing.T) { @@ -809,6 +816,7 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt_scoped_to_key(t ec := newEthConfirmer(t, txStore, ethClient, cfg, evmcfg, ethKeyStore, nil) ctx := tests.Context(t) + latestFinalizedBlockNum := int64(0) // STATE // key 1, tx with nonce 0 is unconfirmed @@ -832,7 +840,7 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt_scoped_to_key(t *(elems[0].Result.(*evmtypes.Receipt)) = txmReceipt2_9 }).Once() - require.NoError(t, ec.CheckForReceipts(ctx, 10)) + require.NoError(t, ec.CheckForReceipts(ctx, 10, latestFinalizedBlockNum)) mustTxBeInState(t, txStore, etx1_0, txmgrcommon.TxUnconfirmed) mustTxBeInState(t, txStore, etx1_1, txmgrcommon.TxUnconfirmed) @@ -850,7 +858,7 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt_scoped_to_key(t *(elems[0].Result.(*evmtypes.Receipt)) = txmReceipt1_1 }).Once() - require.NoError(t, ec.CheckForReceipts(ctx, 11)) + require.NoError(t, ec.CheckForReceipts(ctx, 11, latestFinalizedBlockNum)) mustTxBeInState(t, txStore, etx1_0, txmgrcommon.TxConfirmedMissingReceipt) mustTxBeInState(t, txStore, etx1_1, txmgrcommon.TxConfirmed) @@ -861,9 +869,7 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt(t *testing.T) { t.Parallel() db := pgtest.NewSqlxDB(t) - cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { - c.EVM[0].FinalityDepth = ptr[uint32](50) - }) + cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) {}) txStore := cltest.NewTestTxStore(t, db) ethKeyStore := cltest.NewKeyStore(t, db).Eth() @@ -876,6 +882,7 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt(t *testing.T) { ec := newEthConfirmer(t, txStore, ethClient, cfg, evmcfg, ethKeyStore, nil) ctx := tests.Context(t) + latestFinalizedBlockNum := int64(0) // STATE // eth_txes with nonce 0 has two attempts (broadcast before block 21 and 41) the first of which will get a receipt @@ -949,7 +956,7 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt(t *testing.T) { // PERFORM // Block num of 43 is one higher than the receipt (as would generally be expected) - require.NoError(t, ec.CheckForReceipts(ctx, 43)) + require.NoError(t, ec.CheckForReceipts(ctx, 43, latestFinalizedBlockNum)) // Expected state is that the "top" eth_tx is now confirmed, with the // two below it "confirmed_missing_receipt" and the "bottom" eth_tx also confirmed @@ -1009,7 +1016,7 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt(t *testing.T) { // PERFORM // Block num of 44 is one higher than the receipt (as would generally be expected) - require.NoError(t, ec.CheckForReceipts(ctx, 44)) + require.NoError(t, ec.CheckForReceipts(ctx, 44, latestFinalizedBlockNum)) // Expected state is that the "top" two eth_txes are now confirmed, with the // one below it still "confirmed_missing_receipt" and the bottom one remains confirmed @@ -1038,7 +1045,7 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt(t *testing.T) { // eth_txes with nonce 2 is confirmed // eth_txes with nonce 3 is confirmed - t.Run("continues to leave eth_txes with state 'confirmed_missing_receipt' unchanged if at least one attempt is above EVM.FinalityDepth", func(t *testing.T) { + t.Run("continues to leave eth_txes with state 'confirmed_missing_receipt' unchanged if at least one attempt is above LatestFinalizedBlockNum", func(t *testing.T) { ethClient.On("SequenceAt", mock.Anything, mock.Anything, mock.Anything).Return(evmtypes.Nonce(10), nil) ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { return len(b) == 2 && @@ -1051,9 +1058,11 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt(t *testing.T) { elems[1].Result = &evmtypes.Receipt{} }).Once() + latestFinalizedBlockNum = 30 + // PERFORM // Block num of 80 puts the first attempt (21) below threshold but second attempt (41) still above - require.NoError(t, ec.CheckForReceipts(ctx, 80)) + require.NoError(t, ec.CheckForReceipts(ctx, 80, latestFinalizedBlockNum)) // Expected state is that the "top" two eth_txes are now confirmed, with the // one below it still "confirmed_missing_receipt" and the bottom one remains confirmed @@ -1078,7 +1087,7 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt(t *testing.T) { // eth_txes with nonce 2 is confirmed // eth_txes with nonce 3 is confirmed - t.Run("marks eth_Txes with state 'confirmed_missing_receipt' as 'errored' if a receipt fails to show up and all attempts are buried deeper than EVM.FinalityDepth", func(t *testing.T) { + t.Run("marks eth_Txes with state 'confirmed_missing_receipt' as 'errored' if a receipt fails to show up and all attempts are buried deeper than LatestFinalizedBlockNum", func(t *testing.T) { ethClient.On("SequenceAt", mock.Anything, mock.Anything, mock.Anything).Return(evmtypes.Nonce(10), nil) ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { return len(b) == 2 && @@ -1091,9 +1100,11 @@ func TestEthConfirmer_CheckForReceipts_confirmed_missing_receipt(t *testing.T) { elems[1].Result = &evmtypes.Receipt{} }).Once() + latestFinalizedBlockNum = 50 + // PERFORM // Block num of 100 puts the first attempt (21) and second attempt (41) below threshold - require.NoError(t, ec.CheckForReceipts(ctx, 100)) + require.NoError(t, ec.CheckForReceipts(ctx, 100, latestFinalizedBlockNum)) // Expected state is that the "top" two eth_txes are now confirmed, with the // one below it marked as "fatal_error" and the bottom one remains confirmed @@ -1117,9 +1128,7 @@ func TestEthConfirmer_CheckConfirmedMissingReceipt(t *testing.T) { t.Parallel() db := pgtest.NewSqlxDB(t) - cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { - c.EVM[0].FinalityDepth = ptr[uint32](50) - }) + cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) {}) txStore := cltest.NewTestTxStore(t, db) ethKeyStore := cltest.NewKeyStore(t, db).Eth() @@ -1197,9 +1206,7 @@ func TestEthConfirmer_CheckConfirmedMissingReceipt_batchSendTransactions_fails(t t.Parallel() db := pgtest.NewSqlxDB(t) - cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { - c.EVM[0].FinalityDepth = ptr[uint32](50) - }) + cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) {}) txStore := cltest.NewTestTxStore(t, db) ethKeyStore := cltest.NewKeyStore(t, db).Eth() @@ -1262,7 +1269,6 @@ func TestEthConfirmer_CheckConfirmedMissingReceipt_smallEvmRPCBatchSize_middleBa db := pgtest.NewSqlxDB(t) cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { - c.EVM[0].FinalityDepth = ptr[uint32](50) c.EVM[0].RPCDefaultBatchSize = ptr[uint32](1) }) txStore := cltest.NewTestTxStore(t, db) @@ -2679,9 +2685,10 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) { Hash: testutils.NewHash(), Number: 9, Parent: &evmtypes.Head{ - Number: 8, - Hash: testutils.NewHash(), - Parent: nil, + Number: 8, + Hash: testutils.NewHash(), + IsFinalized: true, + Parent: nil, }, }, } @@ -3216,8 +3223,9 @@ func TestEthConfirmer_ProcessStuckTransactions(t *testing.T) { tx := mustInsertUnconfirmedTxWithBroadcastAttempts(t, txStore, nonce, fromAddress, autoPurgeMinAttempts, blockNum-int64(autoPurgeThreshold), marketGasPrice.Add(oneGwei)) head := evmtypes.Head{ - Hash: testutils.NewHash(), - Number: blockNum, + Hash: testutils.NewHash(), + Number: blockNum, + IsFinalized: true, } ethClient.On("SequenceAt", mock.Anything, mock.Anything, mock.Anything).Return(evmtypes.Nonce(0), nil).Once() ethClient.On("BatchCallContext", mock.Anything, mock.Anything).Return(nil).Once() @@ -3240,8 +3248,9 @@ func TestEthConfirmer_ProcessStuckTransactions(t *testing.T) { require.Equal(t, bumpedFee.Legacy, latestAttempt.TxFee.Legacy) head = evmtypes.Head{ - Hash: testutils.NewHash(), - Number: blockNum + 1, + Hash: testutils.NewHash(), + Number: blockNum + 1, + IsFinalized: true, } ethClient.On("SequenceAt", mock.Anything, mock.Anything, mock.Anything).Return(evmtypes.Nonce(1), nil) ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { diff --git a/core/chains/evm/txmgr/evm_tx_store_test.go b/core/chains/evm/txmgr/evm_tx_store_test.go index dde8e047bfd..dbc9750b7bc 100644 --- a/core/chains/evm/txmgr/evm_tx_store_test.go +++ b/core/chains/evm/txmgr/evm_tx_store_test.go @@ -1117,13 +1117,14 @@ func TestORM_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { ethKeyStore := cltest.NewKeyStore(t, db).Eth() ethClient := evmtest.NewEthClientMockWithDefaultChain(t) _, fromAddress := cltest.MustInsertRandomKeyReturningState(t, ethKeyStore) + latestFinalizedBlockNum := int64(8) // tx state should be confirmed missing receipt - // attempt should be broadcast before cutoff time + // attempt should be before latestFinalizedBlockNum t.Run("successfully mark errored transactions", func(t *testing.T) { etx := mustInsertConfirmedMissingReceiptEthTxWithLegacyAttempt(t, txStore, 1, 7, time.Now(), fromAddress) - err := txStore.MarkOldTxesMissingReceiptAsErrored(tests.Context(t), 10, 2, ethClient.ConfiguredChainID()) + err := txStore.MarkOldTxesMissingReceiptAsErrored(tests.Context(t), 10, latestFinalizedBlockNum, ethClient.ConfiguredChainID()) require.NoError(t, err) etx, err = txStore.FindTxWithAttempts(ctx, etx.ID) @@ -1133,7 +1134,7 @@ func TestORM_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { t.Run("successfully mark errored transactions w/ qopt passing in sql.Tx", func(t *testing.T) { etx := mustInsertConfirmedMissingReceiptEthTxWithLegacyAttempt(t, txStore, 1, 7, time.Now(), fromAddress) - err := txStore.MarkOldTxesMissingReceiptAsErrored(tests.Context(t), 10, 2, ethClient.ConfiguredChainID()) + err := txStore.MarkOldTxesMissingReceiptAsErrored(tests.Context(t), 10, latestFinalizedBlockNum, ethClient.ConfiguredChainID()) require.NoError(t, err) // must run other query outside of postgres transaction so changes are committed From 0ccedfbaf18202b573cf220ed050575c3487db6c Mon Sep 17 00:00:00 2001 From: Farber98 Date: Wed, 3 Jul 2024 14:31:11 -0300 Subject: [PATCH 15/16] mocks for both common and core/evm --- common/txmgr/mocks/tx_manager.go | 2 +- core/chains/evm/txmgr/mocks/config.go | 2 +- core/chains/evm/txmgr/mocks/evm_tx_store.go | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/common/txmgr/mocks/tx_manager.go b/common/txmgr/mocks/tx_manager.go index c7380b76876..502f6fe8a25 100644 --- a/common/txmgr/mocks/tx_manager.go +++ b/common/txmgr/mocks/tx_manager.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.42.2. DO NOT EDIT. package mocks diff --git a/core/chains/evm/txmgr/mocks/config.go b/core/chains/evm/txmgr/mocks/config.go index efaaab68afa..5ec00e960ab 100644 --- a/core/chains/evm/txmgr/mocks/config.go +++ b/core/chains/evm/txmgr/mocks/config.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.42.2. DO NOT EDIT. package mocks diff --git a/core/chains/evm/txmgr/mocks/evm_tx_store.go b/core/chains/evm/txmgr/mocks/evm_tx_store.go index 454d6ab8f20..f6422546128 100644 --- a/core/chains/evm/txmgr/mocks/evm_tx_store.go +++ b/core/chains/evm/txmgr/mocks/evm_tx_store.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.42.2. DO NOT EDIT. package mocks @@ -1065,17 +1065,17 @@ func (_m *EvmTxStore) MarkAllConfirmedMissingReceipt(ctx context.Context, chainI return r0 } -// MarkOldTxesMissingReceiptAsErrored provides a mock function with given fields: ctx, blockNum, finalityDepth, chainID -func (_m *EvmTxStore) MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, finalityDepth uint32, chainID *big.Int) error { - ret := _m.Called(ctx, blockNum, finalityDepth, chainID) +// MarkOldTxesMissingReceiptAsErrored provides a mock function with given fields: ctx, blockNum, latestFinalizedBlockNum, chainID +func (_m *EvmTxStore) MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, latestFinalizedBlockNum int64, chainID *big.Int) error { + ret := _m.Called(ctx, blockNum, latestFinalizedBlockNum, chainID) if len(ret) == 0 { panic("no return value specified for MarkOldTxesMissingReceiptAsErrored") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, int64, uint32, *big.Int) error); ok { - r0 = rf(ctx, blockNum, finalityDepth, chainID) + if rf, ok := ret.Get(0).(func(context.Context, int64, int64, *big.Int) error); ok { + r0 = rf(ctx, blockNum, latestFinalizedBlockNum, chainID) } else { r0 = ret.Error(0) } From f27399aec5ef854151e75ce8a1962985a347a8e9 Mon Sep 17 00:00:00 2001 From: Farber98 Date: Thu, 4 Jul 2024 10:47:24 -0300 Subject: [PATCH 16/16] remove comments that are still referencing to finalityDepth within evm/txmgr --- core/chains/evm/txmgr/evm_tx_store.go | 8 ++++---- core/chains/evm/txmgr/reaper_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/chains/evm/txmgr/evm_tx_store.go b/core/chains/evm/txmgr/evm_tx_store.go index 3b3437af88e..eae8332eff9 100644 --- a/core/chains/evm/txmgr/evm_tx_store.go +++ b/core/chains/evm/txmgr/evm_tx_store.go @@ -34,8 +34,8 @@ import ( var ( ErrKeyNotUpdated = errors.New("evmTxStore: Key not updated") - // ErrCouldNotGetReceipt is the error string we save if we reach our finality depth for a confirmed transaction without ever getting a receipt - // This most likely happened because an external wallet used the account for this nonce + // ErrCouldNotGetReceipt is the error string we save if we reach our LatestFinalizedBlockNum for a confirmed transaction + // without ever getting a receipt. This most likely happened because an external wallet used the account for this nonce ErrCouldNotGetReceipt = "could not get receipt" ) @@ -960,11 +960,11 @@ func (o *evmTxStore) SaveFetchedReceipts(ctx context.Context, r []*evmtypes.Rece // NOTE: We continue to attempt to resend evm.txes in this state on // every head to guard against the extremely rare scenario of nonce gap due to // reorg that excludes the transaction (from another wallet) that had this -// nonce (until finality depth is reached, after which we make the explicit +// nonce (until LatestFinalizedBlockNum is reached, after which we make the explicit // decision to give up). This is done in the EthResender. // // We will continue to try to fetch a receipt for these attempts until all -// attempts are below the finality depth from current head. +// attempts are below the LatestFinalizedBlockNum from current head. func (o *evmTxStore) MarkAllConfirmedMissingReceipt(ctx context.Context, chainID *big.Int) (err error) { var cancel context.CancelFunc ctx, cancel = o.stopCh.Ctx(ctx) diff --git a/core/chains/evm/txmgr/reaper_test.go b/core/chains/evm/txmgr/reaper_test.go index eaa7eecb252..e853fba17ca 100644 --- a/core/chains/evm/txmgr/reaper_test.go +++ b/core/chains/evm/txmgr/reaper_test.go @@ -104,7 +104,7 @@ func TestReaper_ReapTxes(t *testing.T) { err = r.ReapTxes(42) assert.NoError(t, err) - // Now it deleted because the eth_tx was past EVM.FinalityDepth + // Now it deleted because the eth_tx was marked as finalized cltest.AssertCount(t, db, "evm.txes", 0) })