Skip to content

Conversation

@jtimon
Copy link
Contributor

@jtimon jtimon commented Jun 23, 2014

The time is already set in CreateNewBlock and the nonce is set to zero for no reason (never used after that assignment).
In testnet mode the nbits (and thefore target, which is the same with another type) could be affected in testnet mode after finding the block with that additional UpdateTime (which may update nBits), which doesn't seem very safe.
This is not tested yet, but seems straightforward to review and test.

@jtimon
Copy link
Contributor Author

jtimon commented Jun 25, 2014

This is not as risky as it may look like, in fact I think it is pretty save and if anything the previous behavior is the wrong one (not saying it doesn't need testing).

How much time can pass from https://github.com/bitcoin/bitcoin/blob/master/src/miner.cpp#L308 (called from https://github.com/bitcoin/bitcoin/blob/master/src/rpcmining.cpp#L359) to https://github.com/bitcoin/bitcoin/blob/master/src/rpcmining.cpp#L369 ?
The nonce is set to zero in both cases and the bits can only change in the second UpdateTime if we're on testnet.

@sipa
Copy link
Member

sipa commented Jun 25, 2014

Untested ACK. Those calls look redundant.

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4393_8ec7329807a6ce96188c6e6dd3af84602499b07f/ for binaries and test log.
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.

@jgarzik
Copy link
Contributor

jgarzik commented Jun 26, 2014

NAK. Obviously wrong. Read the code. If called frequently, those static vars act as a cache.

@jtimon
Copy link
Contributor Author

jtimon commented Jun 26, 2014

I see, so removing this would cause the cached nTime to be used.
I think removing the nNonce = 0 assignment is safe though, no?
A cached zero should be as good as a fresh one.

@jtimon jtimon closed this Jun 26, 2014
@sipa
Copy link
Member

sipa commented Jun 26, 2014

Undo ACK; You're right, @jgarzik.

@jtimon jtimon deleted the rpcpow branch June 26, 2014 18:59
@laanwj
Copy link
Member

laanwj commented Jun 27, 2014

Sharp catch @jgarzik

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants