Merge #20671: Replace boost::optional with std::optional

fa4435e Replace boost::optional with std::optional (MarcoFalke)
fa7e803 Remove unused MakeOptional (MarcoFalke)
fadd402 psbt: Assert that tx has a value in UpdatePSBTOutput (MarcoFalke)

Pull request description:

  Now that we can use std::optional from the vanilla standard library, drop the third-party boost dependency

ACKs for top commit:
  practicalswift:
    cr ACK fa4435e: patch looks correct!
  laanwj:
    code review ACK fa4435e
  hebasto:
    ACK fa4435e, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 50c5a1a130cac65e043e0177ba5b009fc2ba09343af4e23322ff2eb32184a55f8f2dea66e7a1b9d9acf56bc164eef4e47448750549a07f3b661199ac9bf9afef

Author: laanwj

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();

src/optional.h

@@ -5,22 +5,16 @@
 #ifndef BITCOIN_OPTIONAL_H
 #define BITCOIN_OPTIONAL_H
 
+#include <optional>
 #include <utility>
 
-#include <boost/optional.hpp>
-
 //! Substitute for C++17 std::optional
+//! DEPRECATED use std::optional in new code.
 template <typename T>
-using Optional = boost::optional<T>;
-
-//! Substitute for C++17 std::make_optional
-template <typename T>
-Optional<T> MakeOptional(bool condition, T&& value)
-{
-    return boost::make_optional(condition, std::forward<T>(value));
-}
+using Optional = std::optional<T>;
 
 //! 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

src/psbt.cpp

@@ -3,6 +3,8 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <psbt.h>
+
+#include <util/check.h>
 #include <util/strencodings.h>
 
 
@@ -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.

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;
         }

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;

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()));
@@ -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<int> height = MakeOptional(false, int()); // Height of the specified block or the common ancestor, if the block provided was in a deactivated chain.
+    Optional<int> height;    // Height of the specified block or the common ancestor, if the block provided was in a deactivated chain.
     Optional<int> 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<int> stop_height = MakeOptional(false, int());
+    Optional<int> stop_height;
     uint256 start_block;
     {
         LOCK(pwallet->cs_wallet);

src/wallet/wallet.cpp

@@ -85,9 +85,9 @@ static void UpdateWalletSetting(interfaces::Chain& chain,
                                 std::vector<bilingual_str>& 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));
         }
@@ -4055,8 +4055,7 @@ std::shared_ptr<CWallet> 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<int64_t> time_first_key = MakeOptional(false, int64_t());;
+        Optional<int64_t> 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;

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