From fa2eb383522249a5f4d48726c520cec5de496614 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 27 Jun 2020 10:45:45 -0400 Subject: [PATCH 1/3] interfaces: Remove unused getDefaultChangeType --- src/interfaces/wallet.cpp | 1 - src/interfaces/wallet.h | 3 --- 2 files changed, 4 deletions(-) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index b65eb72b1cc..d5297242821 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -438,7 +438,6 @@ public: bool canGetAddresses() override { return m_wallet->CanGetAddresses(); } bool privateKeysDisabled() override { return m_wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); } OutputType getDefaultAddressType() override { return m_wallet->m_default_address_type; } - OutputType getDefaultChangeType() override { return m_wallet->m_default_change_type; } CAmount getDefaultMaxTxFee() override { return m_wallet->m_default_max_tx_fee; } void remove() override { diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index e2161521f66..44605cac104 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -256,9 +256,6 @@ public: // Get default address type. virtual OutputType getDefaultAddressType() = 0; - // Get default change type. - virtual OutputType getDefaultChangeType() = 0; - //! Get max tx fee. virtual CAmount getDefaultMaxTxFee() = 0; From faddad71f648ed99734f4f8811bd4bc7232ca670 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 27 Jun 2020 11:05:41 -0400 Subject: [PATCH 2/3] Remove confusing OutputType::CHANGE_AUTO --- src/outputtype.h | 8 -------- src/wallet/rpcwallet.cpp | 7 ++++--- src/wallet/wallet.cpp | 24 +++++++++++++++--------- src/wallet/wallet.h | 13 ++++++++----- 4 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/outputtype.h b/src/outputtype.h index 1438f658448..77a16b1d057 100644 --- a/src/outputtype.h +++ b/src/outputtype.h @@ -18,14 +18,6 @@ enum class OutputType { LEGACY, P2SH_SEGWIT, BECH32, - - /** - * Special output type for change outputs only. Automatically choose type - * based on address type setting and the types other of non-change outputs - * (see -changetype option documentation and implementation in - * CWallet::TransactionChangeType for details). - */ - CHANGE_AUTO, }; extern const std::array OUTPUT_TYPES; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c05233d0af5..1994b2090a7 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -306,7 +306,7 @@ static UniValue getrawchangeaddress(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Error: This wallet has no available keys"); } - OutputType output_type = pwallet->m_default_change_type != OutputType::CHANGE_AUTO ? pwallet->m_default_change_type : pwallet->m_default_address_type; + OutputType output_type = pwallet->m_default_change_type.get_value_or(pwallet->m_default_address_type); if (!request.params[0].isNull()) { if (!ParseOutputType(request.params[0].get_str(), output_type)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str())); @@ -2993,10 +2993,11 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f if (options.exists("changeAddress")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both changeAddress and address_type options"); } - coinControl.m_change_type = pwallet->m_default_change_type; - if (!ParseOutputType(options["change_type"].get_str(), *coinControl.m_change_type)) { + OutputType out_type; + if (!ParseOutputType(options["change_type"].get_str(), out_type)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown change type '%s'", options["change_type"].get_str())); } + coinControl.m_change_type.emplace(out_type); } coinControl.fAllowWatchOnly = ParseIncludeWatchonly(options["includeWatching"], *pwallet); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 57eec9baf90..3fd66c14105 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2653,11 +2653,11 @@ static uint32_t GetLocktimeForNewTransaction(interfaces::Chain& chain, const uin return locktime; } -OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vector& vecSend) +OutputType CWallet::TransactionChangeType(const Optional& change_type, const std::vector& vecSend) { // If -changetype is specified, always use that change type. - if (change_type != OutputType::CHANGE_AUTO) { - return change_type; + if (change_type) { + return *change_type; } // if m_default_address_type is legacy, use legacy address as change (even @@ -3826,14 +3826,20 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } } - if (!gArgs.GetArg("-addresstype", "").empty() && !ParseOutputType(gArgs.GetArg("-addresstype", ""), walletInstance->m_default_address_type)) { - error = strprintf(_("Unknown address type '%s'"), gArgs.GetArg("-addresstype", "")); - return nullptr; + if (!gArgs.GetArg("-addresstype", "").empty()) { + if (!ParseOutputType(gArgs.GetArg("-addresstype", ""), walletInstance->m_default_address_type)) { + error = strprintf(_("Unknown address type '%s'"), gArgs.GetArg("-addresstype", "")); + return nullptr; + } } - if (!gArgs.GetArg("-changetype", "").empty() && !ParseOutputType(gArgs.GetArg("-changetype", ""), walletInstance->m_default_change_type)) { - error = strprintf(_("Unknown change type '%s'"), gArgs.GetArg("-changetype", "")); - return nullptr; + if (!gArgs.GetArg("-changetype", "").empty()) { + OutputType out_type; + if (!ParseOutputType(gArgs.GetArg("-changetype", ""), out_type)) { + error = strprintf(_("Unknown change type '%s'"), gArgs.GetArg("-changetype", "")); + return nullptr; + } + walletInstance->m_default_change_type = out_type; } if (gArgs.IsArgSet("-mintxfee")) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 9931671fb4a..e7b480af97e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -105,9 +105,6 @@ class ReserveDestination; //! Default for -addresstype constexpr OutputType DEFAULT_ADDRESS_TYPE{OutputType::BECH32}; -//! Default for -changetype -constexpr OutputType DEFAULT_CHANGE_TYPE{OutputType::CHANGE_AUTO}; - static constexpr uint64_t KNOWN_WALLET_FLAGS = WALLET_FLAG_AVOID_REUSE | WALLET_FLAG_BLANK_WALLET @@ -934,7 +931,7 @@ public: Balance GetBalance(int min_depth = 0, bool avoid_reuse = true) const; CAmount GetAvailableBalance(const CCoinControl* coinControl = nullptr) const; - OutputType TransactionChangeType(OutputType change_type, const std::vector& vecSend); + OutputType TransactionChangeType(const Optional& change_type, const std::vector& vecSend); /** * Insert additional inputs into the transaction by @@ -1012,7 +1009,13 @@ public: CFeeRate m_fallback_fee{DEFAULT_FALLBACK_FEE}; CFeeRate m_discard_rate{DEFAULT_DISCARD_FEE}; OutputType m_default_address_type{DEFAULT_ADDRESS_TYPE}; - OutputType m_default_change_type{DEFAULT_CHANGE_TYPE}; + /** + * Default output type for change outputs. When unset, automatically choose type + * based on address type setting and the types other of non-change outputs + * (see -changetype option documentation and implementation in + * CWallet::TransactionChangeType for details). + */ + Optional m_default_change_type{}; /** Absolute maximum transaction fee (in satoshis) used by default for the wallet */ CAmount m_default_max_tx_fee{DEFAULT_TRANSACTION_MAXFEE}; From fa927ff884ae373c676deed63180a8d238872cdc Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 27 Jun 2020 11:05:03 -0400 Subject: [PATCH 3/3] Enable Wswitch for OutputType --- src/outputtype.cpp | 12 ++++++------ src/wallet/scriptpubkeyman.cpp | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/outputtype.cpp b/src/outputtype.cpp index 871474d56e7..e9788528261 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -42,8 +42,8 @@ const std::string& FormatOutputType(OutputType type) case OutputType::LEGACY: return OUTPUT_TYPE_STRING_LEGACY; case OutputType::P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT; case OutputType::BECH32: return OUTPUT_TYPE_STRING_BECH32; - default: assert(false); - } + } // no default case, so the compiler can warn about missing cases + assert(false); } CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type) @@ -61,8 +61,8 @@ CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type) return witdest; } } - default: assert(false); - } + } // no default case, so the compiler can warn about missing cases + assert(false); } std::vector GetAllDestinationsForKey(const CPubKey& key) @@ -100,6 +100,6 @@ CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, return ScriptHash(witprog); } } - default: assert(false); - } + } // no default case, so the compiler can warn about missing cases + assert(false); } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index d57f99a205f..17da781e3b1 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1900,8 +1900,8 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ desc_prefix = "wpkh(" + xpub + "/84'"; break; } - default: assert(false); - } + } // no default case, so the compiler can warn about missing cases + assert(!desc_prefix.empty()); // Mainnet derives at 0', testnet and regtest derive at 1' if (Params().IsTestChain()) {