miner debugging: faux-mining

theuni commented at 5:32 pm on October 23, 2016: member

I’m not sure if this actually makes sense to include in the mainline code, but I’m PRing here in case others think it may be useful. It has proven to be extremely helpful for me in the last few days while testing various mining software.

This enables anyone to “mine” on any chain, including mainnet in realtime, in order to test mining software. The desired difficulty is set via a bitcoind param -forceminerdifficulty=n. The difficulty is set in bitcoind rather than via a new rpc command or getblocktemplate/submitblock in order to facilitate testing mining/pool software that may be hard to change (for example, in embedded hardware, or pool software running on a different machine).

Backstory

I was recently working on adding segwit commitments to some existing mining software. After doing basic sanity checks, I let it run on testnet (where the difficulty is currently very high) for a few days. I began to notice a subtle race condition that caused a small percentage of blocks to be rejected, with no obvious cause for the breakage.

I didn’t think that my code could’ve caused the problem, and ordinarily I would just re-run on the unpatched code. Problem was, testnet3 has activated segwit, and the unpatched miner could not insert the commitment. So the only way to really simulate (without manually creating lots of peers and transactions in order to stumble upon the problem) would be to mine on mainnet. Obviously that wasn’t realistic.

Solution

Mine on mainnet with a specified difficulty, ignore the work requirement, validate, report success, then throw the block away. This turned out to work beautifully. The time it took me to reproduce the error dropped from ~10hrs to ~30sec.

In this case, specifying the difficulty, rather than simply setting it to the minimum, was crucial. The race condition occurred only after rolling the extranonce a few times.

I believe this approach should work universally. Mining/Pooling software gets a little cranky about stale blocks, but otherwise works fine.

fanquake added the label Mining on Oct 24, 2016

laanwj commented at 7:26 am on October 24, 2016: member

Concept ACK.

Although there looks to be a lot of code (as well as argument changes) to force powLimit to be interpreted as (“7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff”).

What would be the disadvantage of just changing ConsensusParams.powLimit to that value in this case?

Also: maybe this should be allowed for the test networks only, not mainnet?

gmaxwell commented at 8:39 am on October 25, 2016: contributor

This is sort of the idea behind block proposals, but what you’re doing is more drop-in sounds useful to me.

Re: pow limit, I have a proposal in progress to remove checkpoints completely that makes the powlimit an argument as a side effect. https://github.com/gmaxwell/bitcoin/commit/2db190b183c5204da23191ca642c7f6cad412ae3

theuni commented at 7:20 pm on October 25, 2016: member

@laanwj Allowing this to work on mainnet is actually a key part.

There’s a reasonable next target for miner/pool testing: block reward to p2sh/p2wpkh/p2wsh. Most mining/pool software at the moment has only hard-coded support for paying to a pubkey hash. Obviously it would be nice give them the ability to support the other formats, but as-is, it can be tough (and potentially very expensive) to simulate mining for a few days on mainnet.

(Though you could make the argument that a preference should be submitted to in gbt via coinbasetxn, which would include a serialized scriptPubKey or txout with the template, but let’s ignore that for the sake of the example).

This would allow those changes to be made/tested safely, without having to worry about differences in chaings wrt activation status.

As for changing the consensus.powLimit, I was hoping to avoid mixing app logic in at the chain level.

laanwj commented at 9:54 pm on October 25, 2016: member

Yes agreed - I misunderstood the goal here, due to it being a global command-line argument instead of a per-use RPC argument. But you explain why in the OP (though it may still make sense to be able to do this with an RPC too)

in src/main.cpp: in a78f24e498 outdated

3367@@ -3368,16 +3368,16 @@ bool FindUndoPos(CValidationState &state, int nFile, CDiskBlockPos &pos, unsigne
3368     return true;
3369 }
3370 
3371-bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW)
3372+bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, const uint256& powLimit, bool fCheckPOW)

jtimon commented at 10:35 pm on October 27, 2016:

Since you are touching CheckBlockHeader, perhaps you could use the opportunity to remove the param fCheckPOW. Now checking the pow is the only thing CheckBlockHeader does, so if you don’t want to check it just don’t call the function.

in src/rpc/mining.cpp: in 9dafef32f6 outdated

449+            uint32_t nBits;
450+            if (nforcednBits) {
451+                powLimit = uint256S("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
452+                nBits = nforcednBits;
453+            } else {
454+                nBits = GetNextWorkRequired(pindexPrev, &block, Params().GetConsensus());

jtimon commented at 10:43 pm on October 27, 2016:

Tiny style nit: you could be inconsistent in the ordering of declaration and assignments to nBits and powLimit (ie either powLimit or nBits first in the 3 places). Feel free to ignore.

jtimon commented at 10:51 pm on October 27, 2016: contributor

utACK besides the fCheckPOW nit. If the goal is to relay an invalid block on mainnet, neither the custom chains nor the blocksigned ones will help. Regarding CheckProofOfWork changing to taking the powLimit instead of the whole consensus params, I think it’s fine that both do it. Whatever gets merged first will force the other one to rebase, but in a more or less trivial way. What you shouldn’t do to avoid colliding with https://github.com/gmaxwell/bitcoin/commit/2db190b183c5204da23191ca642c7f6cad412ae3 is doing the same for CheckBlock and CheckBlockHeader. Those will have to take both a powLimit and the whole consensusParams (for @gmaxwell will need the struct in CheckBlockHeader()).

theuni commented at 11:37 pm on October 27, 2016: member

@jtimon I believe if we end up with both of these going in, @gmaxwell’s change can simply decide which value to use inside of AcceptBlockHeader() before passing it on. I haven’t looked closely though, that may not be the case. Either way, I’m sure rebasing would be no problem.

And the goal isn’t to relay an invalid block, it is simply to validate an otherwise-valid block that doesn’t provide enough work. If that’s the case, it responds with an “ok” to the miner, tricking it into thinking that the block was accepted. The block is then dropped and forgotten, not relayed.

In doing so, you can “mine” dozens (hundreds/thousands?) of mainnet blocks/sec as a test, using real and un-altered miners/pools.

btcdrak commented at 9:03 am on November 29, 2016: contributor

needs rebase.

mining: allow powLimit to be specified for TestBlockValidity 848d666bc2

mining: pass nBits into TestBlockValidity 78d751bab5

mining: introduce -forceminerdifficulty debugging option

This is intended for debugging mining software only! It allows for real-time
mining on any chain with the specified difficulty. When this option is set,
minimum difficulty blocks will be accepted by the submitblock rpc as long as
they are otherwise valid.

This is very helpful for simulating mining on a real chain, without having to
use real hashpower.

Additionally, it does not require any change in rpc behavior, allowing testing
of miners that cannot be easily changed.

ccc378c681

theuni force-pushed on Dec 2, 2016

theuni commented at 10:40 pm on December 2, 2016: member

Rebased and fixed up @jtimon’s nit.

This can be easily tested with bfgminer, either on testnet or mainnet.

Testnet example: Run bitcoind: ./bitcoind -forceminerdifficulty=493944831 -testnet -daemon -debug

and bfgminer: ./bfgminer --no-local-bitcoin -o 127.0.0.1:18332 -O <rpcuser>:<rpcpass> --generate-to <some address> --scan auto

jtimon commented at 10:48 pm on December 2, 2016: contributor

mhmm, it doesn’t look like you solved any of my two nits (to reiterate I don’t really care much about the syle nit).

btcdrak commented at 12:44 pm on December 23, 2016: contributor

needs rebase

bitkevin commented at 2:47 am on January 8, 2017: contributor

Check out codes:

0git clone https://github.com/theuni/bitcoin.git
1
2cd bitcoin
3git checkout origin/faux-mining
4
5./autogen.sh
6./configure --disable-wallet

build failure:

 0In file included from ./streams.h:13:0,
 1                 from ./net.h:19,
 2                 from ./main.h:16,
 3                 from bench/checkblock.cpp:8:
 4bench/checkblock.cpp: In function ‘void DeserializeAndCheckBlockTest(benchmark::State&)’:
 5bench/checkblock.cpp:50:57: error: invalid initialization of reference of type ‘const uint256&’ from expression of type ‘Consensus::Params’
 6         assert(CheckBlock(block, validationState, params));
 7                                                         ^
 8In file included from bench/checkblock.cpp:8:0:
 9./main.h:467:6: error: in passing argument 3 of ‘bool CheckBlock(const CBlock&, CValidationState&, const uint256&, bool, bool)’
10 bool CheckBlock(const CBlock& block, CValidationState& state, const uint256& powLimit, bool fCheckPOW = true, bool fCheckMerkleRoot = true);
11      ^
12  AR       libbitcoin_server.a
13make[2]: *** [bench/bench_bench_bitcoin-checkblock.o] Error 1
14make[2]: *** Waiting for unfinished jobs....
15make[2]: Leaving directory `/work/zhibiao.pan/bitcoin/theuni/bitcoin/src'
16make[1]: *** [all-recursive] Error 1
17make[1]: Leaving directory `/work/zhibiao.pan/bitcoin/theuni/bitcoin/src'
18make: *** [all-recursive] Error 1

btcdrak commented at 10:29 pm on February 13, 2017: contributor

Needs rebase.

theuni closed this on Sep 29, 2017

DrahtBot locked this on Sep 8, 2021

miner debugging: faux-mining #9000

Backstory

Solution