highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-06-26 08:51:55 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge #17938: Disallow automatic conversion between disparate hash types

Browse Source 
4d7369125a82214ea42b808a32b71b315a5c3c72 Disallow automatic conversion between hash types (Ben Woosley)
fa9ef2cdbed32438bdb32623af6e06f13ecd35e4 Remove an apparently unnecessary conversion (Ben Woosley)
966a22d859db37b1775e2180e5be032fc4fdf483 Explicitly support conversion between equivalent hash types (Ben Woosley)
f32c1e07fd6c174ff3f6406a619550d2f6c19360 Use explicit conversion from WitnessV0KeyHash -> CKeyID (Ben Woosley)
2c54217f913967703b404747133be67cf2f4feac Use explicit conversion from PKHash -> CKeyID (Ben Woosley)
a9e451f144480d7b170e49087df162989d31cd20 Convert CPubKey to WitnessV0KeyHash directly (Ben Woosley)
3fcc46812334074d2c77a6233e8a961cd0785872 Prefer explicit CScriptID construction (Ben Woosley)
0a5ea32ce605984094c5552877cb99bc81654f2c Prefer explicit uint160 conversion (Ben Woosley)

Pull request description:

  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".

ACKs for top commit:
  achow101:
    ACK 4d7369125a82214ea42b808a32b71b315a5c3c72
  meshcollider:
    re-utACK 4d7369125a82214ea42b808a32b71b315a5c3c72

Tree-SHA512: f1b3284ddc6fb6c6e726f2c22668b6d732d45eb5418262ed2b9c728f60be7be43dfb414b6ddd9915025c8dcd7f360dc3b46e997a945a2feb95b0e5c4f05d6b54
This commit is contained in:
Samuel Dobson 2020-06-21 20:04:09 +12:00
commit bd331bd745
No known key found for this signature in database
GPG Key ID: D300116E1C875A3D
11 changed files with 122 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/outputtype.cpp
View File

@ -53,7 +53,7 @@ CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type)
case OutputType::P2SH_SEGWIT:
case OutputType::BECH32: {
if (!key.IsCompressed()) return PKHash(key);
CTxDestination witdest = WitnessV0KeyHash(PKHash(key));
CTxDestination witdest = WitnessV0KeyHash(key);
CScript witprog = GetScriptForDestination(witdest);
if (type == OutputType::P2SH_SEGWIT) {
return ScriptHash(witprog);

2
src/qt/coincontroldialog.cpp
View File

@ -456,7 +456,7 @@ void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel *
{
CPubKey pubkey;
PKHash *pkhash = boost::get<PKHash>(&address);
if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, CKeyID(*pkhash), pubkey))
if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, ToKeyID(*pkhash), pubkey))
{
nBytesInputs += (pubkey.IsCompressed() ? 148 : 180);
}

2
src/rpc/rawtransaction.cpp
View File

@ -595,7 +595,7 @@ static UniValue decodescript(const JSONRPCRequest& request)
if (which_type == TX_PUBKEY) {
segwitScr = GetScriptForDestination(WitnessV0KeyHash(Hash160(solutions_data[0].begin(), solutions_data[0].end())));
} else if (which_type == TX_PUBKEYHASH) {
segwitScr = GetScriptForDestination(WitnessV0KeyHash(solutions_data[0]));
segwitScr = GetScriptForDestination(WitnessV0KeyHash(uint160{solutions_data[0]}));
} else {
// Scripts that are not fit for P2WPKH are encoded as P2WSH.
// Newer segwit program versions should be considered when then become available.

6
src/script/sign.cpp
View File

@ -131,7 +131,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator
}
case TX_SCRIPTHASH:
h160 = uint160(vSolutions[0]);
if (GetCScript(provider, sigdata, h160, scriptRet)) {
if (GetCScript(provider, sigdata, CScriptID{h160}, scriptRet)) {
ret.push_back(std::vector<unsigned char>(scriptRet.begin(), scriptRet.end()));
return true;
}
@ -165,7 +165,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator
case TX_WITNESS_V0_SCRIPTHASH:
CRIPEMD160().Write(&vSolutions[0][0], vSolutions[0].size()).Finalize(h160.begin());
if (GetCScript(provider, sigdata, h160, scriptRet)) {
if (GetCScript(provider, sigdata, CScriptID{h160}, scriptRet)) {
ret.push_back(std::vector<unsigned char>(scriptRet.begin(), scriptRet.end()));
return true;
}
@ -458,7 +458,7 @@ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script)
if (whichtype == TX_SCRIPTHASH) {
auto h160 = uint160(solutions[0]);
CScript subscript;
if (provider.GetCScript(h160, subscript)) {
if (provider.GetCScript(CScriptID{h160}, subscript)) {
whichtype = Solver(subscript, solutions);
if (whichtype == TX_WITNESS_V0_SCRIPTHASH || whichtype == TX_WITNESS_V0_KEYHASH || whichtype == TX_WITNESS_UNKNOWN) return true;
}

6
src/script/signingprovider.cpp
View File

@ -180,10 +180,10 @@ CKeyID GetKeyForDestination(const SigningProvider& store, const CTxDestination&
// Only supports destinations which map to single public keys, i.e. P2PKH,
// P2WPKH, and P2SH-P2WPKH.
if (auto id = boost::get<PKHash>(&dest)) {
return CKeyID(*id);
return ToKeyID(*id);
}
if (auto witness_id = boost::get<WitnessV0KeyHash>(&dest)) {
return CKeyID(*witness_id);
return ToKeyID(*witness_id);
}
if (auto script_hash = boost::get<ScriptHash>(&dest)) {
CScript script;
@ -191,7 +191,7 @@ CKeyID GetKeyForDestination(const SigningProvider& store, const CTxDestination&
CTxDestination inner_dest;
if (store.GetCScript(script_id, script) && ExtractDestination(script, inner_dest)) {
if (auto inner_witness_id = boost::get<WitnessV0KeyHash>(&inner_dest)) {
return CKeyID(*inner_witness_id);
return ToKeyID(*inner_witness_id);
}
}
}

24
src/script/standard.cpp
View File

@ -16,11 +16,27 @@ typedef std::vector<unsigned char> valtype;
bool fAcceptDatacarrier = DEFAULT_ACCEPT_DATACARRIER;
unsigned nMaxDatacarrierBytes = MAX_OP_RETURN_RELAY;
CScriptID::CScriptID(const CScript& in) : uint160(Hash160(in.begin(), in.end())) {}
CScriptID::CScriptID(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {}
CScriptID::CScriptID(const ScriptHash& in) : BaseHash(static_cast<uint160>(in)) {}
ScriptHash::ScriptHash(const CScript& in) : uint160(Hash160(in.begin(), in.end())) {}
ScriptHash::ScriptHash(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {}
ScriptHash::ScriptHash(const CScriptID& in) : BaseHash(static_cast<uint160>(in)) {}
PKHash::PKHash(const CPubKey& pubkey) : uint160(pubkey.GetID()) {}
PKHash::PKHash(const CPubKey& pubkey) : BaseHash(pubkey.GetID()) {}
PKHash::PKHash(const CKeyID& pubkey_id) : BaseHash(pubkey_id) {}
WitnessV0KeyHash::WitnessV0KeyHash(const CPubKey& pubkey) : BaseHash(pubkey.GetID()) {}
WitnessV0KeyHash::WitnessV0KeyHash(const PKHash& pubkey_hash) : BaseHash(static_cast<uint160>(pubkey_hash)) {}
CKeyID ToKeyID(const PKHash& key_hash)
{
return CKeyID{static_cast<uint160>(key_hash)};
}
CKeyID ToKeyID(const WitnessV0KeyHash& key_hash)
{
return CKeyID{static_cast<uint160>(key_hash)};
}
WitnessV0ScriptHash::WitnessV0ScriptHash(const CScript& in)
{
@ -307,7 +323,7 @@ CScript GetScriptForWitness(const CScript& redeemscript)
if (typ == TX_PUBKEY) {
return GetScriptForDestination(WitnessV0KeyHash(Hash160(vSolutions[0].begin(), vSolutions[0].end())));
} else if (typ == TX_PUBKEYHASH) {
return GetScriptForDestination(WitnessV0KeyHash(vSolutions[0]));
return GetScriptForDestination(WitnessV0KeyHash(uint160{vSolutions[0]}));
}
return GetScriptForDestination(WitnessV0ScriptHash(redeemscript));
}

104
src/script/standard.h
View File

@ -18,14 +18,77 @@ static const bool DEFAULT_ACCEPT_DATACARRIER = true;
class CKeyID;
class CScript;
struct ScriptHash;
template<typename HashType>
class BaseHash
{
protected:
HashType m_hash;
public:
BaseHash() : m_hash() {}
BaseHash(const HashType& in) : m_hash(in) {}
unsigned char* begin()
{
return m_hash.begin();
}
const unsigned char* begin() const
{
return m_hash.begin();
}
unsigned char* end()
{
return m_hash.end();
}
const unsigned char* end() const
{
return m_hash.end();
}
operator std::vector<unsigned char>() const
{
return std::vector<unsigned char>{m_hash.begin(), m_hash.end()};
}
std::string ToString() const
{
return m_hash.ToString();
}
bool operator==(const BaseHash<HashType>& other) const noexcept
{
return m_hash == other.m_hash;
}
bool operator!=(const BaseHash<HashType>& other) const noexcept
{
return !(m_hash == other.m_hash);
}
bool operator<(const BaseHash<HashType>& other) const noexcept
{
return m_hash < other.m_hash;
}
size_t size() const
{
return m_hash.size();
}
};
/** A reference to a CScript: the Hash160 of its serialization (see script.h) */
class CScriptID : public uint160
class CScriptID : public BaseHash<uint160>
{
public:
CScriptID() : uint160() {}
CScriptID() : BaseHash() {}
explicit CScriptID(const CScript& in);
CScriptID(const uint160& in) : uint160(in) {}
explicit CScriptID(const uint160& in) : BaseHash(in) {}
explicit CScriptID(const ScriptHash& in);
};
/**
@ -73,41 +136,44 @@ public:
friend bool operator<(const CNoDestination &a, const CNoDestination &b) { return true; }
};
struct PKHash : public uint160
struct PKHash : public BaseHash<uint160>
{
PKHash() : uint160() {}
explicit PKHash(const uint160& hash) : uint160(hash) {}
PKHash() : BaseHash() {}
explicit PKHash(const uint160& hash) : BaseHash(hash) {}
explicit PKHash(const CPubKey& pubkey);
using uint160::uint160;
explicit PKHash(const CKeyID& pubkey_id);
};
CKeyID ToKeyID(const PKHash& key_hash);
struct WitnessV0KeyHash;
struct ScriptHash : public uint160
struct ScriptHash : public BaseHash<uint160>
{
ScriptHash() : uint160() {}
ScriptHash() : BaseHash() {}
// These don't do what you'd expect.
// Use ScriptHash(GetScriptForDestination(...)) instead.
explicit ScriptHash(const WitnessV0KeyHash& hash) = delete;
explicit ScriptHash(const PKHash& hash) = delete;
explicit ScriptHash(const uint160& hash) : uint160(hash) {}
explicit ScriptHash(const uint160& hash) : BaseHash(hash) {}
explicit ScriptHash(const CScript& script);
using uint160::uint160;
explicit ScriptHash(const CScriptID& script);
};
struct WitnessV0ScriptHash : public uint256
struct WitnessV0ScriptHash : public BaseHash<uint256>
{
WitnessV0ScriptHash() : uint256() {}
explicit WitnessV0ScriptHash(const uint256& hash) : uint256(hash) {}
WitnessV0ScriptHash() : BaseHash() {}
explicit WitnessV0ScriptHash(const uint256& hash) : BaseHash(hash) {}
explicit WitnessV0ScriptHash(const CScript& script);
using uint256::uint256;
};
struct WitnessV0KeyHash : public uint160
struct WitnessV0KeyHash : public BaseHash<uint160>
{
WitnessV0KeyHash() : uint160() {}
explicit WitnessV0KeyHash(const uint160& hash) : uint160(hash) {}
using uint160::uint160;
WitnessV0KeyHash() : BaseHash() {}
explicit WitnessV0KeyHash(const uint160& hash) : BaseHash(hash) {}
explicit WitnessV0KeyHash(const CPubKey& pubkey);
explicit WitnessV0KeyHash(const PKHash& pubkey_hash);
};
CKeyID ToKeyID(const WitnessV0KeyHash& key_hash);
//! CTxDestination subtype to encode any future Witness version
struct WitnessUnknown

2
src/wallet/rpcdump.cpp
View File

@ -297,7 +297,7 @@ UniValue importaddress(const JSONRPCRequest& request)
pwallet->ImportScripts(scripts, 0 /* timestamp */);
if (fP2SH) {
scripts.insert(GetScriptForDestination(ScriptHash(CScriptID(redeem_script))));
scripts.insert(GetScriptForDestination(ScriptHash(redeem_script)));
}
pwallet->ImportScriptPubKeys(strLabel, scripts, false /* have_solving_data */, true /* apply_label */, 1 /* timestamp */);

4
src/wallet/rpcwallet.cpp
View File

@ -3517,7 +3517,7 @@ public:
UniValue operator()(const PKHash& pkhash) const
{
CKeyID keyID(pkhash);
CKeyID keyID{ToKeyID(pkhash)};
UniValue obj(UniValue::VOBJ);
CPubKey vchPubKey;
if (provider && provider->GetPubKey(keyID, vchPubKey)) {
@ -3542,7 +3542,7 @@ public:
{
UniValue obj(UniValue::VOBJ);
CPubKey pubkey;
if (provider && provider->GetPubKey(CKeyID(id), pubkey)) {
if (provider && provider->GetPubKey(ToKeyID(id), pubkey)) {
obj.pushKV("pubkey", HexStr(pubkey));
}
return obj;

6
src/wallet/scriptpubkeyman.cpp
View File

@ -573,9 +573,8 @@ bool LegacyScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const std::
SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const
{
CKeyID key_id(pkhash);
CKey key;
if (!GetKey(key_id, key)) {
if (!GetKey(ToKeyID(pkhash), key)) {
return SigningResult::PRIVATE_KEY_NOT_AVAILABLE;
}
@ -2052,9 +2051,8 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message,
return SigningResult::PRIVATE_KEY_NOT_AVAILABLE;
}
CKeyID key_id(pkhash);
CKey key;
if (!keys->GetKey(key_id, key)) {
if (!keys->GetKey(ToKeyID(pkhash), key)) {
return SigningResult::PRIVATE_KEY_NOT_AVAILABLE;
}

6
src/wallet/test/ismine_tests.cpp
View File

@ -167,7 +167,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
CScript witnessscript = GetScriptForDestination(WitnessV0KeyHash(PKHash(pubkeys[0])));
CScript witnessscript = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0]));
scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessscript));
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(witnessscript));
@ -202,7 +202,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(PKHash(pubkeys[0])));
scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0]));
// Keystore implicitly has key and P2SH redeemScript
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(scriptPubKey));
@ -217,7 +217,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(PKHash(uncompressedPubkey)));
scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(uncompressedPubkey));
// Keystore has key, but no P2SH redeemScript
result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey);