From fadd4029dced574778ade228931a7706f92bc676 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 16 Dec 2020 11:28:22 +0100 Subject: [PATCH 1/3] psbt: Assert that tx has a value in UpdatePSBTOutput This was previously done implicitly in boost::optional by BOOST_ASSERT. Also, it was checked at runtime by valgrind if for some reason the assert was disabled. std::optional dereference won't assert, so add the Assert here explicitly. The explicit Assert also helps to document the code better. --- src/psbt.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/psbt.cpp b/src/psbt.cpp index 3fb743e5db2..4db57d3cd09 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -3,6 +3,8 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include + +#include #include @@ -207,7 +209,8 @@ size_t CountPSBTUnsignedInputs(const PartiallySignedTransaction& psbt) { void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index) { - const CTxOut& out = psbt.tx->vout.at(index); + CMutableTransaction& tx = *Assert(psbt.tx); + const CTxOut& out = tx.vout.at(index); PSBTOutput& psbt_out = psbt.outputs.at(index); // Fill a SignatureData with output info @@ -217,7 +220,7 @@ void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransactio // Construct a would-be spend of this output, to update sigdata with. // Note that ProduceSignature is used to fill in metadata (not actual signatures), // so provider does not need to provide any private keys (it can be a HidingSigningProvider). - MutableTransactionSignatureCreator creator(psbt.tx.get_ptr(), /* index */ 0, out.nValue, SIGHASH_ALL); + MutableTransactionSignatureCreator creator(&tx, /* index */ 0, out.nValue, SIGHASH_ALL); ProduceSignature(provider, creator, out.scriptPubKey, sigdata); // Put redeem_script, witness_script, key paths, into PSBTOutput. From fa7e803f3e4df33117927aef6fef9bfaee4f410d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 16 Dec 2020 12:02:27 +0100 Subject: [PATCH 2/3] Remove unused MakeOptional The only use was to work around a compiler warning in an ancient compiler, which we no longer support. --- src/optional.h | 7 ------- src/wallet/rpcwallet.cpp | 5 ++--- src/wallet/wallet.cpp | 3 +-- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/optional.h b/src/optional.h index a382cd7b77c..252d56fd587 100644 --- a/src/optional.h +++ b/src/optional.h @@ -13,13 +13,6 @@ template using Optional = boost::optional; -//! Substitute for C++17 std::make_optional -template -Optional MakeOptional(bool condition, T&& value) -{ - return boost::make_optional(condition, std::forward(value)); -} - //! Substitute for C++17 std::nullopt static auto& nullopt = boost::none; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 94a73b67dfe..614ebd22f6f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1573,8 +1573,7 @@ static RPCHelpMan listsinceblock() LOCK(wallet.cs_wallet); - // The way the 'height' is initialized is just a workaround for the gcc bug #47679 since version 4.6.0. - Optional height = MakeOptional(false, int()); // Height of the specified block or the common ancestor, if the block provided was in a deactivated chain. + Optional height; // Height of the specified block or the common ancestor, if the block provided was in a deactivated chain. Optional altheight; // Height of the specified block, even if it's in a deactivated chain. int target_confirms = 1; isminefilter filter = ISMINE_SPENDABLE; @@ -3597,7 +3596,7 @@ static RPCHelpMan rescanblockchain() } int start_height = 0; - Optional stop_height = MakeOptional(false, int()); + Optional stop_height; uint256 start_block; { LOCK(pwallet->cs_wallet); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 740e671595c..e8a67ede2b5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4055,8 +4055,7 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, const std::st // No need to read and scan block if block was created before // our wallet birthday (as adjusted for block time variability) - // The way the 'time_first_key' is initialized is just a workaround for the gcc bug #47679 since version 4.6.0. - Optional time_first_key = MakeOptional(false, int64_t());; + Optional time_first_key; for (auto spk_man : walletInstance->GetAllScriptPubKeyMans()) { int64_t time = spk_man->GetTimeFirstKey(); if (!time_first_key || time < *time_first_key) time_first_key = time; From fa4435e22f78f632a455016ce00a357009aac059 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 5 Jun 2020 23:02:44 +0200 Subject: [PATCH 3/3] Replace boost::optional with std::optional --- src/bench/wallet_balance.cpp | 2 +- src/optional.h | 9 +++++---- src/wallet/feebumper.cpp | 2 +- src/wallet/fees.cpp | 2 +- src/wallet/rpcwallet.cpp | 2 +- src/wallet/wallet.cpp | 6 +++--- test/lint/lint-includes.sh | 1 - 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index aa436ee3ea3..b385cec085f 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -36,7 +36,7 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY); for (int i = 0; i < 100; ++i) { - generatetoaddress(test_setup.m_node, address_mine.get_value_or(ADDRESS_WATCHONLY)); + generatetoaddress(test_setup.m_node, address_mine.value_or(ADDRESS_WATCHONLY)); generatetoaddress(test_setup.m_node, ADDRESS_WATCHONLY); } SyncWithValidationInterfaceQueue(); diff --git a/src/optional.h b/src/optional.h index 252d56fd587..40c6b6cc31b 100644 --- a/src/optional.h +++ b/src/optional.h @@ -5,15 +5,16 @@ #ifndef BITCOIN_OPTIONAL_H #define BITCOIN_OPTIONAL_H +#include #include -#include - //! Substitute for C++17 std::optional +//! DEPRECATED use std::optional in new code. template -using Optional = boost::optional; +using Optional = std::optional; //! Substitute for C++17 std::nullopt -static auto& nullopt = boost::none; +//! DEPRECATED use std::nullopt in new code. +static auto& nullopt = std::nullopt; #endif // BITCOIN_OPTIONAL_H diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 1bbfa197d75..aea5e8e60a2 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -231,7 +231,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo // Write back transaction mtx = CMutableTransaction(*tx_new); // Mark new tx not replaceable, if requested. - if (!coin_control.m_signal_bip125_rbf.get_value_or(wallet.m_signal_rbf)) { + if (!coin_control.m_signal_bip125_rbf.value_or(wallet.m_signal_rbf)) { for (auto& input : mtx.vin) { if (input.nSequence < 0xfffffffe) input.nSequence = 0xfffffffe; } diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 249bc833c67..e87e315bf47 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -49,7 +49,7 @@ CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_contr // We will use smart fee estimation unsigned int target = coin_control.m_confirm_target ? *coin_control.m_confirm_target : wallet.m_confirm_target; // By default estimates are economical iff we are signaling opt-in-RBF - bool conservative_estimate = !coin_control.m_signal_bip125_rbf.get_value_or(wallet.m_signal_rbf); + bool conservative_estimate = !coin_control.m_signal_bip125_rbf.value_or(wallet.m_signal_rbf); // Allow to override the default fee estimate mode over the CoinControl instance if (coin_control.m_fee_mode == FeeEstimateMode::CONSERVATIVE) conservative_estimate = true; else if (coin_control.m_fee_mode == FeeEstimateMode::ECONOMICAL) conservative_estimate = false; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 614ebd22f6f..ae4e8f28989 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -309,7 +309,7 @@ static RPCHelpMan getrawchangeaddress() throw JSONRPCError(RPC_WALLET_ERROR, "Error: This wallet has no available keys"); } - OutputType output_type = pwallet->m_default_change_type.get_value_or(pwallet->m_default_address_type); + OutputType output_type = pwallet->m_default_change_type.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())); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e8a67ede2b5..ed5de6e852b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -85,9 +85,9 @@ static void UpdateWalletSetting(interfaces::Chain& chain, std::vector& warnings) { if (load_on_startup == nullopt) return; - if (load_on_startup.get() && !AddWalletSetting(chain, wallet_name)) { + if (load_on_startup.value() && !AddWalletSetting(chain, wallet_name)) { warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may not be loaded next node startup.")); - } else if (!load_on_startup.get() && !RemoveWalletSetting(chain, wallet_name)) { + } else if (!load_on_startup.value() && !RemoveWalletSetting(chain, wallet_name)) { warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may still be loaded next node startup.")); } } @@ -3051,7 +3051,7 @@ bool CWallet::CreateTransactionInternal( // to avoid conflicting with other possible uses of nSequence, // and in the spirit of "smallest possible change from prior // behavior." - const uint32_t nSequence = coin_control.m_signal_bip125_rbf.get_value_or(m_signal_rbf) ? MAX_BIP125_RBF_SEQUENCE : (CTxIn::SEQUENCE_FINAL - 1); + const uint32_t nSequence = coin_control.m_signal_bip125_rbf.value_or(m_signal_rbf) ? MAX_BIP125_RBF_SEQUENCE : (CTxIn::SEQUENCE_FINAL - 1); for (const auto& coin : selected_coins) { txNew.vin.push_back(CTxIn(coin.outpoint, CScript(), nSequence)); } diff --git a/test/lint/lint-includes.sh b/test/lint/lint-includes.sh index fde77aea2dc..393f734abeb 100755 --- a/test/lint/lint-includes.sh +++ b/test/lint/lint-includes.sh @@ -60,7 +60,6 @@ EXPECTED_BOOST_INCLUDES=( boost/multi_index/ordered_index.hpp boost/multi_index/sequenced_index.hpp boost/multi_index_container.hpp - boost/optional.hpp boost/preprocessor/cat.hpp boost/preprocessor/stringize.hpp boost/process.hpp