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

[R4R]reannounce local pending transactions #570

Merged
merged 3 commits into from
Nov 25, 2021
Merged

Conversation

KeefeL
Copy link
Contributor

@KeefeL KeefeL commented Nov 19, 2021

Description

When the network traffic is heavy, transactions that have been broadcast may be truncated in peers' tx_pools, while these local pending transactions have been marked as known, the source node won't broadcast them again, so they have no chance to be mined.

We add a timer to check whether the local pending transactions stay in tx_pool exceed a threshold, once transactions exceed the threshold, they will be announced to some peers again. The TxPool.ReannounceTime is used to control the threshold, the default value of TxPool.ReannounceTime is too big to be exceeded, users can config this parameter to enable this feature.

Also need to pay attention, this feature only announce the local pending transactions. If your node is configured TxPool.NoLocals=false, then the transactions sent to this RPC node will be treated as local transactions, otherwise you should add your addresses to the TxPool.Locals, only the transactions from the local addresses will be treated as local transactions.

Rationale

This feature is introduced to handle some extreme situations, it won't affect the original function, and it is disabled by default. Users can enable this feature by setting TxPool.ReannouceTime according to their needs.

Example

Set TxPool.ReannounceTime to 5 minutes:
geth --config ./config.toml --datadir ./node --cache 8000 --rpc.allow-unprotected-txs --txlookuplimit 0 --txpool.reannouncetime 5m

Changes

Notable changes:

  • add --txpool.reannouncetime parameter, default value is 10 years, minimum value is 1 minute.

@unclezoro unclezoro changed the title reannouce local pending transactions [WIP]reannouce local pending transactions Nov 21, 2021
@KeefeL KeefeL changed the title [WIP]reannouce local pending transactions reannouce local pending transactions Nov 22, 2021
@KeefeL KeefeL changed the title reannouce local pending transactions reannounce local pending transactions Nov 22, 2021
@KeefeL KeefeL changed the title reannounce local pending transactions [R4R]reannounce local pending transactions Nov 24, 2021
@@ -398,6 +398,11 @@ var (
Usage: "Maximum amount of time non-executable transaction are queued",
Value: ethconfig.Defaults.TxPool.Lifetime,
}
TxPoolReannounceTimeFlag = cli.DurationFlag{
Name: "txpool.reannouncetime",
Usage: "Minimum amount of time for announcing local pending transactions again",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minimum amount of time sound weried.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

core/tx_pool.go Show resolved Hide resolved
core/tx_pool.go Show resolved Hide resolved
core/tx_pool.go Show resolved Hide resolved
@KeefeL KeefeL merged commit fbc52de into bnb-chain:develop Nov 25, 2021
This was referenced Nov 29, 2021
j75689 pushed a commit that referenced this pull request Nov 29, 2021
* reannouce local pending transactions

* add tests for tx_pool reannouce local pending transactions

* add tests for handler reannounce local pending transactions
KeefeL added a commit to KeefeL/bsc that referenced this pull request Jun 6, 2022
* reannouce local pending transactions

* add tests for tx_pool reannouce local pending transactions

* add tests for handler reannounce local pending transactions
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.

None yet

4 participants