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

Support -checkmempool=N, which runs checks once every N transactions #6776

Merged
merged 1 commit into from Oct 28, 2015

Conversation

Projects
None yet
8 participants
@sipa
Member

sipa commented Oct 7, 2015

No description provided.

@@ -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);

This comment has been minimized.

@dcousens

dcousens Oct 8, 2015

Contributor

I feel like this would be clearer as two (or at least one) if conditionals.

@dcousens

dcousens Oct 8, 2015

Contributor

I feel like this would be clearer as two (or at least one) if conditionals.

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Oct 8, 2015

Contributor

What is the behaviour currently? How often does a sanity check run right now?

Contributor

dcousens commented Oct 8, 2015

What is the behaviour currently? How often does a sanity check run right now?

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Oct 8, 2015

Member
Member

sipa commented Oct 8, 2015

if (nCheckFrequency == 0)
return;
if (insecure_rand() >= nCheckFrequency)

This comment has been minimized.

@dcousens

dcousens Oct 8, 2015

Contributor

What is the range of insecure_rand?

edit:2^32?

@dcousens

dcousens Oct 8, 2015

Contributor

What is the range of insecure_rand?

edit:2^32?

This comment has been minimized.

@sipa

sipa Oct 20, 2015

Member

Yes.

@sipa

sipa Oct 20, 2015

Member

Yes.

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Oct 8, 2015

Contributor

utACK

Contributor

dcousens commented Oct 8, 2015

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; }

This comment has been minimized.

@jonasschnelli

jonasschnelli Oct 8, 2015

Member

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?

@jonasschnelli

jonasschnelli Oct 8, 2015

Member

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?

This comment has been minimized.

@sipa

sipa Oct 8, 2015

Member
@sipa

sipa via email Oct 8, 2015

Member

This comment has been minimized.

@sipa

sipa Oct 8, 2015

Member
@sipa

sipa via email Oct 8, 2015

Member

This comment has been minimized.

@Diapolo

Diapolo Oct 31, 2015

IMHO this value should've taken a small comment :).

@Diapolo

Diapolo Oct 31, 2015

IMHO this value should've taken a small comment :).

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Oct 8, 2015

Member

Nice.
utACK (code review).

Travis issue is unrelated.

Member

jonasschnelli commented Oct 8, 2015

Nice.
utACK (code review).

Travis issue is unrelated.

@laanwj laanwj added the Mempool label Oct 8, 2015

@sdaftuar

This comment has been minimized.

Show comment
Hide comment
@sdaftuar

sdaftuar Oct 20, 2015

Member

ACK. Looks like travis needs to be bumped?

Member

sdaftuar commented Oct 20, 2015

ACK. Looks like travis needs to be bumped?

@morcos

This comment has been minimized.

Show comment
Hide comment
@morcos

morcos Oct 20, 2015

Member

utACK

Member

morcos commented Oct 20, 2015

utACK

@sipa sipa merged commit ab1f560 into bitcoin:master Oct 28, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

sipa added a commit that referenced this pull request Oct 28, 2015

Merge pull request #6776
ab1f560 Support -checkmempool=N, which runs checks on average once every N transactions (Pieter Wuille)
@gmaxwell

This comment has been minimized.

Show comment
Hide comment
@gmaxwell

gmaxwell Oct 28, 2015

Member

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.

Member

gmaxwell commented Oct 28, 2015

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

Auto merge of #2940 - str4d:nu-activation-mempool-expiry, r=str4d
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.

zkbot added a commit to zcash/zcash that referenced this pull request Feb 20, 2018

Auto merge of #2940 - str4d:nu-activation-mempool-expiry, r=str4d
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment