Support -checkmempool=N, which runs checks once every N transactions #6776
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -837,7 +837,10 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) | |||
| InitWarning(_("Warning: Unsupported argument -benchmark ignored, use -debug=bench.")); | |||
|
|
|||
| // Checkmempool and checkblockindex default to true in regtest mode | |||
| mempool.setSanityCheck(GetBoolArg("-checkmempool", chainparams.DefaultConsistencyChecks())); | |||
| int ratio = std::min<int>(std::max<int>(GetArg("-checkmempool", chainparams.DefaultConsistencyChecks() ? 1 : 0), 0), 1000000); | |||
There was a problem hiding this comment.
I feel like this would be clearer as two (or at least one) if conditionals.
|
What is the behaviour currently? How often does a sanity check run right now? |
|
Right now runs for every transaction if -checkmempool is specified,
otherwise never.
So the behaviour for =0 and =1 remains the same, but higher number can be
used for lower test frequency.
|
| if (nCheckFrequency == 0) | ||
| return; | ||
|
|
||
| if (insecure_rand() >= nCheckFrequency) |
There was a problem hiding this comment.
What is the range of insecure_rand?
edit:2^32?
|
utACK |
| @@ -338,7 +338,7 @@ class CTxMemPool | |||
| * check does nothing. | |||
| */ | |||
| void check(const CCoinsViewCache *pcoins) const; | |||
| void setSanityCheck(bool _fSanityCheck) { fSanityCheck = _fSanityCheck; } | |||
| void setSanityCheck(double dFrequency = 1.0) { nCheckFrequency = dFrequency * 4294967296.0; } | |||
There was a problem hiding this comment.
Not sure: but would't it be more clean to use ULONG_MAX here? Also not sure: if using 4294967296 instead of 4294967295(= ULONG_MAX) wouldn't this result in autocast to uint64_t?
There was a problem hiding this comment.
Note the .0, making it a double constant rather than an int.
There was a problem hiding this comment.
Also, ULONG_MAX gives the maximum value of an unsigned long. Here we need
the number of possible values in a uint32_t. That is by definition 2^32.
There was a problem hiding this comment.
IMHO this value should've taken a small comment :).
|
Nice. Travis issue is unrelated. |
|
ACK. Looks like travis needs to be bumped? |
|
utACK |
sipa
added a commit
that referenced
this pull request
Oct 28, 2015
ab1f560 Support -checkmempool=N, which runs checks on average once every N transactions (Pieter Wuille)
|
This has a bug: -checkmempool=1 never checks due to integer overflow in the nCheckFrequency= dFrequency*4294967296 and nCheckFrequency is uint32_t, so the result ends up 0. |
zkbot
added a commit
to zcash/zcash
that referenced
this pull request
Feb 20, 2018
Mempool improvements, branch ID awareness Whenever the local chain tip is updated, transactions in the mempool which commit to an unmineable branch ID (for example, just before a network upgrade activates, where the next block will have a different branch ID) will be removed. Includes commits cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6654 - Only the mempool index change. - bitcoin/bitcoin#6776 - bitcoin/bitcoin#7020 - bitcoin/bitcoin#6915 Part of #2074.
1 task
furszy
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Jun 14, 2020
2105947 Implement helper class for CTxMemPoolEntry constructor (Alex Morcos) 1cef905 Make -checkmempool=1 not fail through int32 overflow (Pieter Wuille) 0f72ff2 Support -checkmempool=N, which runs checks on average once every N transactions (Pieter Wuille) 89483d0 [Bug] Make operator() a const function in CompareTxMemPoolEntryByX (random-zebra) a50ad77 Lower default policy limits (random-zebra) 03f7152 fix locking issue with new mempool limiting (random-zebra) 1598961 Fix stale comment in CTxMemPool::TrimToSize. (random-zebra) 98d0d68 Undo GetMinFee-requires-extra-call-to-hit-0 (random-zebra) 6ad6ee6 Add reasonable test case for mempool trimming (random-zebra) 8dcbb7e Only call TrimToSize once per reorg/blocks disconnect (random-zebra) c20cd38 Implement on-the-fly mempool size limitation. (random-zebra) aee2e17 Print mempool size in KB when adding txn (random-zebra) f7c85fd Add CFeeRate += operator (random-zebra) 5bd2a00 Track (and define) ::minRelayTxFee in CTxMemPool (random-zebra) 0b50f6c Add Mempool Expire function to remove old transactions (random-zebra) d26f5e0 Fix calling mempool directly, instead of pool, in ATMP (random-zebra) fc5eddb Reverse the sort on the mempool's feerate index (random-zebra) 0ce1df0 [BUG] Fix CTxMemPool::check excluding zerocoins from children checks (random-zebra) 1f7bd52 Track transaction packages in CTxMemPoolEntry (random-zebra) 1fd406b TxMemPool: Change mapTx to a boost::multi_index_container (random-zebra) Pull request description: built on top of - [x] #1645 This PR pulls some updates from upstream in the mempool area, adding the required adjustments for legacy zerocoin txes and updating the functional test suite. Specifically, here we: - track mempool descendants (in-mempool transactions that depend on other mempool transactions) - turn `mapTx` into a `boost::multi_index_container` that sorts the mempool on 3 criteria: - transaction hash - fee rate - time in the mempool - Add a max size for the mempool (throwing away the cheapest txs and bumping the min relay fee, when full) - Implement on-the-fly mempool size limit with the flag `-maxmempool` - Implement `-checkmempool=N` to customize the frequency of the mempool check - Implement helper for `CTxMemPoolEntry` for the unit tests. Backports: - bitcoin#6654 - bitcoin#6722 [`*`] - bitcoin#6889 - bitcoin#6771 - bitcoin#6776 - bitcoin#6896 - bitcoin#7020 [`*`] excluding bitcoin@9e93640 as our default minimum tx fee rate of 10k satoshis is only 0,00003 USD at the time of writing. ACKs for top commit: Fuzzbawls: utACK 2105947 furszy: Re utACK 2105947 and merging this nice upgrade :) . Tree-SHA512: 51a7d75bd52f7646d461252c78f0dd9d7e8b5c1c66c22944120bfe293b28f5d48135de339ebf3d8a5b4c61ca5452383ed1b10c417be06dc4a335ac645842ea14
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.