Skip to content

*: add broadcast local txs feature#192

Merged
yysung1123 merged 1 commit into
indexer-v1.15.0from
yysung/broadcast-local-txs-1.15.0
Feb 11, 2025
Merged

*: add broadcast local txs feature#192
yysung1123 merged 1 commit into
indexer-v1.15.0from
yysung/broadcast-local-txs-1.15.0

Conversation

@yysung1123
Copy link
Copy Markdown

@yysung1123 yysung1123 commented Feb 11, 2025

ethereum#30559

The locals-tracking feature has been migrated from pool to TxTracker. As a result, pending local transactions should now be retrieved directly from TxTracker.

Data flow:

  • v1.14.13:
    handler -> txPool -> LegacyPool (subpool) -> filter pending by locals (contains the addresses that submitting local tx before)
  • v1.15.0:
    handler -> txPool -> TxTracker (PendingLocalTxsPublisher) -> query pending txs from LegacyPool and filter them by byAddr (contains the addresses that submitting local tx before)

@yysung1123 yysung1123 force-pushed the yysung/broadcast-local-txs-1.15.0 branch 3 times, most recently from 23223b2 to edd6b15 Compare February 11, 2025 06:43
Comment thread core/txpool/legacypool/legacypool.go Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

could this be removed?

Copy link
Copy Markdown
Author

@yysung1123 yysung1123 Feb 11, 2025

Choose a reason for hiding this comment

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

Here, I followed the approach of Rejournal.
store the config value in legacypool.Config and use it in eth/backend.go for TxTracker
ref.

rejournal := config.TxPool.Rejournal

Comment thread eth/backend.go
}
eth.localTxTracker = locals.New(config.TxPool.Journal, rejournal, eth.blockchain.Config(), eth.txPool, broadcastPendingLocalTx)
stack.RegisterLifecycle(eth.localTxTracker)
eth.txPool.PendingLocalTxsPublisher = eth.localTxTracker
Copy link
Copy Markdown

@vicky-sunshine vicky-sunshine Feb 11, 2025

Choose a reason for hiding this comment

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

Just a note on the main changes in this PR:

The key refactor is in txPool's SubscribePendingLocalTxsEvent:

  • Before: txPool called subPools.SubscribePendingLocalTxsEvent when executing SubscribePendingLocalTxsEvent.
  • After: It now calls PendingLocalTxsPublisher.SubscribePendingLocalTransactions, which is backed by localTxTracker.

Since localTxTracker is optional and the handler only has txPool (but not localTxTracker), we had to pass eth.localTxTracker into txPool at L257.

While the data flow feels a bit indirect (handler → txPool → localTxTracker, instead of handler → localTxTracker), I couldn’t immediately think of a cleaner solution without making broader changes.

Overall, I think this approach is fine, so I’ll approve this.

Reference of old customized commit: bf10d46#diff-4d08b06f78070fa573caaaa4b6b56dd5cc7bbeb03b04d1b93cafa167fc5577e5R374

Copy link
Copy Markdown
Author

@yysung1123 yysung1123 Feb 11, 2025

Choose a reason for hiding this comment

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

Thanks for organizing!

The data flow:

  • before v1.14.13:
    handler -> txPool -> LegacyPool (subpool) -> filter pending by locals (contains the addresses that submitting local tx before)
  • after v1.15.0:
    handler -> txPool -> TxTracker (PendingLocalTxsPublisher) -> query pending txs from LegacyPool and filter them by byAddr (contains the addresses that submitting local tx before)

Comment thread core/txpool/locals/tx_tracker.go Outdated
case <-pendingLocalTxs.C:
lTxs := types.Transactions{}
for addr, lazyTxs := range tracker.pool.Pending(txpool.PendingFilter{OnlyPlainTxs: true}) {
if tracker.byAddr[addr] == nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

check if key exists instead of checking value?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

updated

@yysung1123 yysung1123 force-pushed the yysung/broadcast-local-txs-1.15.0 branch from edd6b15 to 07fb79c Compare February 11, 2025 08:10
Copy link
Copy Markdown

@markya0616 markya0616 left a comment

Choose a reason for hiding this comment

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

LGTM

@yysung1123 yysung1123 merged commit f145fc3 into indexer-v1.15.0 Feb 11, 2025
yysung1123 pushed a commit that referenced this pull request Oct 24, 2025
## Why this should be merged

Addition of recent, functionality constitutes a bump to the minor
version. Doing this on `main` first for consistency and will cut a
release candidate after.

## How this works

Magic

## How this was tested

Super magic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants