From b43adbd2378d3fdd7f947d395d9c6163dca190df Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 11 Feb 2021 11:48:06 +0100 Subject: [PATCH] Merge #20211: Use -Wswitch for TxoutType where possible fa650ca7f19307a9237e64ac311488c8947fc12a Use -Wswitch for TxoutType where possible (MarcoFalke) fa59e0b5bd2aed8380cc9b9e52791f662aecd6a6 test: Add missing script_standard_Solver_success cases (MarcoFalke) Pull request description: This removes unused `default:` cases for all `switch` statements on `TxoutType` and adds the cases (`MULTISIG`, `NULL_DATA`, `NONSTANDARD`) to `ExtractDestination` for clarity. Also, the compiler is now able to use `-Wswitch`. ACKs for top commit: practicalswift: cr ACK fa650ca7f19307a9237e64ac311488c8947fc12a: patch looks correct and `assert(false);` is better than UB :) hebasto: ACK fa650ca7f19307a9237e64ac311488c8947fc12a, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 282458b6523bd8923a0c0f5c423d1db2dce2a2d1b1d1dae455415c6fc995bb41ce82c1f9b0a1c0dcc6d874d171e04c30eca585f147582f52c7048c140358630a --- src/script/sign.cpp | 9 +++------ src/script/standard.cpp | 21 ++++++++++++--------- src/wallet/rpcdump.cpp | 4 ++-- src/wallet/scriptpubkeyman.cpp | 5 ++--- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index d9e460f75cc591..bb120d784c29c7 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -99,8 +99,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator std::vector vSolutions; whichTypeRet = Solver(scriptPubKey, vSolutions); - switch (whichTypeRet) - { + switch (whichTypeRet) { case TxoutType::NONSTANDARD: case TxoutType::NULL_DATA: return false; @@ -151,10 +150,8 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator } return ok; } - - default: - return false; - } + } // no default case, so the compiler can warn about missing cases + assert(false); } static CScript PushAll(const std::vector& values) diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 567e24369bf4b7..36f432017394f6 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -29,8 +29,7 @@ CKeyID ToKeyID(const PKHash& key_hash) const char* GetTxnOutputType(TxoutType t) { - switch (t) - { + switch (t) { case TxoutType::NONSTANDARD: return "nonstandard"; case TxoutType::PUBKEY: return "pubkey"; case TxoutType::PUBKEYHASH: return "pubkeyhash"; @@ -138,7 +137,8 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet) std::vector vSolutions; TxoutType whichType = Solver(scriptPubKey, vSolutions); - if (whichType == TxoutType::PUBKEY) { + switch (whichType) { + case TxoutType::PUBKEY: { CPubKey pubKey(vSolutions[0]); if (!pubKey.IsValid()) return false; @@ -146,18 +146,21 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet) addressRet = PKHash(pubKey); return true; } - else if (whichType == TxoutType::PUBKEYHASH) - { + case TxoutType::PUBKEYHASH: { addressRet = PKHash(uint160(vSolutions[0])); return true; } - else if (whichType == TxoutType::SCRIPTHASH) - { + case TxoutType::SCRIPTHASH: { addressRet = ScriptHash(uint160(vSolutions[0])); return true; } - // Multisig txns have more than one address... - return false; + case TxoutType::MULTISIG: + // Multisig txns have more than one address... + case TxoutType::NULL_DATA: + case TxoutType::NONSTANDARD: + return false; + } // no default case, so the compiler can warn about missing cases + assert(false); } bool ExtractDestinations(const CScript& scriptPubKey, TxoutType& typeRet, std::vector& addressRet, int& nRequiredRet) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index bdc8a81e9be58b..e6d2f59c9249ee 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1109,9 +1109,9 @@ static std::string RecurseImportData(const CScript& script, ImportData& import_d case TxoutType::NULL_DATA: return "unspendable script"; case TxoutType::NONSTANDARD: - default: return "unrecognized script"; - } + } // no default case, so the compiler can warn about missing cases + CHECK_NONFATAL(false); } static UniValue ProcessImportLegacy(ImportData& import_data, std::map& pubkey_map, std::map& privkey_map, std::set& script_pub_keys, bool& have_solving_data, const UniValue& data, std::vector& ordered_pubkeys) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 78a2dbc2bc0938..c9b07789a0be3f 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -90,8 +90,7 @@ IsMineResult IsMineInner(const LegacyScriptPubKeyMan& keystore, const CScript& s TxoutType whichType = Solver(scriptPubKey, vSolutions); CKeyID keyID; - switch (whichType) - { + switch (whichType) { case TxoutType::NONSTANDARD: case TxoutType::NULL_DATA: break; @@ -154,7 +153,7 @@ IsMineResult IsMineInner(const LegacyScriptPubKeyMan& keystore, const CScript& s } break; } - } + } // no default case, so the compiler can warn about missing cases if (ret == IsMineResult::NO && keystore.HaveWatchOnly(scriptPubKey)) { ret = std::max(ret, IsMineResult::WATCH_ONLY);