Running some more fuzzing tests, I noticed a bug in the new getmininginfo information:
gavin$ ./bitcoind -datadir=testnet-box/3 getmininginfo { "blocks" : 218, "currentblocksize" : 1000, "currentblocktx" : 0, "difficulty" : 0.12500000, "errors" : "", "generate" : true, "genproclimit" : 1, "hashespersec" : 0, "pooledtx" : 18446744073709551613, "testnet" : true }
I think hashespersec should be greater than zero, also...
Does #766 fix it?
Haven't tried #766. Test setup is:
3 testnet-in-a-box nodes, with node 1 sending fuzzed transactions to 2. Node 3 is listening in and CPU mining.
valgrind doesn't complain about uninitialized vars (you have to compile openssl and bdb specially to make valgrind not trip on them), and getmininginfo returns reasonable info at first.
But it is reproducible, pooledtx goes wacky after a while.
My best guess is a missing-lock condition reading the variable from getmininginfo while the other thread is updating it. Does it go back to normal after the misread?
hashespersec should be zero, unless the internal reference miner is running.
I guess generate=true, but are all the blocks downloaded?
In the dup I just closed I noticed that the pooledtx went crazy around the same time as a new block was found.
The 'crazy' number 18446744073709551613 is in fact just -3 printed as an unsigned 64 bit int.
The bug is caused by CTransaction::RemoveFromMemoryPool() doing --nPooledTx for every transaction in the newly found block whether or not it was in our memory pool or not.
CBlock::SetBestChain does this: // Delete redundant memory transactions BOOST_FOREACH(CTransaction& tx, vtx) tx.RemoveFromMemoryPool();
and CTransaction::RemoveFromMemoryPool() doesn't check whether we have each transaction or not - it erase()s it anyway, and decrements the counter.
How often does it happen that it tried to remove a transaction that wasn't in the memory pool?
It doesn't. My change comment was sloppy.
oh... what would have been the unsloppy comment?
"Only attempt to remove transactions from memory pool when [...]" would be better.
When a new block is found, the code was attempting to remove each of the transactions in the block from the memory pool of pending transactions. This would quietly succeed, doing nothing, for transactions which weren't in the memory pool, but it was decrementing the pool size counter even if it didn't remove the transaction, leading to the pool size counter wrapping around to a huge value.
So my change checks to see if each transaction is in the memory pool before attempting to remove it from the pool.
ah, thanks. now I understand. I guess this could be a location in the code where I could make a patch that restricts relaying blocks that are not including free transactions. I think it would be good to give this option to full node operators who aren't mining. So far various patches have been included that help raise mining fees. Such a patch would help to encourage miners to include free txs thereby lowering mining fees. I believe it should be down to the node operators to decide/vote on mining fees rather than the developers. I suspect that the value of bitcoin could be affected by such a change. Perhaps the more mining fees go up, the higher the value of a bitcoin increases too? If so, that would explain a motivation for the developers (who probably own substantial bitcoins) to see the mining fees increase.
This isn't the place to discuss the issue.
I would suggest making a thread in the development board here: https://bitcointalk.org/index.php?board=6.0 to discuss it.
[ But I doubt reducing mining fees from their currently very low level would have any impact on the value of bitcoins. A concern is that unless transaction fees increase to match the falling rewards for mining, many miners will stop mining, leaving the network more vulnerable to attack. ]