Skip to content

Conversation

@jonasschnelli
Copy link
Contributor

Reduces checkblocks default to 144 blocks (MIN_BLOCKS_TO_KEEP / 2).

Introduce -checkmaxtx with a default of 100'000 transaction to set a upper bound of amount of transaction to check during the CVerifyDB process.

There is also a new constant MIN_BLOCK_TO_RESPECT_WITH_MAX_TX_CHECK which defines the minimum amount of blocks to check even if the -checkmaxtx check has been fulfilled.

@dcousens
Copy link
Contributor

dcousens commented Jun 2, 2016

Perhaps we should also reduce the number 288. Maybe a limit specified in number of transactions is better?

Or perhaps a limit specified in size on disk? Last 288MB as an example.

if (ShutdownRequested())
return true;
nTxCheckLimitCurrent+=block.vtx.size();
if (nTxCheckLimit > 0 && nTxCheckLimitCurrent >= nTxCheckLimit && chainActive.Height() - pindex->nHeight >= std::min(nCheckDepth, MIN_BLOCK_TO_RESPECT_WITH_MAX_TX_CHECK)) {
Copy link
Member

Choose a reason for hiding this comment

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

The std::min isn't necessary, as chainActive.Height() - pindex->nHeight will never exceed nCheckDepth (we bail out of the loop before that can happen).

@kazcw
Copy link
Contributor

kazcw commented Jun 5, 2016

The relationship between -checkblocks and -checkmaxtx is not clear from the help text. It looks like as implemented it will stop when either limit is reached, modulo the MIN_BLOCK_TO_RESPECT_WITH_MAX_TX_CHECK logic (i.e. they are both maxes). Unless there's a particular benefit to the max-based interface, I think I would prefer if they were both mins, which would also obviate MIN_BLOCK_TO_RESPECT_WITH_MAX_TX_CHECK.

@sipa
Copy link
Member

sipa commented Aug 25, 2016

You could also just get rid of MIN_BLOCK_TO_RESPECT_WITH_MAX_TX_CHECK. The theoretical maximum number of transactions in a block is 16665 anyway, so 100000 transactions effectively already requires 7 blocks anyway. If someone wants to set a lower number of transactions, so bit it.

That would simplify this PR, and have clearer semantics.

@sipa
Copy link
Member

sipa commented Aug 31, 2016

Closing this as superseded by #8611.

@sipa sipa closed this Aug 31, 2016
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants