-
Notifications
You must be signed in to change notification settings - Fork 249
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
Add cli option for limit-per-account in the mempool #815
Add cli option for limit-per-account in the mempool #815
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #815 +/- ##
==========================================
+ Coverage 62.08% 62.53% +0.44%
==========================================
Files 203 203
Lines 18723 18731 +8
==========================================
+ Hits 11624 11713 +89
+ Misses 5997 5920 -77
+ Partials 1102 1098 -4 ☔ View full report in Codecov by Sentry. |
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 👍
Do you think it is worth creating a test for the limit per account ?
would be nice to have for sure, just to ensure that this change works as expected |
Do you mean e2e tests or unit tests. Because there are already some unit tests for the account limit. |
Yeah, It guess that's fine, not worth the whole setup for the tx per account test. |
This PR makes the already existing flags
txpool-limit-per-account
(default 16)txpool-limit
(default 10k)which were previously solo only flags available in testnet and mainnet modes. No unbounded option was added but we can discuss if that is necessary.