Merge #20612: [0.21] final rc3 backports

48134a09ad doc: Update wallet database installation guide for macOS (Hennadii Stepanov) f51e1cb291 build: Use Homebrew's sqlite package if it is available (Hennadii Stepanov) 48f8929aad build, refactor: Check that Homebrew's qt5 package is actually installed (Hennadii Stepanov) 96124a2041 build: Check that Homebrew's berkeley-db4 package is actually installed (Hennadii Stepanov) 61e316e661 Don't set BDB flags when configuring without (Jonas Schnelli) ce13b99020 Add regression test for incorrect decoding (Pieter Wuille) 1caa32e3f2 Improve heuristic hex transaction decoding (Pieter Wuille) 0d3c140c4d test: add coverage for passing fee rate as a string (Jon Atack) 06c84232b3 wallet, bugfix: allow send to take string fee rate values (Jon Atack) bead935470 Send and require SENDADDRV2 before VERACK (Pieter Wuille) 9e806887a8 Don't send 'sendaddrv2' to pre-70016 software (Pieter Wuille) Pull request description: ACKs for top commit: laanwj: ACK 48134a09ad Tree-SHA512: 92f1199b96ab7775f88e882ec7fedf43118a4b8452d1c8d0b1cf072d8de153bbb601c7381bc1c5c80c93803c6f9942d54646e9c74e3a6703ce13854fb383fd5e
2020-12-10 19:17:29 +01:00 · 2020-12-10 19:17:29 +01:00 · 3fee499bc3
parent aa4b8ebfec 48134a09ad
commit 3fee499bc3
12 changed files with 147 additions and 40 deletions
--- a/configure.ac
+++ b/configure.ac
@ -646,16 +646,19 @@ case $host in
         dnl It's safe to add these paths even if the functionality is disabled by
         dnl the user (--without-wallet or --without-gui for example).

-         bdb_prefix=$($BREW --prefix berkeley-db4 2>/dev/null)
-         qt5_prefix=$($BREW --prefix qt5 2>/dev/null)
-         if test x$bdb_prefix != x && test "x$BDB_CFLAGS" = "x" && test "x$BDB_LIBS" = "x"; then
+         if test "x$use_bdb" != xno && $BREW list --versions berkeley-db4 >/dev/null && test "x$BDB_CFLAGS" = "x" && test "x$BDB_LIBS" = "x"; then
+           bdb_prefix=$($BREW --prefix berkeley-db4 2>/dev/null)
           dnl This must precede the call to BITCOIN_FIND_BDB48 below.
           BDB_CFLAGS="-I$bdb_prefix/include"
           BDB_LIBS="-L$bdb_prefix/lib -ldb_cxx-4.8"
         fi
-         if test x$qt5_prefix != x; then
-           PKG_CONFIG_PATH="$qt5_prefix/lib/pkgconfig:$PKG_CONFIG_PATH"
-           export PKG_CONFIG_PATH
+
+         if test "x$use_sqlite" != xno && $BREW list --versions sqlite3 >/dev/null; then
+           export PKG_CONFIG_PATH="$($BREW --prefix sqlite3 2>/dev/null)/lib/pkgconfig:$PKG_CONFIG_PATH"
+         fi
+
+         if $BREW list --versions qt5 >/dev/null; then
+           export PKG_CONFIG_PATH="$($BREW --prefix qt5 2>/dev/null)/lib/pkgconfig:$PKG_CONFIG_PATH"
         fi
       fi
     else
--- a/doc/build-osx.md
+++ b/doc/build-osx.md
@ -19,7 +19,7 @@ Then install [Homebrew](https://brew.sh).

 ## Dependencies
 ```shell
-brew install automake berkeley-db4 libtool boost miniupnpc pkg-config python qt libevent qrencode sqlite
+brew install automake libtool boost miniupnpc pkg-config python qt libevent qrencode
 ```

 If you run into issues, check [Homebrew's troubleshooting page](https://docs.brew.sh/Troubleshooting).
@ -30,7 +30,22 @@ If you want to build the disk image with `make deploy` (.dmg / optional), you ne
 brew install librsvg
 ```

-## Berkeley DB
+The wallet support requires one or both of the dependencies ([*SQLite*](#sqlite) and [*Berkeley DB*](#berkeley-db)) in the sections below.
+To build Bitcoin Core without wallet, see [*Disable-wallet mode*](#disable-wallet-mode).
+
+#### SQLite
+
+Usually, macOS installation already has a suitable SQLite installation.
+Also, the Homebrew package could be installed:
+
+```shell
+brew install sqlite
+```
+
+In that case the Homebrew package will prevail.
+
+#### Berkeley DB
+
 It is recommended to use Berkeley DB 4.8. If you have to build it yourself,
 you can use [this](/contrib/install_db4.sh) script to install it
 like so:
@ -41,7 +56,11 @@ like so:

 from the root of the repository.

-**Note**: You only need Berkeley DB if the wallet is enabled (see [*Disable-wallet mode*](/doc/build-osx.md#disable-wallet-mode)).
+Also, the Homebrew package could be installed:
+
+```shell
+brew install berkeley-db4
+```

 ## Build Bitcoin Core

@ -72,14 +91,14 @@ from the root of the repository.
    make deploy
    ```

-## `disable-wallet` mode
+## Disable-wallet mode
 When the intention is to run only a P2P node without a wallet, Bitcoin Core may be
-compiled in `disable-wallet` mode with:
+compiled in disable-wallet mode with:
 ```shell
 ./configure --disable-wallet
 ```

-In this case there is no dependency on Berkeley DB 4.8 and SQLite.
+In this case there is no dependency on [*Berkeley DB*](#berkeley-db) and [*SQLite*](#sqlite).

 Mining is also possible in disable-wallet mode using the `getblocktemplate` RPC call.

--- a/src/core_read.cpp
+++ b/src/core_read.cpp
@ -119,31 +119,72 @@ static bool CheckTxScriptsSanity(const CMutableTransaction& tx)

 static bool DecodeTx(CMutableTransaction& tx, const std::vector<unsigned char>& tx_data, bool try_no_witness, bool try_witness)
 {
+    // General strategy:
+    // - Decode both with extended serialization (which interprets the 0x0001 tag as a marker for
+    //   the presense of witnesses) and with legacy serialization (which interprets the tag as a
+    //   0-input 1-output incomplete transaction).
+    //   - Restricted by try_no_witness (which disables legacy if false) and try_witness (which
+    //     disables extended if false).
+    //   - Ignore serializations that do not fully consume the hex string.
+    // - If neither succeeds, fail.
+    // - If only one succeeds, return that one.
+    // - If both decode attempts succeed:
+    //   - If only one passes the CheckTxScriptsSanity check, return that one.
+    //   - If neither or both pass CheckTxScriptsSanity, return the extended one.
+
+    CMutableTransaction tx_extended, tx_legacy;
+    bool ok_extended = false, ok_legacy = false;
+
+    // Try decoding with extended serialization support, and remember if the result successfully
+    // consumes the entire input.
    if (try_witness) {
        CDataStream ssData(tx_data, SER_NETWORK, PROTOCOL_VERSION);
        try {
-            ssData >> tx;
-            // If transaction looks sane, we don't try other mode even if requested
-            if (ssData.empty() && (!try_no_witness || CheckTxScriptsSanity(tx))) {
-                return true;
-            }
+            ssData >> tx_extended;
+            if (ssData.empty()) ok_extended = true;
        } catch (const std::exception&) {
            // Fall through.
        }
    }

+    // Optimization: if extended decoding succeeded and the result passes CheckTxScriptsSanity,
+    // don't bother decoding the other way.
+    if (ok_extended && CheckTxScriptsSanity(tx_extended)) {
+        tx = std::move(tx_extended);
+        return true;
+    }
+
+    // Try decoding with legacy serialization, and remember if the result successfully consumes the entire input.
    if (try_no_witness) {
        CDataStream ssData(tx_data, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS);
        try {
-            ssData >> tx;
-            if (ssData.empty()) {
-                return true;
-            }
+            ssData >> tx_legacy;
+            if (ssData.empty()) ok_legacy = true;
        } catch (const std::exception&) {
            // Fall through.
        }
    }

+    // If legacy decoding succeeded and passes CheckTxScriptsSanity, that's our answer, as we know
+    // at this point that extended decoding either failed or doesn't pass the sanity check.
+    if (ok_legacy && CheckTxScriptsSanity(tx_legacy)) {
+        tx = std::move(tx_legacy);
+        return true;
+    }
+
+    // If extended decoding succeeded, and neither decoding passes sanity, return the extended one.
+    if (ok_extended) {
+        tx = std::move(tx_extended);
+        return true;
+    }
+
+    // If legacy decoding succeeded and extended didn't, return the legacy one.
+    if (ok_legacy) {
+        tx = std::move(tx_legacy);
+        return true;
+    }
+
+    // If none succeeded, we failed.
    return false;
 }

--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@ -2364,10 +2364,16 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
            m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::WTXIDRELAY));
        }

-        m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK));
-
        // Signal ADDRv2 support (BIP155).
-        m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2));
+        if (greatest_common_version >= 70016) {
+            // BIP155 defines addrv2 and sendaddrv2 for all protocol versions, but some
+            // implementations reject messages they don't know. As a courtesy, don't send
+            // it to nodes with a version before 70016, as no software is known to support
+            // BIP155 that doesn't announce at least that protocol version number.
+            m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2));
+        }
+
+        m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK));

        pfrom.nServices = nServices;
        pfrom.SetAddrLocal(addrMe);
@ -2540,6 +2546,17 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
        return;
    }

+    if (msg_type == NetMsgType::SENDADDRV2) {
+        if (pfrom.fSuccessfullyConnected) {
+            // Disconnect peers that send SENDADDRV2 message after VERACK; this
+            // must be negotiated between VERSION and VERACK.
+            pfrom.fDisconnect = true;
+            return;
+        }
+        pfrom.m_wants_addrv2 = true;
+        return;
+    }
+
    if (!pfrom.fSuccessfullyConnected) {
        LogPrint(BCLog::NET, "Unsupported message \"%s\" prior to verack from peer=%d\n", SanitizeString(msg_type), pfrom.GetId());
        return;
@ -2607,11 +2624,6 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
        return;
    }

-    if (msg_type == NetMsgType::SENDADDRV2) {
-        pfrom.m_wants_addrv2 = true;
-        return;
-    }
-
    if (msg_type == NetMsgType::SENDHEADERS) {
        LOCK(cs_main);
        State(pfrom.GetId())->fPreferHeaders = true;
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@ -4083,7 +4083,7 @@ static RPCHelpMan send()
                UniValueType(), // outputs (ARR or OBJ, checked later)
                UniValue::VNUM, // conf_target
                UniValue::VSTR, // estimate_mode
-                UniValue::VNUM, // fee_rate
+                UniValueType(), // fee_rate, will be checked by AmountFromValue() in SetFeeEstimateMode()
                UniValue::VOBJ, // options
                }, true
            );
--- a/test/functional/rpc_fundrawtransaction.py
+++ b/test/functional/rpc_fundrawtransaction.py
@ -717,10 +717,10 @@ class RawTransactionsTest(BitcoinTestFramework):

        result = node.fundrawtransaction(rawtx)  # uses self.min_relay_tx_fee (set by settxfee)
        btc_kvb_to_sat_vb = 100000  # (1e5)
-        result1 = node.fundrawtransaction(rawtx, {"fee_rate": 2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee})
+        result1 = node.fundrawtransaction(rawtx, {"fee_rate": str(2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee)})
        result2 = node.fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee})
        result3 = node.fundrawtransaction(rawtx, {"fee_rate": 10 * btc_kvb_to_sat_vb * self.min_relay_tx_fee})
-        result4 = node.fundrawtransaction(rawtx, {"feeRate": 10 * self.min_relay_tx_fee})
+        result4 = node.fundrawtransaction(rawtx, {"feeRate": str(10 * self.min_relay_tx_fee)})
        # Test that funding non-standard "zero-fee" transactions is valid.
        result5 = self.nodes[3].fundrawtransaction(rawtx, {"fee_rate": 0})
        result6 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 0})
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@ -190,11 +190,11 @@ class PSBTTest(BitcoinTestFramework):
        self.log.info("Test walletcreatefundedpsbt fee rate of 10000 sat/vB and 0.1 BTC/kvB produces a total fee at or slightly below -maxtxfee (~0.05290000)")
        res1 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": 10000, "add_inputs": True})
        assert_approx(res1["fee"], 0.055, 0.005)
-        res2 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.1, "add_inputs": True})
+        res2 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": "0.1", "add_inputs": True})
        assert_approx(res2["fee"], 0.055, 0.005)

        self.log.info("Test min fee rate checks with walletcreatefundedpsbt are bypassed, e.g. a fee_rate under 1 sat/vB is allowed")
-        res3 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": 0.99999999, "add_inputs": True})
+        res3 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": "0.99999999", "add_inputs": True})
        assert_approx(res3["fee"], 0.00000381, 0.0000001)
        res4 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"feeRate": 0.00000999, "add_inputs": True})
        assert_approx(res4["fee"], 0.00000381, 0.0000001)
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@ -372,6 +372,13 @@ class RawTransactionsTest(BitcoinTestFramework):
        encrawtx = "01000000010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f505000000000000000000"
        decrawtx = self.nodes[0].decoderawtransaction(encrawtx, False) # decode as non-witness transaction
        assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000'))
+        # known ambiguous transaction in the chain (see https://github.com/bitcoin/bitcoin/issues/20579)
+        encrawtx = "020000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff4b03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000ffffffff03f4c1fb4b0000000016001497cfc76442fe717f2a3f0cc9c175f7561b6619970000000000000000266a24aa21a9ed957d1036a80343e0d1b659497e1b48a38ebe876a056d45965fac4a85cda84e1900000000000000002952534b424c4f434b3a8e092581ab01986cbadc84f4b43f4fa4bb9e7a2e2a0caf9b7cf64d939028e22c0120000000000000000000000000000000000000000000000000000000000000000000000000"
+        decrawtx = self.nodes[0].decoderawtransaction(encrawtx)
+        decrawtx_wit = self.nodes[0].decoderawtransaction(encrawtx, True)
+        assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # fails to decode as non-witness transaction
+        assert_equal(decrawtx, decrawtx_wit) # the witness interpretation should be chosen
+        assert_equal(decrawtx['vin'][0]['coinbase'], "03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000")

        # Basic signrawtransaction test
        addr = self.nodes[1].getnewaddress()
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@ -396,9 +396,9 @@ class P2PInterface(P2PConnection):
        assert message.nVersion >= MIN_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_VERSION_SUPPORTED)
        if message.nVersion >= 70016:
            self.send_message(msg_wtxidrelay())
-        self.send_message(msg_verack())
        if self.support_addrv2:
            self.send_message(msg_sendaddrv2())
+        self.send_message(msg_verack())
        self.nServices = message.nServices

    # Connection helper methods
--- a/test/functional/wallet_basic.py
+++ b/test/functional/wallet_basic.py
@ -235,7 +235,8 @@ class WalletTest(BitcoinTestFramework):
        fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8
        explicit_fee_rate_btc_kvb = Decimal(fee_rate_btc_kvb) / 1000

-        txid = self.nodes[2].sendmany(amounts={address: 10}, fee_rate=fee_rate_sat_vb)
+        # Test passing fee_rate as a string
+        txid = self.nodes[2].sendmany(amounts={address: 10}, fee_rate=str(fee_rate_sat_vb))
        self.nodes[2].generate(1)
        self.sync_all(self.nodes[0:3])
        balance = self.nodes[2].getbalance()
@ -244,6 +245,17 @@ class WalletTest(BitcoinTestFramework):
        node_0_bal += Decimal('10')
        assert_equal(self.nodes[0].getbalance(), node_0_bal)

+        # Test passing fee_rate as an integer
+        amount = Decimal("0.0001")
+        txid = self.nodes[2].sendmany(amounts={address: amount}, fee_rate=fee_rate_sat_vb)
+        self.nodes[2].generate(1)
+        self.sync_all(self.nodes[0:3])
+        balance = self.nodes[2].getbalance()
+        node_2_bal = self.check_fee_amount(balance, node_2_bal - amount, explicit_fee_rate_btc_kvb, self.get_vsize(self.nodes[2].gettransaction(txid)['hex']))
+        assert_equal(balance, node_2_bal)
+        node_0_bal += amount
+        assert_equal(self.nodes[0].getbalance(), node_0_bal)
+
        for key in ["totalFee", "feeRate"]:
            assert_raises_rpc_error(-8, "Unknown named parameter key", self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=1, key=1)

@ -405,7 +417,7 @@ class WalletTest(BitcoinTestFramework):
            amount = 3
            fee_rate_sat_vb = 2
            fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8
-
+            # Test passing fee_rate as an integer
            txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=fee_rate_sat_vb)
            tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])
            self.nodes[0].generate(1)
@ -414,6 +426,19 @@ class WalletTest(BitcoinTestFramework):
            fee = prebalance - postbalance - Decimal(amount)
            assert_fee_amount(fee, tx_size, Decimal(fee_rate_btc_kvb))

+            prebalance = self.nodes[2].getbalance()
+            amount = Decimal("0.001")
+            fee_rate_sat_vb = 1.23
+            fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8
+            # Test passing fee_rate as a string
+            txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=str(fee_rate_sat_vb))
+            tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])
+            self.nodes[0].generate(1)
+            self.sync_all(self.nodes[0:3])
+            postbalance = self.nodes[2].getbalance()
+            fee = prebalance - postbalance - amount
+            assert_fee_amount(fee, tx_size, Decimal(fee_rate_btc_kvb))
+
            for key in ["totalFee", "feeRate"]:
                assert_raises_rpc_error(-8, "Unknown named parameter key", self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=1, key=1)

--- a/test/functional/wallet_bumpfee.py
+++ b/test/functional/wallet_bumpfee.py
@ -149,7 +149,7 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
    self.sync_mempools((rbf_node, peer_node))
    assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool()
    if mode == "fee_rate":
-        bumped_psbt = rbf_node.psbtbumpfee(rbfid, {"fee_rate": NORMAL})
+        bumped_psbt = rbf_node.psbtbumpfee(rbfid, {"fee_rate": str(NORMAL)})
        bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": NORMAL})
    else:
        bumped_psbt = rbf_node.psbtbumpfee(rbfid)
--- a/test/functional/wallet_send.py
+++ b/test/functional/wallet_send.py
@ -256,8 +256,8 @@ class WalletSendTest(BitcoinTestFramework):
        assert res["complete"]

        self.log.info("Test setting explicit fee rate")
-        res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=1, add_to_wallet=False)
-        res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=1, add_to_wallet=False)
+        res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate="1", add_to_wallet=False)
+        res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate="1", add_to_wallet=False)
        assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"], self.nodes[1].decodepsbt(res2["psbt"])["fee"])

        res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=7, add_to_wallet=False)