I'd propose to implement this slightly differently, more straightforward: set the default to invalid values (such as -1 or MAXINT or 0, whatever is most useless) then check for valid values in IsMiningSetup() instead of probing the arguments.

That'd probably make it clearer too, okay...

@laanwj That'd need making the variables globals again.. so maybe this is better after all?

Ugh, yes I now see that CreateNewBlock also queries and parse the arguments every time. That's the same as using globals, just in a hidden way.
At least I now understand your reasoning for doing it that way in IsMiningSetup.

The obvious downside of this approach being ..... once a miner has set them, we lose the ability to select smarter defaults, because whatever they pick could end up being used indefinitely.

"We" shouldn't be setting defaults anyway. That's the point.

If we think that the common settings become wrong for an option there is always the possibility of just renaming the option to force resetting it.

Rebased without "&& true"

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/03528791ea2b870595723642dbac74bfa4f0923e for binaries and test log.

This could happen for one of several reasons:

1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in qa/pull-tester)
2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
4. The test suite fails on either Linux i386 or Win32
5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

I think this needs a documentation update too. Miners upgrading to 0.9.0 will have no idea what to set the defaults to, so the documentation could suggest some sane default values (I get the irony here) and to sources of information with regard to setting the values.
We wouldn't want them to just put some nonsense values.

NAK

This is fundamentally "force X to think" which is unfriendly. We want miners to be thinking about this, agreed; But I see nothing wrong and much good by continuing to provide a sane default mining policy. Let's continue to play nice with our miner friends.

Seems consensus is against this (at least in the current state), so closing for now.

Huh? Most people seemed to support this from the comments... Just realised this was closed without merging :(

Rebased

This probably breaks a bunch of qa/rpc-tests (which run -regtest and which rely on defaults).

The same objections remain...

@jgarzik This does give a randomised selection of not-unreasonable settings.

@luke-jr AFAICS this change requires miners to have something set in their configs, which they may not have set before, by checking mapArgs.count()

Thus it breaks e.g. every miner, including p2pool miners, which do not have those settings in their config file.

Am I missing something?

@jgarzik No, that sounds about right. It breaks miners who have thus far been too negligent to configure their node, until they do so.

@gavinandresen Fixed QA tests by skipping config check on regtest/testnet.

NAK maintained.

This would seem to increase the barrier to entry for p2pool etc. miners and knowingly break currently-working configurations.

Just seems like way too many problems or oddities with this patch:

• Raises the bar for getting p2pool working: Breaks current documentation + requires users to perform lots of research vs. simply taking a useful default.
• Breaks existing, working configs.
• Picking random values for key settings is just odd, and runs counter to normal engineering practice. Less deterministic, makes testing more difficult, and more.

More generally, "pushy" changes that halt software operation until the user educates themselves to a higher degree are distasteful. Bullying the user into learning the intricacies of mining policy is not the way to go.

It is a lot of headache just to avoid providing a sane default.

Closing due to inactivity.

Yes, while I philosophically agree with this pull, and I'd rather get rid of any unnecessary default values any day, it's so inconvenient to miners that we can't really merge it.