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”.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
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.
There is a build error unfortunately
0wallet/scriptpubkeyman.cpp:2054:25: error: no matching function for call to ‘CKeyID::CKeyID(const PKHash&)’
1 CKeyID key_id(pkhash);
2 ^
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.
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.
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.
