Drop unnecessary merkle mutation checks #14716

pull Empact wants to merge 2 commits into bitcoin:master from Empact:unnecessary-merkle-mutation-checks changing 3 files +4 −6

Empact commented at 9:39 AM on November 13, 2018: member
Ok this is consensus code so I'm prepared to see this rejected, but I figure I'd bring it up just in case:
- Don't publish ComputeMerkleRoot declaration in consensus/merkle.h, it's only used in the bench.
- Don't continue checking for mutations in ComputeMerkleRoot if one has already been found.
- Compare mutated to nullptr to clarify that it's a pointer rather than a bool.
I saw a ~3% MerkleRoot bench improvement due either to random chance or the dropped mutation checks.

Don't declare ComputeMerkleRoot in consensus/merkle.h

This reduces surface area, given it's not used anywhere other than bench.

a892420061

Don't continue checking for merkle mutation if we've found one c23df9d805
fanquake added the label Consensus on Nov 13, 2018
MarcoFalke commented at 5:05 PM on November 13, 2018: member

Slightly tend to NACK. If an attacker wants to waste some CPU-milliseconds, they can already achieve it way easier.
gmaxwell commented at 5:51 PM on November 13, 2018: contributor

I don't think we care about making it ever so slightly faster in the presence of a mutated block as that is an exceptionally uncommon case (and one that results in the peer giving us the block being banned). @MarcoFalke mutation is a type of invalidity that doesn't require the attacker make a pow valid block.
Empact commented at 5:56 PM on November 13, 2018: member

Alright I'll close to spare the other reviewers. Thanks, and on to other things.
Empact closed this on Nov 13, 2018
DrahtBot locked this on Sep 8, 2021

Contributors

Labels