Skip to content

Commit

Permalink
Merge pull request #14938 from smartcontractkit/hotfix/BCFR-1065-prev…
Browse files Browse the repository at this point in the history 
…ent-confirmer-false-warning-2.18-cerry-pick

Hotfix/BCFR 1065 prevent confirmer false warning 2.18
  • Loading branch information
@chainchad
chainchad authored Oct 24, 2024
2 parents d21d159 + 1d72e0c commit 571c154
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
View file
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
View file
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 571c154

Please sign in to comment.