From 2a032e83a5e09ae128e8c751779a7d1eebb729ea Mon Sep 17 00:00:00 2001 From: amit-momin <108959691+amit-momin@users.noreply.github.com> Date: Fri, 2 Aug 2024 10:35:40 -0500 Subject: [PATCH] Update AutoPurge config interface and add header for Scroll API (#13999) * Updated AutoPurge heuristic configs to be optional * Added content-type header for Scroll stuck tx API call * Fixed linting * Added changeset --- .changeset/violet-clouds-rhyme.md | 5 ++++ .../evm/config/chain_scoped_transactions.go | 8 +++---- core/chains/evm/config/config.go | 4 ++-- core/chains/evm/txmgr/stuck_tx_detector.go | 23 ++++++++++++++----- .../evm/txmgr/stuck_tx_detector_test.go | 16 ++++++------- 5 files changed, 36 insertions(+), 20 deletions(-) create mode 100644 .changeset/violet-clouds-rhyme.md diff --git a/.changeset/violet-clouds-rhyme.md b/.changeset/violet-clouds-rhyme.md new file mode 100644 index 00000000000..b6db0e85c4f --- /dev/null +++ b/.changeset/violet-clouds-rhyme.md @@ -0,0 +1,5 @@ +--- +"chainlink": minor +--- + +Updated AutoPurge.Threshold and AutoPurge.MinAttempts configs to only be required for heuristic and added content-type header for Scroll API #internal diff --git a/core/chains/evm/config/chain_scoped_transactions.go b/core/chains/evm/config/chain_scoped_transactions.go index 87031a4c66e..27edb12648a 100644 --- a/core/chains/evm/config/chain_scoped_transactions.go +++ b/core/chains/evm/config/chain_scoped_transactions.go @@ -47,12 +47,12 @@ func (a *autoPurgeConfig) Enabled() bool { return *a.c.Enabled } -func (a *autoPurgeConfig) Threshold() uint32 { - return *a.c.Threshold +func (a *autoPurgeConfig) Threshold() *uint32 { + return a.c.Threshold } -func (a *autoPurgeConfig) MinAttempts() uint32 { - return *a.c.MinAttempts +func (a *autoPurgeConfig) MinAttempts() *uint32 { + return a.c.MinAttempts } func (a *autoPurgeConfig) DetectionApiUrl() *url.URL { diff --git a/core/chains/evm/config/config.go b/core/chains/evm/config/config.go index b0a5772f739..3ccdfeea8b8 100644 --- a/core/chains/evm/config/config.go +++ b/core/chains/evm/config/config.go @@ -110,8 +110,8 @@ type Transactions interface { type AutoPurgeConfig interface { Enabled() bool - Threshold() uint32 - MinAttempts() uint32 + Threshold() *uint32 + MinAttempts() *uint32 DetectionApiUrl() *url.URL } diff --git a/core/chains/evm/txmgr/stuck_tx_detector.go b/core/chains/evm/txmgr/stuck_tx_detector.go index 1beb857af8f..5901be0b02d 100644 --- a/core/chains/evm/txmgr/stuck_tx_detector.go +++ b/core/chains/evm/txmgr/stuck_tx_detector.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "math/big" "net/http" @@ -37,8 +38,8 @@ type stuckTxDetectorTxStore interface { type stuckTxDetectorConfig interface { Enabled() bool - Threshold() uint32 - MinAttempts() uint32 + Threshold() *uint32 + MinAttempts() *uint32 DetectionApiUrl() *url.URL } @@ -78,7 +79,7 @@ func NewStuckTxDetector(lggr logger.Logger, chainID *big.Int, chainType chaintyp func (d *stuckTxDetector) LoadPurgeBlockNumMap(ctx context.Context, addresses []common.Address) error { // Skip loading purge block num map if auto-purge feature disabled or Threshold is set to 0 - if !d.cfg.Enabled() || d.cfg.Threshold() == 0 { + if !d.cfg.Enabled() || d.cfg.Threshold() == nil || *d.cfg.Threshold() == 0 { return nil } d.purgeBlockNumLock.Lock() @@ -172,6 +173,11 @@ func (d *stuckTxDetector) FindUnconfirmedTxWithLowestNonce(ctx context.Context, // 4. If 3 is true, check if the latest attempt's gas price is higher than what our gas estimator's GetFee method returns // 5. If 4 is true, the transaction is likely stuck due to overflow func (d *stuckTxDetector) detectStuckTransactionsHeuristic(ctx context.Context, txs []Tx, blockNum int64) ([]Tx, error) { + if d.cfg.Threshold() == nil || d.cfg.MinAttempts() == nil { + err := errors.New("missing required configs for the stuck transaction heuristic. Transactions.AutoPurge.Threshold and Transactions.AutoPurge.MinAttempts are required") + d.lggr.Error(err.Error()) + return txs, err + } d.purgeBlockNumLock.RLock() defer d.purgeBlockNumLock.RUnlock() // Get gas price from internal gas estimator @@ -187,17 +193,17 @@ func (d *stuckTxDetector) detectStuckTransactionsHeuristic(ctx context.Context, d.purgeBlockNumLock.RLock() lastPurgeBlockNum := d.purgeBlockNumMap[tx.FromAddress] d.purgeBlockNumLock.RUnlock() - if lastPurgeBlockNum > blockNum-int64(d.cfg.Threshold()) { + if lastPurgeBlockNum > blockNum-int64(*d.cfg.Threshold()) { continue } // Tx attempts are loaded from newest to oldest oldestBroadcastAttempt, newestBroadcastAttempt, broadcastedAttemptsCount := findBroadcastedAttempts(tx) // 2. Check if Threshold amount of blocks have passed since the oldest attempt's broadcast block num - if *oldestBroadcastAttempt.BroadcastBeforeBlockNum > blockNum-int64(d.cfg.Threshold()) { + if *oldestBroadcastAttempt.BroadcastBeforeBlockNum > blockNum-int64(*d.cfg.Threshold()) { continue } // 3. Check if the transaction has at least MinAttempts amount of broadcasted attempts - if broadcastedAttemptsCount < d.cfg.MinAttempts() { + if broadcastedAttemptsCount < *d.cfg.MinAttempts() { continue } // 4. Check if the newest broadcasted attempt's gas price is higher than what our gas estimator's GetFee method returns @@ -278,6 +284,10 @@ func (d *stuckTxDetector) detectStuckTransactionsScroll(ctx context.Context, txs if err != nil { return nil, fmt.Errorf("failed to make new request with context: %w", err) } + + // Add Content-Type header + postReq.Header.Add("Content-Type", "application/json") + // Send request resp, err := d.httpClient.Do(postReq) if err != nil { @@ -287,6 +297,7 @@ func (d *stuckTxDetector) detectStuckTransactionsScroll(ctx context.Context, txs if resp.StatusCode != 200 { return nil, fmt.Errorf("request failed with status %d", resp.StatusCode) } + // Decode the response into expected type scrollResp := new(scrollResponse) err = json.NewDecoder(resp.Body).Decode(scrollResp) diff --git a/core/chains/evm/txmgr/stuck_tx_detector_test.go b/core/chains/evm/txmgr/stuck_tx_detector_test.go index e980527c989..5f0d73be184 100644 --- a/core/chains/evm/txmgr/stuck_tx_detector_test.go +++ b/core/chains/evm/txmgr/stuck_tx_detector_test.go @@ -78,8 +78,8 @@ func TestStuckTxDetector_LoadPurgeBlockNumMap(t *testing.T) { autoPurgeMinAttempts := uint32(3) autoPurgeCfg := testAutoPurgeConfig{ enabled: true, // Enable auto-purge feature for testing - threshold: autoPurgeThreshold, - minAttempts: autoPurgeMinAttempts, + threshold: &autoPurgeThreshold, + minAttempts: &autoPurgeMinAttempts, } stuckTxDetector := txmgr.NewStuckTxDetector(lggr, testutils.FixtureChainID, "", assets.NewWei(assets.NewEth(100).ToInt()), autoPurgeCfg, feeEstimator, txStore, ethClient) @@ -176,8 +176,8 @@ func TestStuckTxDetector_DetectStuckTransactionsHeuristic(t *testing.T) { autoPurgeMinAttempts := uint32(3) autoPurgeCfg := testAutoPurgeConfig{ enabled: true, // Enable auto-purge feature for testing - threshold: autoPurgeThreshold, - minAttempts: autoPurgeMinAttempts, + threshold: &autoPurgeThreshold, + minAttempts: &autoPurgeMinAttempts, } blockNum := int64(100) stuckTxDetector := txmgr.NewStuckTxDetector(lggr, testutils.FixtureChainID, "", assets.NewWei(assets.NewEth(100).ToInt()), autoPurgeCfg, feeEstimator, txStore, ethClient) @@ -423,12 +423,12 @@ func mustInsertUnconfirmedEthTxWithBroadcastPurgeAttempt(t *testing.T, txStore t type testAutoPurgeConfig struct { enabled bool - threshold uint32 - minAttempts uint32 + threshold *uint32 + minAttempts *uint32 detectionApiUrl *url.URL } func (t testAutoPurgeConfig) Enabled() bool { return t.enabled } -func (t testAutoPurgeConfig) Threshold() uint32 { return t.threshold } -func (t testAutoPurgeConfig) MinAttempts() uint32 { return t.minAttempts } +func (t testAutoPurgeConfig) Threshold() *uint32 { return t.threshold } +func (t testAutoPurgeConfig) MinAttempts() *uint32 { return t.minAttempts } func (t testAutoPurgeConfig) DetectionApiUrl() *url.URL { return t.detectionApiUrl }