Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BCI-3573] - Remove dependence on FinalityDepth in EVM TXM code #13794

Merged
merged 55 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
99cb0b6
removing finality within common/txmgr
Farber98 Jul 3, 2024
72f430e
remove finality from common/txmgr comment
Farber98 Jul 3, 2024
2de27e3
latestFinalizedBlockNum support within evm/txmgr txstore
Farber98 Jul 3, 2024
fe42945
refactor tests after evm/txmgr txstore changes
Farber98 Jul 3, 2024
1d14c4b
mocks for both common and core/evm
Farber98 Jul 3, 2024
6222291
remove comments that are still referencing to finalityDepth within ev…
Farber98 Jul 4, 2024
8e01631
add changeset
Farber98 Jul 9, 2024
b8fead5
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm the…
Farber98 Jul 10, 2024
55b069f
error if no LatestFinalizedHead was found
Farber98 Jul 11, 2024
43b3428
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
Farber98 Jul 11, 2024
9a97763
fix mocks version
Farber98 Jul 11, 2024
8ed35fe
fix mocks version
Farber98 Jul 11, 2024
09c3704
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
Farber98 Jul 11, 2024
1a694f7
mock versioning
Farber98 Jul 11, 2024
7f1b8b5
mock version
Farber98 Jul 11, 2024
e9c8daa
mock version
Farber98 Jul 11, 2024
fdb0d33
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
Farber98 Jul 11, 2024
5f13155
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
Farber98 Jul 12, 2024
b151dbe
mock version
Farber98 Jul 12, 2024
552a89f
mock version
Farber98 Jul 12, 2024
551a8c0
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
Farber98 Jul 12, 2024
7163d10
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
Farber98 Jul 30, 2024
8e91a75
inject head tracker trimmed API
Farber98 Jul 31, 2024
cb57245
fix tests
Farber98 Jul 31, 2024
1188b4b
mocks
Farber98 Jul 31, 2024
a4bb842
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
Farber98 Jul 31, 2024
a0673ce
remove unnecesary tests as finalized head is guaranteed
Farber98 Jul 31, 2024
b10c57d
ht in shell local
Farber98 Jul 31, 2024
a891e36
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
Farber98 Jul 31, 2024
62278e0
remove evm dep from confirmer and use generics instead
Farber98 Aug 1, 2024
2adcd80
make markOldTxesMissingReceiptAsErrored condition inclusive
Farber98 Aug 1, 2024
4fb2252
use already initialized head tracker within shell local
Farber98 Aug 1, 2024
f4ca5d9
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
Farber98 Aug 1, 2024
73aa556
fix mocking, fix unit test
huangzhen1997 Aug 2, 2024
9d99a29
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
huangzhen1997 Aug 2, 2024
242f41c
fix a potential bug
huangzhen1997 Aug 5, 2024
6a361f5
fix bug
huangzhen1997 Aug 5, 2024
f53f260
refactor
huangzhen1997 Aug 5, 2024
b7b7e56
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
huangzhen1997 Aug 5, 2024
6e71ff5
address comments
huangzhen1997 Aug 5, 2024
4663172
fix lint
huangzhen1997 Aug 5, 2024
805975d
rename
huangzhen1997 Aug 6, 2024
14bb96b
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
huangzhen1997 Aug 6, 2024
31d4569
have log back
huangzhen1997 Aug 6, 2024
cdeee6a
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
huangzhen1997 Aug 6, 2024
cebf699
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
huangzhen1997 Aug 6, 2024
0f98a46
update comments
huangzhen1997 Aug 6, 2024
dbbac95
remove nil check
huangzhen1997 Aug 6, 2024
14a5668
minor
huangzhen1997 Aug 6, 2024
2e1e92a
resolve merge conflicts
huangzhen1997 Aug 6, 2024
ebe7d59
fix test
huangzhen1997 Aug 6, 2024
e59e259
grammar
huangzhen1997 Aug 6, 2024
d70f6af
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
huangzhen1997 Aug 6, 2024
b0ed1e0
rephrase
huangzhen1997 Aug 6, 2024
5a079e9
Merge branch 'develop' into BCI-3573-remove-finalitydepth-evm-txm
huangzhen1997 Aug 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/chatty-spiders-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": minor
---

remove dependency on FinalityDepth in EVM TXM code. #internal
66 changes: 47 additions & 19 deletions common/txmgr/confirmer.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ var (
}, []string{"chainID"})
)

type confirmerHeadTracker[HEAD types.Head[BLOCK_HASH], BLOCK_HASH types.Hashable] interface {
LatestAndFinalizedBlock(ctx context.Context) (latest, finalized HEAD, err error)
}

// Confirmer is a broad service which performs four different tasks in sequence on every new longest chain
// Step 1: Mark that all currently pending transaction attempts were broadcast before this block
// Step 2: Check pending transactions for receipts
Expand Down Expand Up @@ -133,14 +137,15 @@ type Confirmer[
ks txmgrtypes.KeyStore[ADDR, CHAIN_ID, SEQ]
enabledAddresses []ADDR

mb *mailbox.Mailbox[HEAD]
stopCh services.StopChan
wg sync.WaitGroup
initSync sync.Mutex
isStarted bool

mb *mailbox.Mailbox[HEAD]
stopCh services.StopChan
wg sync.WaitGroup
initSync sync.Mutex
isStarted bool
nConsecutiveBlocksChainTooShort int
isReceiptNil func(R) bool

headTracker confirmerHeadTracker[HEAD, BLOCK_HASH]
}

func NewConfirmer[
Expand All @@ -164,6 +169,7 @@ func NewConfirmer[
lggr logger.Logger,
isReceiptNil func(R) bool,
stuckTxDetector txmgrtypes.StuckTxDetector[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE],
headTracker confirmerHeadTracker[HEAD, BLOCK_HASH],
) *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE] {
lggr = logger.Named(lggr, "Confirmer")
return &Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]{
Expand All @@ -181,6 +187,7 @@ func NewConfirmer[
mb: mailbox.NewSingle[HEAD](),
isReceiptNil: isReceiptNil,
stuckTxDetector: stuckTxDetector,
headTracker: headTracker,
}
}

Expand Down Expand Up @@ -297,7 +304,20 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) pro
return fmt.Errorf("CheckConfirmedMissingReceipt failed: %w", err)
}

if err := ec.CheckForReceipts(ctx, head.BlockNumber()); err != nil {
_, latestFinalizedHead, err := ec.headTracker.LatestAndFinalizedBlock(ctx)
if err != nil {
return fmt.Errorf("failed to retrieve latest finalized head: %w", err)
}
huangzhen1997 marked this conversation as resolved.
Show resolved Hide resolved

if !latestFinalizedHead.IsValid() {
return fmt.Errorf("latest finalized head is not valid")
}

if latestFinalizedHead.BlockNumber() > head.BlockNumber() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Don't think this log is needed.
The HeadTracker should never send finalzied blocks that are newer than current blocks.
This is likely a clear case of TXM panicking or throwing critical errors. But this should already been handled at HT layer.

ec.lggr.Debugw("processHead received old block", "latestFinalizedHead", latestFinalizedHead.BlockNumber(), "headNum", head.BlockNumber(), "time", time.Since(mark), "id", "confirmer")
}

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

Expand All @@ -318,7 +338,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); err != nil {
if err := ec.EnsureConfirmedTransactionsInLongestChain(ctx, head, latestFinalizedHead.BlockNumber()); err != nil {
return fmt.Errorf("EnsureConfirmedTransactionsInLongestChain failed: %w", err)
}

Expand Down Expand Up @@ -395,8 +415,8 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Che
return
}

// CheckForReceipts finds attempts that are still pending and checks to see if a receipt is present for the given block number
func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) CheckForReceipts(ctx context.Context, blockNum int64) error {
// CheckForReceipts finds attempts that are still pending and checks to see if a receipt is present for the given block number.
func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) CheckForReceipts(ctx context.Context, blockNum int64, latestFinalizedBlockNum int64) error {
attempts, err := ec.txStore.FindTxAttemptsRequiringReceiptFetch(ctx, ec.chainID)
if err != nil {
return fmt.Errorf("FindTxAttemptsRequiringReceiptFetch failed: %w", err)
Expand Down Expand Up @@ -443,7 +463,7 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Che
return fmt.Errorf("unable to mark txes as 'confirmed_missing_receipt': %w", err)
}

if err := ec.txStore.MarkOldTxesMissingReceiptAsErrored(ctx, blockNum, ec.chainConfig.FinalityDepth(), ec.chainID); err != nil {
if err := ec.txStore.MarkOldTxesMissingReceiptAsErrored(ctx, blockNum, latestFinalizedBlockNum, ec.chainID); err != nil {
return fmt.Errorf("unable to confirm buried unconfirmed txes': %w", err)
}
return nil
Expand Down Expand Up @@ -1004,22 +1024,30 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) han
}
}

// EnsureConfirmedTransactionsInLongestChain finds all confirmed txes up to the depth
// EnsureConfirmedTransactionsInLongestChain finds all confirmed txes up to the earliest head
// of the given chain and ensures that every one has a receipt with a block hash that is
// in the given chain.
//
// If any of the confirmed transactions does not have a receipt in the chain, it has been
// re-org'd out and will be rebroadcast.
func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) EnsureConfirmedTransactionsInLongestChain(ctx context.Context, head types.Head[BLOCK_HASH]) error {
if head.ChainLength() < ec.chainConfig.FinalityDepth() {
logArgs := []interface{}{
"chainLength", head.ChainLength(), "finalityDepth", ec.chainConfig.FinalityDepth(),
}
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 {
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 {
if ec.nConsecutiveBlocksChainTooShort > logAfterNConsecutiveBlocksChainTooShort {
warnMsg := "Chain length supplied for re-org detection was shorter than FinalityDepth. Re-org protection is not working properly. This could indicate a problem with the remote RPC endpoint, a compatibility issue with a particular blockchain, a bug with this particular blockchain, heads table being truncated too early, remote node out of sync, or something else. If this happens a lot please raise a bug with the Chainlink team including a log output sample and details of the chain and RPC endpoint you are using."
warnMsg := "Chain length supplied for re-org detection was shorter than 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)...)
} else {
logMsg := "Chain length supplied for re-org detection was shorter than FinalityDepth"
logMsg := "Chain length supplied for re-org detection was shorter than the depth from the latest head to the finalized head"
ec.lggr.Debugw(logMsg, append(logArgs, "nConsecutiveBlocksChainTooShort", ec.nConsecutiveBlocksChainTooShort)...)
}
ec.nConsecutiveBlocksChainTooShort++
Expand Down
22 changes: 11 additions & 11 deletions common/txmgr/types/mocks/tx_store.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion common/txmgr/types/tx_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ type TransactionStore[
HasInProgressTransaction(ctx context.Context, account ADDR, chainID CHAIN_ID) (exists bool, err error)
LoadTxAttempts(ctx context.Context, etx *Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error
MarkAllConfirmedMissingReceipt(ctx context.Context, chainID CHAIN_ID) (err error)
MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, finalityDepth uint32, chainID CHAIN_ID) error
MarkOldTxesMissingReceiptAsErrored(ctx context.Context, blockNum int64, latestFinalizedBlockNum int64, chainID CHAIN_ID) error
PreloadTxes(ctx context.Context, attempts []TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error
SaveConfirmedMissingReceiptAttempt(ctx context.Context, timeout time.Duration, attempt *TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], broadcastAt time.Time) error
SaveInProgressAttempt(ctx context.Context, attempt *TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error
Expand Down
13 changes: 9 additions & 4 deletions core/chains/evm/txmgr/builder.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package txmgr

import (
"context"
"math/big"
"time"

Expand All @@ -13,12 +14,15 @@ import (
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/chaintype"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/forwarders"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas"
httypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/headtracker/types"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/keystore"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller"
evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
)

type latestAndFinalizedBlockHeadTracker interface {
LatestAndFinalizedBlock(ctx context.Context) (latest, finalized *evmtypes.Head, err error)
}

// NewTxm constructs the necessary dependencies for the EvmTxm (broadcaster, confirmer, etc) and returns a new EvmTxManager
func NewTxm(
ds sqlutil.DataSource,
Expand All @@ -33,7 +37,7 @@ func NewTxm(
logPoller logpoller.LogPoller,
keyStore keystore.Eth,
estimator gas.EvmFeeEstimator,
headTracker httypes.HeadTracker,
headTracker latestAndFinalizedBlockHeadTracker,
) (txm TxManager,
err error,
) {
Expand All @@ -55,7 +59,7 @@ func NewTxm(
evmBroadcaster := NewEvmBroadcaster(txStore, txmClient, txmCfg, feeCfg, txConfig, listenerConfig, keyStore, txAttemptBuilder, lggr, checker, chainConfig.NonceAutoSync(), chainConfig.ChainType())
evmTracker := NewEvmTracker(txStore, keyStore, chainID, lggr)
stuckTxDetector := NewStuckTxDetector(lggr, client.ConfiguredChainID(), chainConfig.ChainType(), fCfg.PriceMax(), txConfig.AutoPurge(), estimator, txStore, client)
evmConfirmer := NewEvmConfirmer(txStore, txmClient, txmCfg, feeCfg, txConfig, dbConfig, keyStore, txAttemptBuilder, lggr, stuckTxDetector)
evmConfirmer := NewEvmConfirmer(txStore, txmClient, txmCfg, feeCfg, txConfig, dbConfig, keyStore, txAttemptBuilder, lggr, stuckTxDetector, headTracker)
evmFinalizer := NewEvmFinalizer(lggr, client.ConfiguredChainID(), chainConfig.RPCDefaultBatchSize(), txStore, client, headTracker)
var evmResender *Resender
if txConfig.ResendAfterThreshold() > 0 {
Expand Down Expand Up @@ -116,8 +120,9 @@ func NewEvmConfirmer(
txAttemptBuilder TxAttemptBuilder,
lggr logger.Logger,
stuckTxDetector StuckTxDetector,
headTracker latestAndFinalizedBlockHeadTracker,
) *Confirmer {
return txmgr.NewConfirmer(txStore, client, chainConfig, feeConfig, txConfig, dbConfig, keystore, txAttemptBuilder, lggr, func(r *evmtypes.Receipt) bool { return r == nil }, stuckTxDetector)
return txmgr.NewConfirmer(txStore, client, chainConfig, feeConfig, txConfig, dbConfig, keystore, txAttemptBuilder, lggr, func(r *evmtypes.Receipt) bool { return r == nil }, stuckTxDetector, headTracker)
}

// NewEvmTracker instantiates a new EVM tracker for abandoned transactions
Expand Down
Loading
Loading