Verified as functional on testnet. (You need a new transaction to enter mempool after running bitcoin-cli setblockmaxsize x and before running getblocktemplate, otherwise GBT will just use the cached template.)
This patch was discussed briefly on IRC about 20 hours ago. http://bitcoinstats.com/irc/bitcoin-dev/logs/2015/11/29
607 | + 608 | + LOCK(cs_main); // need to block against getblocktemplate 609 | + 610 | + uint64_t size = nBlockMaxSize; 611 | + if (params[0].get_real() >= 1000) 612 | + size = (params[0].get_int64()); // rejects 0.0 amounts
nit: why the extra parens?
No good reason. Will fix.
Oh, and another thing right next to it: Bitcoin Core uses unsigned ints for nBlockMaxSize, not uint64_t. This shouldn't cause any functional problems as written right now, but it's still probably worth fixing.
@pstratem RE: per GBT call parameter: Possible, but I don't see the use-case for that being compelling enough to overcome my laziness. I think that runtime blockmaxsize adjustment will usually be done by a human pool operator in order to tweak and optimize block propagation times. This would be easiest to perform as a fire-once-and-forget kind of tweak. The option you're suggesting would be useful for pools that want to generate e.g. empty blocks immediately after receiving a block from a peer. That would be useful, but not quite useful enough for me to want to implement it myself.
606 | + ); 607 | + 608 | + LOCK(cs_main); // need to block against getblocktemplate 609 | + 610 | + uint64_t size = nBlockMaxSize; 611 | + if (params[0].get_real() >= 1000)
This accepts unbounded size? What happens when it's above 1MB?
I expect it will make CreateNewBlock fail its own sanity check.
miner.cpp:131: nBlockMaxSize = std::max((unsigned int)1000, std::min((unsigned int)(MAX_BLOCK_SIZE-1000), nBlockMaxSize));
If you pass a value larger than 1000000 to setblockmaxsize, then the variable will be set to that large value. However, when it's used, it's clamped to MAX_BLOCK_SIZE-1000, so block creation continues unimpeded with valid values.
The check against an excessively low block size (< 1000 bytes) wasn't necessary either. I mostly put it in because I didn't want to think about what might go wrong with negative values.
Agree, not really an issue (though the RPC could fail to warn the user that it's not going to do what he requests).
I guess a warning would be useful for clarity about its purpose at least in the help doc.
Can you rename to setmaxblocksize?
Can you rename to setmaxblocksize?
Only if we want to have a different name for the RPC and for the internal variable and command line switch, or if we want to rename the internal variable and command line switch.
Hmm forget about renaming. It is messed up in the master already...
125 | @@ -125,8 +126,7 @@ CBlockTemplate* CreateNewBlock(const CChainParams& chainparams, const CScript& s 126 | pblocktemplate->vTxFees.push_back(-1); // updated at end 127 | pblocktemplate->vTxSigOps.push_back(-1); // updated at end 128 | 129 | - // Largest block you're willing to create: 130 | - unsigned int nBlockMaxSize = GetArg("-blockmaxsize", DEFAULT_BLOCK_MAX_SIZE); 131 | + // Largest block you're willing to create 132 | // Limit to betweeen 1K and MAX_BLOCK_SIZE-1K for sanity: 133 | nBlockMaxSize = std::max((unsigned int)1000, std::min((unsigned int)(MAX_BLOCK_SIZE-1000), nBlockMaxSize));
This line now mutates a global variable. I think you should make a copy first.
It's a "global" variable that is used in exactly one function, which is the same function that you're looking at here (just down 100 lines or so), and part of the same critical section. It's locked with the same lock (cs_main) as the code that mutates it in the RPC I created.
Anyway, if you don't like the way it looks, I can take a look at it again in a few days. Kinda busy with other stuff right now, though.
@jtimon as this is literally only used CreateNewBlock can you also add a parameter to override per getblocktemplate call ?
I also like that solution more. Especially as getblocktemplate is designed to be extended with extra parameters by taking an object, instead of having to tack on another positional argument.
Changing global parameters in RPC calls is something I'd like to avoid, especially if they only apply "hidden parameters" to other RPC calls instead of change processes going on in the background. Stateful APIs are something commonly regretted, it complicates concurrent usage, for example.
laanwj if we make it an argument to CreateNewBlock, then it's of no direct use to sysadmins until pool software is rewritten to take advantage of it. Even then, the use-case that I had in mind (being able to change the block size soft limit from the command line without restarting any processes) would only be possible if the pool software implements something basically the same as what I just implemented, an RPC that mutates state, but in the pool software instead of bitcoind.
As a miner, I've wanted this functionality several times, so I eventually implemented it. A pool developer might want the functionality of having an argument to getblocktemplate, but that's a different feature for a different use-case.
Rebase needed.
Hmm, isn't better to create one generic interface?
# Bigger blocks...
setvariable blockmaxsize 8000000
# Enlarge mempool 300M -> 2G
setvariable maxmempool 2000
# Allow less connections
setvariable maxconnections 4
etc.?
That would be awesome, but would really require a total rewrite of the argument parsing code, as it means reevaluation of interaction with other arguments, constraints, changing global variables (which often were immutable before and thus didn't need a lock but now do...).
I'd really like to see the ability to set global variables via RPC calls.