Skip to content

qa: Remove redundant checkmempool/checkblockindex extra_args #13913

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 11, 2018
Merged

qa: Remove redundant checkmempool/checkblockindex extra_args #13913

merged 1 commit into from
Aug 11, 2018

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Aug 8, 2018

They are already enabled by default for regtest:

bitcoin/src/init.cpp

Lines 1002 to 1007 in df9f712

// Checkmempool and checkblockindex default to true in regtest mode
int ratio = std::min<int>(std::max<int>(gArgs.GetArg("-checkmempool", chainparams.DefaultConsistencyChecks() ? 1 : 0), 0), 1000000);
if (ratio != 0) {
mempool.setSanityCheck(1.0 / ratio);
}
fCheckBlockIndex = gArgs.GetBoolArg("-checkblockindex", chainparams.DefaultConsistencyChecks());

Closes #13912. CC #12138

@maflcko maflcko added the Tests label Aug 8, 2018
@Empact
Copy link
Contributor

Empact commented Aug 8, 2018

Concept ACK. Could be interesting to disable these settings and see which tests require them.

@domob1812
Copy link
Contributor

utACK fa31ca0.

@Empact: Why should any tests "require" these settings at all? They just make sure the tests enforce more things, no?

@Empact
Copy link
Contributor

Empact commented Aug 11, 2018 •
edited
Loading

utACK fa31ca0

@domob1812 I'm not that familiar with the checks, e.g. wrt to whether they have any side-effects, so I was curious to explore the counterfactual - if the checks are explicitly active now, and removing the explicit activation leaves them active by default, what happens if the default changes? Ran the affected tests with regtest fDefaultConsistencyChecks = false, and each passed, so seems good to me.

@Empact Empact mentioned this pull request Aug 11, 2018
Closed
@maflcko maflcko merged commit fa31ca0 into bitcoin:master Aug 11, 2018
maflcko pushed a commit that referenced this pull request Aug 11, 2018
Merge #13913: qa: Remove redundant checkmempool/checkblockindex extra…
a08533c 
…_args

fa31ca0 qa: Remove redundant checkmempool/checkblockindex extra_args (MarcoFalke)

Pull request description:

  They are already enabled by default for regtest:

  https://github.com/bitcoin/bitcoin/blob/df9f71274645a917e2578c52a1c59745bce8112d/src/init.cpp#L1002-L1007

  Closes  #13912. CC #12138

Tree-SHA512: b11a3e8cc4715569f917ab89132f8c8dcae64aebcd7a34182675f86cf7f6e207e3187b7ea01a56c92c8c3af76122b6b995e84f533e134676863f8953dc1f0574
@maflcko maflcko deleted the Mf1808-qaRedundantChecks branch August 11, 2018 10:59
@Bushstar Bushstar mentioned this pull request Aug 13, 2018
Merged
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 27, 2021
@PastaPastaPasta
Merge bitcoin#13913: qa: Remove redundant checkmempool/checkblockinde…
81bf95e 
…x extra_args

fa31ca0 qa: Remove redundant checkmempool/checkblockindex extra_args (MarcoFalke)

Pull request description:

  They are already enabled by default for regtest:

  https://github.com/bitcoin/bitcoin/blob/df9f71274645a917e2578c52a1c59745bce8112d/src/init.cpp#L1002-L1007

  Closes  bitcoin#13912. CC bitcoin#12138

Tree-SHA512: b11a3e8cc4715569f917ab89132f8c8dcae64aebcd7a34182675f86cf7f6e207e3187b7ea01a56c92c8c3af76122b6b995e84f533e134676863f8953dc1f0574
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 28, 2021
@PastaPastaPasta
Merge bitcoin#13913: qa: Remove redundant checkmempool/checkblockinde…
2341749 
…x extra_args

fa31ca0 qa: Remove redundant checkmempool/checkblockindex extra_args (MarcoFalke)

Pull request description:

  They are already enabled by default for regtest:

  https://github.com/bitcoin/bitcoin/blob/df9f71274645a917e2578c52a1c59745bce8112d/src/init.cpp#L1002-L1007

  Closes  bitcoin#13912. CC bitcoin#12138

Tree-SHA512: b11a3e8cc4715569f917ab89132f8c8dcae64aebcd7a34182675f86cf7f6e207e3187b7ea01a56c92c8c3af76122b6b995e84f533e134676863f8953dc1f0574
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 29, 2021
@PastaPastaPasta
Merge bitcoin#13913: qa: Remove redundant checkmempool/checkblockinde…
5edf469 
…x extra_args

fa31ca0 qa: Remove redundant checkmempool/checkblockindex extra_args (MarcoFalke)

Pull request description:

  They are already enabled by default for regtest:

  https://github.com/bitcoin/bitcoin/blob/df9f71274645a917e2578c52a1c59745bce8112d/src/init.cpp#L1002-L1007

  Closes  bitcoin#13912. CC bitcoin#12138

Tree-SHA512: b11a3e8cc4715569f917ab89132f8c8dcae64aebcd7a34182675f86cf7f6e207e3187b7ea01a56c92c8c3af76122b6b995e84f533e134676863f8953dc1f0574
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 29, 2021
@PastaPastaPasta
Merge bitcoin#13913: qa: Remove redundant checkmempool/checkblockinde…
d42488e 
…x extra_args

fa31ca0 qa: Remove redundant checkmempool/checkblockindex extra_args (MarcoFalke)

Pull request description:

  They are already enabled by default for regtest:

  https://github.com/bitcoin/bitcoin/blob/df9f71274645a917e2578c52a1c59745bce8112d/src/init.cpp#L1002-L1007

  Closes  bitcoin#13912. CC bitcoin#12138

Tree-SHA512: b11a3e8cc4715569f917ab89132f8c8dcae64aebcd7a34182675f86cf7f6e207e3187b7ea01a56c92c8c3af76122b6b995e84f533e134676863f8953dc1f0574
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 29, 2021
@PastaPastaPasta
Merge bitcoin#13913: qa: Remove redundant checkmempool/checkblockinde…
fa57557 
…x extra_args

fa31ca0 qa: Remove redundant checkmempool/checkblockindex extra_args (MarcoFalke)

Pull request description:

  They are already enabled by default for regtest:

  https://github.com/bitcoin/bitcoin/blob/df9f71274645a917e2578c52a1c59745bce8112d/src/init.cpp#L1002-L1007

  Closes  bitcoin#13912. CC bitcoin#12138

Tree-SHA512: b11a3e8cc4715569f917ab89132f8c8dcae64aebcd7a34182675f86cf7f6e207e3187b7ea01a56c92c8c3af76122b6b995e84f533e134676863f8953dc1f0574
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 29, 2021
@PastaPastaPasta
Merge bitcoin#13913: qa: Remove redundant checkmempool/checkblockinde…
f33c2c8 
…x extra_args

fa31ca0 qa: Remove redundant checkmempool/checkblockindex extra_args (MarcoFalke)

Pull request description:

  They are already enabled by default for regtest:

  https://github.com/bitcoin/bitcoin/blob/df9f71274645a917e2578c52a1c59745bce8112d/src/init.cpp#L1002-L1007

  Closes  bitcoin#13912. CC bitcoin#12138

Tree-SHA512: b11a3e8cc4715569f917ab89132f8c8dcae64aebcd7a34182675f86cf7f6e207e3187b7ea01a56c92c8c3af76122b6b995e84f533e134676863f8953dc1f0574
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 29, 2021
@PastaPastaPasta
Merge bitcoin#13913: qa: Remove redundant checkmempool/checkblockinde…
b04090d 
…x extra_args

fa31ca0 qa: Remove redundant checkmempool/checkblockindex extra_args (MarcoFalke)

Pull request description:

  They are already enabled by default for regtest:

  https://github.com/bitcoin/bitcoin/blob/df9f71274645a917e2578c52a1c59745bce8112d/src/init.cpp#L1002-L1007

  Closes  bitcoin#13912. CC bitcoin#12138

Tree-SHA512: b11a3e8cc4715569f917ab89132f8c8dcae64aebcd7a34182675f86cf7f6e207e3187b7ea01a56c92c8c3af76122b6b995e84f533e134676863f8953dc1f0574
@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.
Reviewers
No reviews
Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@maflcko @Empact @domob1812