feat(metrics): calculate the real pending tx

[georgehao]

1. Purpose or design rationale of this PR

https://www.notion.so/scrollzkp/monitoring-Show-the-count-of-executable-transactions-on-txpool-graph-85e468811d334eed88466bddd043f3ac

because some pending tx's gasFeeCap less than the current baseFee, so add a metrics to calculate the real pending tx

2. PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

• build: Changes that affect the build system or external dependencies (example scopes: yarn, eslint, typescript)
• ci: Changes to our CI configuration files and scripts (example scopes: vercel, github, cypress)
• docs: Documentation-only changes
• feat: A new feature
• fix: A bug fix
• perf: A code change that improves performance
• refactor: A code change that doesn't fix a bug, or add a feature, or improves performance
• style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
• test: Adding missing tests or correcting existing tests

3. Deployment tag versioning

Has the version in params/version.go been updated?

• This PR doesn't involve a new deployment, git tag, docker image tag, and it doesn't affect traces
• Yes

4. Breaking change label

Does this PR have the breaking-change label?

• This PR is not a breaking change
• Yes

[jonastheis]

Another option could be calling https://github.com/scroll-tech/go-ethereum/blob/develop/core/tx_pool.go#L505 periodically in a separate thread to not block/prolong txpool reorgs.

[0xmountaintop]

Another option could be calling https://github.com/scroll-tech/go-ethereum/blob/develop/core/tx_pool.go#L505 periodically in a separate thread to not block/prolong txpool reorgs.

Yeah I agree with this approach

[omerfirmak]

Another option could be calling https://github.com/scroll-tech/go-ethereum/blob/develop/core/tx_pool.go#L505 periodically in a separate thread to not block/prolong txpool reorgs.

Even if this is called in another thread, it will hold the txpool lock and potentially interfere with block building and/or tx pool reorgs.

[georgehao]

Another option could be calling https://github.com/scroll-tech/go-ethereum/blob/develop/core/tx_pool.go#L505 periodically in a separate thread to not block/prolong txpool reorgs.

Even if this is called in another thread, it will hold the txpool lock and potentially interfere with block building and/or tx pool reorgs.

maybe we can recover pending := w.eth.TxPool().PendingWithMax(false, w.config.MaxAccountsNum) to pending := w.eth.TxPool().PendingWithMax(true, w.config.MaxAccountsNum)

go-ethereum/miner/scroll_worker.go

Line 445 in 4b85bbc

pending := w.eth.TxPool().PendingWithMax(false, w.config.MaxAccountsNum)

[jonastheis]

Counting it in the worker might not actually yield the correct number of pending transactions depending on how w.config.MaxAccountsNum is configured. Which makes it quite complex to assess whether the number is correct or not. Also imo it seems out of place as it is a txpool metric and not every node might be running a worker in the future.

Let's not over complicate things. Why not just start simple with this approach and add an additional metric to see how long it blocks the txpool. If it turns out to be significant or we feel it interferes with block building and/or tx pool reorgs, we can still go the route @omerfirmak suggested and you implemented in c5df443

[georgehao]

w.config.MaxAccountsNum

w.config.MaxAccountsNum is worker.config.MaxAccountsNum = math.MaxInt

[jonastheis]

Yes, by default worker.config.MaxAccountsNum = math.MaxInt but it's configurable with --miner.maxaccountsnum flag. This makes the result rely on side effects that don't have anything to do with what we're trying to achieve, makes it unreliable and much harder to reason about/complicated.

Also --miner.maxaccountsnum is set here in deployment: https://github.com/scroll-tech/testnet/blob/f459a3e4e558f456f3e60ffad07db70b8d2c0ad0/core/l2geth/l2geth_run.sh#L150

[georgehao]

Yes, by default worker.config.MaxAccountsNum = math.MaxInt but it's configurable with --miner.maxaccountsnum flag. This makes the result rely on side effects that don't have anything to do with what we're trying to achieve, makes it unreliable and much harder to reason about/complicated.

Also --miner.maxaccountsnum is set here in deployment: https://github.com/scroll-tech/testnet/blob/f459a3e4e558f456f3e60ffad07db70b8d2c0ad0/core/l2geth/l2geth_run.sh#L150

you are right

[georgehao]

So I'll follow @jonastheis 's approach, add another commit

[Thegaram]

This works, but Jonas's suggestion would be more robust.

[omerfirmak]

can we add a metric on how long statsWithMinBaseFee takes to run? Since it might interfere with worker, it is good to know if it is holding the txpool lock for a long period.

[georgehao]

can we add a metric on how long statsWithMinBaseFee takes to run? Since it might interfere with worker, it is good to know if it is holding the txpool lock for a long period.

add cd5ed7a