-
Notifications
You must be signed in to change notification settings - Fork 867
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
Prioritize with nonce distance #2505
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments
...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java
Outdated
Show resolved
Hide resolved
...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
…ort keys aren't working as I expect Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
…l check for dropping Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
…ders Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
Signed-off-by: Justin Florentine <justin.florentine@consensys.net>
e5a4b55
to
d6162a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Waiting till after london to merge, and then observe running for a while. |
@@ -436,7 +432,7 @@ public void transactionDroppedEventDoesNotFireAfterUnsubscribe() { | |||
transactionPool.addLocalTransaction(TX2); | |||
|
|||
assertThat(result.get()).isNotNull(); | |||
serviceImpl.removeTransactionAddedListener(id); | |||
serviceImpl.removeTransactionDroppedListener(id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢
This reverts commit 1f4f131.
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: Justin Florentine <justin.florentine@consensys.net> Co-authored-by: matkt <karim.t2am@gmail.com>
PR description
Prevent multiple transactions from the same address from evicting others which are waiting in a full transaction pool. When we have to evict a transaction from a full pool, we now also consider how far it is from the lowest nonce from the same address already in the pool. We are also increasing the size of the pending transaction pool, so it should support the new 10MB maximum message size previously introduced by pr #2485 .
Fixed Issue(s)
Changelog
Default value for
--tx-pool-max-size
is now 32K.