Skip to content

core/txpool/legacypool: fix validTxMeter to count transactions #32845

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 10, 2025
+12 −5
Merged

core/txpool/legacypool: fix validTxMeter to count transactions #32845

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a9a6192
core/txpool/legacypool: fix validTxMeter to count transactions (not a…
cskiraly Oct 7, 2025
ad6b106
core/txpool/legacypool: count only valid as valid
cskiraly Oct 9, 2025
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
2 changes: 1 addition & 1 deletion core/txpool/legacypool/legacypool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,7 @@ func (pool *LegacyPool) Add(txs []*types.Transaction, sync bool) []error {
pool.mu.Lock()
newErrs, dirtyAddrs := pool.addTxsLocked(news)
pool.mu.Unlock()
validTxMeter.Mark(int64(len(news)))
Copy link
Member

Choose a reason for hiding this comment

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

won't this count all txs in "news" including the ones that error on stateful checks?

Copy link
Member

Choose a reason for hiding this comment

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

It's a valid point raised by Matt.

maybe change like this?

diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go
index 80a9faf23f..9274def280 100644
--- a/core/txpool/legacypool/legacypool.go
+++ b/core/txpool/legacypool/legacypool.go
@@ -985,16 +985,22 @@ func (pool *LegacyPool) Add(txs []*types.Transaction, sync bool) []error {
 // addTxsLocked attempts to queue a batch of transactions if they are valid.
 // The transaction pool lock must be held.
 func (pool *LegacyPool) addTxsLocked(txs []*types.Transaction) ([]error, *accountSet) {
-	dirty := newAccountSet(pool.signer)
-	errs := make([]error, len(txs))
+	var (
+		dirty = newAccountSet(pool.signer)
+		errs  = make([]error, len(txs))
+		valid int
+	)
 	for i, tx := range txs {
 		replaced, err := pool.add(tx)
 		errs[i] = err
 		if err == nil && !replaced {
 			dirty.addTx(tx)
 		}
+		if err == nil {
+			valid += 1
+		}
 	}
-	validTxMeter.Mark(int64(len(dirty.accounts)))
+	validTxMeter.Mark(int64(valid))
 	return errs, dirty
 }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In first instance I just wanted to TX level, without changing essence, but I think you are right.
Will fix later today, but I do want to count both valid and invalid. Maybe a new counter here for those that return err.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

add in addTxsLocked updates counters on err, while it simply returns for those that are added, so it seems ok to update with valid here.


var nilSlot = 0
for _, err := range newErrs {
Expand Down Expand Up @@ -994,7 +995,6 @@ func (pool *LegacyPool) addTxsLocked(txs []*types.Transaction) ([]error, *accoun
dirty.addTx(tx)
}
}
validTxMeter.Mark(int64(len(dirty.accounts)))
return errs, dirty
}

Expand Down