Merge bitcoin/bitcoin#28754: [26.x] Backports for rc2

e4e84790f6 doc: update manual pages for v26.0rc2 (fanquake)
0b189a9092 build: bump version to v26.0rc2 (fanquake)
e097d4cb53 gui: fix crash on selecting "Mask values" in transaction view (Sebastian Falbesoner)
05e8874554 guix: update signapple (fanquake)
deccc50631 guix: Zip needs to include all files with time as SOURCE_DATE_EPOCH (Andrew Chow)
fe57abd7e9 test: add coverage for snapshot chainstate not matching AssumeUTXO parameters (pablomartin4btc)
b761a58171 assumeutxo, blockstorage: prevent core dump on invalid hash (pablomartin4btc)
d3ebf6e9fc [test] Test i2p private key constraints (Vasil Dimov)
1f11784aac [net] Check i2p private key constraints (dergoegge)
6544ffa01f bugfix: Mark CNoDestination and PubKeyDestination constructor explicit (MarcoFalke)

Pull request description:

  Backports for v26.0rc2:
  * #28695
  * #28698
  * #28728
  * #28757
  * #28759
  * https://github.com/bitcoin-core/gui/pull/774

ACKs for top commit:
  josibake:
    ACK e4e84790f6
  hebasto:
    re-ACK e4e84790f6, only a backport of https://github.com/bitcoin-core/gui/pull/774 added since my [recent](https://github.com/bitcoin/bitcoin/pull/28754#pullrequestreview-1707143194) review.
  TheCharlatan:
    Re-ACK e4e84790f6

Tree-SHA512: 4b95afd26b8bf91250cb883423de8b274cefa48dc474734f5900aeb756eee3a6c656116efcfa2caff3c250678c16b70cc6b7a5d840018dc7e2c1e8161622cd61

configure.ac

@@ -2,7 +2,7 @@ AC_PREREQ([2.69])
 define(_CLIENT_VERSION_MAJOR, 26)
 define(_CLIENT_VERSION_MINOR, 0)
 define(_CLIENT_VERSION_BUILD, 0)
-define(_CLIENT_VERSION_RC, 1)
+define(_CLIENT_VERSION_RC, 2)
 define(_CLIENT_VERSION_IS_RELEASE, true)
 define(_COPYRIGHT_YEAR, 2023)
 define(_COPYRIGHT_HOLDERS,[The %s developers])

contrib/guix/libexec/codesign.sh

@@ -86,7 +86,11 @@ mkdir -p "$DISTSRC"
             signapple apply dist/Bitcoin-Qt.app codesignatures/osx/dist
 
             # Make a .zip from dist/
-            zip "${OUTDIR}/${DISTNAME}-${HOST}.zip" dist/*
+            cd dist/
+            find . -print0 \
+                | xargs -0r touch --no-dereference --date="@${SOURCE_DATE_EPOCH}"
+            find . | sort \
+                | zip -X@ "${OUTDIR}/${DISTNAME}-${HOST}.zip"
             ;;
         *)
             exit 1

contrib/guix/manifest.scm

@@ -19,7 +19,6 @@
              ((gnu packages python) #:select (python-minimal))
              ((gnu packages python-build) #:select (python-tomli))
              ((gnu packages python-crypto) #:select (python-asn1crypto))
-             ((gnu packages python-web) #:select (python-requests))
              ((gnu packages tls) #:select (openssl))
              ((gnu packages version-control) #:select (git-minimal))
              (guix build-system cmake)
@@ -445,7 +444,7 @@ and endian independent.")
     (license license:expat)))
 
 (define-public python-signapple
-  (let ((commit "8a945a2e7583be2665cf3a6a89d665b70ecd1ab6"))
+  (let ((commit "7a96b4171a360abf0f0f56e499f8f9ed2116280d"))
     (package
       (name "python-signapple")
       (version (git-version "0.1" "1" commit))
@@ -458,14 +457,13 @@ and endian independent.")
          (file-name (git-file-name name commit))
          (sha256
           (base32
-           "0fr1hangvfyiwflca6jg5g8zvg3jc9qr7vd2c12ff89pznf38dlg"))))
+           "0aa4k180jnpal15yhncnm3g3z9gzmi7qb25q5l0kaj444a1p2pm4"))))
       (build-system python-build-system)
       (propagated-inputs
        `(("python-asn1crypto" ,python-asn1crypto)
          ("python-oscrypto" ,python-oscrypto)
          ("python-certvalidator" ,python-certvalidator)
          ("python-elfesteem" ,python-elfesteem)
-         ("python-requests" ,python-requests)
          ("python-macholib" ,python-macholib)))
       ;; There are no tests, but attempting to run python setup.py test leads to
       ;; problems, just disable the test

doc/man/bitcoin-cli.1

@@ -1,7 +1,7 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-CLI "1" "October 2023" "bitcoin-cli v26.0.0rc1" "User Commands"
+.TH BITCOIN-CLI "1" "October 2023" "bitcoin-cli v26.0.0rc2" "User Commands"
 .SH NAME
-bitcoin-cli \- manual page for bitcoin-cli v26.0.0rc1
+bitcoin-cli \- manual page for bitcoin-cli v26.0.0rc2
 .SH SYNOPSIS
 .B bitcoin-cli
 [\fI\,options\/\fR] \fI\,<command> \/\fR[\fI\,params\/\fR]  \fI\,Send command to Bitcoin Core\/\fR
@@ -15,7 +15,7 @@ bitcoin-cli \- manual page for bitcoin-cli v26.0.0rc1
 .B bitcoin-cli
 [\fI\,options\/\fR] \fI\,help <command>      Get help for a command\/\fR
 .SH DESCRIPTION
-Bitcoin Core RPC client version v26.0.0rc1
+Bitcoin Core RPC client version v26.0.0rc2
 .SH OPTIONS
 .HP
 \-?

doc/man/bitcoin-qt.1

@@ -1,12 +1,12 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-QT "1" "October 2023" "bitcoin-qt v26.0.0rc1" "User Commands"
+.TH BITCOIN-QT "1" "October 2023" "bitcoin-qt v26.0.0rc2" "User Commands"
 .SH NAME
-bitcoin-qt \- manual page for bitcoin-qt v26.0.0rc1
+bitcoin-qt \- manual page for bitcoin-qt v26.0.0rc2
 .SH SYNOPSIS
 .B bitcoin-qt
 [\fI\,command-line options\/\fR]
 .SH DESCRIPTION
-Bitcoin Core version v26.0.0rc1
+Bitcoin Core version v26.0.0rc2
 .SH OPTIONS
 .HP
 \-?

doc/man/bitcoin-tx.1

@@ -1,7 +1,7 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-TX "1" "October 2023" "bitcoin-tx v26.0.0rc1" "User Commands"
+.TH BITCOIN-TX "1" "October 2023" "bitcoin-tx v26.0.0rc2" "User Commands"
 .SH NAME
-bitcoin-tx \- manual page for bitcoin-tx v26.0.0rc1
+bitcoin-tx \- manual page for bitcoin-tx v26.0.0rc2
 .SH SYNOPSIS
 .B bitcoin-tx
 [\fI\,options\/\fR] \fI\,<hex-tx> \/\fR[\fI\,commands\/\fR]  \fI\,Update hex-encoded bitcoin transaction\/\fR
@@ -9,7 +9,7 @@ bitcoin-tx \- manual page for bitcoin-tx v26.0.0rc1
 .B bitcoin-tx
 [\fI\,options\/\fR] \fI\,-create \/\fR[\fI\,commands\/\fR]   \fI\,Create hex-encoded bitcoin transaction\/\fR
 .SH DESCRIPTION
-Bitcoin Core bitcoin\-tx utility version v26.0.0rc1
+Bitcoin Core bitcoin\-tx utility version v26.0.0rc2
 .SH OPTIONS
 .HP
 \-?

doc/man/bitcoin-util.1

@@ -1,12 +1,12 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-UTIL "1" "October 2023" "bitcoin-util v26.0.0rc1" "User Commands"
+.TH BITCOIN-UTIL "1" "October 2023" "bitcoin-util v26.0.0rc2" "User Commands"
 .SH NAME
-bitcoin-util \- manual page for bitcoin-util v26.0.0rc1
+bitcoin-util \- manual page for bitcoin-util v26.0.0rc2
 .SH SYNOPSIS
 .B bitcoin-util
 [\fI\,options\/\fR] [\fI\,commands\/\fR]  \fI\,Do stuff\/\fR
 .SH DESCRIPTION
-Bitcoin Core bitcoin\-util utility version v26.0.0rc1
+Bitcoin Core bitcoin\-util utility version v26.0.0rc2
 .SH OPTIONS
 .HP
 \-?

doc/man/bitcoin-wallet.1

@@ -1,9 +1,9 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-WALLET "1" "October 2023" "bitcoin-wallet v26.0.0rc1" "User Commands"
+.TH BITCOIN-WALLET "1" "October 2023" "bitcoin-wallet v26.0.0rc2" "User Commands"
 .SH NAME
-bitcoin-wallet \- manual page for bitcoin-wallet v26.0.0rc1
+bitcoin-wallet \- manual page for bitcoin-wallet v26.0.0rc2
 .SH DESCRIPTION
-Bitcoin Core bitcoin\-wallet version v26.0.0rc1
+Bitcoin Core bitcoin\-wallet version v26.0.0rc2
 .PP
 bitcoin\-wallet is an offline tool for creating and interacting with Bitcoin Core wallet files.
 By default bitcoin\-wallet will act on wallets in the default mainnet wallet directory in the datadir.

doc/man/bitcoind.1

@@ -1,12 +1,12 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIND "1" "October 2023" "bitcoind v26.0.0rc1" "User Commands"
+.TH BITCOIND "1" "October 2023" "bitcoind v26.0.0rc2" "User Commands"
 .SH NAME
-bitcoind \- manual page for bitcoind v26.0.0rc1
+bitcoind \- manual page for bitcoind v26.0.0rc2
 .SH SYNOPSIS
 .B bitcoind
 [\fI\,options\/\fR]                     \fI\,Start Bitcoin Core\/\fR
 .SH DESCRIPTION
-Bitcoin Core version v26.0.0rc1
+Bitcoin Core version v26.0.0rc2
 .SH OPTIONS
 .HP
 \-?

src/addresstype.h

@@ -13,15 +13,16 @@
 #include <variant>
 #include <algorithm>
 
-class CNoDestination {
+class CNoDestination
+{
 private:
     CScript m_script;
 
 public:
     CNoDestination() = default;
-    CNoDestination(const CScript& script) : m_script(script) {}
+    explicit CNoDestination(const CScript& script) : m_script(script) {}
 
-    const CScript& GetScript() const { return m_script; }
+    const CScript& GetScript() const LIFETIMEBOUND { return m_script; }
 
     friend bool operator==(const CNoDestination& a, const CNoDestination& b) { return a.GetScript() == b.GetScript(); }
     friend bool operator<(const CNoDestination& a, const CNoDestination& b) { return a.GetScript() < b.GetScript(); }
@@ -32,7 +33,7 @@ private:
     CPubKey m_pubkey;
 
 public:
-    PubKeyDestination(const CPubKey& pubkey) : m_pubkey(pubkey) {}
+    explicit PubKeyDestination(const CPubKey& pubkey) : m_pubkey(pubkey) {}
 
     const CPubKey& GetPubKey() const LIFETIMEBOUND { return m_pubkey; }
 

src/bench/wallet_create_tx.cpp

@@ -94,13 +94,14 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type
     }
 
     // Generate destinations
-    CScript dest = GetScriptForDestination(getNewDestination(wallet, output_type));
+    const auto dest{getNewDestination(wallet, output_type)};
 
     // Generate chain; each coinbase will have two outputs to fill-up the wallet
     const auto& params = Params();
+    const CScript coinbase_out{GetScriptForDestination(dest)};
     unsigned int chain_size = 5000; // 5k blocks means 10k UTXO for the wallet (minus 200 due COINBASE_MATURITY)
     for (unsigned int i = 0; i < chain_size; ++i) {
-        generateFakeBlock(params, test_setup->m_node, wallet, dest);
+        generateFakeBlock(params, test_setup->m_node, wallet, coinbase_out);
     }
 
     // Check available balance
@@ -185,4 +186,4 @@ static void WalletAvailableCoins(benchmark::Bench& bench) { AvailableCoins(bench
 
 BENCHMARK(WalletCreateTxUseOnlyPresetInputs, benchmark::PriorityLevel::LOW)
 BENCHMARK(WalletCreateTxUsePresetInputsAndCoinSelection, benchmark::PriorityLevel::LOW)
-BENCHMARK(WalletAvailableCoins, benchmark::PriorityLevel::LOW);
+BENCHMARK(WalletAvailableCoins, benchmark::PriorityLevel::LOW);

src/i2p.cpp

@@ -384,11 +384,26 @@ Binary Session::MyDestination() const
     static constexpr size_t CERT_LEN_POS = 385;
 
     uint16_t cert_len;
+
+    if (m_private_key.size() < CERT_LEN_POS + sizeof(cert_len)) {
+        throw std::runtime_error(strprintf("The private key is too short (%d < %d)",
+                                           m_private_key.size(),
+                                           CERT_LEN_POS + sizeof(cert_len)));
+    }
+
     memcpy(&cert_len, &m_private_key.at(CERT_LEN_POS), sizeof(cert_len));
     cert_len = be16toh(cert_len);
 
     const size_t dest_len = DEST_LEN_BASE + cert_len;
 
+    if (dest_len > m_private_key.size()) {
+        throw std::runtime_error(strprintf("Certificate length (%d) designates that the private key should "
+                                           "be %d bytes, but it is only %d bytes",
+                                           cert_len,
+                                           dest_len,
+                                           m_private_key.size()));
+    }
+
     return Binary{m_private_key.begin(), m_private_key.begin() + dest_len};
 }
 

src/node/blockstorage.cpp

@@ -387,7 +387,12 @@ bool BlockManager::LoadBlockIndex(const std::optional<uint256>& snapshot_blockha
     }
 
     if (snapshot_blockhash) {
-        const AssumeutxoData au_data = *Assert(GetParams().AssumeutxoForBlockhash(*snapshot_blockhash));
+        const std::optional<AssumeutxoData> maybe_au_data = GetParams().AssumeutxoForBlockhash(*snapshot_blockhash);
+        if (!maybe_au_data) {
+            m_opts.notifications.fatalError(strprintf("Assumeutxo data not found for the given blockhash '%s'.", snapshot_blockhash->ToString()));
+            return false;
+        }
+        const AssumeutxoData& au_data = *Assert(maybe_au_data);
         m_snapshot_height = au_data.height;
         CBlockIndex* base{LookupBlockIndex(*snapshot_blockhash)};
 

src/qt/transactionview.cpp

@@ -531,6 +531,9 @@ void TransactionView::showDetails()
         TransactionDescDialog *dlg = new TransactionDescDialog(selection.at(0));
         dlg->setAttribute(Qt::WA_DeleteOnClose);
         m_opened_dialogs.append(dlg);
+        connect(dlg, &QObject::destroyed, [this, dlg] {
+            m_opened_dialogs.removeOne(dlg);
+        });
         dlg->show();
     }
 }

src/qt/walletmodel.cpp

@@ -187,8 +187,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
             setAddress.insert(rcp.address);
             ++nAddresses;
 
-            CScript scriptPubKey = GetScriptForDestination(DecodeDestination(rcp.address.toStdString()));
+            CRecipient recipient{DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount};
-            CRecipient recipient = {scriptPubKey, rcp.amount, rcp.fSubtractFeeFromAmount};
             vecSend.push_back(recipient);
 
             total += rcp.amount;

src/test/i2p_tests.cpp

@@ -9,6 +9,7 @@
 #include <test/util/logging.h>
 #include <test/util/net.h>
 #include <test/util/setup_common.h>
+#include <util/readwritefile.h>
 #include <util/threadinterrupt.h>
 
 #include <boost/test/unit_test.hpp>
@@ -125,4 +126,47 @@ BOOST_AUTO_TEST_CASE(listen_ok_accept_fail)
     }
 }
 
+BOOST_AUTO_TEST_CASE(damaged_private_key)
+{
+    const auto CreateSockOrig = CreateSock;
+
+    CreateSock = [](const CService&) {
+        return std::make_unique<StaticContentsSock>("HELLO REPLY RESULT=OK VERSION=3.1\n"
+                                                    "SESSION STATUS RESULT=OK DESTINATION=\n");
+    };
+
+    const auto i2p_private_key_file = m_args.GetDataDirNet() / "test_i2p_private_key_damaged";
+
+    for (const auto& [file_contents, expected_error] : std::vector<std::tuple<std::string, std::string>>{
+             {"", "The private key is too short (0 < 387)"},
+
+             {"abcd", "The private key is too short (4 < 387)"},
+
+             {std::string(386, '\0'), "The private key is too short (386 < 387)"},
+
+             {std::string(385, '\0') + '\0' + '\1',
+              "Certificate length (1) designates that the private key should be 388 bytes, but it is only "
+              "387 bytes"},
+
+             {std::string(385, '\0') + '\0' + '\5' + "abcd",
+              "Certificate length (5) designates that the private key should be 392 bytes, but it is only "
+              "391 bytes"}}) {
+        BOOST_REQUIRE(WriteBinaryFile(i2p_private_key_file, file_contents));
+
+        CThreadInterrupt interrupt;
+        i2p::sam::Session session(i2p_private_key_file, CService{}, &interrupt);
+
+        {
+            ASSERT_DEBUG_LOG("Creating persistent SAM session");
+            ASSERT_DEBUG_LOG(expected_error);
+
+            i2p::Connection conn;
+            bool proxy_error;
+            BOOST_CHECK(!session.Connect(CService{}, conn, proxy_error));
+        }
+    }
+
+    CreateSock = CreateSockOrig;
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/wallet/test/spend_tests.cpp

@@ -78,7 +78,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup)
 
     // Try to create a tx that spends more than what preset inputs + wallet selected inputs are covering for.
     // The wallet can cover up to 200 BTC, and the tx target is 299 BTC.
-    std::vector<CRecipient> recipients = {{GetScriptForDestination(*Assert(wallet->GetNewDestination(OutputType::BECH32, "dummy"))),
+    std::vector<CRecipient> recipients{{*Assert(wallet->GetNewDestination(OutputType::BECH32, "dummy")),
                                            /*nAmount=*/299 * COIN, /*fSubtractFeeFromAmount=*/true}};
     CCoinControl coin_control;
     coin_control.m_allow_other_inputs = true;

src/wallet/test/wallet_tests.cpp

@@ -605,7 +605,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
     // returns the coin associated with the change address underneath the
     // coinbaseKey pubkey, even though the change address has a different
     // pubkey.
-    AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, /*subtract_fee=*/false});
+    AddTx(CRecipient{PubKeyDestination{{}}, 1 * COIN, /*subtract_fee=*/false});
     {
         LOCK(wallet->cs_wallet);
         list = ListCoins(*wallet);

test/functional/feature_assumeutxo.py

@@ -33,6 +33,8 @@ Interesting starting states could be loading a snapshot when the current chain t
 - TODO: Not an ancestor or a descendant of the snapshot block and has more work
 
 """
+from shutil import rmtree
+
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -107,6 +109,22 @@ class AssumeutxoTest(BitcoinTestFramework):
                 f.write(valid_snapshot_contents[(32 + 8 + offset + len(content)):])
             expected_error(log_msg=f"[snapshot] bad snapshot content hash: expected 61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53, got {wrong_hash}")
 
+    def test_invalid_chainstate_scenarios(self):
+        self.log.info("Test different scenarios of invalid snapshot chainstate in datadir")
+
+        self.log.info("  - snapshot chainstate refering to a block that is not in the assumeutxo parameters")
+        self.stop_node(0)
+        chainstate_snapshot_path = self.nodes[0].chain_path / "chainstate_snapshot"
+        chainstate_snapshot_path.mkdir()
+        with open(chainstate_snapshot_path / "base_blockhash", 'wb') as f:
+            f.write(b'z' * 32)
+        expected_error = f"Error: A fatal internal error occurred, see debug.log for details"
+        self.nodes[0].assert_start_raises_init_error(expected_msg=expected_error)
+
+        # resurrect node again
+        rmtree(chainstate_snapshot_path)
+        self.start_node(0)
+
     def run_test(self):
         """
         Bring up two (disconnected) nodes, mine some new blocks on the first,
@@ -166,6 +184,7 @@ class AssumeutxoTest(BitcoinTestFramework):
         assert_equal(n0.getblockchaininfo()["blocks"], FINAL_HEIGHT)
 
         self.test_invalid_snapshot_scenarios(dump_output['path'])
+        self.test_invalid_chainstate_scenarios()
 
         self.log.info(f"Loading snapshot into second node from {dump_output['path']}")
         loaded = n1.loadtxoutset(dump_output['path'])