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

Cherry-pick of stuck transaction detection fix for Scroll #1257

Merged
merged 3 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/violet-clouds-rhyme.md
Original file line number Diff line number Diff line change
@@ -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
8 changes: 4 additions & 4 deletions core/chains/evm/config/chain_scoped_transactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions core/chains/evm/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ type Transactions interface {

type AutoPurgeConfig interface {
Enabled() bool
Threshold() uint32
MinAttempts() uint32
Threshold() *uint32
MinAttempts() *uint32
DetectionApiUrl() *url.URL
}

Expand Down
23 changes: 17 additions & 6 deletions core/chains/evm/txmgr/stuck_tx_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"math/big"
"net/http"
Expand Down Expand Up @@ -37,8 +38,8 @@ type stuckTxDetectorTxStore interface {

type stuckTxDetectorConfig interface {
Enabled() bool
Threshold() uint32
MinAttempts() uint32
Threshold() *uint32
MinAttempts() *uint32
DetectionApiUrl() *url.URL
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
16 changes: 8 additions & 8 deletions core/chains/evm/txmgr/stuck_tx_detector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 }
Loading