This bases the script/standard hash types, TxDestination-related and CScriptID on a base template which does not silently convert the underlying uintN type.
Inspired by and built on #17924. Commits are small and focused to ease review.
Note some of these changes may be relative to existing bugs of the same sort as #17924. See particularly "Convert CPubKey to WitnessV0KeyHash directly" and "Remove an apparently unnecessary conversion".
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
concept ACK, will review deeper if base is merged
Code review ACK 730267fd969cded98982b6c6762080dca2332939
@instagibbs rebased now that #17924 is merged
@instagibbs how about a review?
Concept ACK, I like the idea of using explicit and separate types here to prevent non-sensible conversions, if you can use language features to avoid bugs that's very good.
Concept ACK: explicit conversion is better (and safer!) than implicit conversion
Travis sad.
ACK 4b7de7c402270388794203496728c4baa56c4877
ACK 4b7de7c402270388794203496728c4baa56c4877
Code review ACK 4b7de7c402270388794203496728c4baa56c4877
There is a build error unfortunately
wallet/scriptpubkeyman.cpp:2054:25: error: no matching function for call to ‘CKeyID::CKeyID(const PKHash&)’
CKeyID key_id(pkhash);
^
Rebased, and the fix is in https://github.com/bitcoin/bitcoin/pull/17938/commits/1236555761b79c44f4bc9676260321b8c7095a6f
I'm not sure what's up with Appveyor, and I'm happy to squash.
I think it makes sense to squash fixup commits that fix a compile-failure. Otherwise it is not possible to compile earlier commits.
The round-tripping through PKHash has no effect, and is
potentially misleading as such.
These types are equivalent, in data etc, so they need only their
data cast across.
Note a function is used rather than a casting
operator as CKeyID is defined at a lower level than script/standard
These types are equivalent, in data etc, so they need only their
data cast across.
ScriptHash <-> CScriptID
CKeyID -> PKHash
PKHash -> WitnessV0KeyHash
CScript -> CScriptID -> ScriptHash is unnecessary because
ScriptHash and CScriptID do the same thing.
A templated BaseHash does not allow for automatic conversion, thus
conversions much be explicitly allowed / whitelisted, which will
reduce the risk of unintended conversions.
Squashed the fix and rebased.
ACK 4d7369125a82214ea42b808a32b71b315a5c3c72
Only change since last is to handle a newly introduced conversion. That's also a good example of the benefits that this PR brings.
re-utACK 4d7369125a82214ea42b808a32b71b315a5c3c72
