From 2af9ffaa79386e2177440ebf9d75918f864f29db Mon Sep 17 00:00:00 2001 From: James Walker Date: Fri, 23 Feb 2024 16:37:45 -0500 Subject: [PATCH 01/41] implement MoveInProgressToFatalError --- common/txmgr/address_state.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 2bbb777c634..1c0a4d2d747 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -1,6 +1,7 @@ package txmgr import ( + "fmt" "sync" "time" @@ -226,7 +227,24 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveUn } // MoveInProgressToFatalError moves the in-progress transaction to the fatal error state. +// If there is no in-progress transaction, an error is returned. func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveInProgressToFatalError(txError null.String) error { + as.Lock() + defer as.Unlock() + + tx := as.inprogressTx + if tx == nil { + return fmt.Errorf("move_in_progress_to_fatal_error: no transaction in progress") + } + + tx.State = TxFatalError + tx.Sequence = nil + tx.TxAttempts = nil + tx.InitialBroadcastAt = nil + tx.Error = txError + as.fatalErroredTxs[tx.ID] = tx + as.inprogressTx = nil + return nil } From 1aa98d389f15be93da78c45481e956b025eeea1b Mon Sep 17 00:00:00 2001 From: James Walker Date: Fri, 23 Feb 2024 16:39:33 -0500 Subject: [PATCH 02/41] implement MoveConfirmedMissingReceiptToFatalError --- common/txmgr/address_state.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 1c0a4d2d747..99b6b9d9f79 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -249,10 +249,26 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveIn } // MoveConfirmedMissingReceiptToFatalError moves the confirmed missing receipt transaction to the fatal error state. +// If there is no confirmed missing receipt transaction with the given ID, an error is returned. func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveConfirmedMissingReceiptToFatalError( - etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], - txError null.String, + txID int64, txError null.String, ) error { + as.Lock() + defer as.Unlock() + + tx, ok := as.confirmedMissingReceiptTxs[txID] + if !ok || tx == nil { + return fmt.Errorf("move_confirmed_missing_receipt_to_fatal_error: no confirmed_missing_receipt transaction with ID %d", txID) + } + + tx.State = TxFatalError + tx.Sequence = nil + tx.TxAttempts = nil + tx.InitialBroadcastAt = nil + tx.Error = txError + as.fatalErroredTxs[tx.ID] = tx + delete(as.confirmedMissingReceiptTxs, tx.ID) + return nil } From 3c301dd3656d8e862061969b493b8824ed343e54 Mon Sep 17 00:00:00 2001 From: James Walker Date: Fri, 23 Feb 2024 16:44:01 -0500 Subject: [PATCH 03/41] implement MoveUnconfirmedToConfirmedMissingReceipt --- common/txmgr/address_state.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 99b6b9d9f79..74c77ad86fe 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -273,7 +273,28 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveCo } // MoveUnconfirmedToConfirmedMissingReceipt moves the unconfirmed transaction to the confirmed missing receipt state. -func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveUnconfirmedToConfirmedMissingReceipt(attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { +// If there is no unconfirmed transaction with the given ID, an error is returned. +func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveUnconfirmedToConfirmedMissingReceipt(txAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { + as.Lock() + defer as.Unlock() + + tx, ok := as.unconfirmedTxs[txAttempt.TxID] + if !ok || tx == nil { + return fmt.Errorf("move_unconfirmed_to_confirmed_missing_receipt: no unconfirmed transaction with ID %d", txAttempt.TxID) + } + if tx.BroadcastAt.Before(broadcastAt) { + tx.BroadcastAt = &broadcastAt + } + tx.State = TxConfirmedMissingReceipt + if len(tx.TxAttempts) == 0 { + tx.TxAttempts = []txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + } + txAttempt.State = txmgrtypes.TxAttemptBroadcast + tx.TxAttempts = append(tx.TxAttempts, txAttempt) + + as.confirmedMissingReceiptTxs[tx.ID] = tx + delete(as.unconfirmedTxs, tx.ID) + return nil } From 659b2fb51844b92f878c22fd55c09a9a86df6963 Mon Sep 17 00:00:00 2001 From: James Walker Date: Fri, 23 Feb 2024 16:46:08 -0500 Subject: [PATCH 04/41] implement MoveInProgressToConfirmedMissingReceipt --- common/txmgr/address_state.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 74c77ad86fe..b4aba85a34a 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -299,7 +299,28 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveUn } // MoveInProgressToConfirmedMissingReceipt moves the in-progress transaction to the confirmed missing receipt state. -func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveInProgressToConfirmedMissingReceipt(attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { +// If there is no in-progress transaction, an error is returned. +func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveInProgressToConfirmedMissingReceipt(txAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { + as.Lock() + defer as.Unlock() + + tx := as.inprogressTx + if tx == nil { + return fmt.Errorf("move_in_progress_to_confirmed_missing_receipt: no transaction in progress") + } + if tx.BroadcastAt.Before(broadcastAt) { + tx.BroadcastAt = &broadcastAt + } + tx.State = TxConfirmedMissingReceipt + if len(tx.TxAttempts) == 0 { + tx.TxAttempts = []txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + } + txAttempt.State = txmgrtypes.TxAttemptBroadcast + tx.TxAttempts = append(tx.TxAttempts, txAttempt) + + as.confirmedMissingReceiptTxs[tx.ID] = tx + as.inprogressTx = nil + return nil } From debfdc989e844c6afc0ebda738da5a3ffc2c3e5c Mon Sep 17 00:00:00 2001 From: James Walker Date: Fri, 23 Feb 2024 16:48:53 -0500 Subject: [PATCH 05/41] implement MoveConfirmedToUnconfirmed --- common/txmgr/address_state.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index b4aba85a34a..394b1afd039 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -325,6 +325,32 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveIn } // MoveConfirmedToUnconfirmed moves the confirmed transaction to the unconfirmed state. -func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveConfirmedToUnconfirmed(attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { +func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveConfirmedToUnconfirmed(txAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { + as.Lock() + defer as.Unlock() + + if txAttempt.State != txmgrtypes.TxAttemptBroadcast { + return fmt.Errorf("move_confirmed_to_unconfirmed: attempt must be in broadcast state") + } + + tx, ok := as.confirmedTxs[txAttempt.TxID] + if !ok || tx == nil { + return fmt.Errorf("move_confirmed_to_unconfirmed: no confirmed transaction with ID %d", txAttempt.TxID) + } + tx.State = TxUnconfirmed + + // Delete the receipt from the attempt + txAttempt.Receipts = nil + // Reset the broadcast information for the attempt + txAttempt.State = txmgrtypes.TxAttemptInProgress + txAttempt.BroadcastBeforeBlockNum = nil + if len(tx.TxAttempts) == 0 { + tx.TxAttempts = []txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + } + tx.TxAttempts = append(tx.TxAttempts, txAttempt) + + as.unconfirmedTxs[tx.ID] = tx + delete(as.confirmedTxs, tx.ID) + return nil } From e8ae7f4e20ee096fc4275146f59a34a60ad53483 Mon Sep 17 00:00:00 2001 From: James Walker Date: Mon, 26 Feb 2024 21:23:20 -0500 Subject: [PATCH 06/41] implement MoveUnstartedToFatalError --- common/txmgr/address_state.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 394b1afd039..bb8b56b564a 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -219,10 +219,25 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveUn } // MoveUnstartedToFatalError moves the unstarted transaction to the fatal error state. +// It returns an error if there is no unstarted transaction with the given ID. func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveUnstartedToFatalError( - etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], - txError null.String, + txID int64, txError null.String, ) error { + as.Lock() + defer as.Unlock() + + tx := as.unstartedTxs.RemoveTxByID(txID) + if tx == nil { + return fmt.Errorf("move_unstarted_to_fatal_error: no unstarted transaction with ID %d", txID) + } + + tx.State = TxFatalError + tx.Sequence = nil + tx.TxAttempts = nil + tx.InitialBroadcastAt = nil + tx.Error = txError + as.fatalErroredTxs[tx.ID] = tx + return nil } From 66be6d3cb610afdf9d143cfde3161ec37a639582 Mon Sep 17 00:00:00 2001 From: James Walker Date: Mon, 26 Feb 2024 21:25:00 -0500 Subject: [PATCH 07/41] implement UpdateTxFatalError --- common/txmgr/inmemory_store.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index cd783a25210..b82d6ff0be0 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -178,6 +178,37 @@ func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveR // UpdateTxFatalError updates a transaction to fatal_error. func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) UpdateTxFatalError(ctx context.Context, tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { + if tx.State != TxInProgress && tx.State != TxUnstarted { + return fmt.Errorf("update_tx_fatal_error: can only transition to fatal_error from in_progress, transaction is currently %s", tx.State) + } + if !tx.Error.Valid { + return fmt.Errorf("update_tx_fatal_error: expected error field to be set") + } + + ms.addressStatesLock.RLock() + defer ms.addressStatesLock.RUnlock() + as, ok := ms.addressStates[tx.FromAddress] + if !ok { + return fmt.Errorf("update_tx_fatal_error: %w", ErrAddressNotFound) + } + + // Persist to persistent storage + if err := ms.txStore.UpdateTxFatalError(ctx, tx); err != nil { + return fmt.Errorf("update_tx_fatal_error: %w", err) + } + + // Update in memory store + switch tx.State { + case TxInProgress: + if err := as.MoveInProgressToFatalError(tx.Error); err != nil { + return fmt.Errorf("update_tx_fatal_error: %w", err) + } + case TxUnstarted: + if err := as.MoveUnstartedToFatalError(tx.ID, tx.Error); err != nil { + return fmt.Errorf("update_tx_fatal_error: %w", err) + } + } + return nil } From 5153d24ab572d357831674d23e142c38397ead4c Mon Sep 17 00:00:00 2001 From: James Walker Date: Tue, 27 Feb 2024 14:47:25 -0500 Subject: [PATCH 08/41] implement SaveConfirmedMissingReceiptAttempts --- common/txmgr/inmemory_store.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index cd783a25210..481e41dee4e 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -314,7 +314,23 @@ func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Prelo return nil } func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveConfirmedMissingReceiptAttempt(ctx context.Context, timeout time.Duration, attempt *txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { - return nil + ms.addressStatesLock.RLock() + defer ms.addressStatesLock.RUnlock() + as, ok := ms.addressStates[attempt.Tx.FromAddress] + if !ok { + return fmt.Errorf("save_confirmed_missing_receipt_attempt: %w", ErrAddressNotFound) + } + if attempt.State != txmgrtypes.TxAttemptInProgress { + return fmt.Errorf("expected state to be in_progress") + } + + // Persist to persistent storage + if err := ms.txStore.SaveConfirmedMissingReceiptAttempt(ctx, timeout, attempt, broadcastAt); err != nil { + return err + } + + // Update in memory store + return as.MoveInProgressToConfirmedMissingReceipt(*attempt, broadcastAt) } func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveInProgressAttempt(ctx context.Context, attempt *txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { return nil From f91d5abd090bb611e1cb218d8b3bd240cb58edc7 Mon Sep 17 00:00:00 2001 From: James Walker Date: Tue, 27 Feb 2024 14:57:48 -0500 Subject: [PATCH 09/41] implement apply find and delete for address state --- common/txmgr/address_state.go | 131 +++++++++++++++++++++++++++++++++- 1 file changed, 128 insertions(+), 3 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index bb8b56b564a..27e4795b2ab 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -137,6 +137,31 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) ApplyT fn func(*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]), txIDs ...int64, ) { + as.Lock() + defer as.Unlock() + + // if txStates is empty then apply the filter to only the as.allTransactions map + if len(txStates) == 0 { + as.applyToTxs(as.allTxs, fn, txIDs...) + return + } + + for _, txState := range txStates { + switch txState { + case TxInProgress: + if as.inprogressTx != nil { + fn(as.inprogressTx) + } + case TxUnconfirmed: + as.applyToTxs(as.unconfirmedTxs, fn, txIDs...) + case TxConfirmedMissingReceipt: + as.applyToTxs(as.confirmedMissingReceiptTxs, fn, txIDs...) + case TxConfirmed: + as.applyToTxs(as.confirmedTxs, fn, txIDs...) + case TxFatalError: + as.applyToTxs(as.fatalErroredTxs, fn, txIDs...) + } + } } // FetchTxAttempts returns all attempts for the given transactions that match the given filters. @@ -153,16 +178,42 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FetchT return nil } -// FetchTxs returns all transactions that match the given filters. +// FindTxs returns all transactions that match the given filters. // If txIDs are provided, only the transactions with those IDs are considered. // If no txIDs are provided, all transactions are considered. // If no txStates are provided, all transactions are considered. -func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FetchTxs( +func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindTxs( txStates []txmgrtypes.TxState, filter func(*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool, txIDs ...int64, ) []txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] { - return nil + as.RLock() + defer as.RUnlock() + + // if txStates is empty then apply the filter to only the as.allTransactions map + if len(txStates) == 0 { + return as.findTxs(as.allTxs, filter, txIDs...) + } + + var txs []txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + for _, txState := range txStates { + switch txState { + case TxInProgress: + if as.inprogressTx != nil && filter(as.inprogressTx) { + txs = append(txs, *as.inprogressTx) + } + case TxUnconfirmed: + txs = append(txs, as.findTxs(as.unconfirmedTxs, filter, txIDs...)...) + case TxConfirmedMissingReceipt: + txs = append(txs, as.findTxs(as.confirmedMissingReceiptTxs, filter, txIDs...)...) + case TxConfirmed: + txs = append(txs, as.findTxs(as.confirmedTxs, filter, txIDs...)...) + case TxFatalError: + txs = append(txs, as.findTxs(as.fatalErroredTxs, filter, txIDs...)...) + } + } + + return txs } // PruneUnstartedTxQueue removes the transactions with the given IDs from the unstarted transaction queue. @@ -171,6 +222,10 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) PruneU // DeleteTxs removes the transactions with the given IDs from the address state. func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) DeleteTxs(txs ...txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { + as.Lock() + defer as.Unlock() + + as.deleteTxs(txs...) } // PeekNextUnstartedTx returns the next unstarted transaction in the queue without removing it from the unstarted queue. @@ -369,3 +424,73 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveCo return nil } + +func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) applyToTxs( + txIDsToTx map[int64]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], + fn func(*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]), + txIDs ...int64, +) { + // if txIDs is not empty then only apply the filter to those transactions + if len(txIDs) > 0 { + for _, txID := range txIDs { + tx := txIDsToTx[txID] + if tx != nil { + fn(tx) + } + } + return + } + + // if txIDs is empty then apply the filter to all transactions + for _, tx := range txIDsToTx { + fn(tx) + } +} + +func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) findTxs( + txIDsToTx map[int64]*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], + filter func(*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool, + txIDs ...int64, +) []txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] { + as.RLock() + defer as.RUnlock() + + var txs []txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + // if txIDs is not empty then only apply the filter to those transactions + if len(txIDs) > 0 { + for _, txID := range txIDs { + tx := txIDsToTx[txID] + if tx != nil && filter(tx) { + txs = append(txs, *tx) + } + } + return txs + } + + // if txIDs is empty then apply the filter to all transactions + for _, tx := range txIDsToTx { + if filter(tx) { + txs = append(txs, *tx) + } + } + + return txs +} + +func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) deleteTxs(txs ...txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { + for _, tx := range txs { + if tx.IdempotencyKey != nil { + delete(as.idempotencyKeyToTx, *tx.IdempotencyKey) + } + txID := tx.ID + if as.inprogressTx != nil && as.inprogressTx.ID == txID { + as.inprogressTx = nil + } + delete(as.allTxs, txID) + delete(as.unconfirmedTxs, txID) + delete(as.confirmedMissingReceiptTxs, txID) + delete(as.confirmedTxs, txID) + delete(as.fatalErroredTxs, txID) + as.unstartedTxs.RemoveTxByID(txID) + } +} From 42c82ac7bf91b7e12329329dfa50d54fb165da50 Mon Sep 17 00:00:00 2001 From: James Walker Date: Tue, 27 Feb 2024 15:03:12 -0500 Subject: [PATCH 10/41] implement MarkOldTxesMissingReceiptAsErrored --- common/txmgr/inmemory_store.go | 89 +++++++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index cd783a25210..36db017fe8a 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -339,7 +339,94 @@ func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MarkA return nil } func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, finalityDepth uint32, chainID CHAIN_ID) error { - return nil + if ms.chainID.String() != chainID.String() { + return fmt.Errorf("mark_old_txes_missing_receipt_as_errored: %w", ErrInvalidChainID) + } + + // Persist to persistent storage + if err := ms.txStore.MarkOldTxesMissingReceiptAsErrored(ctx, blockNum, finalityDepth, chainID); err != nil { + return err + } + + // Update in memory store + type result struct { + ID int64 + Sequence SEQ + FromAddress ADDR + MaxBroadcastBeforeBlockNum int64 + TxHashes []TX_HASH + } + var resultsLock sync.Mutex + var results []result + wg := sync.WaitGroup{} + errsLock := sync.Mutex{} + var errs error + ms.addressStatesLock.RLock() + defer ms.addressStatesLock.RUnlock() + for _, as := range ms.addressStates { + wg.Add(1) + go func(as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) { + filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { + if tx.TxAttempts == nil || len(tx.TxAttempts) == 0 { + return false + } + if tx.State != TxConfirmedMissingReceipt { + return false + } + attempt := tx.TxAttempts[0] + if attempt.BroadcastBeforeBlockNum == nil { + return false + } + + return *attempt.BroadcastBeforeBlockNum < blockNum-int64(finalityDepth) + } + states := []txmgrtypes.TxState{TxConfirmedMissingReceipt} + txs := as.FindTxs(states, filter) + for _, tx := range txs { + if err := as.MoveConfirmedMissingReceiptToFatalError(tx.ID, null.StringFrom(ErrCouldNotGetReceipt.Error())); err != nil { + err = fmt.Errorf("mark_old_txes_missing_receipt_as_errored: address: %s: %w", as.fromAddress, err) + errsLock.Lock() + errs = errors.Join(errs, err) + errsLock.Unlock() + continue + } + hashes := make([]TX_HASH, len(tx.TxAttempts)) + maxBroadcastBeforeBlockNum := int64(0) + for i, attempt := range tx.TxAttempts { + hashes[i] = attempt.Hash + if attempt.BroadcastBeforeBlockNum != nil { + if *attempt.BroadcastBeforeBlockNum > maxBroadcastBeforeBlockNum { + maxBroadcastBeforeBlockNum = *attempt.BroadcastBeforeBlockNum + } + } + } + rr := result{ + ID: tx.ID, + Sequence: *tx.Sequence, + FromAddress: tx.FromAddress, + MaxBroadcastBeforeBlockNum: maxBroadcastBeforeBlockNum, + TxHashes: hashes, + } + resultsLock.Lock() + results = append(results, rr) + resultsLock.Unlock() + } + wg.Done() + }(as) + } + wg.Wait() + + for _, r := range results { + ms.lggr.Criticalw(fmt.Sprintf("eth_tx with ID %v expired without ever getting a receipt for any of our attempts. "+ + "Current block height is %v, transaction was broadcast before block height %v. This transaction may not have not been sent and will be marked as fatally errored. "+ + "This can happen if there is another instance of chainlink running that is using the same private key, or if "+ + "an external wallet has been used to send a transaction from account %s with nonce %v."+ + " Please note that Chainlink requires exclusive ownership of it's private keys and sharing keys across multiple"+ + " chainlink instances, or using the chainlink keys with an external wallet is NOT SUPPORTED and WILL lead to missed transactions", + r.ID, blockNum, r.MaxBroadcastBeforeBlockNum, r.FromAddress, r.Sequence), "ethTxID", r.ID, "sequence", r.Sequence, "fromAddress", r.FromAddress, "txHashes", r.TxHashes) + } + + return errs } func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) deepCopyTx( From ef987f42fb73fd44dddb057d02720d9c30b84ec0 Mon Sep 17 00:00:00 2001 From: James Walker Date: Tue, 27 Feb 2024 15:05:29 -0500 Subject: [PATCH 11/41] implement ReapTxHistory --- common/txmgr/inmemory_store.go | 60 ++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index cd783a25210..901b6eb09ce 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -259,6 +259,66 @@ func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Prune } func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) ReapTxHistory(ctx context.Context, minBlockNumberToKeep int64, timeThreshold time.Time, chainID CHAIN_ID) error { + if ms.chainID.String() != chainID.String() { + return fmt.Errorf("reap_tx_history: %w", ErrInvalidChainID) + } + + // Persist to persistent storage + if err := ms.txStore.ReapTxHistory(ctx, minBlockNumberToKeep, timeThreshold, chainID); err != nil { + return err + } + + // Update in memory store + states := []txmgrtypes.TxState{TxConfirmed} + filterFn := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { + if tx.TxAttempts == nil || len(tx.TxAttempts) == 0 { + return false + } + for _, attempt := range tx.TxAttempts { + if attempt.Receipts == nil || len(attempt.Receipts) == 0 { + continue + } + if attempt.Receipts[0].GetBlockNumber() == nil { + continue + } + if attempt.Receipts[0].GetBlockNumber().Int64() >= minBlockNumberToKeep { + continue + } + if tx.CreatedAt.After(timeThreshold) { + continue + } + return tx.State == TxConfirmed + } + return false + } + + wg := sync.WaitGroup{} + ms.addressStatesLock.RLock() + defer ms.addressStatesLock.RUnlock() + for _, as := range ms.addressStates { + wg.Add(1) + go func(as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) { + as.DeleteTxs(as.FindTxs(states, filterFn)...) + wg.Done() + }(as) + } + wg.Wait() + + filterFn = func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { + return tx.State == TxFatalError && tx.CreatedAt.Before(timeThreshold) + } + states = []txmgrtypes.TxState{TxFatalError} + ms.addressStatesLock.RLock() + defer ms.addressStatesLock.RUnlock() + for _, as := range ms.addressStates { + wg.Add(1) + go func(as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) { + as.DeleteTxs(as.FindTxs(states, filterFn)...) + wg.Done() + }(as) + } + wg.Wait() + return nil } func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) CountTransactionsByState(_ context.Context, state txmgrtypes.TxState, chainID CHAIN_ID) (uint32, error) { From a65204ac8dfaf6deff1581e4a1b508d967562881 Mon Sep 17 00:00:00 2001 From: James Walker Date: Tue, 27 Feb 2024 15:07:36 -0500 Subject: [PATCH 12/41] implement DeleteInProgressAttempt --- common/txmgr/inmemory_store.go | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index cd783a25210..7360e153fd4 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -266,6 +266,41 @@ func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Count } func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) DeleteInProgressAttempt(ctx context.Context, attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { + if attempt.State != txmgrtypes.TxAttemptInProgress { + return fmt.Errorf("delete_in_progress_attempt: expected attempt to be in_progress") + } + if attempt.ID == 0 { + return fmt.Errorf("delete_in_progress_attempt: expected attempt to have an ID") + } + + // Check if fromaddress enabled + ms.addressStatesLock.RLock() + defer ms.addressStatesLock.RUnlock() + as, ok := ms.addressStates[attempt.Tx.FromAddress] + if !ok { + return fmt.Errorf("delete_in_progress_attempt: %w", ErrAddressNotFound) + } + + // Persist to persistent storage + if err := ms.txStore.DeleteInProgressAttempt(ctx, attempt); err != nil { + return fmt.Errorf("delete_in_progress_attempt: %w", err) + } + + // Update in memory store + filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { + if tx.TxAttempts == nil || len(tx.TxAttempts) == 0 { + return false + } + + for _, a := range tx.TxAttempts { + if a.ID == attempt.ID { + return true + } + } + return false + } + as.DeleteTxs(as.FindTxs(nil, filter)...) + return nil } From b971ee651c37b7f6c94547f759812e1c4a9c4d71 Mon Sep 17 00:00:00 2001 From: James Walker Date: Tue, 27 Feb 2024 15:33:30 -0500 Subject: [PATCH 13/41] implement SaveReplacementInProgressAttempt --- common/txmgr/inmemory_store.go | 36 ++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index cd783a25210..dea20161dad 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -173,6 +173,42 @@ func (ms *InMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveR oldAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], replacementAttempt *txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], ) error { + if oldAttempt.State != txmgrtypes.TxAttemptInProgress || replacementAttempt.State != txmgrtypes.TxAttemptInProgress { + return fmt.Errorf("save_replacement_in_progress_attempt: expected attempts to be in_progress") + } + if oldAttempt.ID == 0 { + return fmt.Errorf("save_replacement_in_progress_attempt: expected oldattempt to have an ID") + } + + ms.addressStatesLock.RLock() + defer ms.addressStatesLock.RUnlock() + as, ok := ms.addressStates[oldAttempt.Tx.FromAddress] + if !ok { + return fmt.Errorf("save_replacement_in_progress_attempt: %w", ErrAddressNotFound) + } + + // Persist to persistent storage + if err := ms.txStore.SaveReplacementInProgressAttempt(ctx, oldAttempt, replacementAttempt); err != nil { + return fmt.Errorf("save_replacement_in_progress_attempt: %w", err) + } + + // Update in memory store + tx, err := as.PeekInProgressTx() + if tx == nil { + return fmt.Errorf("save_replacement_in_progress_attempt: %w", err) + } + + var found bool + for i := 0; i < len(tx.TxAttempts); i++ { + if tx.TxAttempts[i].ID == oldAttempt.ID { + tx.TxAttempts[i] = *replacementAttempt + found = true + } + } + if !found { + tx.TxAttempts = append(tx.TxAttempts, *replacementAttempt) + } + return nil } From b43bdd3657864b805a6bb977a7757b2ef066e04f Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 29 Feb 2024 15:49:21 -0500 Subject: [PATCH 14/41] refactor MoveXXXToFatal to MoveTxToFatal --- common/txmgr/address_state.go | 85 ++++++++++++----------------------- 1 file changed, 29 insertions(+), 56 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 27e4795b2ab..0280b26f942 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -273,72 +273,33 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveUn return nil } -// MoveUnstartedToFatalError moves the unstarted transaction to the fatal error state. -// It returns an error if there is no unstarted transaction with the given ID. -func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveUnstartedToFatalError( +// MoveTxToFatalError moves a transaction to the fatal error state. +// It returns an error if there is no transaction with the given ID. +// It returns an error if the transaction is not in an expected state. +func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveTxToFatalError( txID int64, txError null.String, ) error { as.Lock() defer as.Unlock() - tx := as.unstartedTxs.RemoveTxByID(txID) + tx := as.allTxs[txID] if tx == nil { - return fmt.Errorf("move_unstarted_to_fatal_error: no unstarted transaction with ID %d", txID) + return fmt.Errorf("move_tx_to_fatal_error: no transaction with ID %d", txID) } - tx.State = TxFatalError - tx.Sequence = nil - tx.TxAttempts = nil - tx.InitialBroadcastAt = nil - tx.Error = txError - as.fatalErroredTxs[tx.ID] = tx - - return nil -} - -// MoveInProgressToFatalError moves the in-progress transaction to the fatal error state. -// If there is no in-progress transaction, an error is returned. -func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveInProgressToFatalError(txError null.String) error { - as.Lock() - defer as.Unlock() - - tx := as.inprogressTx - if tx == nil { - return fmt.Errorf("move_in_progress_to_fatal_error: no transaction in progress") + as.moveTxToFatalError(tx, txError) + + switch tx.State { + case TxUnstarted: + _ = as.unstartedTxs.RemoveTxByID(txID) + case TxInProgress: + as.inprogressTx = nil + case TxConfirmedMissingReceipt: + delete(as.confirmedMissingReceiptTxs, tx.ID) + default: + return fmt.Errorf("move_tx_to_fatal_error: transaction with ID %d is in an unexpected state: %s", txID, tx.State) } - tx.State = TxFatalError - tx.Sequence = nil - tx.TxAttempts = nil - tx.InitialBroadcastAt = nil - tx.Error = txError - as.fatalErroredTxs[tx.ID] = tx - as.inprogressTx = nil - - return nil -} - -// MoveConfirmedMissingReceiptToFatalError moves the confirmed missing receipt transaction to the fatal error state. -// If there is no confirmed missing receipt transaction with the given ID, an error is returned. -func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MoveConfirmedMissingReceiptToFatalError( - txID int64, txError null.String, -) error { - as.Lock() - defer as.Unlock() - - tx, ok := as.confirmedMissingReceiptTxs[txID] - if !ok || tx == nil { - return fmt.Errorf("move_confirmed_missing_receipt_to_fatal_error: no confirmed_missing_receipt transaction with ID %d", txID) - } - - tx.State = TxFatalError - tx.Sequence = nil - tx.TxAttempts = nil - tx.InitialBroadcastAt = nil - tx.Error = txError - as.fatalErroredTxs[tx.ID] = tx - delete(as.confirmedMissingReceiptTxs, tx.ID) - return nil } @@ -494,3 +455,15 @@ func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) delete as.unstartedTxs.RemoveTxByID(txID) } } + +func (as *AddressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTxToFatalError( + tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], + txError null.String, +) { + tx.State = TxFatalError + tx.Sequence = nil + tx.TxAttempts = nil + tx.InitialBroadcastAt = nil + tx.Error = txError + as.fatalErroredTxs[tx.ID] = tx +} From fbd76d62fbad7501b5ff80043841799414372859 Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 7 Mar 2024 17:00:55 -0500 Subject: [PATCH 15/41] address comments --- common/txmgr/address_state.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 57aee39d421..791dafdd7b3 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -166,6 +166,8 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) applyT as._applyToTxs(as.confirmedTxs, fn, txIDs...) case TxFatalError: as._applyToTxs(as.fatalErroredTxs, fn, txIDs...) + default: + panic("apply_to_txs_by_state: unknown transaction state") } } } @@ -216,6 +218,8 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) findTx txs = append(txs, as._findTxs(as.confirmedTxs, filter, txIDs...)...) case TxFatalError: txs = append(txs, as._findTxs(as.fatalErroredTxs, filter, txIDs...)...) + default: + panic("find_txs: unknown transaction state") } } @@ -302,6 +306,10 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTx as.inprogressTx = nil case TxConfirmedMissingReceipt: delete(as.confirmedMissingReceiptTxs, tx.ID) + case TxUnconfirmed: + delete(as.unconfirmedTxs, tx.ID) + case TxConfirmed: + delete(as.confirmedTxs, tx.ID) default: return fmt.Errorf("move_tx_to_fatal_error: transaction with ID %d is in an unexpected state: %s", txID, tx.State) } From 8d3960f67159ee86da72b139e4b4541f321dea9d Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 7 Mar 2024 18:33:50 -0500 Subject: [PATCH 16/41] address comments --- common/txmgr/address_state.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 791dafdd7b3..5376d16fb71 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -327,13 +327,13 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveUn if !ok || tx == nil { return fmt.Errorf("move_unconfirmed_to_confirmed_missing_receipt: no unconfirmed transaction with ID %d", txAttempt.TxID) } + if len(tx.TxAttempts) == 0 { + return fmt.Errorf("move_unconfirmed_to_confirmed_missing_receipt: no attempts for transaction with ID %d", txAttempt.TxID) + } if tx.BroadcastAt.Before(broadcastAt) { tx.BroadcastAt = &broadcastAt } tx.State = TxConfirmedMissingReceipt - if len(tx.TxAttempts) == 0 { - tx.TxAttempts = []txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} - } txAttempt.State = txmgrtypes.TxAttemptBroadcast tx.TxAttempts = append(tx.TxAttempts, txAttempt) @@ -353,13 +353,13 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveIn if tx == nil { return fmt.Errorf("move_in_progress_to_confirmed_missing_receipt: no transaction in progress") } + if len(tx.TxAttempts) == 0 { + return fmt.Errorf("move_in_progress_to_confirmed_missing_receipt: no attempts for transaction with ID %d", tx.ID) + } if tx.BroadcastAt.Before(broadcastAt) { tx.BroadcastAt = &broadcastAt } tx.State = TxConfirmedMissingReceipt - if len(tx.TxAttempts) == 0 { - tx.TxAttempts = []txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} - } txAttempt.State = txmgrtypes.TxAttemptBroadcast tx.TxAttempts = append(tx.TxAttempts, txAttempt) @@ -382,6 +382,9 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveCo if !ok || tx == nil { return fmt.Errorf("move_confirmed_to_unconfirmed: no confirmed transaction with ID %d", txAttempt.TxID) } + if len(tx.TxAttempts) == 0 { + return fmt.Errorf("move_confirmed_to_unconfirmed: no attempts for transaction with ID %d", txAttempt.TxID) + } tx.State = TxUnconfirmed // Delete the receipt from the attempt @@ -389,9 +392,6 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveCo // Reset the broadcast information for the attempt txAttempt.State = txmgrtypes.TxAttemptInProgress txAttempt.BroadcastBeforeBlockNum = nil - if len(tx.TxAttempts) == 0 { - tx.TxAttempts = []txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} - } tx.TxAttempts = append(tx.TxAttempts, txAttempt) as.unconfirmedTxs[tx.ID] = tx From 18b8eb37c38d5b870ac2003ab51976bf782bcc80 Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 7 Mar 2024 18:46:46 -0500 Subject: [PATCH 17/41] address new comments --- common/txmgr/address_state.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 5376d16fb71..3b775b7d7b7 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -138,6 +138,8 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) findTx // If txIDs are provided, only the transactions with those IDs are considered. // If no txIDs are provided, all transactions in the given states are considered. // If no txStates are provided, all transactions are considered. +// This method does not handle transactions in the UnstartedTx state. +// Any transaction states that are unknown will cause a panic including UnstartedTx. func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) applyToTxsByState( txStates []txmgrtypes.TxState, fn func(*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]), @@ -286,6 +288,7 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveUn // moveTxToFatalError moves a transaction to the fatal error state. // It returns an error if there is no transaction with the given ID. // It returns an error if the transaction is not in an expected state. +// Unknown transaction states will cause a panic this includes Unconfirmed and Confirmed transactions. func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTxToFatalError( txID int64, txError null.String, ) error { @@ -306,12 +309,8 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTx as.inprogressTx = nil case TxConfirmedMissingReceipt: delete(as.confirmedMissingReceiptTxs, tx.ID) - case TxUnconfirmed: - delete(as.unconfirmedTxs, tx.ID) - case TxConfirmed: - delete(as.confirmedTxs, tx.ID) default: - return fmt.Errorf("move_tx_to_fatal_error: transaction with ID %d is in an unexpected state: %s", txID, tx.State) + panic("move_tx_to_fatal_error: unknown transaction state") } return nil From afe628079d743dd6a50a18841bfbc0bf72aa59e3 Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 13 Mar 2024 11:00:42 -0400 Subject: [PATCH 18/41] initial implementation of SaveReplacementInProgressAttempt --- common/txmgr/inmemory_store.go | 8 ++- .../evm/txmgr/evm_inmemory_store_test.go | 62 +++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 9f6373a68ab..757a6ee0507 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "math/big" + "sync" "time" "github.com/google/uuid" @@ -46,7 +47,8 @@ type inMemoryStore[ keyStore txmgrtypes.KeyStore[ADDR, CHAIN_ID, SEQ] persistentTxStore txmgrtypes.TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE] - addressStates map[ADDR]*addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] + addressStatesLock sync.RWMutex + addressStates map[ADDR]*addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] } // NewInMemoryStore returns a new inMemoryStore @@ -186,12 +188,12 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveR } // Persist to persistent storage - if err := ms.txStore.SaveReplacementInProgressAttempt(ctx, oldAttempt, replacementAttempt); err != nil { + if err := ms.persistentTxStore.SaveReplacementInProgressAttempt(ctx, oldAttempt, replacementAttempt); err != nil { return fmt.Errorf("save_replacement_in_progress_attempt: %w", err) } // Update in memory store - tx, err := as.PeekInProgressTx() + tx, err := as.peekInProgressTx() if tx == nil { return fmt.Errorf("save_replacement_in_progress_attempt: %w", err) } diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index a102ee1c996..b6a858a2c99 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -1,14 +1,76 @@ package txmgr_test import ( + "context" + "math/big" "testing" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/smartcontractkit/chainlink-common/pkg/logger" + commontxmgr "github.com/smartcontractkit/chainlink/v2/common/txmgr" + txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" + + evmgas "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmtxmgr "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/internal/cltest" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/evmtest" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" ) +func TestInMemoryStore_SaveReplacementInProgressAttempt(t *testing.T) { + t.Parallel() + + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := context.Background() + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + t.Run("successfully replace tx attempt", func(t *testing.T) { + // Insert a transaction into persistent store + inTx := mustInsertInProgressEthTxWithAttempt(t, persistentStore, 123, fromAddress) + oldAttempt := inTx.TxAttempts[0] + newAttempt := cltest.NewDynamicFeeEthTxAttempt(t, inTx.ID) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + err := inMemoryStore.SaveReplacementInProgressAttempt( + testutils.Context(t), + oldAttempt, + &newAttempt, + ) + require.NoError(t, err) + + expTx, err := persistentStore.FindTxWithAttempts(inTx.ID) + require.NoError(t, err) + fn := func(tx *evmtxmgr.Tx) bool { return true } + actTxs := inMemoryStore.XXXTestFindTxs(nil, fn, inTx.ID) + require.Equal(t, 1, len(actTxs)) + actTx := actTxs[0] + assertTxEqual(t, expTx, actTx) + assert.Equal(t, txmgrtypes.TxAttemptBroadcast, actTx.TxAttempts[0].State) + }) +} + // assertTxEqual asserts that two transactions are equal func assertTxEqual(t *testing.T, exp, act evmtxmgr.Tx) { assert.Equal(t, exp.ID, act.ID) From 5af210628f8aaf2011c3bae26759a4a514551a9c Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 13 Mar 2024 16:24:24 -0400 Subject: [PATCH 19/41] update tests for SaveReplacementInProgressAttempt --- common/txmgr/address_state.go | 55 ++++++++- common/txmgr/inmemory_store.go | 41 +++---- .../evm/txmgr/evm_inmemory_store_test.go | 105 ++++++++++++++---- 3 files changed, 156 insertions(+), 45 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 791b16b2d42..9e488d789b6 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -1,6 +1,7 @@ package txmgr import ( + "errors" "fmt" "sync" "time" @@ -244,14 +245,64 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) delete as._deleteTxs(txs...) } +// deleteTxAttempts removes the attempts with the given IDs from the address state. +// It removes the attempts from the hash lookup map and from the transaction. +// If an attempt is not found in the hash lookup map, it is ignored. +// If a transaction is not found in the allTxs map, it is ignored. +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) deleteTxAttempts(txAttempts ...txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { + as.Lock() + defer as.Unlock() + + for _, txAttempt := range txAttempts { + // remove the attempt from the hash lookup map + delete(as.attemptHashToTxAttempt, txAttempt.Hash) + // remove the attempt from the transaction + if tx := as.allTxs[txAttempt.TxID]; tx != nil { + var removeIndex int + for i := 0; i < len(tx.TxAttempts); i++ { + if tx.TxAttempts[i].ID == txAttempt.ID { + removeIndex = i + break + } + } + tx.TxAttempts = append(tx.TxAttempts[:removeIndex], tx.TxAttempts[removeIndex+1:]...) + } + } +} + +// addTxAttempt adds the given attempt to the transaction which matches its TxID. +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) addTxAttempts(txAttempts ...txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { + as.Lock() + defer as.Unlock() + + var errs error + for _, txAttempt := range txAttempts { + tx := as.allTxs[txAttempt.TxID] + if tx == nil { + errs = errors.Join(errs, fmt.Errorf("no transaction with ID %d", txAttempt.TxID)) + continue + } + + // add the attempt to the transaction + if tx.TxAttempts == nil { + tx.TxAttempts = []txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + } + tx.TxAttempts = append(tx.TxAttempts, txAttempt) + // add the attempt to the hash lookup map + as.attemptHashToTxAttempt[txAttempt.Hash] = &txAttempt + } + + return errs +} + // peekNextUnstartedTx returns the next unstarted transaction in the queue without removing it from the unstarted queue. func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) peekNextUnstartedTx() (*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) { return nil, nil } // peekInProgressTx returns the in-progress transaction without removing it from the in-progress state. -func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) peekInProgressTx() (*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) { - return nil, nil +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) peekInProgressTx() *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] { + return nil } // addTxToUnstarted adds the given transaction to the unstarted queue. diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 757a6ee0507..e48fb068e61 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -174,17 +174,29 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveR replacementAttempt *txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], ) error { if oldAttempt.State != txmgrtypes.TxAttemptInProgress || replacementAttempt.State != txmgrtypes.TxAttemptInProgress { - return fmt.Errorf("save_replacement_in_progress_attempt: expected attempts to be in_progress") + return fmt.Errorf("expected attempts to be in_progress") } if oldAttempt.ID == 0 { - return fmt.Errorf("save_replacement_in_progress_attempt: expected oldattempt to have an ID") + return fmt.Errorf("expected oldAttempt to have an ID") } ms.addressStatesLock.RLock() defer ms.addressStatesLock.RUnlock() - as, ok := ms.addressStates[oldAttempt.Tx.FromAddress] - if !ok { - return fmt.Errorf("save_replacement_in_progress_attempt: %w", ErrAddressNotFound) + filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { + return true + } + var as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] + var tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + for _, vas := range ms.addressStates { + txs := vas.findTxs(nil, filter, oldAttempt.TxID) + if len(txs) == 1 { + tx = &txs[0] + as = vas + break + } + } + if tx == nil { + return fmt.Errorf("save_replacement_in_progress_attempt: %w", ErrTxnNotFound) } // Persist to persistent storage @@ -193,20 +205,11 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveR } // Update in memory store - tx, err := as.peekInProgressTx() - if tx == nil { - return fmt.Errorf("save_replacement_in_progress_attempt: %w", err) - } - - var found bool - for i := 0; i < len(tx.TxAttempts); i++ { - if tx.TxAttempts[i].ID == oldAttempt.ID { - tx.TxAttempts[i] = *replacementAttempt - found = true - } - } - if !found { - tx.TxAttempts = append(tx.TxAttempts, *replacementAttempt) + // delete the old attempt + as.deleteTxAttempts(oldAttempt) + // add the new attempt + if err := as.addTxAttempts(*replacementAttempt); err != nil { + return fmt.Errorf("save_replacement_in_progress_attempt: failed to add a replacement transaction attempt: %w", err) } return nil diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index b6a858a2c99..9b20ed5da0d 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -25,35 +25,35 @@ import ( func TestInMemoryStore_SaveReplacementInProgressAttempt(t *testing.T) { t.Parallel() - db := pgtest.NewSqlxDB(t) - _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) - persistentStore := cltest.NewTestTxStore(t, db, dbcfg) - kst := cltest.NewKeyStore(t, db, dbcfg) - _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) - - ethClient := evmtest.NewEthClientMockWithDefaultChain(t) - lggr := logger.TestSugared(t) - chainID := ethClient.ConfiguredChainID() - ctx := context.Background() - - inMemoryStore, err := commontxmgr.NewInMemoryStore[ - *big.Int, - common.Address, common.Hash, common.Hash, - *evmtypes.Receipt, - evmtypes.Nonce, - evmgas.EvmFee, - ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) - require.NoError(t, err) - t.Run("successfully replace tx attempt", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := context.Background() + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + // Insert a transaction into persistent store inTx := mustInsertInProgressEthTxWithAttempt(t, persistentStore, 123, fromAddress) - oldAttempt := inTx.TxAttempts[0] - newAttempt := cltest.NewDynamicFeeEthTxAttempt(t, inTx.ID) // Insert the transaction into the in-memory store require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) - err := inMemoryStore.SaveReplacementInProgressAttempt( + oldAttempt := inTx.TxAttempts[0] + newAttempt := cltest.NewDynamicFeeEthTxAttempt(t, inTx.ID) + err = inMemoryStore.SaveReplacementInProgressAttempt( testutils.Context(t), oldAttempt, &newAttempt, @@ -67,7 +67,64 @@ func TestInMemoryStore_SaveReplacementInProgressAttempt(t *testing.T) { require.Equal(t, 1, len(actTxs)) actTx := actTxs[0] assertTxEqual(t, expTx, actTx) - assert.Equal(t, txmgrtypes.TxAttemptBroadcast, actTx.TxAttempts[0].State) + assert.Equal(t, txmgrtypes.TxAttemptInProgress, actTx.TxAttempts[0].State) + assert.Equal(t, newAttempt.Hash, actTx.TxAttempts[0].Hash) + assert.NotEqual(t, oldAttempt.ID, actTx.TxAttempts[0].ID) + }) + + t.Run("error parity for in-memory vs persistent store", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := context.Background() + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx := mustInsertInProgressEthTxWithAttempt(t, persistentStore, 124, fromAddress) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + oldAttempt := inTx.TxAttempts[0] + newAttempt := cltest.NewDynamicFeeEthTxAttempt(t, inTx.ID) + + t.Run("error when old attempt is not in progress", func(t *testing.T) { + oldAttempt.State = txmgrtypes.TxAttemptBroadcast + expErr := persistentStore.SaveReplacementInProgressAttempt(testutils.Context(t), oldAttempt, &newAttempt) + actErr := inMemoryStore.SaveReplacementInProgressAttempt(testutils.Context(t), oldAttempt, &newAttempt) + assert.Equal(t, expErr, actErr) + oldAttempt.State = txmgrtypes.TxAttemptInProgress + }) + + t.Run("error when new attempt is not in progress", func(t *testing.T) { + newAttempt.State = txmgrtypes.TxAttemptBroadcast + expErr := persistentStore.SaveReplacementInProgressAttempt(testutils.Context(t), oldAttempt, &newAttempt) + actErr := inMemoryStore.SaveReplacementInProgressAttempt(testutils.Context(t), oldAttempt, &newAttempt) + assert.Equal(t, expErr, actErr) + newAttempt.State = txmgrtypes.TxAttemptInProgress + }) + + t.Run("error when old attempt id is 0", func(t *testing.T) { + originalID := oldAttempt.ID + oldAttempt.ID = 0 + expErr := persistentStore.SaveReplacementInProgressAttempt(testutils.Context(t), oldAttempt, &newAttempt) + actErr := inMemoryStore.SaveReplacementInProgressAttempt(testutils.Context(t), oldAttempt, &newAttempt) + assert.Equal(t, expErr, actErr) + oldAttempt.ID = originalID + }) }) } From 82521c3d3710fb5f7421f40ce2ec4b30b994494d Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 13 Mar 2024 18:09:43 -0400 Subject: [PATCH 20/41] implement tests for DeleteInProgressAttempt --- common/txmgr/address_state.go | 25 +++++ common/txmgr/inmemory_store.go | 34 +++--- .../evm/txmgr/evm_inmemory_store_test.go | 103 ++++++++++++++++++ 3 files changed, 145 insertions(+), 17 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 791b16b2d42..e8a2f7c100d 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -244,6 +244,31 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) delete as._deleteTxs(txs...) } +// deleteTxAttempts removes the attempts with the given IDs from the address state. +// It removes the attempts from the hash lookup map and from the transaction. +// If an attempt is not found in the hash lookup map, it is ignored. +// If a transaction is not found in the allTxs map, it is ignored. +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) deleteTxAttempts(txAttempts ...txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { + as.Lock() + defer as.Unlock() + + for _, txAttempt := range txAttempts { + // remove the attempt from the hash lookup map + delete(as.attemptHashToTxAttempt, txAttempt.Hash) + // remove the attempt from the transaction + if tx := as.allTxs[txAttempt.TxID]; tx != nil { + var removeIndex int + for i := 0; i < len(tx.TxAttempts); i++ { + if tx.TxAttempts[i].ID == txAttempt.ID { + removeIndex = i + break + } + } + tx.TxAttempts = append(tx.TxAttempts[:removeIndex], tx.TxAttempts[removeIndex+1:]...) + } + } +} + // peekNextUnstartedTx returns the next unstarted transaction in the queue without removing it from the unstarted queue. func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) peekNextUnstartedTx() (*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) { return nil, nil diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 33d7185cf0f..97c0c753444 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -267,7 +267,7 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Count func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) DeleteInProgressAttempt(ctx context.Context, attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { if attempt.State != txmgrtypes.TxAttemptInProgress { - return fmt.Errorf("delete_in_progress_attempt: expected attempt to be in_progress") + return fmt.Errorf("DeleteInProgressAttempt: expected attempt state to be in_progress") } if attempt.ID == 0 { return fmt.Errorf("delete_in_progress_attempt: expected attempt to have an ID") @@ -276,9 +276,21 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Delet // Check if fromaddress enabled ms.addressStatesLock.RLock() defer ms.addressStatesLock.RUnlock() - as, ok := ms.addressStates[attempt.Tx.FromAddress] - if !ok { - return fmt.Errorf("delete_in_progress_attempt: %w", ErrAddressNotFound) + filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { + return true + } + var as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] + var tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + for _, vas := range ms.addressStates { + txs := vas.findTxs(nil, filter, attempt.TxID) + if len(txs) == 1 { + tx = &txs[0] + as = vas + break + } + } + if tx == nil { + return fmt.Errorf("delete_in_progress_attempt: %w", ErrTxnNotFound) } // Persist to persistent storage @@ -287,19 +299,7 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Delet } // Update in memory store - filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { - if tx.TxAttempts == nil || len(tx.TxAttempts) == 0 { - return false - } - - for _, a := range tx.TxAttempts { - if a.ID == attempt.ID { - return true - } - } - return false - } - as.deleteTxs(as.findTxs(nil, filter)...) + as.deleteTxAttempts(attempt) return nil } diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index a102ee1c996..6513e7d3779 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -1,14 +1,117 @@ package txmgr_test import ( + "context" + "math/big" "testing" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/smartcontractkit/chainlink-common/pkg/logger" + commontxmgr "github.com/smartcontractkit/chainlink/v2/common/txmgr" + txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" + + evmgas "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmtxmgr "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/internal/cltest" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/evmtest" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" ) +func TestInMemoryStore_DeleteInProgressAttempt(t *testing.T) { + t.Parallel() + + t.Run("successfully replace tx attempt", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := context.Background() + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx := mustInsertInProgressEthTxWithAttempt(t, persistentStore, 1, fromAddress) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + oldAttempt := inTx.TxAttempts[0] + err = inMemoryStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) + require.NoError(t, err) + + expTx, err := persistentStore.FindTxWithAttempts(inTx.ID) + require.NoError(t, err) + assert.Equal(t, 0, len(expTx.TxAttempts)) + + fn := func(tx *evmtxmgr.Tx) bool { return true } + actTxs := inMemoryStore.XXXTestFindTxs(nil, fn, inTx.ID) + require.Equal(t, 1, len(actTxs)) + actTx := actTxs[0] + assertTxEqual(t, expTx, actTx) + }) + + t.Run("error parity for in-memory vs persistent store", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := context.Background() + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx := mustInsertInProgressEthTxWithAttempt(t, persistentStore, 124, fromAddress) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + oldAttempt := inTx.TxAttempts[0] + t.Run("error when attempt is not in progress", func(t *testing.T) { + oldAttempt.State = txmgrtypes.TxAttemptBroadcast + expErr := persistentStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) + actErr := inMemoryStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) + assert.Equal(t, expErr, actErr) + oldAttempt.State = txmgrtypes.TxAttemptInProgress + }) + + t.Run("error when attempt has 0 id", func(t *testing.T) { + originalID := oldAttempt.ID + oldAttempt.ID = 0 + expErr := persistentStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) + actErr := inMemoryStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) + assert.Equal(t, expErr, actErr) + oldAttempt.ID = originalID + }) + }) +} + // assertTxEqual asserts that two transactions are equal func assertTxEqual(t *testing.T, exp, act evmtxmgr.Tx) { assert.Equal(t, exp.ID, act.ID) From f74f45a8c41eccd951ef030a265fb0b42ccc6d23 Mon Sep 17 00:00:00 2001 From: James Walker Date: Mon, 18 Mar 2024 18:35:56 -0400 Subject: [PATCH 21/41] implement tests for MarkOldTxesMissingReceiptAsErrored --- common/txmgr/address_state.go | 12 +- common/txmgr/inmemory_store.go | 94 ++++++++-------- .../evm/txmgr/evm_inmemory_store_test.go | 106 +++++++++++++++++- 3 files changed, 157 insertions(+), 55 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 791b16b2d42..7de25828656 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -303,18 +303,24 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTx if tx == nil { return fmt.Errorf("move_tx_to_fatal_error: no transaction with ID %d", txID) } + originalState := tx.State + // Move the transaction to the fatal error state as._moveTxToFatalError(tx, txError) - switch tx.State { + // Remove the transaction from its original state + switch originalState { case TxUnstarted: _ = as.unstartedTxs.RemoveTxByID(txID) case TxInProgress: as.inprogressTx = nil case TxConfirmedMissingReceipt: delete(as.confirmedMissingReceiptTxs, tx.ID) + case TxFatalError: + // Already in fatal error state + return nil default: - panic("move_tx_to_fatal_error: unknown transaction state") + panic(fmt.Sprintf("move_tx_to_fatal_error: unknown transaction state: %q", tx.State)) } return nil @@ -476,7 +482,7 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) _moveT ) { tx.State = TxFatalError tx.Sequence = nil - tx.TxAttempts = nil + tx.BroadcastAt = nil tx.InitialBroadcastAt = nil tx.Error = txError as.fatalErroredTxs[tx.ID] = tx diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index da00990847d..0fb299dc808 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -340,7 +340,7 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MarkA } func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, finalityDepth uint32, chainID CHAIN_ID) error { if ms.chainID.String() != chainID.String() { - return fmt.Errorf("mark_old_txes_missing_receipt_as_errored: %w", ErrInvalidChainID) + return nil } // Persist to persistent storage @@ -356,65 +356,59 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MarkO MaxBroadcastBeforeBlockNum int64 TxHashes []TX_HASH } - var resultsLock sync.Mutex var results []result - wg := sync.WaitGroup{} - errsLock := sync.Mutex{} + cutoff := blockNum - int64(finalityDepth) + if cutoff <= 0 { + return nil + } + filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { + if len(tx.TxAttempts) == 0 { + return false + } + var maxBroadcastBeforeBlockNum int64 + for i := 0; i < len(tx.TxAttempts); i++ { + txAttempt := tx.TxAttempts[i] + if txAttempt.BroadcastBeforeBlockNum == nil { + continue + } + if *txAttempt.BroadcastBeforeBlockNum > maxBroadcastBeforeBlockNum { + maxBroadcastBeforeBlockNum = *txAttempt.BroadcastBeforeBlockNum + } + } + return maxBroadcastBeforeBlockNum < cutoff + } var errs error ms.addressStatesLock.RLock() defer ms.addressStatesLock.RUnlock() for _, as := range ms.addressStates { - wg.Add(1) - go func(as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) { - filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { - if tx.TxAttempts == nil || len(tx.TxAttempts) == 0 { - return false - } - if tx.State != TxConfirmedMissingReceipt { - return false - } - attempt := tx.TxAttempts[0] - if attempt.BroadcastBeforeBlockNum == nil { - return false - } - - return *attempt.BroadcastBeforeBlockNum < blockNum-int64(finalityDepth) + states := []txmgrtypes.TxState{TxConfirmedMissingReceipt} + txs := as.findTxs(states, filter) + for _, tx := range txs { + if err := as.moveTxToFatalError(tx.ID, null.StringFrom(ErrCouldNotGetReceipt.Error())); err != nil { + err = fmt.Errorf("mark_old_txes_missing_receipt_as_errored: address: %s: %w", as.fromAddress, err) + errs = errors.Join(errs, err) + continue } - states := []txmgrtypes.TxState{TxConfirmedMissingReceipt} - txs := as.findTxs(states, filter) - for _, tx := range txs { - if err := as.moveTxToFatalError(tx.ID, null.StringFrom(ErrCouldNotGetReceipt.Error())); err != nil { - err = fmt.Errorf("mark_old_txes_missing_receipt_as_errored: address: %s: %w", as.fromAddress, err) - errsLock.Lock() - errs = errors.Join(errs, err) - errsLock.Unlock() - continue - } - hashes := make([]TX_HASH, len(tx.TxAttempts)) - maxBroadcastBeforeBlockNum := int64(0) - for i, attempt := range tx.TxAttempts { - hashes[i] = attempt.Hash - if attempt.BroadcastBeforeBlockNum != nil { - if *attempt.BroadcastBeforeBlockNum > maxBroadcastBeforeBlockNum { - maxBroadcastBeforeBlockNum = *attempt.BroadcastBeforeBlockNum - } + hashes := make([]TX_HASH, len(tx.TxAttempts)) + maxBroadcastBeforeBlockNum := int64(0) + for i, attempt := range tx.TxAttempts { + hashes[i] = attempt.Hash + if attempt.BroadcastBeforeBlockNum != nil { + if *attempt.BroadcastBeforeBlockNum > maxBroadcastBeforeBlockNum { + maxBroadcastBeforeBlockNum = *attempt.BroadcastBeforeBlockNum } } - rr := result{ - ID: tx.ID, - Sequence: *tx.Sequence, - FromAddress: tx.FromAddress, - MaxBroadcastBeforeBlockNum: maxBroadcastBeforeBlockNum, - TxHashes: hashes, - } - resultsLock.Lock() - results = append(results, rr) - resultsLock.Unlock() } - wg.Done() - }(as) + rr := result{ + ID: tx.ID, + Sequence: *tx.Sequence, + FromAddress: tx.FromAddress, + MaxBroadcastBeforeBlockNum: maxBroadcastBeforeBlockNum, + TxHashes: hashes, + } + results = append(results, rr) + } } - wg.Wait() for _, r := range results { ms.lggr.Criticalw(fmt.Sprintf("eth_tx with ID %v expired without ever getting a receipt for any of our attempts. "+ diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index a102ee1c996..2409ae45175 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -1,14 +1,111 @@ package txmgr_test import ( + "context" + "math/big" "testing" + "time" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/smartcontractkit/chainlink-common/pkg/logger" + commontxmgr "github.com/smartcontractkit/chainlink/v2/common/txmgr" + txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" + + evmgas "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmtxmgr "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/internal/cltest" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/evmtest" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" ) +func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { + t.Parallel() + blockNum := int64(10) + finalityDepth := uint32(2) + + t.Run("successfully mark errored transaction", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := context.Background() + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx := mustInsertConfirmedMissingReceiptEthTxWithLegacyAttempt(t, persistentStore, 1, 7, time.Now(), fromAddress) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + err = inMemoryStore.MarkOldTxesMissingReceiptAsErrored(testutils.Context(t), blockNum, finalityDepth, chainID) + require.NoError(t, err) + + expTx, err := persistentStore.FindTxWithAttempts(inTx.ID) + require.NoError(t, err) + require.Len(t, expTx.TxAttempts, 1) + + fn := func(tx *evmtxmgr.Tx) bool { return true } + actTxs := inMemoryStore.XXXTestFindTxs(nil, fn, inTx.ID) + require.Equal(t, 1, len(actTxs)) + actTx := actTxs[0] + + assertTxEqual(t, expTx, actTx) + assert.Equal(t, txmgrtypes.TxAttemptBroadcast, actTx.TxAttempts[0].State) + assert.Equal(t, commontxmgr.TxFatalError, actTx.State) + }) + + t.Run("error parity for in-memory vs persistent store", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := context.Background() + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx := mustInsertConfirmedMissingReceiptEthTxWithLegacyAttempt(t, persistentStore, 1, 7, time.Now(), fromAddress) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + t.Run("error when old attempt is not in progress", func(t *testing.T) { + wrongChainID := big.NewInt(123) + expErr := persistentStore.MarkOldTxesMissingReceiptAsErrored(testutils.Context(t), blockNum, finalityDepth, wrongChainID) + actErr := inMemoryStore.MarkOldTxesMissingReceiptAsErrored(testutils.Context(t), blockNum, finalityDepth, wrongChainID) + assert.Equal(t, expErr, actErr) + }) + }) +} + // assertTxEqual asserts that two transactions are equal func assertTxEqual(t *testing.T, exp, act evmtxmgr.Tx) { assert.Equal(t, exp.ID, act.ID) @@ -20,7 +117,12 @@ func assertTxEqual(t *testing.T, exp, act evmtxmgr.Tx) { assert.Equal(t, exp.Value, act.Value) assert.Equal(t, exp.FeeLimit, act.FeeLimit) assert.Equal(t, exp.Error, act.Error) - assert.Equal(t, exp.BroadcastAt, act.BroadcastAt) + if exp.BroadcastAt != nil { + require.NotNil(t, act.BroadcastAt) + assert.Equal(t, exp.BroadcastAt.Unix(), act.BroadcastAt.Unix()) + } else { + assert.Equal(t, exp.BroadcastAt, act.BroadcastAt) + } assert.Equal(t, exp.InitialBroadcastAt, act.InitialBroadcastAt) assert.Equal(t, exp.CreatedAt, act.CreatedAt) assert.Equal(t, exp.State, act.State) @@ -33,7 +135,7 @@ func assertTxEqual(t *testing.T, exp, act evmtxmgr.Tx) { assert.Equal(t, exp.SignalCallback, act.SignalCallback) assert.Equal(t, exp.CallbackCompleted, act.CallbackCompleted) - require.Len(t, exp.TxAttempts, len(act.TxAttempts)) + require.Equal(t, len(exp.TxAttempts), len(act.TxAttempts)) for i := 0; i < len(exp.TxAttempts); i++ { assertTxAttemptEqual(t, exp.TxAttempts[i], act.TxAttempts[i]) } From 051ebe2b7988a456ab9ccc4487fbab1ce989c395 Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 20 Mar 2024 22:55:30 -0400 Subject: [PATCH 22/41] add panic --- common/txmgr/inmemory_store.go | 2 +- .../evm/txmgr/evm_inmemory_store_test.go | 34 ------------------- core/scripts/go.mod | 6 ++-- core/scripts/go.sum | 12 +++---- go.mod | 6 ++-- go.sum | 12 +++---- integration-tests/go.mod | 6 ++-- integration-tests/go.sum | 12 +++---- integration-tests/load/go.mod | 6 ++-- integration-tests/load/go.sum | 12 +++---- 10 files changed, 37 insertions(+), 71 deletions(-) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 0fb299dc808..a40b9009d41 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -340,7 +340,7 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MarkA } func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, finalityDepth uint32, chainID CHAIN_ID) error { if ms.chainID.String() != chainID.String() { - return nil + panic(fmt.Sprintf(ErrInvalidChainID.Error()+": %s", chainID.String())) } // Persist to persistent storage diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index 2409ae45175..c5b121f9369 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -70,40 +70,6 @@ func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { assert.Equal(t, txmgrtypes.TxAttemptBroadcast, actTx.TxAttempts[0].State) assert.Equal(t, commontxmgr.TxFatalError, actTx.State) }) - - t.Run("error parity for in-memory vs persistent store", func(t *testing.T) { - db := pgtest.NewSqlxDB(t) - _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) - persistentStore := cltest.NewTestTxStore(t, db, dbcfg) - kst := cltest.NewKeyStore(t, db, dbcfg) - _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) - - ethClient := evmtest.NewEthClientMockWithDefaultChain(t) - lggr := logger.TestSugared(t) - chainID := ethClient.ConfiguredChainID() - ctx := context.Background() - - inMemoryStore, err := commontxmgr.NewInMemoryStore[ - *big.Int, - common.Address, common.Hash, common.Hash, - *evmtypes.Receipt, - evmtypes.Nonce, - evmgas.EvmFee, - ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) - require.NoError(t, err) - - // Insert a transaction into persistent store - inTx := mustInsertConfirmedMissingReceiptEthTxWithLegacyAttempt(t, persistentStore, 1, 7, time.Now(), fromAddress) - // Insert the transaction into the in-memory store - require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) - - t.Run("error when old attempt is not in progress", func(t *testing.T) { - wrongChainID := big.NewInt(123) - expErr := persistentStore.MarkOldTxesMissingReceiptAsErrored(testutils.Context(t), blockNum, finalityDepth, wrongChainID) - actErr := inMemoryStore.MarkOldTxesMissingReceiptAsErrored(testutils.Context(t), blockNum, finalityDepth, wrongChainID) - assert.Equal(t, expErr, actErr) - }) - }) } // assertTxEqual asserts that two transactions are equal diff --git a/core/scripts/go.mod b/core/scripts/go.mod index 9b7952c10a5..135e0de6aff 100644 --- a/core/scripts/go.mod +++ b/core/scripts/go.mod @@ -157,8 +157,8 @@ require ( github.com/gorilla/securecookie v1.1.2 // indirect github.com/gorilla/sessions v1.2.2 // indirect github.com/gorilla/websocket v1.5.1 // indirect - github.com/grafana/pyroscope-go v1.0.4 // indirect - github.com/grafana/pyroscope-go/godeltaprof v0.1.4 // indirect + github.com/grafana/pyroscope-go v1.1.1 // indirect + github.com/grafana/pyroscope-go/godeltaprof v0.1.6 // indirect github.com/graph-gophers/dataloader v5.0.0+incompatible // indirect github.com/graph-gophers/graphql-go v1.3.0 // indirect github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 // indirect @@ -195,7 +195,7 @@ require ( github.com/jmhodges/levigo v1.0.0 // indirect github.com/jpillora/backoff v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect - github.com/klauspost/compress v1.17.2 // indirect + github.com/klauspost/compress v1.17.3 // indirect github.com/klauspost/cpuid/v2 v2.2.5 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect diff --git a/core/scripts/go.sum b/core/scripts/go.sum index 077f4538d01..fac1d4f5463 100644 --- a/core/scripts/go.sum +++ b/core/scripts/go.sum @@ -658,10 +658,10 @@ github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/ad github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/gorilla/websocket v1.5.1 h1:gmztn0JnHVt9JZquRuzLw3g4wouNVzKL15iLr/zn/QY= github.com/gorilla/websocket v1.5.1/go.mod h1:x3kM2JMyaluk02fnUJpQuwD2dCS5NDG2ZHL0uE0tcaY= -github.com/grafana/pyroscope-go v1.0.4 h1:oyQX0BOkL+iARXzHuCdIF5TQ7/sRSel1YFViMHC7Bm0= -github.com/grafana/pyroscope-go v1.0.4/go.mod h1:0d7ftwSMBV/Awm7CCiYmHQEG8Y44Ma3YSjt+nWcWztY= -github.com/grafana/pyroscope-go/godeltaprof v0.1.4 h1:mDsJ3ngul7UfrHibGQpV66PbZ3q1T8glz/tK3bQKKEk= -github.com/grafana/pyroscope-go/godeltaprof v0.1.4/go.mod h1:1HSPtjU8vLG0jE9JrTdzjgFqdJ/VgN7fvxBNq3luJko= +github.com/grafana/pyroscope-go v1.1.1 h1:PQoUU9oWtO3ve/fgIiklYuGilvsm8qaGhlY4Vw6MAcQ= +github.com/grafana/pyroscope-go v1.1.1/go.mod h1:Mw26jU7jsL/KStNSGGuuVYdUq7Qghem5P8aXYXSXG88= +github.com/grafana/pyroscope-go/godeltaprof v0.1.6 h1:nEdZ8louGAplSvIJi1HVp7kWvFvdiiYg3COLlTwJiFo= +github.com/grafana/pyroscope-go/godeltaprof v0.1.6/go.mod h1:Tk376Nbldo4Cha9RgiU7ik8WKFkNpfds98aUzS8omLE= github.com/graph-gophers/dataloader v5.0.0+incompatible h1:R+yjsbrNq1Mo3aPG+Z/EKYrXrXXUNJHOgbRt+U6jOug= github.com/graph-gophers/dataloader v5.0.0+incompatible/go.mod h1:jk4jk0c5ZISbKaMe8WsVopGB5/15GvGHMdMdPtwlRp4= github.com/graph-gophers/graphql-go v1.3.0 h1:Eb9x/q6MFpCLz7jBCiP/WTxjSDrYLR1QY41SORZyNJ0= @@ -868,8 +868,8 @@ github.com/klauspost/compress v1.9.7/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0 github.com/klauspost/compress v1.11.4/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= github.com/klauspost/compress v1.12.3/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg= github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= -github.com/klauspost/compress v1.17.2 h1:RlWWUY/Dr4fL8qk9YG7DTZ7PDgME2V4csBXA8L/ixi4= -github.com/klauspost/compress v1.17.2/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= +github.com/klauspost/compress v1.17.3 h1:qkRjuerhUU1EmXLYGkSH6EZL+vPSxIrYjLNAK4slzwA= +github.com/klauspost/compress v1.17.3/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= github.com/klauspost/cpuid v1.2.1/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.2.5 h1:0E5MSMDEoAulmXNFquVs//DdoomxaoTY1kUhbc/qbZg= diff --git a/go.mod b/go.mod index bc500ce3b40..4833ea2c284 100644 --- a/go.mod +++ b/go.mod @@ -30,7 +30,7 @@ require ( github.com/gorilla/securecookie v1.1.2 github.com/gorilla/sessions v1.2.2 github.com/gorilla/websocket v1.5.1 - github.com/grafana/pyroscope-go v1.0.4 + github.com/grafana/pyroscope-go v1.1.1 github.com/graph-gophers/dataloader v5.0.0+incompatible github.com/graph-gophers/graphql-go v1.3.0 github.com/hashicorp/consul/sdk v0.14.1 @@ -210,7 +210,7 @@ require ( github.com/google/go-tpm v0.9.0 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/gorilla/context v1.1.1 // indirect - github.com/grafana/pyroscope-go/godeltaprof v0.1.4 // indirect + github.com/grafana/pyroscope-go/godeltaprof v0.1.6 // indirect github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 // indirect github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.0 // indirect github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.3 // indirect @@ -240,7 +240,7 @@ require ( github.com/jackpal/go-nat-pmp v1.0.2 // indirect github.com/jmhodges/levigo v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect - github.com/klauspost/compress v1.17.2 // indirect + github.com/klauspost/compress v1.17.3 // indirect github.com/klauspost/cpuid/v2 v2.2.5 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect diff --git a/go.sum b/go.sum index 0477adbed3c..30335639468 100644 --- a/go.sum +++ b/go.sum @@ -649,10 +649,10 @@ github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/ad github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/gorilla/websocket v1.5.1 h1:gmztn0JnHVt9JZquRuzLw3g4wouNVzKL15iLr/zn/QY= github.com/gorilla/websocket v1.5.1/go.mod h1:x3kM2JMyaluk02fnUJpQuwD2dCS5NDG2ZHL0uE0tcaY= -github.com/grafana/pyroscope-go v1.0.4 h1:oyQX0BOkL+iARXzHuCdIF5TQ7/sRSel1YFViMHC7Bm0= -github.com/grafana/pyroscope-go v1.0.4/go.mod h1:0d7ftwSMBV/Awm7CCiYmHQEG8Y44Ma3YSjt+nWcWztY= -github.com/grafana/pyroscope-go/godeltaprof v0.1.4 h1:mDsJ3ngul7UfrHibGQpV66PbZ3q1T8glz/tK3bQKKEk= -github.com/grafana/pyroscope-go/godeltaprof v0.1.4/go.mod h1:1HSPtjU8vLG0jE9JrTdzjgFqdJ/VgN7fvxBNq3luJko= +github.com/grafana/pyroscope-go v1.1.1 h1:PQoUU9oWtO3ve/fgIiklYuGilvsm8qaGhlY4Vw6MAcQ= +github.com/grafana/pyroscope-go v1.1.1/go.mod h1:Mw26jU7jsL/KStNSGGuuVYdUq7Qghem5P8aXYXSXG88= +github.com/grafana/pyroscope-go/godeltaprof v0.1.6 h1:nEdZ8louGAplSvIJi1HVp7kWvFvdiiYg3COLlTwJiFo= +github.com/grafana/pyroscope-go/godeltaprof v0.1.6/go.mod h1:Tk376Nbldo4Cha9RgiU7ik8WKFkNpfds98aUzS8omLE= github.com/graph-gophers/dataloader v5.0.0+incompatible h1:R+yjsbrNq1Mo3aPG+Z/EKYrXrXXUNJHOgbRt+U6jOug= github.com/graph-gophers/dataloader v5.0.0+incompatible/go.mod h1:jk4jk0c5ZISbKaMe8WsVopGB5/15GvGHMdMdPtwlRp4= github.com/graph-gophers/graphql-go v1.3.0 h1:Eb9x/q6MFpCLz7jBCiP/WTxjSDrYLR1QY41SORZyNJ0= @@ -861,8 +861,8 @@ github.com/klauspost/compress v1.10.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYs github.com/klauspost/compress v1.11.4/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= github.com/klauspost/compress v1.12.3/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg= github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= -github.com/klauspost/compress v1.17.2 h1:RlWWUY/Dr4fL8qk9YG7DTZ7PDgME2V4csBXA8L/ixi4= -github.com/klauspost/compress v1.17.2/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= +github.com/klauspost/compress v1.17.3 h1:qkRjuerhUU1EmXLYGkSH6EZL+vPSxIrYjLNAK4slzwA= +github.com/klauspost/compress v1.17.3/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= github.com/klauspost/cpuid v1.2.1/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.2.5 h1:0E5MSMDEoAulmXNFquVs//DdoomxaoTY1kUhbc/qbZg= diff --git a/integration-tests/go.mod b/integration-tests/go.mod index a85871465ac..cb4814f7358 100644 --- a/integration-tests/go.mod +++ b/integration-tests/go.mod @@ -235,8 +235,8 @@ require ( github.com/grafana/gomemcache v0.0.0-20231023152154-6947259a0586 // indirect github.com/grafana/loki v1.6.2-0.20231215164305-b51b7d7b5503 // indirect github.com/grafana/loki/pkg/push v0.0.0-20231201111602-11ef833ed3e4 // indirect - github.com/grafana/pyroscope-go v1.0.4 // indirect - github.com/grafana/pyroscope-go/godeltaprof v0.1.4 // indirect + github.com/grafana/pyroscope-go v1.1.1 // indirect + github.com/grafana/pyroscope-go/godeltaprof v0.1.6 // indirect github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd // indirect github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 // indirect @@ -289,7 +289,7 @@ require ( github.com/json-iterator/go v1.1.12 // indirect github.com/julienschmidt/httprouter v1.3.0 // indirect github.com/kelseyhightower/envconfig v1.4.0 // indirect - github.com/klauspost/compress v1.17.2 // indirect + github.com/klauspost/compress v1.17.3 // indirect github.com/klauspost/cpuid/v2 v2.2.5 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect diff --git a/integration-tests/go.sum b/integration-tests/go.sum index e45709fee48..d3573e4ead3 100644 --- a/integration-tests/go.sum +++ b/integration-tests/go.sum @@ -859,10 +859,10 @@ github.com/grafana/loki v1.6.2-0.20231215164305-b51b7d7b5503 h1:gdrsYbmk8822v6qv github.com/grafana/loki v1.6.2-0.20231215164305-b51b7d7b5503/go.mod h1:d8seWXCEXkL42mhuIJYcGi6DxfehzoIpLrMQWJojvOo= github.com/grafana/loki/pkg/push v0.0.0-20231201111602-11ef833ed3e4 h1:wQ0FnSeebhJIBkgYOD06Mxk9HV2KhtEG0hp/7R+5RUQ= github.com/grafana/loki/pkg/push v0.0.0-20231201111602-11ef833ed3e4/go.mod h1:f3JSoxBTPXX5ec4FxxeC19nTBSxoTz+cBgS3cYLMcr0= -github.com/grafana/pyroscope-go v1.0.4 h1:oyQX0BOkL+iARXzHuCdIF5TQ7/sRSel1YFViMHC7Bm0= -github.com/grafana/pyroscope-go v1.0.4/go.mod h1:0d7ftwSMBV/Awm7CCiYmHQEG8Y44Ma3YSjt+nWcWztY= -github.com/grafana/pyroscope-go/godeltaprof v0.1.4 h1:mDsJ3ngul7UfrHibGQpV66PbZ3q1T8glz/tK3bQKKEk= -github.com/grafana/pyroscope-go/godeltaprof v0.1.4/go.mod h1:1HSPtjU8vLG0jE9JrTdzjgFqdJ/VgN7fvxBNq3luJko= +github.com/grafana/pyroscope-go v1.1.1 h1:PQoUU9oWtO3ve/fgIiklYuGilvsm8qaGhlY4Vw6MAcQ= +github.com/grafana/pyroscope-go v1.1.1/go.mod h1:Mw26jU7jsL/KStNSGGuuVYdUq7Qghem5P8aXYXSXG88= +github.com/grafana/pyroscope-go/godeltaprof v0.1.6 h1:nEdZ8louGAplSvIJi1HVp7kWvFvdiiYg3COLlTwJiFo= +github.com/grafana/pyroscope-go/godeltaprof v0.1.6/go.mod h1:Tk376Nbldo4Cha9RgiU7ik8WKFkNpfds98aUzS8omLE= github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd h1:PpuIBO5P3e9hpqBD0O/HjhShYuM6XE0i/lbE6J94kww= github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd/go.mod h1:M5qHK+eWfAv8VR/265dIuEpL3fNfeC21tXXp9itM24A= github.com/graph-gophers/dataloader v5.0.0+incompatible h1:R+yjsbrNq1Mo3aPG+Z/EKYrXrXXUNJHOgbRt+U6jOug= @@ -1109,8 +1109,8 @@ github.com/klauspost/compress v1.10.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYs github.com/klauspost/compress v1.11.4/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= github.com/klauspost/compress v1.12.3/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg= github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= -github.com/klauspost/compress v1.17.2 h1:RlWWUY/Dr4fL8qk9YG7DTZ7PDgME2V4csBXA8L/ixi4= -github.com/klauspost/compress v1.17.2/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= +github.com/klauspost/compress v1.17.3 h1:qkRjuerhUU1EmXLYGkSH6EZL+vPSxIrYjLNAK4slzwA= +github.com/klauspost/compress v1.17.3/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= github.com/klauspost/cpuid v1.2.1/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.2.5 h1:0E5MSMDEoAulmXNFquVs//DdoomxaoTY1kUhbc/qbZg= diff --git a/integration-tests/load/go.mod b/integration-tests/load/go.mod index f432efaa614..ca5713cb830 100644 --- a/integration-tests/load/go.mod +++ b/integration-tests/load/go.mod @@ -217,8 +217,8 @@ require ( github.com/grafana/gomemcache v0.0.0-20231023152154-6947259a0586 // indirect github.com/grafana/loki v1.6.2-0.20231215164305-b51b7d7b5503 // indirect github.com/grafana/loki/pkg/push v0.0.0-20231201111602-11ef833ed3e4 // indirect - github.com/grafana/pyroscope-go v1.0.4 // indirect - github.com/grafana/pyroscope-go/godeltaprof v0.1.4 // indirect + github.com/grafana/pyroscope-go v1.1.1 // indirect + github.com/grafana/pyroscope-go/godeltaprof v0.1.6 // indirect github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd // indirect github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 // indirect @@ -272,7 +272,7 @@ require ( github.com/json-iterator/go v1.1.12 // indirect github.com/julienschmidt/httprouter v1.3.0 // indirect github.com/kelseyhightower/envconfig v1.4.0 // indirect - github.com/klauspost/compress v1.17.2 // indirect + github.com/klauspost/compress v1.17.3 // indirect github.com/klauspost/cpuid/v2 v2.2.5 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect diff --git a/integration-tests/load/go.sum b/integration-tests/load/go.sum index 1a5729f5ebd..fb658486260 100644 --- a/integration-tests/load/go.sum +++ b/integration-tests/load/go.sum @@ -850,10 +850,10 @@ github.com/grafana/loki v1.6.2-0.20231215164305-b51b7d7b5503 h1:gdrsYbmk8822v6qv github.com/grafana/loki v1.6.2-0.20231215164305-b51b7d7b5503/go.mod h1:d8seWXCEXkL42mhuIJYcGi6DxfehzoIpLrMQWJojvOo= github.com/grafana/loki/pkg/push v0.0.0-20231201111602-11ef833ed3e4 h1:wQ0FnSeebhJIBkgYOD06Mxk9HV2KhtEG0hp/7R+5RUQ= github.com/grafana/loki/pkg/push v0.0.0-20231201111602-11ef833ed3e4/go.mod h1:f3JSoxBTPXX5ec4FxxeC19nTBSxoTz+cBgS3cYLMcr0= -github.com/grafana/pyroscope-go v1.0.4 h1:oyQX0BOkL+iARXzHuCdIF5TQ7/sRSel1YFViMHC7Bm0= -github.com/grafana/pyroscope-go v1.0.4/go.mod h1:0d7ftwSMBV/Awm7CCiYmHQEG8Y44Ma3YSjt+nWcWztY= -github.com/grafana/pyroscope-go/godeltaprof v0.1.4 h1:mDsJ3ngul7UfrHibGQpV66PbZ3q1T8glz/tK3bQKKEk= -github.com/grafana/pyroscope-go/godeltaprof v0.1.4/go.mod h1:1HSPtjU8vLG0jE9JrTdzjgFqdJ/VgN7fvxBNq3luJko= +github.com/grafana/pyroscope-go v1.1.1 h1:PQoUU9oWtO3ve/fgIiklYuGilvsm8qaGhlY4Vw6MAcQ= +github.com/grafana/pyroscope-go v1.1.1/go.mod h1:Mw26jU7jsL/KStNSGGuuVYdUq7Qghem5P8aXYXSXG88= +github.com/grafana/pyroscope-go/godeltaprof v0.1.6 h1:nEdZ8louGAplSvIJi1HVp7kWvFvdiiYg3COLlTwJiFo= +github.com/grafana/pyroscope-go/godeltaprof v0.1.6/go.mod h1:Tk376Nbldo4Cha9RgiU7ik8WKFkNpfds98aUzS8omLE= github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd h1:PpuIBO5P3e9hpqBD0O/HjhShYuM6XE0i/lbE6J94kww= github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd/go.mod h1:M5qHK+eWfAv8VR/265dIuEpL3fNfeC21tXXp9itM24A= github.com/graph-gophers/dataloader v5.0.0+incompatible h1:R+yjsbrNq1Mo3aPG+Z/EKYrXrXXUNJHOgbRt+U6jOug= @@ -1098,8 +1098,8 @@ github.com/klauspost/compress v1.10.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYs github.com/klauspost/compress v1.11.4/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= github.com/klauspost/compress v1.12.3/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg= github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= -github.com/klauspost/compress v1.17.2 h1:RlWWUY/Dr4fL8qk9YG7DTZ7PDgME2V4csBXA8L/ixi4= -github.com/klauspost/compress v1.17.2/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= +github.com/klauspost/compress v1.17.3 h1:qkRjuerhUU1EmXLYGkSH6EZL+vPSxIrYjLNAK4slzwA= +github.com/klauspost/compress v1.17.3/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= github.com/klauspost/cpuid v1.2.1/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.2.5 h1:0E5MSMDEoAulmXNFquVs//DdoomxaoTY1kUhbc/qbZg= From cbacd55f3a47dc59dc93fbce7e79ffc869c865d8 Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 20 Mar 2024 23:06:33 -0400 Subject: [PATCH 23/41] fix linter issue --- common/txmgr/address_state.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 9e488d789b6..46f96ab8bfd 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -276,7 +276,8 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) addTxA defer as.Unlock() var errs error - for _, txAttempt := range txAttempts { + for i := 0; i < len(txAttempts); i++ { + txAttempt := txAttempts[i] tx := as.allTxs[txAttempt.TxID] if tx == nil { errs = errors.Join(errs, fmt.Errorf("no transaction with ID %d", txAttempt.TxID)) From da6d235042eda6a0f097a22e177fbbaea2732414 Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 21 Mar 2024 01:13:55 -0400 Subject: [PATCH 24/41] address comments --- common/txmgr/address_state.go | 37 +++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 791b16b2d42..063086ac6e2 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -113,7 +113,7 @@ func newAddressState[ case TxFatalError: as.fatalErroredTxs[tx.ID] = &tx default: - panic("unknown transaction state") + panic(fmt.Sprintf("unknown transaction state: %q", tx.State)) } as.allTxs[tx.ID] = &tx if tx.IdempotencyKey != nil { @@ -142,8 +142,7 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) findTx // If txIDs are provided, only the transactions with those IDs are considered. // If no txIDs are provided, all transactions in the given states are considered. // If no txStates are provided, all transactions are considered. -// This method does not handle transactions in the UnstartedTx state. -// Any transaction states that are unknown will cause a panic including UnstartedTx. +// Any transaction states that are unknown will cause a panic. func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) applyToTxsByState( txStates []txmgrtypes.TxState, fn func(*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]), @@ -172,8 +171,15 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) applyT as._applyToTxs(as.confirmedTxs, fn, txIDs...) case TxFatalError: as._applyToTxs(as.fatalErroredTxs, fn, txIDs...) + case TxUnstarted: + nfn := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { + if tx.State == TxUnstarted { + fn(tx) + } + } + as._applyToTxs(as.allTxs, nfn, txIDs...) default: - panic("apply_to_txs_by_state: unknown transaction state") + panic(fmt.Sprintf("unknown transaction state: %q", txState)) } } } @@ -224,8 +230,13 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) findTx txs = append(txs, as._findTxs(as.confirmedTxs, filter, txIDs...)...) case TxFatalError: txs = append(txs, as._findTxs(as.fatalErroredTxs, filter, txIDs...)...) + case TxUnstarted: + fn := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { + return tx.State == TxUnstarted && filter(tx) + } + txs = append(txs, as._findTxs(as.allTxs, fn, txIDs...)...) default: - panic("find_txs: unknown transaction state") + panic(fmt.Sprintf("unknown transaction state: %q", txState)) } } @@ -314,7 +325,7 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTx case TxConfirmedMissingReceipt: delete(as.confirmedMissingReceiptTxs, tx.ID) default: - panic("move_tx_to_fatal_error: unknown transaction state") + panic(fmt.Sprintf("unknown transaction state: %q", tx.State)) } return nil @@ -322,23 +333,15 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTx // moveUnconfirmedToConfirmedMissingReceipt moves the unconfirmed transaction to the confirmed missing receipt state. // If there is no unconfirmed transaction with the given ID, an error is returned. -func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveUnconfirmedToConfirmedMissingReceipt(txAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveUnconfirmedToConfirmedMissingReceipt(txID int64) error { as.Lock() defer as.Unlock() - tx, ok := as.unconfirmedTxs[txAttempt.TxID] + tx, ok := as.unconfirmedTxs[txID] if !ok || tx == nil { - return fmt.Errorf("move_unconfirmed_to_confirmed_missing_receipt: no unconfirmed transaction with ID %d", txAttempt.TxID) - } - if len(tx.TxAttempts) == 0 { - return fmt.Errorf("move_unconfirmed_to_confirmed_missing_receipt: no attempts for transaction with ID %d", txAttempt.TxID) - } - if tx.BroadcastAt.Before(broadcastAt) { - tx.BroadcastAt = &broadcastAt + return fmt.Errorf("move_unconfirmed_to_confirmed_missing_receipt: no unconfirmed transaction with ID %d", txID) } tx.State = TxConfirmedMissingReceipt - txAttempt.State = txmgrtypes.TxAttemptBroadcast - tx.TxAttempts = append(tx.TxAttempts, txAttempt) as.confirmedMissingReceiptTxs[tx.ID] = tx delete(as.unconfirmedTxs, tx.ID) From 39566e998aac0fc10b365f4b88e6d08cb741117f Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 21 Mar 2024 01:24:03 -0400 Subject: [PATCH 25/41] address comments --- core/chains/evm/txmgr/evm_inmemory_store_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index c5b121f9369..7ff98d4900e 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -1,7 +1,6 @@ package txmgr_test import ( - "context" "math/big" "testing" "time" @@ -31,14 +30,14 @@ func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { t.Run("successfully mark errored transaction", func(t *testing.T) { db := pgtest.NewSqlxDB(t) _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) - persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + persistentStore := cltest.NewTestTxStore(t, db) kst := cltest.NewKeyStore(t, db, dbcfg) _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) ethClient := evmtest.NewEthClientMockWithDefaultChain(t) lggr := logger.TestSugared(t) chainID := ethClient.ConfiguredChainID() - ctx := context.Background() + ctx := testutils.Context(t) inMemoryStore, err := commontxmgr.NewInMemoryStore[ *big.Int, @@ -54,10 +53,10 @@ func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { // Insert the transaction into the in-memory store require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) - err = inMemoryStore.MarkOldTxesMissingReceiptAsErrored(testutils.Context(t), blockNum, finalityDepth, chainID) + err = inMemoryStore.MarkOldTxesMissingReceiptAsErrored(ctx, blockNum, finalityDepth, chainID) require.NoError(t, err) - expTx, err := persistentStore.FindTxWithAttempts(inTx.ID) + expTx, err := persistentStore.FindTxWithAttempts(ctx, inTx.ID) require.NoError(t, err) require.Len(t, expTx.TxAttempts, 1) From 53e065706cc98788244067afd432e8d11bfef6b4 Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 21 Mar 2024 01:26:03 -0400 Subject: [PATCH 26/41] address comments --- .../evm/txmgr/evm_inmemory_store_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index 6513e7d3779..35c6aaab3a1 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -28,14 +28,14 @@ func TestInMemoryStore_DeleteInProgressAttempt(t *testing.T) { t.Run("successfully replace tx attempt", func(t *testing.T) { db := pgtest.NewSqlxDB(t) _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) - persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + persistentStore := cltest.NewTestTxStore(t, db) kst := cltest.NewKeyStore(t, db, dbcfg) _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) ethClient := evmtest.NewEthClientMockWithDefaultChain(t) lggr := logger.TestSugared(t) chainID := ethClient.ConfiguredChainID() - ctx := context.Background() + ctx := testutils.Context(t) inMemoryStore, err := commontxmgr.NewInMemoryStore[ *big.Int, @@ -52,10 +52,10 @@ func TestInMemoryStore_DeleteInProgressAttempt(t *testing.T) { require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) oldAttempt := inTx.TxAttempts[0] - err = inMemoryStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) + err = inMemoryStore.DeleteInProgressAttempt(ctx, oldAttempt) require.NoError(t, err) - expTx, err := persistentStore.FindTxWithAttempts(inTx.ID) + expTx, err := persistentStore.FindTxWithAttempts(ctx, inTx.ID) require.NoError(t, err) assert.Equal(t, 0, len(expTx.TxAttempts)) @@ -69,7 +69,7 @@ func TestInMemoryStore_DeleteInProgressAttempt(t *testing.T) { t.Run("error parity for in-memory vs persistent store", func(t *testing.T) { db := pgtest.NewSqlxDB(t) _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) - persistentStore := cltest.NewTestTxStore(t, db, dbcfg) + persistentStore := cltest.NewTestTxStore(t, db) kst := cltest.NewKeyStore(t, db, dbcfg) _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) @@ -95,8 +95,8 @@ func TestInMemoryStore_DeleteInProgressAttempt(t *testing.T) { oldAttempt := inTx.TxAttempts[0] t.Run("error when attempt is not in progress", func(t *testing.T) { oldAttempt.State = txmgrtypes.TxAttemptBroadcast - expErr := persistentStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) - actErr := inMemoryStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) + expErr := persistentStore.DeleteInProgressAttempt(ctx, oldAttempt) + actErr := inMemoryStore.DeleteInProgressAttempt(ctx, oldAttempt) assert.Equal(t, expErr, actErr) oldAttempt.State = txmgrtypes.TxAttemptInProgress }) @@ -104,8 +104,8 @@ func TestInMemoryStore_DeleteInProgressAttempt(t *testing.T) { t.Run("error when attempt has 0 id", func(t *testing.T) { originalID := oldAttempt.ID oldAttempt.ID = 0 - expErr := persistentStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) - actErr := inMemoryStore.DeleteInProgressAttempt(testutils.Context(t), oldAttempt) + expErr := persistentStore.DeleteInProgressAttempt(ctx, oldAttempt) + actErr := inMemoryStore.DeleteInProgressAttempt(ctx, oldAttempt) assert.Equal(t, expErr, actErr) oldAttempt.ID = originalID }) From 06938060844a07ee69d9c8102c3712f30e82b4a8 Mon Sep 17 00:00:00 2001 From: Jim W Date: Thu, 21 Mar 2024 21:22:05 -0400 Subject: [PATCH 27/41] TXM In-memory: step 3-04-UpdateTxForRebroadcast (#12231) * implement UpdateTxForRebroadcast * implement tests for UpdateTxForRebroadcast * address some comments * address comments --- common/txmgr/address_state.go | 19 +-- common/txmgr/inmemory_store.go | 19 ++- .../evm/txmgr/evm_inmemory_store_test.go | 117 +++++++++++++++++- 3 files changed, 144 insertions(+), 11 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 063086ac6e2..24ccf795eb0 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -381,24 +381,27 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveCo defer as.Unlock() if txAttempt.State != txmgrtypes.TxAttemptBroadcast { - return fmt.Errorf("move_confirmed_to_unconfirmed: attempt must be in broadcast state") + return fmt.Errorf("attempt must be in broadcast state") } tx, ok := as.confirmedTxs[txAttempt.TxID] if !ok || tx == nil { - return fmt.Errorf("move_confirmed_to_unconfirmed: no confirmed transaction with ID %d", txAttempt.TxID) + return fmt.Errorf("no confirmed transaction with ID %d", txAttempt.TxID) } if len(tx.TxAttempts) == 0 { - return fmt.Errorf("move_confirmed_to_unconfirmed: no attempts for transaction with ID %d", txAttempt.TxID) + return fmt.Errorf("no attempts for transaction with ID %d", txAttempt.TxID) } tx.State = TxUnconfirmed // Delete the receipt from the attempt - txAttempt.Receipts = nil - // Reset the broadcast information for the attempt - txAttempt.State = txmgrtypes.TxAttemptInProgress - txAttempt.BroadcastBeforeBlockNum = nil - tx.TxAttempts = append(tx.TxAttempts, txAttempt) + for i := 0; i < len(tx.TxAttempts); i++ { + if tx.TxAttempts[i].ID == txAttempt.ID { + tx.TxAttempts[i].Receipts = nil + tx.TxAttempts[i].State = txmgrtypes.TxAttemptInProgress + tx.TxAttempts[i].BroadcastBeforeBlockNum = nil + break + } + } as.unconfirmedTxs[tx.ID] = tx delete(as.confirmedTxs, tx.ID) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index bd4e9a2f3a6..e5c99516b25 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "math/big" + "sync" "time" "github.com/google/uuid" @@ -47,7 +48,8 @@ type inMemoryStore[ keyStore txmgrtypes.KeyStore[ADDR, CHAIN_ID, SEQ] persistentTxStore txmgrtypes.TxStore[ADDR, CHAIN_ID, TX_HASH, BLOCK_HASH, R, SEQ, FEE] - addressStates map[ADDR]*addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] + addressStatesLock sync.RWMutex + addressStates map[ADDR]*addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] } // NewInMemoryStore returns a new inMemoryStore @@ -331,7 +333,20 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveS return nil } func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) UpdateTxForRebroadcast(ctx context.Context, etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], etxAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { - return nil + ms.addressStatesLock.RLock() + defer ms.addressStatesLock.RUnlock() + as, ok := ms.addressStates[etx.FromAddress] + if !ok { + return nil + } + + // Persist to persistent storage + if err := ms.persistentTxStore.UpdateTxForRebroadcast(ctx, etx, etxAttempt); err != nil { + return err + } + + // Update in memory store + return as.moveConfirmedToUnconfirmed(etxAttempt) } func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) IsTxFinalized(ctx context.Context, blockHeight int64, txID int64, chainID CHAIN_ID) (bool, error) { return false, nil diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index a102ee1c996..b0860b5b0ed 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -1,14 +1,129 @@ package txmgr_test import ( + "math/big" "testing" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/smartcontractkit/chainlink-common/pkg/logger" + commontxmgr "github.com/smartcontractkit/chainlink/v2/common/txmgr" + txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" + + evmgas "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmtxmgr "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/internal/cltest" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/evmtest" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" ) +func TestInMemoryStore_UpdateTxForRebroadcast(t *testing.T) { + t.Parallel() + + t.Run("delete all receipts for transaction", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := testutils.Context(t) + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx := mustInsertConfirmedEthTxWithReceipt(t, persistentStore, fromAddress, 777, 1) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + txAttempt := inTx.TxAttempts[0] + err = inMemoryStore.UpdateTxForRebroadcast(ctx, inTx, txAttempt) + require.NoError(t, err) + + expTx, err := persistentStore.FindTxWithAttempts(ctx, inTx.ID) + require.NoError(t, err) + + fn := func(tx *evmtxmgr.Tx) bool { return true } + actTxs := inMemoryStore.XXXTestFindTxs(nil, fn, inTx.ID) + require.Equal(t, 1, len(actTxs)) + actTx := actTxs[0] + assertTxEqual(t, expTx, actTx) + assert.Equal(t, commontxmgr.TxUnconfirmed, actTx.State) + assert.Equal(t, txmgrtypes.TxAttemptInProgress, actTx.TxAttempts[0].State) + assert.Nil(t, actTx.TxAttempts[0].BroadcastBeforeBlockNum) + assert.Equal(t, 0, len(actTx.TxAttempts[0].Receipts)) + }) + + t.Run("error parity for in-memory vs persistent store", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := testutils.Context(t) + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx := mustInsertConfirmedEthTxWithReceipt(t, persistentStore, fromAddress, 777, 1) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + txAttempt := inTx.TxAttempts[0] + + t.Run("error when attempt is not in Broadcast state", func(t *testing.T) { + txAttempt.State = txmgrtypes.TxAttemptInProgress + expErr := persistentStore.UpdateTxForRebroadcast(ctx, inTx, txAttempt) + actErr := inMemoryStore.UpdateTxForRebroadcast(ctx, inTx, txAttempt) + assert.Error(t, expErr) + assert.Error(t, actErr) + txAttempt.State = txmgrtypes.TxAttemptBroadcast + }) + t.Run("error when transaction is not in confirmed state", func(t *testing.T) { + inTx.State = commontxmgr.TxUnconfirmed + expErr := persistentStore.UpdateTxForRebroadcast(ctx, inTx, txAttempt) + actErr := inMemoryStore.UpdateTxForRebroadcast(ctx, inTx, txAttempt) + assert.Error(t, expErr) + assert.Error(t, actErr) + inTx.State = commontxmgr.TxConfirmed + }) + t.Run("wrong fromAddress has no error", func(t *testing.T) { + inTx.FromAddress = common.Address{} + expErr := persistentStore.UpdateTxForRebroadcast(ctx, inTx, txAttempt) + actErr := inMemoryStore.UpdateTxForRebroadcast(ctx, inTx, txAttempt) + assert.Equal(t, expErr, actErr) + assert.Nil(t, actErr) + inTx.FromAddress = fromAddress + }) + + }) +} + // assertTxEqual asserts that two transactions are equal func assertTxEqual(t *testing.T, exp, act evmtxmgr.Tx) { assert.Equal(t, exp.ID, act.ID) @@ -33,7 +148,7 @@ func assertTxEqual(t *testing.T, exp, act evmtxmgr.Tx) { assert.Equal(t, exp.SignalCallback, act.SignalCallback) assert.Equal(t, exp.CallbackCompleted, act.CallbackCompleted) - require.Len(t, exp.TxAttempts, len(act.TxAttempts)) + require.Equal(t, len(exp.TxAttempts), len(act.TxAttempts)) for i := 0; i < len(exp.TxAttempts); i++ { assertTxAttemptEqual(t, exp.TxAttempts[i], act.TxAttempts[i]) } From 9d59e02c4557d8b12e7e4198c3301e203dd30308 Mon Sep 17 00:00:00 2001 From: Jim W Date: Thu, 21 Mar 2024 22:09:46 -0400 Subject: [PATCH 28/41] TXM In-memory: step 3-04-MarkAllConfirmedMissingReceipt (#12232) * implement MarkAllConfirmedMissingReceipt * implement tests for MarkAllConfirmedMissingReceipt * add panic if incorrect chainID * address comments * address comments --- common/txmgr/inmemory_store.go | 48 ++++++++++++++- .../evm/txmgr/evm_inmemory_store_test.go | 58 +++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index e5c99516b25..a0cbb22aae5 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -356,7 +356,53 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) FindT return nil, nil } func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MarkAllConfirmedMissingReceipt(ctx context.Context, chainID CHAIN_ID) error { - return nil + if ms.chainID.String() != chainID.String() { + panic("invalid chain ID") + } + + // Persist to persistent storage + if err := ms.persistentTxStore.MarkAllConfirmedMissingReceipt(ctx, chainID); err != nil { + return err + } + + // Update in memory store + var errs error + ms.addressStatesLock.RLock() + defer ms.addressStatesLock.RUnlock() + for _, as := range ms.addressStates { + // Get the max confirmed sequence + filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { return true } + states := []txmgrtypes.TxState{TxConfirmed} + txs := as.findTxs(states, filter) + var maxConfirmedSequence SEQ + for _, tx := range txs { + if tx.Sequence == nil { + continue + } + if (*tx.Sequence).Int64() > maxConfirmedSequence.Int64() { + maxConfirmedSequence = *tx.Sequence + } + } + + // Mark all unconfirmed txs with a sequence less than the max confirmed sequence as confirmed_missing_receipt + filter = func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { + if tx.Sequence == nil { + return false + } + + return (*tx.Sequence).Int64() < maxConfirmedSequence.Int64() + } + states = []txmgrtypes.TxState{TxUnconfirmed} + txs = as.findTxs(states, filter) + for _, tx := range txs { + if err := as.moveUnconfirmedToConfirmedMissingReceipt(tx.ID); err != nil { + err = fmt.Errorf("mark_all_confirmed_missing_receipt: address: %s: %w", as.fromAddress, err) + errs = errors.Join(errs, err) + } + } + } + + return errs } func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, finalityDepth uint32, chainID CHAIN_ID) error { return nil diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index b0860b5b0ed..c231e2cb6a9 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -120,7 +120,65 @@ func TestInMemoryStore_UpdateTxForRebroadcast(t *testing.T) { assert.Nil(t, actErr) inTx.FromAddress = fromAddress }) + }) +} + +func TestInMemoryStore_MarkAllConfirmedMissingReceipt(t *testing.T) { + t.Parallel() + + t.Run("successfully mark all confirmed missing receipt", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := testutils.Context(t) + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // create transaction 0 that is unconfirmed (block 7) + // Insert a transaction into persistent store + blocknum := int64(7) + inTx_0 := cltest.MustInsertUnconfirmedEthTx(t, persistentStore, 0, fromAddress) + inTxAttempt_0 := newBroadcastLegacyEthTxAttempt(t, inTx_0.ID, int64(1)) + inTxAttempt_0.BroadcastBeforeBlockNum = &blocknum + require.NoError(t, persistentStore.InsertTxAttempt(ctx, &inTxAttempt_0)) + assert.Equal(t, commontxmgr.TxUnconfirmed, inTx_0.State) + // Insert the transaction into the in-memory store + inTx_0.TxAttempts = []evmtxmgr.TxAttempt{inTxAttempt_0} + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx_0)) + + // create transaction 1 that is confirmed (block 77) + inTx_1 := mustInsertConfirmedEthTxBySaveFetchedReceipts(t, persistentStore, fromAddress, 1, 77, *chainID) + assert.Equal(t, commontxmgr.TxConfirmed, inTx_1.State) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx_1)) + + // mark transaction 0 as confirmed missing receipt + err = inMemoryStore.MarkAllConfirmedMissingReceipt(ctx, chainID) + require.NoError(t, err) + + expTx, err := persistentStore.FindTxWithAttempts(ctx, inTx_0.ID) + require.NoError(t, err) + + fn := func(tx *evmtxmgr.Tx) bool { return true } + actTxs := inMemoryStore.XXXTestFindTxs(nil, fn, inTx_0.ID) + require.Equal(t, 1, len(actTxs)) + actTx := actTxs[0] + assert.Equal(t, commontxmgr.TxConfirmedMissingReceipt, actTx.State) + assert.Equal(t, txmgrtypes.TxAttemptBroadcast, actTx.TxAttempts[0].State) + assertTxEqual(t, expTx, actTx) }) } From d22d62ca92ab9cf1e7f105d4f0a7a6892277f96c Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 21 Mar 2024 23:13:48 -0400 Subject: [PATCH 29/41] implement tests for ReapTxHistory --- common/txmgr/address_state.go | 85 ++++++++--- common/txmgr/inmemory_store.go | 48 ++---- .../evm/txmgr/evm_inmemory_store_test.go | 144 ++++++++++++++++++ 3 files changed, 225 insertions(+), 52 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 24ccf795eb0..3a960db8f4d 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -247,12 +247,51 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) findTx func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) pruneUnstartedTxQueue(ids []int64) { } +// reapConfirmedTxs removes confirmed transactions that are older than the given time threshold. +// It also removes confirmed transactions that are older than the given block number threshold. +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) reapConfirmedTxs(minBlockNumberToKeep int64, timeThreshold time.Time) { + as.Lock() + defer as.Unlock() + + for _, tx := range as.confirmedTxs { + if len(tx.TxAttempts) == 0 { + continue + } + if tx.CreatedAt.After(timeThreshold) { + continue + } + + for i := 0; i < len(tx.TxAttempts); i++ { + if len(tx.TxAttempts[i].Receipts) == 0 { + continue + } + if tx.TxAttempts[i].Receipts[0].GetBlockNumber() == nil || tx.TxAttempts[i].Receipts[0].GetBlockNumber().Int64() >= minBlockNumberToKeep { + continue + } + as._deleteTx(tx.ID) + } + } +} + +// reapFatalErroredTxs removes fatal errored transactions that are older than the given time threshold. +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) reapFatalErroredTxs(timeThreshold time.Time) { + as.Lock() + defer as.Unlock() + + for _, tx := range as.fatalErroredTxs { + if tx.CreatedAt.After(timeThreshold) { + continue + } + as._deleteTx(tx.ID) + } +} + // deleteTxs removes the transactions with the given IDs from the address state. -func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) deleteTxs(txs ...txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) deleteTxs(txIDs ...int64) { as.Lock() defer as.Unlock() - as._deleteTxs(txs...) + as._deleteTxs(txIDs...) } // peekNextUnstartedTx returns the next unstarted transaction in the queue without removing it from the unstarted queue. @@ -458,22 +497,34 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) _findT return txs } -func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) _deleteTxs(txs ...txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { - for _, tx := range txs { - if tx.IdempotencyKey != nil { - delete(as.idempotencyKeyToTx, *tx.IdempotencyKey) - } - txID := tx.ID - if as.inprogressTx != nil && as.inprogressTx.ID == txID { - as.inprogressTx = nil - } - delete(as.allTxs, txID) - delete(as.unconfirmedTxs, txID) - delete(as.confirmedMissingReceiptTxs, txID) - delete(as.confirmedTxs, txID) - delete(as.fatalErroredTxs, txID) - as.unstartedTxs.RemoveTxByID(txID) +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) _deleteTxs(txIDs ...int64) { + for _, txID := range txIDs { + as._deleteTx(txID) + } +} + +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) _deleteTx(txID int64) { + tx, ok := as.allTxs[txID] + if !ok { + return + } + + for i := 0; i < len(tx.TxAttempts); i++ { + txAttemptHash := tx.TxAttempts[i].Hash + delete(as.attemptHashToTxAttempt, txAttemptHash) + } + if tx.IdempotencyKey != nil { + delete(as.idempotencyKeyToTx, *tx.IdempotencyKey) + } + if as.inprogressTx != nil && as.inprogressTx.ID == txID { + as.inprogressTx = nil } + as.unstartedTxs.RemoveTxByID(txID) + delete(as.unconfirmedTxs, txID) + delete(as.confirmedMissingReceiptTxs, txID) + delete(as.confirmedTxs, txID) + delete(as.fatalErroredTxs, txID) + delete(as.allTxs, txID) } func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) _moveTxToFatalError( diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 15cbaa23755..5d42071de23 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -81,11 +81,21 @@ func NewInMemoryStore[ ms.maxUnstarted = 10000 } + addressesToTxs := map[ADDR][]txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + // populate all enabled addresses + enabledAddresses, err := keyStore.EnabledAddressesForChain(ctx, chainID) + if err != nil { + return nil, fmt.Errorf("new_in_memory_store: %w", err) + } + for _, addr := range enabledAddresses { + addressesToTxs[addr] = []txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + } + txs, err := persistentTxStore.GetAllTransactions(ctx, chainID) if err != nil { return nil, fmt.Errorf("address_state: initialization: %w", err) } - addressesToTxs := map[ADDR][]txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + for _, tx := range txs { at, exists := addressesToTxs[tx.FromAddress] if !exists { @@ -276,43 +286,11 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) ReapT } // Update in memory store - states := []txmgrtypes.TxState{TxConfirmed} - filterFn := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { - if tx.TxAttempts == nil || len(tx.TxAttempts) == 0 { - return false - } - for _, attempt := range tx.TxAttempts { - if attempt.Receipts == nil || len(attempt.Receipts) == 0 { - continue - } - if attempt.Receipts[0].GetBlockNumber() == nil { - continue - } - if attempt.Receipts[0].GetBlockNumber().Int64() >= minBlockNumberToKeep { - continue - } - if tx.CreatedAt.After(timeThreshold) { - continue - } - return tx.State == TxConfirmed - } - return false - } - - ms.addressStatesLock.RLock() - defer ms.addressStatesLock.RUnlock() - for _, as := range ms.addressStates { - as.deleteTxs(as.findTxs(states, filterFn)...) - } - - filterFn = func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { - return tx.State == TxFatalError && tx.CreatedAt.Before(timeThreshold) - } - states = []txmgrtypes.TxState{TxFatalError} ms.addressStatesLock.RLock() defer ms.addressStatesLock.RUnlock() for _, as := range ms.addressStates { - as.deleteTxs(as.findTxs(states, filterFn)...) + as.reapConfirmedTxs(minBlockNumberToKeep, timeThreshold) + as.reapFatalErroredTxs(timeThreshold) } return nil diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index b0860b5b0ed..43b43b4a41a 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -3,10 +3,12 @@ package txmgr_test import ( "math/big" "testing" + "time" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/guregu/null.v4" "github.com/smartcontractkit/chainlink-common/pkg/logger" commontxmgr "github.com/smartcontractkit/chainlink/v2/common/txmgr" @@ -15,12 +17,154 @@ import ( evmgas "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmtxmgr "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" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/evmtest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" ) +func TestInMemoryStore_ReapTxHistory(t *testing.T) { + t.Parallel() + + t.Run("reap all confirmed txs", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := testutils.Context(t) + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx_0 := cltest.MustInsertConfirmedEthTxWithLegacyAttempt(t, persistentStore, 7, 1, fromAddress) + r_0 := mustInsertEthReceipt(t, persistentStore, 1, utils.NewHash(), inTx_0.TxAttempts[0].Hash) + inTx_0.TxAttempts[0].Receipts = append(inTx_0.TxAttempts[0].Receipts, evmtxmgr.DbReceiptToEvmReceipt(&r_0)) + inTx_1 := cltest.MustInsertConfirmedEthTxWithLegacyAttempt(t, persistentStore, 8, 2, fromAddress) + r_1 := mustInsertEthReceipt(t, persistentStore, 2, utils.NewHash(), inTx_1.TxAttempts[0].Hash) + inTx_1.TxAttempts[0].Receipts = append(inTx_1.TxAttempts[0].Receipts, evmtxmgr.DbReceiptToEvmReceipt(&r_1)) + inTx_2 := cltest.MustInsertConfirmedEthTxWithLegacyAttempt(t, persistentStore, 9, 3, fromAddress) + r_2 := mustInsertEthReceipt(t, persistentStore, 3, utils.NewHash(), inTx_2.TxAttempts[0].Hash) + inTx_2.TxAttempts[0].Receipts = append(inTx_2.TxAttempts[0].Receipts, evmtxmgr.DbReceiptToEvmReceipt(&r_2)) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx_0)) + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx_1)) + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx_2)) + + minBlockNumberToKeep := int64(3) + timeThreshold := inTx_2.CreatedAt + expErr := persistentStore.ReapTxHistory(ctx, minBlockNumberToKeep, timeThreshold, chainID) + actErr := inMemoryStore.ReapTxHistory(ctx, minBlockNumberToKeep, timeThreshold, chainID) + require.NoError(t, expErr) + require.NoError(t, actErr) + + fn := func(tx *evmtxmgr.Tx) bool { return true } + // Check that the transactions were reaped in persistent store + expTx_0, err := persistentStore.FindTxWithAttempts(ctx, inTx_0.ID) + require.Error(t, err) + require.Equal(t, int64(0), expTx_0.ID) + expTx_1, err := persistentStore.FindTxWithAttempts(ctx, inTx_1.ID) + require.Error(t, err) + require.Equal(t, int64(0), expTx_1.ID) + // Check that the transactions were reaped in in-memory store + actTxs_0 := inMemoryStore.XXXTestFindTxs(nil, fn, inTx_0.ID) + require.Equal(t, 0, len(actTxs_0)) + actTxs_1 := inMemoryStore.XXXTestFindTxs(nil, fn, inTx_1.ID) + require.Equal(t, 0, len(actTxs_1)) + + // Check that the transaction was not reaped + expTx_2, err := persistentStore.FindTxWithAttempts(ctx, inTx_2.ID) + require.NoError(t, err) + require.Equal(t, inTx_2.ID, expTx_2.ID) + actTxs_2 := inMemoryStore.XXXTestFindTxs(nil, fn, inTx_2.ID) + require.Equal(t, 1, len(actTxs_2)) + assertTxEqual(t, expTx_2, actTxs_2[0]) + }) + t.Run("reap all fatal error txs", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := testutils.Context(t) + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx_0 := cltest.NewEthTx(fromAddress) + inTx_0.Error = null.StringFrom("something exploded") + inTx_0.State = commontxmgr.TxFatalError + inTx_0.CreatedAt = time.Unix(1000, 0) + require.NoError(t, persistentStore.InsertTx(ctx, &inTx_0)) + inTx_1 := cltest.NewEthTx(fromAddress) + inTx_1.Error = null.StringFrom("something exploded") + inTx_1.State = commontxmgr.TxFatalError + inTx_1.CreatedAt = time.Unix(2000, 0) + require.NoError(t, persistentStore.InsertTx(ctx, &inTx_1)) + inTx_2 := cltest.NewEthTx(fromAddress) + inTx_2.Error = null.StringFrom("something exploded") + inTx_2.State = commontxmgr.TxFatalError + inTx_2.CreatedAt = time.Unix(3000, 0) + require.NoError(t, persistentStore.InsertTx(ctx, &inTx_2)) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx_0)) + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx_1)) + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx_2)) + + minBlockNumberToKeep := int64(3) + timeThreshold := time.Unix(2500, 0) // Only reap txs created before this time + expErr := persistentStore.ReapTxHistory(ctx, minBlockNumberToKeep, timeThreshold, chainID) + actErr := inMemoryStore.ReapTxHistory(ctx, minBlockNumberToKeep, timeThreshold, chainID) + require.NoError(t, expErr) + require.NoError(t, actErr) + + fn := func(tx *evmtxmgr.Tx) bool { return true } + // Check that the transactions were reaped in persistent store + expTx_0, err := persistentStore.FindTxWithAttempts(ctx, inTx_0.ID) + require.Error(t, err) + require.Equal(t, int64(0), expTx_0.ID) + expTx_1, err := persistentStore.FindTxWithAttempts(ctx, inTx_1.ID) + require.Error(t, err) + require.Equal(t, int64(0), expTx_1.ID) + // Check that the transactions were reaped in in-memory store + actTxs_0 := inMemoryStore.XXXTestFindTxs(nil, fn, inTx_0.ID) + require.Equal(t, 0, len(actTxs_0)) + actTxs_1 := inMemoryStore.XXXTestFindTxs(nil, fn, inTx_1.ID) + require.Equal(t, 0, len(actTxs_1)) + + // Check that the transaction was not reaped + expTx_2, err := persistentStore.FindTxWithAttempts(ctx, inTx_2.ID) + require.NoError(t, err) + require.Equal(t, inTx_2.ID, expTx_2.ID) + actTxs_2 := inMemoryStore.XXXTestFindTxs(nil, fn, inTx_2.ID) + require.Equal(t, 1, len(actTxs_2)) + assertTxEqual(t, expTx_2, actTxs_2[0]) + }) +} + func TestInMemoryStore_UpdateTxForRebroadcast(t *testing.T) { t.Parallel() From 6a0b5ad1f64331ec1709af02e9cc84675cefb474 Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 21 Mar 2024 23:14:16 -0400 Subject: [PATCH 30/41] panic if wrong chain ID --- common/txmgr/inmemory_store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 5d42071de23..b592fb48d04 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -277,7 +277,7 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Prune func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) ReapTxHistory(ctx context.Context, minBlockNumberToKeep int64, timeThreshold time.Time, chainID CHAIN_ID) error { if ms.chainID.String() != chainID.String() { - return fmt.Errorf("reap_tx_history: %w", ErrInvalidChainID) + panic("invalid chain ID") } // Persist to persistent storage From 3381426389457dc23ab2a33e93ac1bbf85474879 Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 21 Mar 2024 23:27:13 -0400 Subject: [PATCH 31/41] remove merge artifacts --- .../evm/txmgr/evm_inmemory_store_test.go | 67 ++++++++++++------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index 606a33e9935..ef259402b84 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -22,19 +22,10 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" ) -<<<<<<< HEAD func TestInMemoryStore_DeleteInProgressAttempt(t *testing.T) { t.Parallel() t.Run("successfully replace tx attempt", func(t *testing.T) { -======= -func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { - t.Parallel() - blockNum := int64(10) - finalityDepth := uint32(2) - - t.Run("successfully mark errored transaction", func(t *testing.T) { ->>>>>>> jtw/step-3-04 db := pgtest.NewSqlxDB(t) _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) persistentStore := cltest.NewTestTxStore(t, db) @@ -56,34 +47,22 @@ func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { require.NoError(t, err) // Insert a transaction into persistent store -<<<<<<< HEAD inTx := mustInsertInProgressEthTxWithAttempt(t, persistentStore, 1, fromAddress) // Insert the transaction into the in-memory store require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) oldAttempt := inTx.TxAttempts[0] err = inMemoryStore.DeleteInProgressAttempt(ctx, oldAttempt) -======= - inTx := mustInsertConfirmedMissingReceiptEthTxWithLegacyAttempt(t, persistentStore, 1, 7, time.Now(), fromAddress) - // Insert the transaction into the in-memory store - require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) - - err = inMemoryStore.MarkOldTxesMissingReceiptAsErrored(ctx, blockNum, finalityDepth, chainID) ->>>>>>> jtw/step-3-04 require.NoError(t, err) expTx, err := persistentStore.FindTxWithAttempts(ctx, inTx.ID) require.NoError(t, err) -<<<<<<< HEAD assert.Equal(t, 0, len(expTx.TxAttempts)) -======= ->>>>>>> jtw/step-3-04 fn := func(tx *evmtxmgr.Tx) bool { return true } actTxs := inMemoryStore.XXXTestFindTxs(nil, fn, inTx.ID) require.Equal(t, 1, len(actTxs)) actTx := actTxs[0] -<<<<<<< HEAD assertTxEqual(t, expTx, actTx) }) @@ -130,12 +109,54 @@ func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { assert.Equal(t, expErr, actErr) oldAttempt.ID = originalID }) -======= + }) +} + +func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { + t.Parallel() + blockNum := int64(10) + finalityDepth := uint32(2) + + t.Run("successfully mark errored transaction", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := testutils.Context(t) + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx := mustInsertConfirmedMissingReceiptEthTxWithLegacyAttempt(t, persistentStore, 1, 7, time.Now(), fromAddress) + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + err = inMemoryStore.MarkOldTxesMissingReceiptAsErrored(ctx, blockNum, finalityDepth, chainID) + require.NoError(t, err) + + expTx, err := persistentStore.FindTxWithAttempts(ctx, inTx.ID) + require.NoError(t, err) + + fn := func(tx *evmtxmgr.Tx) bool { return true } + actTxs := inMemoryStore.XXXTestFindTxs(nil, fn, inTx.ID) + require.Equal(t, 1, len(actTxs)) + actTx := actTxs[0] assertTxEqual(t, expTx, actTx) assert.Equal(t, txmgrtypes.TxAttemptBroadcast, actTx.TxAttempts[0].State) assert.Equal(t, commontxmgr.TxFatalError, actTx.State) ->>>>>>> jtw/step-3-04 }) } From 0f1c0059b25c3f5dae6816552569c0de7df6d056 Mon Sep 17 00:00:00 2001 From: James Walker Date: Fri, 22 Mar 2024 14:12:21 -0400 Subject: [PATCH 32/41] fix in memory address stuff --- common/txmgr/inmemory_store.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index ea3bf9ada7a..b25c47a13c2 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -81,11 +81,21 @@ func NewInMemoryStore[ ms.maxUnstarted = 10000 } + addressesToTxs := map[ADDR][]txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + // populate all enabled addresses + enabledAddresses, err := keyStore.EnabledAddressesForChain(ctx, chainID) + if err != nil { + return nil, fmt.Errorf("new_in_memory_store: %w", err) + } + for _, addr := range enabledAddresses { + addressesToTxs[addr] = []txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + } + txs, err := persistentTxStore.GetAllTransactions(ctx, chainID) if err != nil { return nil, fmt.Errorf("address_state: initialization: %w", err) } - addressesToTxs := map[ADDR][]txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} + for _, tx := range txs { at, exists := addressesToTxs[tx.FromAddress] if !exists { From 26d432a77da1ecd6a9fd4803c3d6b99cd32c6614 Mon Sep 17 00:00:00 2001 From: James Walker Date: Fri, 22 Mar 2024 14:14:57 -0400 Subject: [PATCH 33/41] fix merge conflict --- .../evm/txmgr/evm_inmemory_store_test.go | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index 42a777f82c4..734ac5588ac 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -24,19 +24,10 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" ) -<<<<<<< HEAD func TestInMemoryStore_ReapTxHistory(t *testing.T) { t.Parallel() t.Run("reap all confirmed txs", func(t *testing.T) { -======= -func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { - t.Parallel() - blockNum := int64(10) - finalityDepth := uint32(2) - - t.Run("successfully mark errored transaction", func(t *testing.T) { ->>>>>>> jtw/step-3-04 db := pgtest.NewSqlxDB(t) _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) persistentStore := cltest.NewTestTxStore(t, db) @@ -58,7 +49,6 @@ func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { require.NoError(t, err) // Insert a transaction into persistent store -<<<<<<< HEAD inTx_0 := cltest.MustInsertConfirmedEthTxWithLegacyAttempt(t, persistentStore, 7, 1, fromAddress) r_0 := mustInsertEthReceipt(t, persistentStore, 1, utils.NewHash(), inTx_0.TxAttempts[0].Hash) inTx_0.TxAttempts[0].Receipts = append(inTx_0.TxAttempts[0].Receipts, evmtxmgr.DbReceiptToEvmReceipt(&r_0)) @@ -172,7 +162,36 @@ func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { actTxs_2 := inMemoryStore.XXXTestFindTxs(nil, fn, inTx_2.ID) require.Equal(t, 1, len(actTxs_2)) assertTxEqual(t, expTx_2, actTxs_2[0]) -======= + }) +} + +func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { + t.Parallel() + blockNum := int64(10) + finalityDepth := uint32(2) + + t.Run("successfully mark errored transaction", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := testutils.Context(t) + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store inTx := mustInsertConfirmedMissingReceiptEthTxWithLegacyAttempt(t, persistentStore, 1, 7, time.Now(), fromAddress) // Insert the transaction into the in-memory store require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) @@ -191,7 +210,6 @@ func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { assertTxEqual(t, expTx, actTx) assert.Equal(t, txmgrtypes.TxAttemptBroadcast, actTx.TxAttempts[0].State) assert.Equal(t, commontxmgr.TxFatalError, actTx.State) ->>>>>>> jtw/step-3-04 }) } From b6341838743b8aed5aa7715c6714abde9b8d5ebc Mon Sep 17 00:00:00 2001 From: James Walker Date: Mon, 25 Mar 2024 21:40:58 -0400 Subject: [PATCH 34/41] implement tests for SaveConfirmedMissingReceiptAttempt --- common/txmgr/address_state.go | 34 ++++++++----- common/txmgr/inmemory_store.go | 22 +++++++-- .../evm/txmgr/evm_inmemory_store_test.go | 48 +++++++++++++++++++ 3 files changed, 88 insertions(+), 16 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 1c0d7cbfb66..bb9bea2ea2f 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -355,28 +355,40 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveUn return nil } -// moveInProgressToConfirmedMissingReceipt moves the in-progress transaction to the confirmed missing receipt state. -// If there is no in-progress transaction, an error is returned. -func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveInProgressToConfirmedMissingReceipt(txAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { +// moveInProgressToConfirmedMissingReceipt moves transaction to the confirmed missing receipt state. +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTxToConfirmedMissingReceipt(txAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { as.Lock() defer as.Unlock() - tx := as.inprogressTx - if tx == nil { - return fmt.Errorf("move_in_progress_to_confirmed_missing_receipt: no transaction in progress") + tx, ok := as.allTxs[txAttempt.TxID] + if !ok { + return fmt.Errorf("move_in_progress_to_confirmed_missing_receipt: no transaction with ID %d", txAttempt.TxID) } if len(tx.TxAttempts) == 0 { return fmt.Errorf("move_in_progress_to_confirmed_missing_receipt: no attempts for transaction with ID %d", tx.ID) } - if tx.BroadcastAt.Before(broadcastAt) { + if tx.BroadcastAt != nil && tx.BroadcastAt.Before(broadcastAt) { tx.BroadcastAt = &broadcastAt } - tx.State = TxConfirmedMissingReceipt - txAttempt.State = txmgrtypes.TxAttemptBroadcast - tx.TxAttempts = append(tx.TxAttempts, txAttempt) + for i := 0; i < len(tx.TxAttempts); i++ { + if tx.TxAttempts[i].ID == txAttempt.ID { + attempt := tx.TxAttempts[i] + attempt.State = txmgrtypes.TxAttemptBroadcast + } + } + + switch tx.State { + case TxInProgress: + as.inprogressTx = nil + case TxUnconfirmed: + delete(as.unconfirmedTxs, tx.ID) + default: + panic(fmt.Sprintf("unknown transaction state: %q", tx.State)) + } + + tx.State = TxConfirmedMissingReceipt as.confirmedMissingReceiptTxs[tx.ID] = tx - as.inprogressTx = nil return nil } diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 47819231b44..b1e276336e1 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -333,21 +333,33 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Prelo func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveConfirmedMissingReceiptAttempt(ctx context.Context, timeout time.Duration, attempt *txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { ms.addressStatesLock.RLock() defer ms.addressStatesLock.RUnlock() - as, ok := ms.addressStates[attempt.Tx.FromAddress] - if !ok { - return fmt.Errorf("save_confirmed_missing_receipt_attempt: %w", ErrAddressNotFound) - } + if attempt.State != txmgrtypes.TxAttemptInProgress { return fmt.Errorf("expected state to be in_progress") } + var tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] + var as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] + filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { return true } + for _, vas := range ms.addressStates { + txs := vas.findTxs(nil, filter, attempt.TxID) + if len(txs) != 0 { + tx = &txs[0] + as = vas + break + } + } + if tx == nil { + return fmt.Errorf("save_confirmed_missing_receipt_attempt: %w", ErrTxnNotFound) + } + // Persist to persistent storage if err := ms.persistentTxStore.SaveConfirmedMissingReceiptAttempt(ctx, timeout, attempt, broadcastAt); err != nil { return err } // Update in memory store - return as.moveInProgressToConfirmedMissingReceipt(*attempt, broadcastAt) + return as.moveTxToConfirmedMissingReceipt(*attempt, broadcastAt) } func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveInProgressAttempt(ctx context.Context, attempt *txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { return nil diff --git a/core/chains/evm/txmgr/evm_inmemory_store_test.go b/core/chains/evm/txmgr/evm_inmemory_store_test.go index 217da333005..82152100acb 100644 --- a/core/chains/evm/txmgr/evm_inmemory_store_test.go +++ b/core/chains/evm/txmgr/evm_inmemory_store_test.go @@ -22,6 +22,54 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" ) +func TestInMemoryStore_SaveConfirmedMissingReceiptAttempt(t *testing.T) { + t.Parallel() + + t.Run("updates attempt to 'broadcast' and transaction to 'confirm_missing_receipt'", func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + _, dbcfg, evmcfg := evmtxmgr.MakeTestConfigs(t) + persistentStore := cltest.NewTestTxStore(t, db) + kst := cltest.NewKeyStore(t, db, dbcfg) + _, fromAddress := cltest.MustInsertRandomKey(t, kst.Eth()) + + ethClient := evmtest.NewEthClientMockWithDefaultChain(t) + lggr := logger.TestSugared(t) + chainID := ethClient.ConfiguredChainID() + ctx := testutils.Context(t) + + inMemoryStore, err := commontxmgr.NewInMemoryStore[ + *big.Int, + common.Address, common.Hash, common.Hash, + *evmtypes.Receipt, + evmtypes.Nonce, + evmgas.EvmFee, + ](ctx, lggr, chainID, kst.Eth(), persistentStore, evmcfg.Transactions()) + require.NoError(t, err) + + // Insert a transaction into persistent store + inTx := mustInsertUnconfirmedEthTxWithAttemptState(t, persistentStore, 1, fromAddress, txmgrtypes.TxAttemptInProgress) + now := time.Now() + // Insert the transaction into the in-memory store + require.NoError(t, inMemoryStore.XXXTestInsertTx(fromAddress, &inTx)) + + defaultDuration := 5 * time.Second + err = inMemoryStore.SaveConfirmedMissingReceiptAttempt(ctx, defaultDuration, &inTx.TxAttempts[0], now) + require.NoError(t, err) + + expTx, err := persistentStore.FindTxWithAttempts(ctx, inTx.ID) + require.NoError(t, err) + + fn := func(tx *evmtxmgr.Tx) bool { return true } + actTxs := inMemoryStore.XXXTestFindTxs(nil, fn, inTx.ID) + require.Equal(t, 1, len(actTxs)) + actTx := actTxs[0] + + assertTxEqual(t, expTx, actTx) + assert.Equal(t, txmgrtypes.TxAttemptBroadcast, actTx.TxAttempts[0].State) + assert.Equal(t, commontxmgr.TxConfirmedMissingReceipt, actTx.State) + }) +} + func TestInMemoryStore_MarkOldTxesMissingReceiptAsErrored(t *testing.T) { t.Parallel() blockNum := int64(10) From f2c2e0b0a6d95f3efdbfd10553e2537ec23931b6 Mon Sep 17 00:00:00 2001 From: James Walker Date: Mon, 25 Mar 2024 22:01:27 -0400 Subject: [PATCH 35/41] update --- common/txmgr/address_state.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index bb9bea2ea2f..9ff5967b0a1 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -377,18 +377,10 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTx attempt.State = txmgrtypes.TxAttemptBroadcast } } - - switch tx.State { - case TxInProgress: - as.inprogressTx = nil - case TxUnconfirmed: - delete(as.unconfirmedTxs, tx.ID) - default: - panic(fmt.Sprintf("unknown transaction state: %q", tx.State)) - } - tx.State = TxConfirmedMissingReceipt + as.confirmedMissingReceiptTxs[tx.ID] = tx + delete(as.unconfirmedTxs, tx.ID) return nil } From 6deab0d829bae0e2b1d26e5fa6b21dd0dad6ac27 Mon Sep 17 00:00:00 2001 From: Jim W Date: Mon, 1 Apr 2024 16:25:40 -0400 Subject: [PATCH 36/41] Update common/txmgr/address_state.go Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com> --- common/txmgr/address_state.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index a0809de7cb6..995e344271b 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -247,8 +247,7 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) findTx func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) pruneUnstartedTxQueue(ids []int64) { } -// reapConfirmedTxs removes confirmed transactions that are older than the given time threshold. -// It also removes confirmed transactions that are older than the given block number threshold. +// reapConfirmedTxs removes confirmed transactions that are older than the given time threshold and have receipts older than the given block number threshold. func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) reapConfirmedTxs(minBlockNumberToKeep int64, timeThreshold time.Time) { as.Lock() defer as.Unlock() From 8ae9d727c09b6878fbb675ea125ba883fc3c1256 Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 3 Apr 2024 11:09:55 -0400 Subject: [PATCH 37/41] address coments --- common/txmgr/address_state.go | 31 ++++++++++++++++--------------- common/txmgr/inmemory_store.go | 4 ++-- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 4e0e7aa4a74..b9fc2cbd70e 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -293,27 +293,28 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) delete as._deleteTxs(txIDs...) } -// deleteTxAttempts removes the attempts with the given IDs from the address state. +// deleteTxAttempt removes the attempt with a given ID from the transaction with the given ID. // It removes the attempts from the hash lookup map and from the transaction. // If an attempt is not found in the hash lookup map, it is ignored. // If a transaction is not found in the allTxs map, it is ignored. -func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) deleteTxAttempts(txAttempts ...txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) { +// No error is returned if the transaction or attempt is not found. +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) deleteTxAttempt(txID, txAttemptID int64) { as.Lock() defer as.Unlock() - for _, txAttempt := range txAttempts { - // remove the attempt from the hash lookup map - delete(as.attemptHashToTxAttempt, txAttempt.Hash) - // remove the attempt from the transaction - if tx := as.allTxs[txAttempt.TxID]; tx != nil { - var removeIndex int - for i := 0; i < len(tx.TxAttempts); i++ { - if tx.TxAttempts[i].ID == txAttempt.ID { - removeIndex = i - break - } - } - tx.TxAttempts = append(tx.TxAttempts[:removeIndex], tx.TxAttempts[removeIndex+1:]...) + tx, ok := as.allTxs[txID] + if !ok || tx == nil { + return + } + + for i := 0; i < len(tx.TxAttempts); i++ { + txAttempt := tx.TxAttempts[i] + if txAttempt.ID == txAttemptID { + // remove the attempt from the hash lookup map + delete(as.attemptHashToTxAttempt, txAttempt.Hash) + // remove the attempt from the transaction + tx.TxAttempts = append(tx.TxAttempts[:i], tx.TxAttempts[i+1:]...) + break } } } diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 635bf1f7d79..b2c8e83a431 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -304,7 +304,7 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Delet return fmt.Errorf("DeleteInProgressAttempt: expected attempt state to be in_progress") } if attempt.ID == 0 { - return fmt.Errorf("delete_in_progress_attempt: expected attempt to have an ID") + return fmt.Errorf("DeleteInProgressAttempt: expected attempt to have an id") } // Check if fromaddress enabled @@ -333,7 +333,7 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Delet } // Update in memory store - as.deleteTxAttempts(attempt) + as.deleteTxAttempt(attempt.TxID, attempt.ID) return nil } From 3a939927338f0e765a1e40486831603353e0d905 Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 3 Apr 2024 11:18:32 -0400 Subject: [PATCH 38/41] cleanup --- common/txmgr/address_state.go | 9 +++++++++ common/txmgr/inmemory_store.go | 12 +++--------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index b9fc2cbd70e..0b82b7030c8 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -198,6 +198,15 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) findTx return nil } +// hasTx returns true if the transaction with the given ID is in the address state. +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) hasTx(txID int64) bool { + as.RLock() + defer as.RUnlock() + + _, ok := as.allTxs[txID] + return ok +} + // findTxs returns all transactions that match the given filters. // If txIDs are provided, only the transactions with those IDs are considered. // If no txIDs are provided, all transactions are considered. diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index b2c8e83a431..773c91bbea1 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -310,21 +310,15 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Delet // Check if fromaddress enabled ms.addressStatesLock.RLock() defer ms.addressStatesLock.RUnlock() - filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { - return true - } var as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] - var tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] for _, vas := range ms.addressStates { - txs := vas.findTxs(nil, filter, attempt.TxID) - if len(txs) == 1 { - tx = &txs[0] + if vas.hasTx(attempt.TxID) { as = vas break } } - if tx == nil { - return fmt.Errorf("delete_in_progress_attempt: %w", ErrTxnNotFound) + if as == nil { + return fmt.Errorf("delete_in_progress_attempt: %w: %q", ErrTxnNotFound, attempt.TxID) } // Persist to persistent storage From 9978e9ba4f3530d2b470acfe92fc41efca9469fb Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 3 Apr 2024 14:48:31 -0400 Subject: [PATCH 39/41] address comments --- common/txmgr/address_state.go | 30 ++++++++++++------------------ common/txmgr/inmemory_store.go | 2 +- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index 1d73e8a58f1..17d7dc0fe28 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -1,7 +1,6 @@ package txmgr import ( - "errors" "fmt" "sync" "time" @@ -330,29 +329,24 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) delete } // addTxAttempt adds the given attempt to the transaction which matches its TxID. -func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) addTxAttempts(txAttempts ...txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) addTxAttempt(txAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { as.Lock() defer as.Unlock() - var errs error - for i := 0; i < len(txAttempts); i++ { - txAttempt := txAttempts[i] - tx := as.allTxs[txAttempt.TxID] - if tx == nil { - errs = errors.Join(errs, fmt.Errorf("no transaction with ID %d", txAttempt.TxID)) - continue - } + tx, ok := as.allTxs[txAttempt.TxID] + if !ok || tx == nil { + return fmt.Errorf("no transaction with ID %d", txAttempt.TxID) + } - // add the attempt to the transaction - if tx.TxAttempts == nil { - tx.TxAttempts = []txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} - } - tx.TxAttempts = append(tx.TxAttempts, txAttempt) - // add the attempt to the hash lookup map - as.attemptHashToTxAttempt[txAttempt.Hash] = &txAttempt + // add the attempt to the transaction + if tx.TxAttempts == nil { + tx.TxAttempts = []txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{} } + tx.TxAttempts = append(tx.TxAttempts, txAttempt) + // add the attempt to the hash lookup map + as.attemptHashToTxAttempt[txAttempt.Hash] = &txAttempt - return errs + return nil } // peekNextUnstartedTx returns the next unstarted transaction in the queue without removing it from the unstarted queue. diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 2b13ba058ad..7bcac570888 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -219,7 +219,7 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveR // delete the old attempt as.deleteTxAttempt(oldAttempt.TxID, oldAttempt.ID) // add the new attempt - if err := as.addTxAttempts(*replacementAttempt); err != nil { + if err := as.addTxAttempt(*replacementAttempt); err != nil { return fmt.Errorf("save_replacement_in_progress_attempt: failed to add a replacement transaction attempt: %w", err) } From 7de69a4f44c23dbea1b55aed08d06709b6f6ddcc Mon Sep 17 00:00:00 2001 From: James Walker Date: Wed, 3 Apr 2024 16:12:32 -0400 Subject: [PATCH 40/41] clean up --- common/txmgr/address_state.go | 46 ++++++++++++++++++++-------------- common/txmgr/inmemory_store.go | 12 +++------ 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/common/txmgr/address_state.go b/common/txmgr/address_state.go index ff76bdc6daf..c35ab05b6de 100644 --- a/common/txmgr/address_state.go +++ b/common/txmgr/address_state.go @@ -420,40 +420,40 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveUn if !ok || tx == nil { return fmt.Errorf("move_unconfirmed_to_confirmed_missing_receipt: no unconfirmed transaction with ID %d", txID) } - tx.State = TxConfirmedMissingReceipt - - as.confirmedMissingReceiptTxs[tx.ID] = tx - delete(as.unconfirmedTxs, tx.ID) + as._moveUnconfirmedToConfirmedMissingReceipt(tx) return nil } -// moveInProgressToConfirmedMissingReceipt moves transaction to the confirmed missing receipt state. -func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTxToConfirmedMissingReceipt(txAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error { +// moveTxAttemptToBroadcast moves the transaction attempt to the broadcast state. +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) moveTxAttemptToBroadcast( + txAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], + broadcastAt time.Time, +) error { as.Lock() defer as.Unlock() tx, ok := as.allTxs[txAttempt.TxID] - if !ok { + if !ok || tx == nil { return fmt.Errorf("move_in_progress_to_confirmed_missing_receipt: no transaction with ID %d", txAttempt.TxID) } - if len(tx.TxAttempts) == 0 { - return fmt.Errorf("move_in_progress_to_confirmed_missing_receipt: no attempts for transaction with ID %d", tx.ID) - } - if tx.BroadcastAt != nil && tx.BroadcastAt.Before(broadcastAt) { - tx.BroadcastAt = &broadcastAt - } + var found bool for i := 0; i < len(tx.TxAttempts); i++ { if tx.TxAttempts[i].ID == txAttempt.ID { - attempt := tx.TxAttempts[i] - attempt.State = txmgrtypes.TxAttemptBroadcast + tx.TxAttempts[i].State = txmgrtypes.TxAttemptBroadcast + as.attemptHashToTxAttempt[txAttempt.Hash] = &tx.TxAttempts[i] + found = true + break } } - tx.State = TxConfirmedMissingReceipt - - as.confirmedMissingReceiptTxs[tx.ID] = tx - delete(as.unconfirmedTxs, tx.ID) + if !found { + return fmt.Errorf("move_in_progress_to_confirmed_missing_receipt: transaction with ID %d has no attempt with ID: %q", txAttempt.TxID, txAttempt.ID) + } + if tx.BroadcastAt != nil && tx.BroadcastAt.Before(broadcastAt) { + tx.BroadcastAt = &broadcastAt + } + as._moveUnconfirmedToConfirmedMissingReceipt(tx) return nil } @@ -582,3 +582,11 @@ func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) _moveT tx.Error = txError as.fatalErroredTxs[tx.ID] = tx } + +func (as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) _moveUnconfirmedToConfirmedMissingReceipt( + tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], +) { + tx.State = TxConfirmedMissingReceipt + as.confirmedMissingReceiptTxs[tx.ID] = tx + delete(as.unconfirmedTxs, tx.ID) +} diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index ee188340a49..784009784ba 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -384,19 +384,15 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveC return fmt.Errorf("expected state to be in_progress") } - var tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] var as *addressState[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] - filter := func(tx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) bool { return true } for _, vas := range ms.addressStates { - txs := vas.findTxs(nil, filter, attempt.TxID) - if len(txs) != 0 { - tx = &txs[0] + if vas.hasTx(attempt.TxID) { as = vas break } } - if tx == nil { - return fmt.Errorf("save_confirmed_missing_receipt_attempt: %w", ErrTxnNotFound) + if as == nil { + return fmt.Errorf("save_confirmed_missing_receipt_attempt: %w: %q", ErrTxnNotFound, attempt.TxID) } // Persist to persistent storage @@ -405,7 +401,7 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveC } // Update in memory store - return as.moveTxToConfirmedMissingReceipt(*attempt, broadcastAt) + return as.moveTxAttemptToBroadcast(*attempt, broadcastAt) } func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) SaveInProgressAttempt(ctx context.Context, attempt *txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { return nil From 4a48dc42e5bd3169c842440b1cd717ff09089c93 Mon Sep 17 00:00:00 2001 From: James Walker Date: Thu, 4 Apr 2024 09:17:28 -0400 Subject: [PATCH 41/41] check if transaction exists --- common/txmgr/inmemory_store.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/txmgr/inmemory_store.go b/common/txmgr/inmemory_store.go index 38c41c8fd08..f66e32f83cc 100644 --- a/common/txmgr/inmemory_store.go +++ b/common/txmgr/inmemory_store.go @@ -208,6 +208,9 @@ func (ms *inMemoryStore[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Updat if !ok { return fmt.Errorf("update_tx_fatal_error: %w", ErrAddressNotFound) } + if !as.hasTx(tx.ID) { + return fmt.Errorf("update_tx_fatal_error: %w: %q", ErrTxnNotFound, tx.ID) + } // Persist to persistent storage if err := ms.persistentTxStore.UpdateTxFatalError(ctx, tx); err != nil {