Skip to content

Commit

Permalink
TXM Configmer logs warning only if the provided chain does not contai…
Browse files Browse the repository at this point in the history
…n finalized block (#14914)

(cherry picked from commit 47aaff4)
  • Loading branch information
dhaidashenko committed Oct 24, 2024
1 parent d21d159 commit 1d72e0c
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 26 deletions.
27 changes: 14 additions & 13 deletions common/txmgr/confirmer.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) pro
ec.lggr.Debugw("Finished RebroadcastWhereNecessary", "headNum", head.BlockNumber(), "time", time.Since(mark), "id", "confirmer")
mark = time.Now()

if err := ec.EnsureConfirmedTransactionsInLongestChain(ctx, head, latestFinalizedHead.BlockNumber()); err != nil {
if err := ec.EnsureConfirmedTransactionsInLongestChain(ctx, head); err != nil {
return fmt.Errorf("EnsureConfirmedTransactionsInLongestChain failed: %w", err)
}

Expand Down Expand Up @@ -1072,19 +1072,20 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) han
//
// If any of the confirmed transactions does not have a receipt in the chain, it has been
// re-org'd out and will be rebroadcast.
func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) EnsureConfirmedTransactionsInLongestChain(ctx context.Context, head types.Head[BLOCK_HASH], latestFinalizedHeadNumber int64) error {
func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) EnsureConfirmedTransactionsInLongestChain(ctx context.Context, head types.Head[BLOCK_HASH]) error {
logArgs := []interface{}{
"chainLength", head.ChainLength(), "latestFinalizedHead number", latestFinalizedHeadNumber,
}

if head.BlockNumber() < latestFinalizedHeadNumber {
errMsg := "current head is shorter than latest finalized head"
ec.lggr.Errorw(errMsg, append(logArgs, "head block number", head.BlockNumber())...)
return errors.New(errMsg)
}

calculatedFinalityDepth := uint32(head.BlockNumber() - latestFinalizedHeadNumber)
if head.ChainLength() < calculatedFinalityDepth {
"chainLength", head.ChainLength(),
}

//Here, we rely on the finalized block provided in the chain instead of the one
//provided via a dedicated method to avoid the false warning of the chain being
//too short. When `FinalityTagBypass = true,` HeadTracker tracks `finality depth
//+ history depth` to prevent excessive CPU usage. Thus, the provided chain may
//be shorter than the chain from the latest to the latest finalized, marked with
//a tag. A proper fix of this issue and complete switch to finality tag support
//will be introduced in BCFR-620
latestFinalized := head.LatestFinalizedHead()
if latestFinalized == nil || !latestFinalized.IsValid() {
if ec.nConsecutiveBlocksChainTooShort > logAfterNConsecutiveBlocksChainTooShort {
warnMsg := "Chain length supplied for re-org detection was shorter than the depth from the latest head to the finalized head. Re-org protection is not working properly. This could indicate a problem with the remote RPC endpoint, a compatibility issue with a particular blockchain, a bug with this particular blockchain, heads table being truncated too early, remote node out of sync, or something else. If this happens a lot please raise a bug with the Chainlink team including a log output sample and details of the chain and RPC endpoint you are using."
ec.lggr.Warnw(warnMsg, append(logArgs, "nConsecutiveBlocksChainTooShort", ec.nConsecutiveBlocksChainTooShort)...)
Expand Down
21 changes: 8 additions & 13 deletions core/chains/evm/txmgr/confirmer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2742,11 +2742,6 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) {
gconfig, config := newTestChainScopedConfig(t)
ec := newEthConfirmer(t, txStore, ethClient, gconfig, config, ethKeyStore, nil)

latestFinalizedHead := evmtypes.Head{
Number: 8,
Hash: testutils.NewHash(),
}

h8 := &evmtypes.Head{
Number: 8,
Hash: testutils.NewHash(),
Expand All @@ -2762,14 +2757,14 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) {
}
head.Parent.Store(h9)
t.Run("does nothing if there aren't any transactions", func(t *testing.T) {
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head, latestFinalizedHead.BlockNumber()))
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head))
})

t.Run("does nothing to unconfirmed transactions", func(t *testing.T) {
etx := cltest.MustInsertUnconfirmedEthTxWithBroadcastLegacyAttempt(t, txStore, 0, fromAddress)

// Do the thing
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head, latestFinalizedHead.BlockNumber()))
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head))

etx, err := txStore.FindTxWithAttempts(ctx, etx.ID)
require.NoError(t, err)
Expand All @@ -2781,7 +2776,7 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) {
mustInsertEthReceipt(t, txStore, head.Number, head.Hash, etx.TxAttempts[0].Hash)

// Do the thing
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head, latestFinalizedHead.BlockNumber()))
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head))

etx, err := txStore.FindTxWithAttempts(ctx, etx.ID)
require.NoError(t, err)
Expand All @@ -2794,7 +2789,7 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) {
mustInsertEthReceipt(t, txStore, h8.Number-1, testutils.NewHash(), etx.TxAttempts[0].Hash)

// Do the thing
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head, latestFinalizedHead.BlockNumber()))
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head))

etx, err := txStore.FindTxWithAttempts(ctx, etx.ID)
require.NoError(t, err)
Expand All @@ -2815,7 +2810,7 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) {
}), fromAddress).Return(commonclient.Successful, nil).Once()

// Do the thing
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head, latestFinalizedHead.BlockNumber()))
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head))

etx, err := txStore.FindTxWithAttempts(ctx, etx.ID)
require.NoError(t, err)
Expand All @@ -2838,7 +2833,7 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) {
commonclient.Successful, nil).Once()

// Do the thing
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head, latestFinalizedHead.BlockNumber()))
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head))

etx, err := txStore.FindTxWithAttempts(ctx, etx.ID)
require.NoError(t, err)
Expand Down Expand Up @@ -2873,7 +2868,7 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) {
}), fromAddress).Return(commonclient.Successful, nil).Once()

// Do the thing
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head, latestFinalizedHead.BlockNumber()))
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head))

etx, err := txStore.FindTxWithAttempts(ctx, etx.ID)
require.NoError(t, err)
Expand All @@ -2893,7 +2888,7 @@ func TestEthConfirmer_EnsureConfirmedTransactionsInLongestChain(t *testing.T) {
// Add receipt that is higher than head
mustInsertEthReceipt(t, txStore, head.Number+1, testutils.NewHash(), attempt.Hash)

require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head, latestFinalizedHead.BlockNumber()))
require.NoError(t, ec.EnsureConfirmedTransactionsInLongestChain(tests.Context(t), head))

etx, err := txStore.FindTxWithAttempts(ctx, etx.ID)
require.NoError(t, err)
Expand Down

0 comments on commit 1d72e0c

Please sign in to comment.