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

Add feature to TXM to detect and purge stuck transactions #12881

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
246b04d
Added the EVM stuck tx detector component
amit-momin Apr 18, 2024
d47d88f
Added stuck tx handling in Confirmer
amit-momin Apr 19, 2024
8c713dc
Fixed linting
amit-momin Apr 19, 2024
05c5a11
Fixed toml config decoding
amit-momin Apr 19, 2024
360d9fa
Fixed config tests
amit-momin Apr 19, 2024
0a04fff
Fixed web resolver config tests
amit-momin Apr 19, 2024
d26473c
Fixed config docs test
amit-momin Apr 19, 2024
1541c5a
Added zkEVM overflow detection and added unit tests for the detector
amit-momin Apr 23, 2024
1d4113d
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin Apr 23, 2024
c71ac14
Fixed broken tests after merge
amit-momin Apr 23, 2024
2e46dde
Reverted AutoPurgeConfig validation changes and fixed config tests
amit-momin Apr 23, 2024
8185dc8
Added changeset and fixed config validation logic
amit-momin Apr 23, 2024
20bfb9d
Fixed linting
amit-momin Apr 23, 2024
82e3a5d
Fixed linting
amit-momin Apr 23, 2024
db5264e
Fixed purge attempt builder and added tests
amit-momin Apr 24, 2024
0d5b318
Updated evm.txes contraint to allow non-null nonce if fatal_error
amit-momin Apr 24, 2024
25105a1
Added confirmer test
amit-momin Apr 24, 2024
491f9e3
Adjusted confirmer test to better reflect actual process
amit-momin Apr 24, 2024
338ab26
Fixed linting
amit-momin Apr 24, 2024
dc17b98
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin Apr 25, 2024
468b73b
Added purge block num loading on Confirmer startup
amit-momin Apr 25, 2024
736eb89
Updated EVM tx store mock
amit-momin Apr 25, 2024
b2902c6
Fixed linting and testdata
amit-momin Apr 25, 2024
6eb5dc5
Updated stuck tx fatal error messages
amit-momin Apr 25, 2024
a5663ee
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin Apr 25, 2024
0cea37b
Updated sql migration file sequence
amit-momin Apr 25, 2024
a3cd9b7
Skipped loading purge block num if auto-purge feature disabled and fi…
amit-momin Apr 26, 2024
02306fe
Fixed linting
amit-momin Apr 26, 2024
a13605b
Renamed function and moved log
amit-momin Apr 26, 2024
c2deae5
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin Apr 26, 2024
a5ea4aa
Added stricter config validation
amit-momin Apr 26, 2024
1cfb5cc
Fixed linting
amit-momin Apr 27, 2024
335033a
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin Apr 29, 2024
a5ba3a0
Updated auto-purge feature configs to adhere to config naming standards
amit-momin Apr 29, 2024
7513d36
Fixed config doc test and updated changeset
amit-momin Apr 29, 2024
4b41b4b
Updated Scroll and zkEVM config defaults and linted common config
amit-momin Apr 29, 2024
cc5764d
Updated config doc
amit-momin Apr 29, 2024
b1faa07
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin Apr 30, 2024
d67d79b
Generated config doc and fixed linting
amit-momin Apr 30, 2024
c4062f5
Updated config description for AutoPurge.MinAttempts
amit-momin May 9, 2024
54b9c7c
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin May 9, 2024
d244012
Fixed sql migration conflict
amit-momin May 9, 2024
0915400
Fixed linting
amit-momin May 9, 2024
c95d261
Updated stuck tx detector to use PriceMax config and added comments
amit-momin May 13, 2024
db5e5d9
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin May 13, 2024
31d69ba
Fixed linting
amit-momin May 13, 2024
90ec4a3
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin May 22, 2024
d81da52
Updated DetectionApiUrl config example
amit-momin May 22, 2024
a33460b
Merge branch 'develop' into BCI-2941-Implement-auto-purge-stuck-trans…
amit-momin May 23, 2024
c08504b
Fixed issues from latest merge
amit-momin May 23, 2024
88826a7
Renumbered sql migration file
amit-momin May 23, 2024
e400a36
Fixed testdata
amit-momin May 24, 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
Prev Previous commit
Next Next commit
Skipped loading purge block num if auto-purge feature disabled and fi…
…xed test
  • Loading branch information
amit-momin committed Apr 26, 2024
commit a3cd9b7c252541af90e0712beca1a1650f97c280
3 changes: 1 addition & 2 deletions common/txmgr/confirmer.go
prashantkumar1982 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,7 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) sta
if err != nil {
return fmt.Errorf("Confirmer: failed to load EnabledAddressesForChain: %w", err)
}
err = ec.stuckTxDetector.LoadPurgeBlockNumMap(ctx, ec.enabledAddresses)
if err != nil {
if err = ec.stuckTxDetector.LoadPurgeBlockNumMap(ctx, ec.enabledAddresses); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

does it make sense to have LoadPurgeBlockNumMap in the NewStuckTxDetector initialization?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered this to avoid another method the confirmer needs to call but this method requires a context. I wouldn't have anything to wire this up to during initialization and didn't want to introduce a context to the TXM builder method just to pass it down to the detector.

Copy link
Contributor

Choose a reason for hiding this comment

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

You actually need to call this from startInternal(), which can be called multiple times during the lifetime of the Confirmer. So doing it in NewStuckTxDetector() will just be redundant. You still have to call it from here

ec.lggr.Debugf("Confirmer: failed to load the last purged block num for enabled addresses. Process can continue as normal but purge rate limiting may be affected.")
}

Expand Down
3 changes: 3 additions & 0 deletions core/chains/evm/txmgr/stuck_tx_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ func NewStuckTxDetector(lggr logger.Logger, chainID *big.Int, chainType config.C
}

func (d *stuckTxDetector) LoadPurgeBlockNumMap(ctx context.Context, addresses []common.Address) error {
if !d.cfg.AutoPurgeStuckTxs() {
return nil
}
d.purgeBlockNumLock.Lock()
defer d.purgeBlockNumLock.Unlock()
// Ok to reset the map here since this method could be reloaded with a new list of from addresses
Expand Down
17 changes: 13 additions & 4 deletions core/chains/evm/txmgr/stuck_tx_detector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,26 @@ func TestStuckTxDetector_LoadPurgeBlockNumMap(t *testing.T) {
t.Parallel()

db := pgtest.NewSqlxDB(t)
_, config := newTestChainScopedConfig(t)
txStore := cltest.NewTestTxStore(t, db)
ethKeyStore := cltest.NewKeyStore(t, db).Eth()
ctx := testutils.Context(t)
blockNum := int64(100)
marketGasPrice := assets.GWei(15)


lggr := logger.Test(t)
ethClient := evmtest.NewEthClientMockWithDefaultChain(t)
feeEstimator := gasmocks.NewEvmFeeEstimator(t)
stuckTxDetector := txmgr.NewStuckTxDetector(lggr, testutils.FixtureChainID, "", config.EVM().Transactions().AutoPurge(), feeEstimator, txStore, ethClient)
marketGasPrice := assets.GWei(15)
fee := gas.EvmFee{Legacy: marketGasPrice}
feeEstimator.On("GetFee", mock.Anything, []byte{}, uint64(0), mock.Anything).Return(fee, uint64(0), nil)
autoPurgeThreshold := uint32(5)
autoPurgeMinAttempts := uint32(3)
autoPurgeCfg := testAutoPurgeConfig{
autoPurgeStuckTxs: true, // Enable auto-purge feature for testing
autoPurgeThreshold: autoPurgeThreshold,
autoPurgeMinAttempts: autoPurgeMinAttempts,
}
stuckTxDetector := txmgr.NewStuckTxDetector(lggr, testutils.FixtureChainID, "", autoPurgeCfg, feeEstimator, txStore, ethClient)

t.Run("purge num map loaded on startup rate limits new purges on startup", func(t *testing.T) {
_, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore)
Expand All @@ -85,7 +94,7 @@ func TestStuckTxDetector_LoadPurgeBlockNumMap(t *testing.T) {
// Create attempts broadcasted autoPurgeThreshold block ago to ensure broadcast block num check is not being triggered
// Create autoPurgeMinAttempts number of attempts to ensure the broadcast attempt count check is not being triggered
// Create attempts so that the latest has a higher gas price than the market to ensure the gas price check is not being triggered
mustInsertUnconfirmedTxWithBroadcastAttempts(t, txStore, 1, fromAddress, 3, blockNum-5, marketGasPrice.Add(oneGwei))
mustInsertUnconfirmedTxWithBroadcastAttempts(t, txStore, 1, fromAddress, autoPurgeMinAttempts, blockNum-int64(autoPurgeThreshold), marketGasPrice.Add(oneGwei))

// Run detection logic on autoPurgeThreshold blocks past the latest broadcast attempt
txs, err := stuckTxDetector.DetectStuckTransactions(ctx, enabledAddresses, blockNum)
Expand Down
Loading