Remove extra nonce reset logic in IncrementExtraNonce #9627

pull jtimon wants to merge 1 commits into bitcoin:master from jtimon:upstream-elements-unused-extranonce changing 1 files +1 −7
  1. jtimon commented at 4:02 AM on January 25, 2017: contributor

    These are unused variables that can be removed. And IncrementExtraNonce() should be never called with uint256() == pblock->hashPrevBlock.

    Should be easy review for @instagibbs @luke-jr and @morcos.

  2. Remove extra nonce reset logic in IncrementExtraNonce
    I assume this was added to prevent deanonymization, but that
    doesn't matter anymore since its only used on regtest.
    f3ded166e1
  3. gmaxwell commented at 7:38 AM on January 25, 2017: contributor

    There is no unused variable here.

    True, resetting the nonce hardly matters for regtest, but that won't stop others from looking at that code when writing their own mining software.

    I also don't understand why the assert is being added, all it is checking is that the block template header is filled out correctly. Sure. It is, though nothing in that function depends on it being so.

    If someone believed that it wasn't possible to call Increment twice with the same prior block, that is not true: the caller can hit the nInnerLoopCount comparison, continue the loop without solving a block and call IncrementExtraNonce to do its job while staying on the same block.

    Actually, I think that assert would introduce an infrequently triggered crash in the regtest miner code. CS_MAIN is not held during the entire duration of the mining. If you have two regtest nodes mining at once, you might accept a block between the CreateNewBlock and the IncrementExtraNonce, resulting in the template and the pindexPrev disagreeing on the identity of the prior block.

  4. jtimon commented at 5:21 PM on January 25, 2017: contributor

    I meant the variable hashPrevBlock is not needed, but I missed it is static. I was only wondering why the assert wasn't simply assert(pblock->hashPrevBlock != uint256());. Sorry, it seems I misunderstood the changes and should probably not have opened this PR.

    In the commmit description, @TheBlueMatt says:

    I assume this was added to prevent deanonymization, but that doesn't matter anymore since its only used on regtest.

  5. TheBlueMatt commented at 5:49 PM on January 25, 2017: member

    I dont recall the point here (I believe it was something to do with cleaning up code for later refactors in elements), but the point of the code block, I'm pretty sure was to prevent the walking extranonce patterns which appear to deanonymize (or at least allow you to bucket) early miners.

  6. jtimon commented at 6:28 PM on January 25, 2017: contributor

    Thanks for clarifying and sorry for the distraction. Closing.

  7. jtimon closed this on Jan 25, 2017

  8. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-17 15:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me