Calculated nBits will be replaced by the following GetNextWorkRequire… #13042

1. 
   qshuai commented at 12:46 pm on April 20, 2018: none

   There are two reasons to remove the code(as you see):

   • The calculated nBits will be replaced by the following GetNextWorkRequired(.) function. So calculating difficulty(nBits) in UpdateTime() function is meaningless. The main chain does not matter. But testnet will exec twice.
   • UpdateTime() function should remain single role for updating timestamp item of block.
2. Calculated nBits will be replaced by the following GetNextWorkRequired(.) function f404765c8b
3. laanwj added the label Refactoring on Apr 23, 2018
4. 
   qshuai commented at 6:34 am on April 24, 2018: none

   As mentioned above(testnet will exec twice), the referenced code is here.

   0UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev); 
   1pblock->nBits          = GetNextWorkRequired(pindexPrev, pblock, chainparams.GetConsensus());
   

5. 
   jonasschnelli commented at 8:18 am on April 26, 2018: contributor

   utACK f404765c8ba6ca16f0c7d9e274fd64cf28675a6b. pblock->nBits = GetNextWorkRequired() is indeed called twice in that case.
6. jonasschnelli added the label Mining on Apr 26, 2018
7. laanwj requested review from luke-jr on Apr 26, 2018
8. in src/rpc/mining.cpp:533 in f404765c8b

   528@@ -529,7 +529,7 @@ UniValue getblocktemplate(const JSONRPCRequest& request)
   529     const Consensus::Params& consensusParams = Params().GetConsensus();
   530 
   531     // Update nTime
   532-    UpdateTime(pblock, consensusParams, pindexPrev);
   533+    UpdateTime(pblock, pindexPrev);
   534     pblock->nNonce = 0;
   

   ---

   ---

   

   MarcoFalke commented at 11:55 am on April 26, 2018:

   You never update nBits here, whereas it should.

   ---

   

   qshuai commented at 4:02 pm on April 26, 2018:

   @MarcoFalke nBits updating depends on whether CreateNewBlock() is called. That is to say, nBits will be updated when CreateNewBlock() is called. It indicates the miner is mining the same height block at the current time at least if CreateNewBlock() is not called. So the nBits needs not to be update, just use cached blocktemplate. The following is the condition of calling CreateNewBlock():

   0if (pindexPrev != chainActive.Tip() ||
   1        (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5) ||
   2        fLastTemplateSupportsSegwit != fSupportsSegwit)
   

   ---

   

   MarcoFalke commented at 4:14 pm on April 26, 2018:

   Then, why would you need to call UpdateTime at all?

   ---

   

   qshuai commented at 4:43 pm on April 26, 2018:

   UpdateTime is responsible for updating nTime field.

   ---

   

   MarcoFalke commented at 9:00 pm on April 26, 2018:

   But it is already called in CreateNewBlock just before GetNextWorkRequired. So if you can skip the GetNextWorkRequired in UpdateTime (with the reason that it was called twice through CNB), you can also skip UpdateTime, no?

   ---

   

   qshuai commented at 0:17 am on April 27, 2018:

   Firstly, UpdateTime must be called regardless of CreateNewBlock: https://github.com/bitcoin/bitcoin/blob/master/src/rpc/mining.cpp#L506-L532

   Secondly, GetNextWorkRequired and TestBlockValidity use the nTime field. So it may be necessary to update nTime field in advance.

   ---

   

   sdaftuar commented at 12:30 pm on April 27, 2018:

   If you don’t update nBits here, then this patch would change behavior for getblocktemplate users on testnet, right? Previously the calculated difficulty could go down if enough time passes (even using a cached block), whereas after this patch that could no longer happen.

   ---

   

   qshuai commented at 2:28 pm on April 27, 2018:

   @sdaftuar Yeah, that is true. I did not consider it before now. So it works although it is a bad code smell.
9. 
   MarcoFalke commented at 2:54 pm on May 18, 2018: member

   What is the status of this? I think the feedback wasn’t addressed and I suggest closing
10. qshuai closed this on May 20, 2018

    ---

11. MarcoFalke locked this on Sep 8, 2021

Contributors
qshuai jonasschnelli MarcoFalke sdaftuar

Review Requested
luke-jr

Labels
Refactoring Mining