Merge #6074: backport: merge bitcoin#18344, #20867, #20286, #21359, #21910, #19651, #21934, #22722, #20295, #23702, partial bitcoin#23706 (rpc backports)

b23d94b14f partial bitcoin#23706: getblockfrompeer followups (Kittywhiskers Van Gogh)
7e5cc5e375 merge bitcoin#23702: Add missing optional to getblockfrompeer (Kittywhiskers Van Gogh)
c294457b52 merge bitcoin#20295: getblockfrompeer (Kittywhiskers Van Gogh)
63ac87f011 merge bitcoin#22722: update estimatesmartfee rpc to return max of estimateSmartFee, mempoolMinFee and minRelayTxFee. (Kittywhiskers Van Gogh)
07e4c2cdd0 merge bitcoin#21934: Include versionbits signalling details during LOCKED_IN (Kittywhiskers Van Gogh)
960e7687d4 merge bitcoin#19651: importdescriptors update existing (Kittywhiskers Van Gogh)
1f31823fed merge bitcoin#21910: remove redundant fOnlySafe argument (Kittywhiskers Van Gogh)
69c5aa8947 merge bitcoin#21359: include_unsafe option for fundrawtransaction (Kittywhiskers Van Gogh)
169dce7e50 merge bitcoin#20286: deprecate addresses and reqSigs from rpc outputs (Kittywhiskers Van Gogh)
7cddf70c58 merge bitcoin#20867: Support up to 20 keys for multisig under Segwit context (Kittywhiskers Van Gogh)
7c59923845 merge bitcoin#18344: Fix nit in getblockchaininfo (Kittywhiskers Van Gogh)
ec0803a0f5 refactor: align `TxToUniv` argument list with upstream (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Closes https://github.com/dashpay/dash/issues/6000

  * `TxToUniv`'s argument list needed to be rearranged to match upstream as closely as possible (i.e. placing Dash-specific arguments at the end of the list to allow for code to be backported unmodified, relying on default arguments instead of having to modify each invocation to insert the default argument in between).

    This was due to a new `TxToUniv` variant being introduced in [bitcoin#20286](https://github.com/bitcoin/bitcoin/pull/20286)

  * The maximum number of public keys in a multisig remains the same. The upper limit for bare multisigs is and always has been `MAX_PUBKEYS_PER_MULTISIG` ([source](19512988c6/src/script/interpreter.cpp (L1143-L1144))), which has always been 20 ([source](19512988c6/src/script/script.h (L28-L29))). The limit of up to 16 comes from P2SH overhead ([source](19512988c6/src/script/descriptor.cpp (L877-L880))) and that hasn't changed.

    In effect, what [bitcoin#20867](https://github.com/bitcoin/bitcoin/pull/20867) does to Dash Core is change the error from "too many public keys" (as we'll be testing against the bare multisig limit) to "excessive redeemScript size" ([source](19512988c6/src/rpc/util.cpp (L223-L225))) (which is the _true_ limitation).

  * Backporting [bitcoin#21934](https://github.com/bitcoin/bitcoin/pull/21934) required a minor change in the condition needed to emit `activation_height` as `has_signal` in the preceding block will evaluate true for both `STARTED` and `LOCKED_IN` while earlier it would only for `STARTED`.

  * In [bitcoin#22722](https://github.com/bitcoin/bitcoin/pull/22722), a `self.stop_node` had to be added to account for the absurd fee warning that a new test condition introduces.

  * [bitcoin#23706](https://github.com/bitcoin/bitcoin/pull/23706) is partial due to commits in it that rely on RPC type enforcement, which is currently not implemented in Dash Core.

  * `feature_dip3_deterministicmns.py` depends on the reporting of `addresses` to validate multisigs containing expected payees. There is no replacement RPC call to report the pubkeys that compose a multisig address. In the interm, the `-deprecatedrpc=addresses` flag has been enabled to allow the test to run otherwise unmodified.

  ## Breaking Changes

  * The following RPCs:  `gettxout`, `getrawtransaction`, `decoderawtransaction`, `decodescript`, `gettransaction`, and REST endpoints: `/rest/tx`, `/rest/getutxos`, `/rest/block` deprecated the following fields (which are no longer returned in the responses by default): `addresses`, `reqSigs`.

    The `-deprecatedrpc=addresses` flag must be passed for these fields to be included in the RPC response. Note that these fields are attributes of the `scriptPubKey` object returned in the RPC response. However, in the response of `decodescript` these fields are top-level attributes, and included again as attributes of the `scriptPubKey` object.

  * When creating a hex-encoded Dash transaction using the `dash-tx` utility with the `-json` option set, the following fields: `addresses`, `reqSigs` are no longer returned in the tx output of the response.

  * The error message for attempts at making multisigs with >16 pubkeys will change to an "excessive redeemScript size" instead of the previous "too many public keys".

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK b23d94b14f
  PastaPastaPasta:
    utACK b23d94b14f

Tree-SHA512: 659d9ba3a0a9f8594b307a7056ab172309d5a0d9efec605bc4b345f7e0fb1032ad303af9e8f51dbd6549e82d0b738ad41eae8a5b3aebf59081f39df0d4b5ec7c

doc/REST-interface.md

@@ -98,11 +98,8 @@ $ curl localhost:19998/rest/getutxos/checkmempool/b2cdfd7b89def827ff8af7cd9bff76
          "scriptPubKey" : {
             "asm" : "OP_DUP OP_HASH160 1c7cebb529b86a04c683dfa87be49de35bcf589e OP_EQUALVERIFY OP_CHECKSIG",
             "hex" : "76a9141c7cebb529b86a04c683dfa87be49de35bcf589e88ac",
-            "reqSigs" : 1,
             "type" : "pubkeyhash",
-            "addresses" : [
+            "address" : "mi7as51dvLJsizWnTMurtRmrP8hG2m1XvD"
-               "mi7as51dvLJsizWnTMurtRmrP8hG2m1XvD"
-            ]
          }
       }
    ]

doc/release-notes-20286.md

@@ -0,0 +1,16 @@
+Updated RPCs
+------------
+
+- The following RPCs:  `gettxout`, `getrawtransaction`, `decoderawtransaction`,
+  `decodescript`, `gettransaction`, and REST endpoints: `/rest/tx`,
+  `/rest/getutxos`, `/rest/block` deprecated the following fields (which are no
+  longer returned in the responses by default): `addresses`, `reqSigs`.
+  The `-deprecatedrpc=addresses` flag must be passed for these fields to be
+  included in the RPC response. Note that these fields are attributes of
+  the `scriptPubKey` object returned in the RPC response. However, in the response
+  of `decodescript` these fields are top-level attributes, and included again as attributes
+  of the `scriptPubKey` object.
+
+- When creating a hex-encoded Dash transaction using the `dash-tx` utility
+  with the `-json` option set, the following fields: `addresses`, `reqSigs` are no longer
+  returned in the tx output of the response.

doc/release-notes-21359.md

@@ -0,0 +1,7 @@
+Wallet
+------
+
+- The `fundrawtransaction`, `send` and `walletcreatefundedpsbt` RPCs now support an `include_unsafe` option
+  that when `true` allows using unsafe inputs to fund the transaction.
+  Note that the resulting transaction may become invalid if one of the unsafe inputs disappears.
+  If that happens, the transaction must be funded with different inputs and republished.

src/Makefile.am

@@ -287,12 +287,12 @@ BITCOIN_CORE_H = \
   rpc/blockchain.h \
   rpc/client.h \
   rpc/mining.h \
-  rpc/net.h \
   rpc/protocol.h \
   rpc/rawtransaction_util.h \
   rpc/register.h \
   rpc/request.h \
   rpc/server.h \
+  rpc/server_util.h \
   rpc/util.h \
   saltedhasher.h \
   scheduler.h \
@@ -510,6 +510,7 @@ libbitcoin_server_a_SOURCES = \
   rpc/quorums.cpp \
   rpc/rawtransaction.cpp \
   rpc/server.cpp \
+  rpc/server_util.cpp \
   script/sigcache.cpp \
   shutdown.cpp \
   spork.cpp \

src/bitcoin-tx.cpp

@@ -676,7 +676,7 @@ static void MutateTx(CMutableTransaction& tx, const std::string& command,
 static void OutputTxJSON(const CTransaction& tx)
 {
     UniValue entry(UniValue::VOBJ);
-    TxToUniv(tx, uint256(), entry);
+    TxToUniv(tx, uint256(), /* include_addresses */ false, entry);
 
     std::string jsonOutput = entry.write(4);
     tfm::format(std::cout, "%s\n", jsonOutput);

src/coinjoin/client.cpp

@@ -1538,7 +1538,7 @@ bool CCoinJoinClientSession::CreateCollateralTransaction(CMutableTransaction& tx
     CCoinControl coin_control;
     coin_control.nCoinType = CoinType::ONLY_COINJOIN_COLLATERAL;
 
-    m_wallet.AvailableCoins(vCoins, true, &coin_control);
+    m_wallet.AvailableCoins(vCoins, &coin_control);
 
     if (vCoins.empty()) {
         strReason = strprintf("%s requires a collateral transaction and could not locate an acceptable input!", gCoinJoinName);

src/core_io.h

@@ -45,8 +45,8 @@ UniValue ValueFromAmount(const CAmount amount);
 std::string FormatScript(const CScript& script);
 std::string EncodeHexTx(const CTransaction& tx);
 std::string SighashToStr(unsigned char sighash_type);
-void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex);
+void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex, bool include_addresses);
 void ScriptToUniv(const CScript& script, UniValue& out, bool include_address);
-void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, const CSpentIndexTxInfo* ptxSpentInfo = nullptr, const CTxUndo* txundo = nullptr);
+void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_addresses, UniValue& entry, bool include_hex = true, const CTxUndo* txundo = nullptr, const CSpentIndexTxInfo* ptxSpentInfo = nullptr);
 
 #endif // BITCOIN_CORE_IO_H

src/core_write.cpp

@@ -166,10 +166,13 @@ void ScriptToUniv(const CScript& script, UniValue& out, bool include_address)
     }
 }
 
+// TODO: from v21 ("addresses" and "reqSigs" deprecated) this method should be refactored to remove the `include_addresses` option
+// this method can also be combined with `ScriptToUniv` as they will overlap
 void ScriptPubKeyToUniv(const CScript& scriptPubKey,
-                        UniValue& out, bool fIncludeHex)
+                        UniValue& out, bool fIncludeHex, bool include_addresses)
 {
     TxoutType type;
+    CTxDestination address;
     std::vector<CTxDestination> addresses;
     int nRequired;
 
@@ -182,17 +185,22 @@ void ScriptPubKeyToUniv(const CScript& scriptPubKey,
         return;
     }
 
-    out.pushKV("reqSigs", nRequired);
+    if (ExtractDestination(scriptPubKey, address)) {
+        out.pushKV("address", EncodeDestination(address));
+    }
     out.pushKV("type", GetTxnOutputType(type));
 
-    UniValue a(UniValue::VARR);
+    if (include_addresses) {
-    for (const CTxDestination& addr : addresses) {
+        UniValue a(UniValue::VARR);
-        a.push_back(EncodeDestination(addr));
+        for (const CTxDestination& addr : addresses) {
+            a.push_back(EncodeDestination(addr));
+        }
+        out.pushKV("addresses", a);
+        out.pushKV("reqSigs", nRequired);
     }
-    out.pushKV("addresses", a);
 }
 
-void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, const CSpentIndexTxInfo* ptxSpentInfo, const CTxUndo* txundo)
+void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_addresses, UniValue& entry, bool include_hex, const CTxUndo* txundo, const CSpentIndexTxInfo* ptxSpentInfo)
 {
     uint256 txid = tx.GetHash();
     entry.pushKV("txid", txid.GetHex());
@@ -260,7 +268,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry,
         out.pushKV("n", (int64_t)i);
 
         UniValue o(UniValue::VOBJ);
-        ScriptPubKeyToUniv(txout.scriptPubKey, o, true);
+        ScriptPubKeyToUniv(txout.scriptPubKey, o, true, include_addresses);
         out.pushKV("scriptPubKey", o);
 
         // Add spent information if spentindex is enabled

src/net_processing.cpp

@@ -398,6 +398,7 @@ public:
 
     /** Implement PeerManager */
     void CheckForStaleTipAndEvictPeers() override;
+    std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override;
     bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     bool IgnoresIncomingTxs() override { return m_ignore_incoming_txs; }
     void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);;
@@ -1862,6 +1863,39 @@ bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex)
         (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, m_chainparams.GetConsensus()) < STALE_RELAY_AGE_LIMIT);
 }
 
+std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBlockIndex& block_index)
+{
+    if (fImporting) return "Importing...";
+    if (fReindex) return "Reindexing...";
+
+    LOCK(cs_main);
+    // Ensure this peer exists and hasn't been disconnected
+    CNodeState* state = State(peer_id);
+    if (state == nullptr) return "Peer does not exist";
+
+    // Mark block as in-flight unless it already is (for this peer).
+    // If a block was already in-flight for a different peer, its BLOCKTXN
+    // response will be dropped.
+    const uint256& hash{block_index.GetBlockHash()};
+    if (!MarkBlockAsInFlight(peer_id, hash, &block_index)) return "Already requested from this peer";
+
+    // Construct message to request the block
+    std::vector<CInv> invs{CInv(MSG_BLOCK, hash)};
+
+    // Send block request message to the peer
+    bool success = m_connman.ForNode(peer_id, [this, &invs](CNode* node) {
+        const CNetMsgMaker msgMaker(node->GetCommonVersion());
+        this->m_connman.PushMessage(node, msgMaker.Make(NetMsgType::GETDATA, invs));
+        return true;
+    });
+
+    if (!success) return "Peer not fully connected";
+
+    LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n",
+                 hash.ToString(), peer_id);
+    return std::nullopt;
+}
+
 std::unique_ptr<PeerManager> PeerManager::make(const CChainParams& chainparams, CConnman& connman, AddrMan& addrman, BanMan* banman,
                                                CScheduler &scheduler, ChainstateManager& chainman, CTxMemPool& pool,
                                                CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync,

src/net_processing.h

@@ -66,6 +66,15 @@ public:
                                              const std::unique_ptr<LLMQContext>& llmq_ctx, bool ignore_incoming_txs);
     virtual ~PeerManager() { }
 
+    /**
+     * Attempt to manually fetch block from a given peer. We must already have the header.
+     *
+     * @param[in]  peer_id      The peer id
+     * @param[in]  block_index  The blockindex
+     * @returns std::nullopt if a request was successfully made, otherwise an error message
+     */
+    virtual std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) = 0;
+
     /** Get statistics from node state */
     virtual bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const = 0;
 

src/rest.cpp

@@ -19,6 +19,7 @@
 #include <rpc/blockchain.h>
 #include <rpc/protocol.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <streams.h>
 #include <sync.h>
 #include <txmempool.h>

src/rpc/blockchain.cpp

@@ -22,6 +22,8 @@
 #include <llmq/context.h>
 #include <node/blockstorage.h>
 #include <node/coinstats.h>
+#include <net.h>
+#include <net_processing.h>
 #include <node/context.h>
 #include <node/utxo_snapshot.h>
 #include <policy/feerate.h>
@@ -29,6 +31,7 @@
 #include <policy/policy.h>
 #include <primitives/transaction.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <script/descriptor.h>
 #include <streams.h>
@@ -73,67 +76,6 @@ static CUpdatedBlock latestblock GUARDED_BY(cs_blockchange);
 
 extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, CTxMemPool& mempool, CChainState& active_chainstate, llmq::CChainLocksHandler& clhandler, llmq::CInstantSendManager& isman, UniValue& entry);
 
-NodeContext& EnsureAnyNodeContext(const CoreContext& context)
-{
-    auto* const node_context = GetContext<NodeContext>(context);
-    if (!node_context) {
-        throw JSONRPCError(RPC_INTERNAL_ERROR, "Node context not found");
-    }
-    return *node_context;
-}
-
-CTxMemPool& EnsureMemPool(const NodeContext& node)
-{
-    if (!node.mempool) {
-        throw JSONRPCError(RPC_CLIENT_MEMPOOL_DISABLED, "Mempool disabled or instance not found");
-    }
-    return *node.mempool;
-}
-
-CTxMemPool& EnsureAnyMemPool(const CoreContext& context)
-{
-    return EnsureMemPool(EnsureAnyNodeContext(context));
-}
-
-ChainstateManager& EnsureChainman(const NodeContext& node)
-{
-    if (!node.chainman) {
-        throw JSONRPCError(RPC_INTERNAL_ERROR, "Node chainman not found");
-    }
-    return *node.chainman;
-}
-
-ChainstateManager& EnsureAnyChainman(const CoreContext& context)
-{
-    return EnsureChainman(EnsureAnyNodeContext(context));
-}
-
-CBlockPolicyEstimator& EnsureFeeEstimator(const NodeContext& node)
-{
-    if (!node.fee_estimator) {
-        throw JSONRPCError(RPC_INTERNAL_ERROR, "Fee estimation disabled");
-    }
-    return *node.fee_estimator;
-}
-
-CBlockPolicyEstimator& EnsureAnyFeeEstimator(const CoreContext& context)
-{
-    return EnsureFeeEstimator(EnsureAnyNodeContext(context));
-}
-
-LLMQContext& EnsureLLMQContext(const NodeContext& node)
-{
-    if (!node.llmq_ctx) {
-        throw JSONRPCError(RPC_INTERNAL_ERROR, "Node LLMQ context not found");
-    }
-    return *node.llmq_ctx;
-}
-
-LLMQContext& EnsureAnyLLMQContext(const CoreContext& context)
-{
-    return EnsureLLMQContext(EnsureAnyNodeContext(context));
-}
-
 /* Calculate the difficulty for a given block index.
  */
 double GetDifficulty(const CBlockIndex* blockindex)
@@ -226,7 +168,7 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIn
             // coinbase transaction (i == 0) doesn't have undo data
             const CTxUndo* txundo = (have_undo && i) ? &blockUndo.vtxundo.at(i - 1) : nullptr;
             UniValue objTx(UniValue::VOBJ);
-            TxToUniv(*tx, uint256(), objTx, true, nullptr, txundo);
+            TxToUniv(*tx, uint256(), objTx, true, txundo);
             bool fLocked = isman.IsLocked(tx->GetHash());
             objTx.pushKV("instantlock", fLocked || result["chainlock"].get_bool());
             objTx.pushKV("instantlock_internal", fLocked);
@@ -825,6 +767,52 @@ static RPCHelpMan getmempoolentry()
     };
 }
 
+static RPCHelpMan getblockfrompeer()
+{
+    return RPCHelpMan{
+        "getblockfrompeer",
+        "\nAttempt to fetch block from a given peer.\n"
+        "\nWe must have the header for this block, e.g. using submitheader.\n"
+        "\nReturns {} if a block-request was successfully scheduled\n",
+        {
+            {"block_hash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash to try to fetch"},
+            {"peer_id", RPCArg::Type::NUM, RPCArg::Optional::NO, "The peer to fetch it from (see getpeerinfo for peer IDs)"},
+        },
+        RPCResult{RPCResult::Type::OBJ, "", "",
+        {
+            {RPCResult::Type::STR, "warnings", /*optional=*/true, "any warnings"},
+        }},
+        RPCExamples{
+            HelpExampleCli("getblockfrompeer", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\" 0")
+            + HelpExampleRpc("getblockfrompeer", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\" 0")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    const NodeContext& node = EnsureAnyNodeContext(request.context);
+    ChainstateManager& chainman = EnsureChainman(node);
+    PeerManager& peerman = EnsurePeerman(node);
+
+    const uint256& block_hash{ParseHashV(request.params[0], "block_hash")};
+    const NodeId peer_id{request.params[1].get_int64()};
+
+    const CBlockIndex* const index = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(block_hash););
+
+    if (!index) {
+        throw JSONRPCError(RPC_MISC_ERROR, "Block header missing");
+    }
+
+    UniValue result = UniValue::VOBJ;
+
+    if (index->nStatus & BLOCK_HAVE_DATA) {
+        result.pushKV("warnings", "Block already downloaded");
+    } else if (const auto err{peerman.FetchBlock(peer_id, *index)}) {
+        throw JSONRPCError(RPC_MISC_ERROR, err.value());
+    }
+    return result;
+},
+    };
+}
+
 static RPCHelpMan getblockhashes()
 {
     return RPCHelpMan{"getblockhashes",
@@ -1513,11 +1501,12 @@ static RPCHelpMan gettxout()
                 {RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT},
                 {RPCResult::Type::OBJ, "scriptPubKey", "",
                     {
-                        {RPCResult::Type::STR_HEX, "asm", ""},
+                        {RPCResult::Type::STR, "asm", ""},
                         {RPCResult::Type::STR_HEX, "hex", ""},
-                        {RPCResult::Type::NUM, "reqSigs", "Number of required signatures"},
+                        {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"},
                         {RPCResult::Type::STR_HEX, "type", "The type, eg pubkeyhash"},
-                        {RPCResult::Type::ARR, "addresses", "Array of Dash addresses",
+                        {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"},
+                        {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses",
                             {{RPCResult::Type::STR, "address", "Dash address"}}},
                     }},
                 {RPCResult::Type::BOOL, "coinbase", "Coinbase or not"},
@@ -1649,8 +1638,8 @@ static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, const std:
     case ThresholdState::ACTIVE: bip9.pushKV("status", "active"); break;
     case ThresholdState::FAILED: bip9.pushKV("status", "failed"); break;
     }
-    if (ThresholdState::STARTED == thresholdState)
+    const bool has_signal = (ThresholdState::STARTED == thresholdState || ThresholdState::LOCKED_IN == thresholdState);
-    {
+    if (has_signal) {
         bip9.pushKV("bit", consensusParams.vDeployments[id].bit);
     }
     bip9.pushKV("start_time", consensusParams.vDeployments[id].nStartTime);
@@ -1661,18 +1650,19 @@ static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, const std:
     }
     int64_t since_height = g_versionbitscache.StateSinceHeight(active_chain_tip, consensusParams, id);
     bip9.pushKV("since", since_height);
-    if (ThresholdState::STARTED == thresholdState)
+    if (has_signal) {
-    {
         UniValue statsUV(UniValue::VOBJ);
         BIP9Stats statsStruct = g_versionbitscache.Statistics(active_chain_tip, consensusParams, id);
         statsUV.pushKV("period", statsStruct.period);
-        statsUV.pushKV("threshold", statsStruct.threshold);
         statsUV.pushKV("elapsed", statsStruct.elapsed);
         statsUV.pushKV("count", statsStruct.count);
-        statsUV.pushKV("possible", statsStruct.possible);
+        if (ThresholdState::LOCKED_IN != thresholdState) {
+            statsUV.pushKV("threshold", statsStruct.threshold);
+            statsUV.pushKV("possible", statsStruct.possible);
+        }
         bip9.pushKV("statistics", statsUV);
     }
-    else if (ThresholdState::LOCKED_IN == thresholdState) {
+    if (ThresholdState::LOCKED_IN == thresholdState) {
         bip9.pushKV("activation_height", since_height + static_cast<int>(consensusParams.vDeployments[id].nWindowSize));
     }
     bip9.pushKV("min_activation_height", consensusParams.vDeployments[id].min_activation_height);
@@ -1718,7 +1708,7 @@ RPCHelpMan getblockchaininfo()
                     {RPCResult::Type::OBJ, "bip9", "status of bip9 softforks (only for \"bip9\" type)",
                     {
                         {RPCResult::Type::STR, "status", "one of \"defined\", \"started\", \"locked_in\", \"active\", \"failed\""},
-                        {RPCResult::Type::NUM, "bit", "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" status)"},
+                        {RPCResult::Type::NUM, "bit", "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" and \"locked_in\" status)"},
                         {RPCResult::Type::NUM_TIME, "start_time", "the minimum median time past of a block at which the bit gains its meaning"},
                         {RPCResult::Type::NUM_TIME, "timeout", "the median time past of a block at which the deployment is considered failed if not yet locked in"},
                         {RPCResult::Type::BOOL, "ehf", "returns true for EHF activated forks"},
@@ -1726,13 +1716,13 @@ RPCHelpMan getblockchaininfo()
                         {RPCResult::Type::NUM, "since", "height of the first block to which the status applies"},
                         {RPCResult::Type::NUM, "activation_height", "expected activation height for this softfork (only for \"locked_in\" status)"},
                         {RPCResult::Type::NUM, "min_activation_height", "minimum height of blocks for which the rules may be enforced"},
-                        {RPCResult::Type::OBJ, "statistics", "numeric statistics about BIP9 signalling for a softfork",
+                        {RPCResult::Type::OBJ, "statistics", "numeric statistics about signalling for a softfork (only for \"started\" and \"locked_in\" status)",
                         {
-                            {RPCResult::Type::NUM, "period", "the length in blocks of the BIP9 signalling period"},
+                            {RPCResult::Type::NUM, "period", "the length in blocks of the signalling period"},
-                            {RPCResult::Type::NUM, "threshold", "the number of blocks with the version bit set required to activate the feature"},
+                            {RPCResult::Type::NUM, "threshold", "the number of blocks with the version bit set required to activate the feature (only for \"started\" status)"},
                             {RPCResult::Type::NUM, "elapsed", "the number of blocks elapsed since the beginning of the current period"},
                             {RPCResult::Type::NUM, "count", "the number of blocks with the version bit set in the current period"},
-                            {RPCResult::Type::BOOL, "possible", "returns false if there are not enough blocks left in this period to pass activation threshold"},
+                            {RPCResult::Type::BOOL, "possible", "returns false if there are not enough blocks left in this period to pass activation threshold (only for \"started\" status)"},
                         }},
                     }},
                     {RPCResult::Type::NUM, "height", "height of the first block which the rules are or will be enforced (only for \"buried\" type, or \"bip9\" type with \"active\" status)"},
@@ -2264,6 +2254,16 @@ void CalculatePercentilesBySize(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], s
     }
 }
 
+void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex)
+{
+    ScriptPubKeyToUniv(scriptPubKey, out, fIncludeHex, IsDeprecatedRPCEnabled("addresses"));
+}
+
+void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, const CTxUndo* txundo, const CSpentIndexTxInfo* ptxSpentInfo)
+{
+    TxToUniv(tx, hashBlock, IsDeprecatedRPCEnabled("addresses"), entry, include_hex, txundo, ptxSpentInfo);
+}
+
 template<typename T>
 static inline bool SetHasKeys(const std::set<T>& set) {return false;}
 template<typename T, typename Tk, typename... Args>
@@ -3046,6 +3046,7 @@ static const CRPCCommand commands[] =
     { "blockchain",         "getbestchainlock",       &getbestchainlock,       {} },
     { "blockchain",         "getblockcount",          &getblockcount,          {} },
     { "blockchain",         "getblock",               &getblock,               {"blockhash","verbosity|verbose"} },
+    { "blockchain",         "getblockfrompeer",       &getblockfrompeer,       {"block_hash", "peer_id"}},
     { "blockchain",         "getblockhashes",         &getblockhashes,         {"high","low"} },
     { "blockchain",         "getblockhash",           &getblockhash,           {"height"} },
     { "blockchain",         "getblockheader",         &getblockheader,         {"blockhash","verbose"} },

src/rpc/blockchain.h

@@ -6,7 +6,7 @@
 #define BITCOIN_RPC_BLOCKCHAIN_H
 
 #include <amount.h>
-#include <context.h>
+#include <core_io.h>
 #include <streams.h>
 #include <sync.h>
 
@@ -17,12 +17,9 @@ extern RecursiveMutex cs_main;
 
 class CBlock;
 class CBlockIndex;
-class CBlockPolicyEstimator;
 class CChainState;
 class CTxMemPool;
-class ChainstateManager;
 class UniValue;
-struct LLMQContext;
 struct NodeContext;
 namespace llmq {
 class CChainLocksHandler;
@@ -57,15 +54,8 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex
 /** Used by getblockstats to get feerates at different percentiles by weight  */
 void CalculatePercentilesBySize(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector<std::pair<CAmount, int64_t>>& scores, int64_t total_size);
 
-NodeContext& EnsureAnyNodeContext(const CoreContext& context);
+void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex);
-CTxMemPool& EnsureMemPool(const NodeContext& node);
+void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, const CTxUndo* txundo = nullptr, const CSpentIndexTxInfo* ptxSpentInfo = nullptr);
-CTxMemPool& EnsureAnyMemPool(const CoreContext& context);
-ChainstateManager& EnsureChainman(const NodeContext& node);
-ChainstateManager& EnsureAnyChainman(const CoreContext& context);
-CBlockPolicyEstimator& EnsureFeeEstimator(const NodeContext& node);
-CBlockPolicyEstimator& EnsureAnyFeeEstimator(const CoreContext& context);
-LLMQContext& EnsureLLMQContext(const NodeContext& node);
-LLMQContext& EnsureAnyLLMQContext(const CoreContext& context);
 
 /**
  * Helper to create UTXO snapshots given a chainstate and a file handle.

src/rpc/client.cpp

@@ -66,6 +66,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "getbalance", 2, "addlocked" },
     { "getbalance", 3, "include_watchonly" },
     { "getbalance", 4, "avoid_reuse" },
+    { "getblockfrompeer", 1, "peer_id" },
     { "getchaintips", 0, "count" },
     { "getchaintips", 1, "branchlen" },
     { "getblockhash", 0, "height" },

src/rpc/coinjoin.cpp

@@ -7,8 +7,8 @@
 #include <coinjoin/context.h>
 #include <coinjoin/server.h>
 #include <rpc/blockchain.h>
-#include <rpc/net.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <util/strencodings.h>
 

src/rpc/evo.cpp

@@ -24,6 +24,7 @@
 #include <node/context.h>
 #include <rpc/blockchain.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <util/moneystr.h>
 #include <util/translation.h>

src/rpc/governance.cpp

@@ -16,8 +16,8 @@
 #include <node/context.h>
 #include <net.h>
 #include <rpc/blockchain.h>
-#include <rpc/net.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <governance/common.h>
 #include <util/strencodings.h>

src/rpc/masternode.cpp

@@ -15,8 +15,8 @@
 #include <net.h>
 #include <netbase.h>
 #include <rpc/blockchain.h>
-#include <rpc/net.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <univalue.h>
 #include <util/strencodings.h>
@@ -187,7 +187,7 @@ static RPCHelpMan masternode_outputs()
     coin_control.nCoinType = CoinType::ONLY_MASTERNODE_COLLATERAL;
     {
         LOCK(wallet->cs_wallet);
-        wallet->AvailableCoins(vPossibleCoins, true, &coin_control);
+        wallet->AvailableCoins(vPossibleCoins, &coin_control);
     }
     UniValue outputsArr(UniValue::VARR);
     for (const auto& out : vPossibleCoins) {

src/rpc/mining.cpp

@@ -26,8 +26,8 @@
 #include <pow.h>
 #include <rpc/blockchain.h>
 #include <rpc/mining.h>
-#include <rpc/net.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <script/descriptor.h>
 #include <script/script.h>
@@ -1142,6 +1142,8 @@ static RPCHelpMan estimatesmartfee()
     RPCTypeCheckArgument(request.params[0], UniValue::VNUM);
 
     CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context);
+    const NodeContext& node = EnsureAnyNodeContext(request.context);
+    const CTxMemPool& mempool = EnsureMemPool(node);
 
     unsigned int max_target = fee_estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
     unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
@@ -1157,7 +1159,10 @@ static RPCHelpMan estimatesmartfee()
     UniValue result(UniValue::VOBJ);
     UniValue errors(UniValue::VARR);
     FeeCalculation feeCalc;
-    CFeeRate feeRate = fee_estimator.estimateSmartFee(conf_target, &feeCalc, conservative);
+    CFeeRate feeRate{fee_estimator.estimateSmartFee(conf_target, &feeCalc, conservative)};
+    CFeeRate min_mempool_feerate{mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000)};
+    CFeeRate min_relay_feerate{::minRelayTxFee};
+    feeRate = std::max({feeRate, min_mempool_feerate, min_relay_feerate});
     if (feeRate != CFeeRate(0)) {
         result.pushKV("feerate", ValueFromAmount(feeRate.GetFeePerK()));
     } else {

src/rpc/misc.cpp

@@ -19,8 +19,8 @@
 #include <net.h>
 #include <node/context.h>
 #include <rpc/blockchain.h>
-#include <rpc/net.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <scheduler.h>
 #include <script/descriptor.h>

src/rpc/net.cpp

@@ -9,7 +9,6 @@
 #include <chainparams.h>
 #include <clientversion.h>
 #include <core_io.h>
-#include <net.h>
 #include <net_permissions.h>
 #include <net_processing.h>
 #include <net_types.h> // For banmap_t
@@ -18,12 +17,12 @@
 #include <policy/settings.h>
 #include <rpc/blockchain.h>
 #include <rpc/protocol.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <sync.h>
 #include <timedata.h>
 #include <util/strencodings.h>
 #include <util/string.h>
-#include <util/system.h>
 #include <util/translation.h>
 #include <validation.h>
 #include <version.h>
@@ -40,22 +39,6 @@ const std::vector<std::string> CONNECTION_TYPE_DOC{
         "feeler (short-lived automatic connection for testing addresses)"
 };
 
-CConnman& EnsureConnman(const NodeContext& node)
-{
-    if (!node.connman) {
-        throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
-    }
-    return *node.connman;
-}
-
-PeerManager& EnsurePeerman(const NodeContext& node)
-{
-    if (!node.peerman) {
-        throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
-    }
-    return *node.peerman;
-}
-
 static RPCHelpMan getconnectioncount()
 {
     return RPCHelpMan{"getconnectioncount",

src/rpc/net.h

@@ -1,15 +0,0 @@
-// Copyright (c) 2021 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#ifndef BITCOIN_RPC_NET_H
-#define BITCOIN_RPC_NET_H
-
-class CConnman;
-class PeerManager;
-struct NodeContext;
-
-CConnman& EnsureConnman(const NodeContext& node);
-PeerManager& EnsurePeerman(const NodeContext& node);
-
-#endif // BITCOIN_RPC_NET_H

src/rpc/quorums.cpp

@@ -7,8 +7,8 @@
 #include <index/txindex.h>
 #include <node/context.h>
 #include <rpc/blockchain.h>
-#include <rpc/net.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <validation.h>
 

src/rpc/rawtransaction.cpp

@@ -28,6 +28,7 @@
 #include <rpc/blockchain.h>
 #include <rpc/rawtransaction_util.h>
 #include <rpc/server.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <script/script.h>
 #include <script/sign.h>
@@ -84,7 +85,7 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, CTxMemPool& mempo
         }
     }
 
-    TxToUniv(tx, uint256(), entry, true, &txSpentInfo);
+    TxToUniv(tx, uint256(), entry, true, /* txundo = */ nullptr, &txSpentInfo);
 
     bool chainLock = false;
     if (!hashBlock.IsNull()) {
@@ -168,9 +169,10 @@ static RPCHelpMan getrawtransaction()
                                     {
                                         {RPCResult::Type::STR, "asm", "the asm"},
                                         {RPCResult::Type::STR, "hex", "the hex"},
-                                        {RPCResult::Type::NUM, "reqSigs", "The required sigs"},
+                                        {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"},
                                         {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"},
-                                        {RPCResult::Type::ARR, "addresses", "",
+                                        {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"},
+                                        {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses",
                                         {
                                             {RPCResult::Type::STR, "address", "Dash address"},
                                         }},
@@ -827,9 +829,10 @@ static RPCHelpMan decoderawtransaction()
                                 {
                                     {RPCResult::Type::STR, "asm", "the asm"},
                                     {RPCResult::Type::STR_HEX, "hex", "the hex"},
-                                    {RPCResult::Type::NUM, "reqSigs", "The required sigs"},
+                                    {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"},
                                     {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"},
-                                    {RPCResult::Type::ARR, "addresses", "",
+                                    {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"},
+                                    {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses",
                                     {
                                         {RPCResult::Type::STR, "address", "Dash address"},
                                     }},
@@ -883,8 +886,9 @@ static RPCHelpMan decodescript()
                     {
                         {RPCResult::Type::STR, "asm", "Script public key"},
                         {RPCResult::Type::STR, "type", "The output type (e.g. "+GetAllOutputTypes()+")"},
-                        {RPCResult::Type::NUM, "reqSigs", "The required signatures"},
+                        {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"},
-                        {RPCResult::Type::ARR, "addresses", "",
+                        {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"},
+                        {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses",
                         {
                             {RPCResult::Type::STR, "address", "Dash address"},
                         }},

src/rpc/server.cpp

@@ -9,6 +9,7 @@
 #include <chainparams.h>
 #include <node/context.h>
 #include <rpc/blockchain.h>
+#include <rpc/server_util.h>
 #include <rpc/util.h>
 #include <shutdown.h>
 #include <sync.h>

src/rpc/server_util.cpp

@@ -0,0 +1,93 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <rpc/server_util.h>
+
+#include <net_processing.h>
+#include <node/context.h>
+#include <policy/fees.h>
+#include <rpc/protocol.h>
+#include <rpc/request.h>
+#include <txmempool.h>
+#include <util/system.h>
+#include <validation.h>
+
+#include <any>
+
+NodeContext& EnsureAnyNodeContext(const CoreContext& context)
+{
+    auto* const node_context = GetContext<NodeContext>(context);
+    if (!node_context) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Node context not found");
+    }
+    return *node_context;
+}
+
+CTxMemPool& EnsureMemPool(const NodeContext& node)
+{
+    if (!node.mempool) {
+        throw JSONRPCError(RPC_CLIENT_MEMPOOL_DISABLED, "Mempool disabled or instance not found");
+    }
+    return *node.mempool;
+}
+
+CTxMemPool& EnsureAnyMemPool(const CoreContext& context)
+{
+    return EnsureMemPool(EnsureAnyNodeContext(context));
+}
+
+ChainstateManager& EnsureChainman(const NodeContext& node)
+{
+    if (!node.chainman) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Node chainman not found");
+    }
+    return *node.chainman;
+}
+
+ChainstateManager& EnsureAnyChainman(const CoreContext& context)
+{
+    return EnsureChainman(EnsureAnyNodeContext(context));
+}
+
+CBlockPolicyEstimator& EnsureFeeEstimator(const NodeContext& node)
+{
+    if (!node.fee_estimator) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Fee estimation disabled");
+    }
+    return *node.fee_estimator;
+}
+
+CBlockPolicyEstimator& EnsureAnyFeeEstimator(const CoreContext& context)
+{
+    return EnsureFeeEstimator(EnsureAnyNodeContext(context));
+}
+
+LLMQContext& EnsureLLMQContext(const NodeContext& node)
+{
+    if (!node.llmq_ctx) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Node LLMQ context not found");
+    }
+    return *node.llmq_ctx;
+}
+
+LLMQContext& EnsureAnyLLMQContext(const CoreContext& context)
+{
+    return EnsureLLMQContext(EnsureAnyNodeContext(context));
+}
+
+CConnman& EnsureConnman(const NodeContext& node)
+{
+    if (!node.connman) {
+        throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
+    }
+    return *node.connman;
+}
+
+PeerManager& EnsurePeerman(const NodeContext& node)
+{
+    if (!node.peerman) {
+        throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
+    }
+    return *node.peerman;
+}

src/rpc/server_util.h

@@ -0,0 +1,30 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_RPC_SERVER_UTIL_H
+#define BITCOIN_RPC_SERVER_UTIL_H
+
+#include <context.h>
+
+class CBlockPolicyEstimator;
+class CConnman;
+class ChainstateManager;
+class CTxMemPool;
+class PeerManager;
+struct NodeContext;
+struct LLMQContext;
+
+NodeContext& EnsureAnyNodeContext(const CoreContext& context);
+CTxMemPool& EnsureMemPool(const NodeContext& node);
+CTxMemPool& EnsureAnyMemPool(const CoreContext& context);
+ChainstateManager& EnsureChainman(const NodeContext& node);
+ChainstateManager& EnsureAnyChainman(const CoreContext& context);
+CBlockPolicyEstimator& EnsureFeeEstimator(const NodeContext& node);
+CBlockPolicyEstimator& EnsureAnyFeeEstimator(const CoreContext& context);
+LLMQContext& EnsureLLMQContext(const NodeContext& node);
+LLMQContext& EnsureAnyLLMQContext(const CoreContext& context);
+CConnman& EnsureConnman(const NodeContext& node);
+PeerManager& EnsurePeerman(const NodeContext& node);
+
+#endif // BITCOIN_RPC_SERVER_UTIL_H

src/rpc/util.cpp

@@ -277,8 +277,8 @@ CTxDestination AddAndGetMultisigDestination(const int required, const std::vecto
     if ((int)pubkeys.size() < required) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("not enough keys supplied (got %u keys, but need at least %d to redeem)", pubkeys.size(), required));
     }
-    if (pubkeys.size() > 16) {
+    if (pubkeys.size() > MAX_PUBKEYS_PER_MULTISIG) {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "Number of keys involved in the multisignature address creation > 16\nReduce the number");
+        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Number of keys involved in the multisignature address creation > %d\nReduce the number", MAX_PUBKEYS_PER_MULTISIG));
     }
 
     script_out = GetScriptForMultisig(required, pubkeys);

src/script/descriptor.cpp

@@ -978,8 +978,8 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
             providers.emplace_back(std::move(pk));
             key_exp_index++;
         }
-        if (providers.empty() || providers.size() > 16) {
+        if (providers.empty() || providers.size() > MAX_PUBKEYS_PER_MULTISIG) {
-            error = strprintf("Cannot have %u keys in multisig; must have between 1 and 16 keys, inclusive", providers.size());
+            error = strprintf("Cannot have %u keys in multisig; must have between 1 and %d keys, inclusive", providers.size(), MAX_PUBKEYS_PER_MULTISIG);
             return nullptr;
         } else if (thres < 1) {
             error = strprintf("Multisig threshold cannot be %d, must be at least 1", thres);
@@ -995,6 +995,7 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
             }
         }
         if (ctx == ParseScriptContext::P2SH) {
+            // This limits the maximum number of compressed pubkeys to 15.
             if (script_size + 3 > MAX_SCRIPT_ELEMENT_SIZE) {
                 error = strprintf("P2SH script is too large, %d bytes is larger than %d bytes", script_size + 3, MAX_SCRIPT_ELEMENT_SIZE);
                 return nullptr;

src/script/interpreter.cpp

@@ -221,7 +221,7 @@ bool static CheckPubKeyEncoding(const valtype &vchPubKey, unsigned int flags, co
     return true;
 }
 
-bool static CheckMinimalPush(const valtype& data, opcodetype opcode) {
+bool CheckMinimalPush(const valtype& data, opcodetype opcode) {
     // Excludes OP_1NEGATE, OP_1-16 since they are by definition minimal
     assert(0 <= opcode && opcode <= OP_PUSHDATA4);
     if (data.size() == 0) {

src/script/interpreter.h

@@ -179,6 +179,8 @@ using MutableTransactionSignatureChecker = GenericTransactionSignatureChecker<CM
 bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* error = nullptr);
 bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = nullptr);
 
+bool CheckMinimalPush(const std::vector<unsigned char>& data, opcodetype opcode);
+
 int FindAndDelete(CScript& script, const CScript& b);
 
 #endif // BITCOIN_SCRIPT_INTERPRETER_H

src/script/standard.cpp

@@ -70,21 +70,53 @@ static constexpr bool IsSmallInteger(opcodetype opcode)
     return opcode >= OP_1 && opcode <= OP_16;
 }
 
-static bool MatchMultisig(const CScript& script, unsigned int& required, std::vector<valtype>& pubkeys)
+static constexpr bool IsPushdataOp(opcodetype opcode)
+{
+    return opcode > OP_FALSE && opcode <= OP_PUSHDATA4;
+}
+
+static constexpr bool IsValidMultisigKeyCount(int n_keys)
+{
+    return n_keys > 0 && n_keys <= MAX_PUBKEYS_PER_MULTISIG;
+}
+
+static bool GetMultisigKeyCount(opcodetype opcode, valtype data, int& count)
+{
+    if (IsSmallInteger(opcode)) {
+        count = CScript::DecodeOP_N(opcode);
+        return IsValidMultisigKeyCount(count);
+    }
+
+    if (IsPushdataOp(opcode)) {
+        if (!CheckMinimalPush(data, opcode)) return false;
+        try {
+            count = CScriptNum(data, /* fRequireMinimal = */ true).getint();
+            return IsValidMultisigKeyCount(count);
+        } catch (const scriptnum_error&) {
+            return false;
+        }
+    }
+
+    return false;
+}
+
+static bool MatchMultisig(const CScript& script, int& required_sigs, std::vector<valtype>& pubkeys)
 {
     opcodetype opcode;
     valtype data;
+    int num_keys;
+
     CScript::const_iterator it = script.begin();
     if (script.size() < 1 || script.back() != OP_CHECKMULTISIG) return false;
 
-    if (!script.GetOp(it, opcode, data) || !IsSmallInteger(opcode)) return false;
+    if (!script.GetOp(it, opcode, data) || !GetMultisigKeyCount(opcode, data, required_sigs)) return false;
-    required = CScript::DecodeOP_N(opcode);
     while (script.GetOp(it, opcode, data) && CPubKey::ValidSize(data)) {
         pubkeys.emplace_back(std::move(data));
     }
-    if (!IsSmallInteger(opcode)) return false;
+    if (!GetMultisigKeyCount(opcode, data, num_keys)) return false;
-    unsigned int keys = CScript::DecodeOP_N(opcode);
+
-    if (pubkeys.size() != keys || keys < required) return false;
+    if (pubkeys.size() != static_cast<unsigned long>(num_keys) || num_keys < required_sigs) return false;
+
     return (it + 1 == script.end());
 }
 
@@ -121,12 +153,12 @@ TxoutType Solver(const CScript& scriptPubKey, std::vector<std::vector<unsigned c
         return TxoutType::PUBKEYHASH;
     }
 
-    unsigned int required;
+    int required;
     std::vector<std::vector<unsigned char>> keys;
     if (MatchMultisig(scriptPubKey, required, keys)) {
-        vSolutionsRet.push_back({static_cast<unsigned char>(required)}); // safe as required is in range 1..16
+        vSolutionsRet.push_back({static_cast<unsigned char>(required)}); // safe as required is in range 1..20
         vSolutionsRet.insert(vSolutionsRet.end(), keys.begin(), keys.end());
-        vSolutionsRet.push_back({static_cast<unsigned char>(keys.size())}); // safe as size is in range 1..16
+        vSolutionsRet.push_back({static_cast<unsigned char>(keys.size())}); // safe as size is in range 1..20
         return TxoutType::MULTISIG;
     }
 
@@ -157,7 +189,6 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet)
         return true;
     }
     case TxoutType::MULTISIG:
-        // Multisig txns have more than one address...
     case TxoutType::NULL_DATA:
     case TxoutType::NONSTANDARD:
         return false;
@@ -165,6 +196,7 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet)
     assert(false);
 }
 
+// TODO: from v21 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed
 bool ExtractDestinations(const CScript& scriptPubKey, TxoutType& typeRet, std::vector<CTxDestination>& addressRet, int& nRequiredRet)
 {
     addressRet.clear();
@@ -240,10 +272,11 @@ CScript GetScriptForMultisig(int nRequired, const std::vector<CPubKey>& keys)
 {
     CScript script;
 
-    script << CScript::EncodeOP_N(nRequired);
+    script << nRequired;
     for (const CPubKey& key : keys)
         script << ToByteVector(key);
-    script << CScript::EncodeOP_N(keys.size()) << OP_CHECKMULTISIG;
+    script << keys.size() << OP_CHECKMULTISIG;
+
     return script;
 }
 

src/script/standard.h

@@ -134,6 +134,8 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet)
  * and nRequiredRet with the n required to spend. For other destinations,
  * addressRet is populated with a single value and nRequiredRet is set to 1.
  * Returns true if successful.
+ *
+ * TODO: from v21 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed
  */
 bool ExtractDestinations(const CScript& scriptPubKey, TxoutType& typeRet, std::vector<CTxDestination>& addressRet, int& nRequiredRet);
 

src/test/descriptor_tests.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <pubkey.h>
 #include <script/descriptor.h>
 #include <script/sign.h>
 #include <script/standard.h>
@@ -27,6 +28,14 @@ void CheckUnparsable(const std::string& prv, const std::string& pub, const std::
     BOOST_CHECK_EQUAL(error, expected_error);
 }
 
+/** Check that the script is inferred as non-standard */
+void CheckInferRaw(const CScript& script)
+{
+    FlatSigningProvider dummy_provider;
+    std::unique_ptr<Descriptor> desc = InferDescriptor(script, dummy_provider);
+    BOOST_CHECK(desc->ToString().rfind("raw(", 0) == 0);
+}
+
 constexpr int DEFAULT = 0;
 constexpr int RANGE = 1; // Expected to be ranged descriptor
 constexpr int HARDENED = 2; // Derivation needs access to private keys
@@ -327,8 +336,7 @@ BOOST_AUTO_TEST_CASE(descriptor_test)
     CheckUnparsable("multi(0,L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1,5KYZdUEo39z3FPrtuX2QbbwGnNP5zTd7yyr2SC1j299sBCnWjss)", "multi(0,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "Multisig threshold cannot be 0, must be at least 1"); // Threshold of 0
     CheckUnparsable("multi(3,L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1,5KYZdUEo39z3FPrtuX2QbbwGnNP5zTd7yyr2SC1j299sBCnWjss)", "multi(3,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "Multisig threshold cannot be larger than the number of keys; threshold is 3 but only 2 keys specified"); // Threshold larger than number of keys
     CheckUnparsable("multi(3,KzoAz5CanayRKex3fSLQ2BwJpN7U52gZvxMyk78nDMHuqrUxuSJy,KwGNz6YCCQtYvFzMtrC6D3tKTKdBBboMrLTsjr2NYVBwapCkn7Mr,KxogYhiNfwxuswvXV66eFyKcCpm7dZ7TqHVqujHAVUjJxyivxQ9X,L2BUNduTSyZwZjwNHynQTF14mv2uz2NRq5n5sYWTb4FkkmqgEE9f)", "multi(3,03669b8afcec803a0d323e9a17f3ea8e68e8abe5a278020a929adbec52421adbd0,0260b2003c386519fc9eadf2b5cf124dd8eea4c4e68d5e154050a9346ea98ce600,0362a74e399c39ed5593852a30147f2959b56bb827dfa3e60e464b02ccf87dc5e8,0261345b53de74a4d721ef877c255429961b7e43714171ac06168d7e08c542a8b8)", "Cannot have 4 pubkeys in bare multisig; only at most 3 pubkeys"); // Threshold larger than number of keys
-    CheckUnparsable("sh(multi(16,KzoAz5CanayRKex3fSLQ2BwJpN7U52gZvxMyk78nDMHuqrUxuSJy,KwGNz6YCCQtYvFzMtrC6D3tKTKdBBboMrLTsjr2NYVBwapCkn7Mr,KxogYhiNfwxuswvXV66eFyKcCpm7dZ7TqHVqujHAVUjJxyivxQ9X,L2BUNduTSyZwZjwNHynQTF14mv2uz2NRq5n5sYWTb4FkkmqgEE9f,L1okJGHGn1kFjdXHKxXjwVVtmCMR2JA5QsbKCSpSb7ReQjezKeoD,KxDCNSST75HFPaW5QKpzHtAyaCQC7p9Vo3FYfi2u4dXD1vgMiboK,L5edQjFtnkcf5UWURn6UuuoFrabgDQUHdheKCziwN42aLwS3KizU,KzF8UWFcEC7BYTq8Go1xVimMkDmyNYVmXV5PV7RuDicvAocoPB8i,L3nHUboKG2w4VSJ5jYZ5CBM97oeK6YuKvfZxrefdShECcjEYKMWZ,KyjHo36dWkYhimKmVVmQTq3gERv3pnqA4xFCpvUgbGDJad7eS8WE,KwsfyHKRUTZPQtysN7M3tZ4GXTnuov5XRgjdF2XCG8faAPmFruRF,KzCUbGhN9LJhdeFfL9zQgTJMjqxdBKEekRGZX24hXdgCNCijkkap,KzgpMBwwsDLwkaC5UrmBgCYaBD2WgZ7PBoGYXR8KT7gCA9UTN5a3,KyBXTPy4T7YG4q9tcAM3LkvfRpD1ybHMvcJ2ehaWXaSqeGUxEdkP,KzJDe9iwJRPtKP2F2AoN6zBgzS7uiuAwhWCfGdNeYJ3PC1HNJ8M8,L1xbHrxynrqLKkoYc4qtoQPx6uy5qYXR5ZDYVYBSRmCV5piU3JG9,L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1))","sh(multi(16,03669b8afcec803a0d323e9a17f3ea8e68e8abe5a278020a929adbec52421adbd0,0260b2003c386519fc9eadf2b5cf124dd8eea4c4e68d5e154050a9346ea98ce600,0362a74e399c39ed5593852a30147f2959b56bb827dfa3e60e464b02ccf87dc5e8,0261345b53de74a4d721ef877c255429961b7e43714171ac06168d7e08c542a8b8,02da72e8b46901a65d4374fe6315538d8f368557dda3a1dcf9ea903f3afe7314c8,0318c82dd0b53fd3a932d16e0ba9e278fcc937c582d5781be626ff16e201f72286,0297ccef1ef99f9d73dec9ad37476ddb232f1238aff877af19e72ba04493361009,02e502cfd5c3f972fe9a3e2a18827820638f96b6f347e54d63deb839011fd5765d,03e687710f0e3ebe81c1037074da939d409c0025f17eb86adb9427d28f0f7ae0e9,02c04d3a5274952acdbc76987f3184b346a483d43be40874624b29e3692c1df5af,02ed06e0f418b5b43a7ec01d1d7d27290fa15f75771cb69b642a51471c29c84acd,036d46073cbb9ffee90473f3da429abc8de7f8751199da44485682a989a4bebb24,02f5d1ff7c9029a80a4e36b9a5497027ef7f3e73384a4a94fbfe7c4e9164eec8bc,02e41deffd1b7cce11cde209a781adcffdabd1b91c0ba0375857a2bfd9302419f3,02d76625f7956a7fc505ab02556c23ee72d832f1bac391bcd2d3abce5710a13d06,0399eb0a5487515802dc14544cf10b3666623762fbed2ec38a3975716e2c29c232,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", "Cannot have 17 keys in multisig; must have between 1 and 16 keys, inclusive"); // Cannot have more than 16 keys in a multisig
+    CheckUnparsable("sh(multi(16,KzoAz5CanayRKex3fSLQ2BwJpN7U52gZvxMyk78nDMHuqrUxuSJy,KwGNz6YCCQtYvFzMtrC6D3tKTKdBBboMrLTsjr2NYVBwapCkn7Mr,KxogYhiNfwxuswvXV66eFyKcCpm7dZ7TqHVqujHAVUjJxyivxQ9X,L2BUNduTSyZwZjwNHynQTF14mv2uz2NRq5n5sYWTb4FkkmqgEE9f,L1okJGHGn1kFjdXHKxXjwVVtmCMR2JA5QsbKCSpSb7ReQjezKeoD,KxDCNSST75HFPaW5QKpzHtAyaCQC7p9Vo3FYfi2u4dXD1vgMiboK,L5edQjFtnkcf5UWURn6UuuoFrabgDQUHdheKCziwN42aLwS3KizU,KzF8UWFcEC7BYTq8Go1xVimMkDmyNYVmXV5PV7RuDicvAocoPB8i,L3nHUboKG2w4VSJ5jYZ5CBM97oeK6YuKvfZxrefdShECcjEYKMWZ,KyjHo36dWkYhimKmVVmQTq3gERv3pnqA4xFCpvUgbGDJad7eS8WE,KwsfyHKRUTZPQtysN7M3tZ4GXTnuov5XRgjdF2XCG8faAPmFruRF,KzCUbGhN9LJhdeFfL9zQgTJMjqxdBKEekRGZX24hXdgCNCijkkap,KzgpMBwwsDLwkaC5UrmBgCYaBD2WgZ7PBoGYXR8KT7gCA9UTN5a3,KyBXTPy4T7YG4q9tcAM3LkvfRpD1ybHMvcJ2ehaWXaSqeGUxEdkP,KzJDe9iwJRPtKP2F2AoN6zBgzS7uiuAwhWCfGdNeYJ3PC1HNJ8M8,L1xbHrxynrqLKkoYc4qtoQPx6uy5qYXR5ZDYVYBSRmCV5piU3JG9,L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1))","sh(multi(16,03669b8afcec803a0d323e9a17f3ea8e68e8abe5a278020a929adbec52421adbd0,0260b2003c386519fc9eadf2b5cf124dd8eea4c4e68d5e154050a9346ea98ce600,0362a74e399c39ed5593852a30147f2959b56bb827dfa3e60e464b02ccf87dc5e8,0261345b53de74a4d721ef877c255429961b7e43714171ac06168d7e08c542a8b8,02da72e8b46901a65d4374fe6315538d8f368557dda3a1dcf9ea903f3afe7314c8,0318c82dd0b53fd3a932d16e0ba9e278fcc937c582d5781be626ff16e201f72286,0297ccef1ef99f9d73dec9ad37476ddb232f1238aff877af19e72ba04493361009,02e502cfd5c3f972fe9a3e2a18827820638f96b6f347e54d63deb839011fd5765d,03e687710f0e3ebe81c1037074da939d409c0025f17eb86adb9427d28f0f7ae0e9,02c04d3a5274952acdbc76987f3184b346a483d43be40874624b29e3692c1df5af,02ed06e0f418b5b43a7ec01d1d7d27290fa15f75771cb69b642a51471c29c84acd,036d46073cbb9ffee90473f3da429abc8de7f8751199da44485682a989a4bebb24,02f5d1ff7c9029a80a4e36b9a5497027ef7f3e73384a4a94fbfe7c4e9164eec8bc,02e41deffd1b7cce11cde209a781adcffdabd1b91c0ba0375857a2bfd9302419f3,02d76625f7956a7fc505ab02556c23ee72d832f1bac391bcd2d3abce5710a13d06,0399eb0a5487515802dc14544cf10b3666623762fbed2ec38a3975716e2c29c232,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", "P2SH script is too large, 581 bytes is larger than 520 bytes"); // Cannot have more than 15 keys in a P2SH multisig, or we exceed maximum push size
-
     // Check for invalid nesting of structures
     CheckUnparsable("sh(XJvEUEcFWCHCyruc8ZX5exPZaGe4UR7gC5FHrhwPnQGDs1uWCsT2)", "sh(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)", "A function is needed within P2SH"); // P2SH needs a script, not a key
     CheckUnparsable("sh(sh(pk(XJvEUEcFWCHCyruc8ZX5exPZaGe4UR7gC5FHrhwPnQGDs1uWCsT2)))", "sh(sh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)))", "Can only have sh() at top level"); // Cannot embed P2SH inside P2SH
@@ -346,6 +354,27 @@ BOOST_AUTO_TEST_CASE(descriptor_test)
     CheckUnparsable("", "addr(asdf)", "Address is not valid"); // Invalid address
     CheckUnparsable("", "raw(asdf)", "Raw script is not hex"); // Invalid script
     CheckUnparsable("", "raw(Ü)#00000000", "Invalid characters in payload"); // Invalid chars
+
+    // A 2of4 but using a direct push rather than OP_2
+    CScript nonminimalmultisig;
+    CKey keys[4];
+    nonminimalmultisig << std::vector<unsigned char>{2};
+    for (int i = 0; i < 4; i++) {
+        keys[i].MakeNewKey(true);
+        nonminimalmultisig << ToByteVector(keys[i].GetPubKey());
+    }
+    nonminimalmultisig << 4 << OP_CHECKMULTISIG;
+    CheckInferRaw(nonminimalmultisig);
+
+    // A 2of4 but using a direct push rather than OP_4
+    nonminimalmultisig.clear();
+    nonminimalmultisig << 2;
+    for (int i = 0; i < 4; i++) {
+        keys[i].MakeNewKey(true);
+        nonminimalmultisig << ToByteVector(keys[i].GetPubKey());
+    }
+    nonminimalmultisig << std::vector<unsigned char>{4} << OP_CHECKMULTISIG;
+    CheckInferRaw(nonminimalmultisig);
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/test/fuzz/script.cpp

@@ -125,9 +125,11 @@ FUZZ_TARGET_INIT(script, initialize_script)
     (void)ScriptToAsmStr(script, true);
 
     UniValue o1(UniValue::VOBJ);
-    ScriptPubKeyToUniv(script, o1, true);
+    ScriptPubKeyToUniv(script, o1, true, true);
+    ScriptPubKeyToUniv(script, o1, true, false);
     UniValue o2(UniValue::VOBJ);
-    ScriptPubKeyToUniv(script, o2, false);
+    ScriptPubKeyToUniv(script, o2, false, true);
+    ScriptPubKeyToUniv(script, o2, false, false);
     UniValue o3(UniValue::VOBJ);
     ScriptToUniv(script, o3, true);
     UniValue o4(UniValue::VOBJ);

src/test/fuzz/transaction.cpp

@@ -94,7 +94,9 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction)
     (void)AreInputsStandard(tx, coins_view_cache);
 
     UniValue u(UniValue::VOBJ);
-    TxToUniv(tx, /* hashBlock */ {}, u);
+    TxToUniv(tx, /* hashBlock */ {}, /* include_addresses */ true, u);
+    TxToUniv(tx, /* hashBlock */ {}, /* include_addresses */ false, u);
     static const uint256 u256_max(uint256S("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"));
-    TxToUniv(tx, u256_max, u);
+    TxToUniv(tx, u256_max, /* include_addresses */ true, u);
+    TxToUniv(tx, u256_max, /* include_addresses */ false, u);
 }

src/validation.cpp

@@ -4170,6 +4170,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, Block
     // This requires some new chain data structure to efficiently look up if a
     // block is in a chain leading to a candidate for best tip, despite not
     // being such a candidate itself.
+    // Note that this would break the getblockfrompeer RPC
 
     // TODO: deal better with return value and error conditions for duplicate
     // and unrequested blocks.

src/wallet/coincontrol.h

@@ -40,6 +40,8 @@ public:
     CTxDestination destChange = CNoDestination();
     //! If false, only selected inputs are used
     bool m_add_inputs = true;
+    //! If false, only safe inputs will be used
+    bool m_include_unsafe_inputs = false;
     //! If false, allows unselected inputs, but requires all selected inputs be used if fAllowOtherInputs is true (default)
     bool fAllowOtherInputs = false;
     //! If false, only include as many inputs as necessary to fulfill a coin selection request. Only usable together with fAllowOtherInputs

src/wallet/coinselection.h

@@ -65,8 +65,7 @@ struct CoinEligibilityFilter
     /** Minimum number of confirmations for outputs that we sent to ourselves.
      * We may use unconfirmed UTXOs sent from ourselves, e.g. change outputs. */
     const int conf_mine;
-    /** Minimum number of confirmations for outputs received from a different
+    /** Minimum number of confirmations for outputs received from a different wallet. */
-     * wallet. We never spend unconfirmed foreign outputs as we cannot rely on these funds yet. */
     const int conf_theirs;
     /** Maximum number of unconfirmed ancestors aggregated across all UTXOs in an OutputGroup. */
     const uint64_t max_ancestors;

src/wallet/rpcdump.cpp

@@ -1750,9 +1750,8 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue&
         // Check if the wallet already contains the descriptor
         auto existing_spk_manager = pwallet->GetDescriptorScriptPubKeyMan(w_desc);
         if (existing_spk_manager) {
-            LOCK(existing_spk_manager->cs_desc_man);
+            if (!existing_spk_manager->CanUpdateToWalletDescriptor(w_desc, error)) {
-            if (range_start > existing_spk_manager->GetWalletDescriptor().range_start) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, error);
-                throw JSONRPCError(RPC_INVALID_PARAMS, strprintf("range_start can only decrease; current range = [%d,%d]", existing_spk_manager->GetWalletDescriptor().range_start, existing_spk_manager->GetWalletDescriptor().range_end));
             }
         }
 
@@ -1769,16 +1768,16 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue&
             } else {
                 pwallet->AddActiveScriptPubKeyMan(spk_manager->GetID(), internal);
             }
+        } else {
+            if (w_desc.descriptor->GetOutputType()) {
+                pwallet->DeactivateScriptPubKeyMan(spk_manager->GetID(), internal);
+            }
         }
 
         result.pushKV("success", UniValue(true));
     } catch (const UniValue& e) {
         result.pushKV("success", UniValue(false));
         result.pushKV("error", e);
-    } catch (...) {
-        result.pushKV("success", UniValue(false));
-
-        result.pushKV("error", JSONRPCError(RPC_MISC_ERROR, "Missing required fields"));
     }
     if (warnings.size()) result.pushKV("warnings", warnings);
     return result;

src/wallet/rpcwallet.cpp

@@ -1811,7 +1811,7 @@ static RPCHelpMan gettransaction()
 
     if (verbose) {
         UniValue decoded(UniValue::VOBJ);
-        TxToUniv(*wtx.tx, uint256(), decoded, false);
+        TxToUniv(*wtx.tx, uint256(), pwallet->chain().rpcEnableDeprecated("addresses"), decoded, false);
         entry.pushKV("decoded", decoded);
     }
 
@@ -3332,9 +3332,10 @@ static RPCHelpMan listunspent()
         coinControl.m_avoid_address_reuse = false;
         coinControl.m_min_depth = nMinDepth;
         coinControl.m_max_depth = nMaxDepth;
+        coinControl.m_include_unsafe_inputs = include_unsafe;
 
         LOCK(pwallet->cs_wallet);
-        pwallet->AvailableCoins(vecOutputs, !include_unsafe, &coinControl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount);
+        pwallet->AvailableCoins(vecOutputs, &coinControl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount);
     }
 
     LOCK(pwallet->cs_wallet);
@@ -3418,6 +3419,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
         RPCTypeCheckObj(options,
             {
                 {"add_inputs", UniValueType(UniValue::VBOOL)},
+                {"include_unsafe", UniValueType(UniValue::VBOOL)},
                 {"add_to_wallet", UniValueType(UniValue::VBOOL)},
                 {"changeAddress", UniValueType(UniValue::VSTR)},
                 {"change_address", UniValueType(UniValue::VSTR)},
@@ -3464,8 +3466,11 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
             lockUnspents = (options.exists("lock_unspents") ? options["lock_unspents"] : options["lockUnspents"]).get_bool();
         }
 
-        if (options.exists("feeRate"))
+        if (options.exists("include_unsafe")) {
-        {
+            coinControl.m_include_unsafe_inputs = options["include_unsafe"].get_bool();
+        }
+
+        if (options.exists("feeRate")) {
             if (options.exists("conf_target")) {
                 throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and feeRate");
             }
@@ -3530,6 +3535,9 @@ static RPCHelpMan fundrawtransaction()
                     {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}",
                         {
                             {"add_inputs", RPCArg::Type::BOOL, /* default */ "true", "For a transaction with existing inputs, automatically include more if they are not enough."},
+                            {"include_unsafe", RPCArg::Type::BOOL, /* default */ "false", "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n"
+                                                          "Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n"
+                                                          "If that happens, you will need to fund the transaction with different inputs and republish it."},
                             {"changeAddress", RPCArg::Type::STR, /* default */ "pool address", "The Dash address to receive the change"},
                             {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"},
                             {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only.\n"
@@ -4199,6 +4207,9 @@ static RPCHelpMan send()
             {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
                 {
                     {"add_inputs", RPCArg::Type::BOOL, /* default */ "false", "If inputs are specified, automatically include more if they are not enough."},
+                    {"include_unsafe", RPCArg::Type::BOOL, /* default */ "false", "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n"
+                                                          "Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n"
+                                                          "If that happens, you will need to fund the transaction with different inputs and republish it."},
                     {"add_to_wallet", RPCArg::Type::BOOL, /* default */ "true", "When false, returns a serialized transaction which will not be added to the wallet or broadcast"},
                     {"change_address", RPCArg::Type::STR_HEX, /* default */ "pool address", "The Dash address to receive the change"},
                     {"change_position", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"},
@@ -4533,6 +4544,9 @@ static RPCHelpMan walletcreatefundedpsbt()
             {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
                 {
                     {"add_inputs", RPCArg::Type::BOOL, /* default */ "false", "If inputs are specified, automatically include more if they are not enough."},
+                    {"include_unsafe", RPCArg::Type::BOOL, /* default */ "false", "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n"
+                                                    "Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n"
+                                                    "If that happens, you will need to fund the transaction with different inputs and republish it."},
                     {"changeAddress", RPCArg::Type::STR_HEX, /* default */ "pool address", "The Dash address to receive the change"},
                     {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"},
                     {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only"},

src/wallet/scriptpubkeyman.cpp

@@ -2007,6 +2007,12 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const
     AssertLockHeld(cs_desc_man);
     assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));
 
+    // Check if provided key already exists
+    if (m_map_keys.find(pubkey.GetID()) != m_map_keys.end() ||
+        m_map_crypted_keys.find(pubkey.GetID()) != m_map_crypted_keys.end()) {
+        return true;
+    }
+
     if (m_storage.HasEncryptionKeys()) {
         if (m_storage.IsLocked()) {
             return false;
@@ -2442,3 +2448,37 @@ void DescriptorScriptPubKeyMan::UpgradeDescriptorCache()
         throw std::runtime_error(std::string(__func__) + ": writing cache items failed");
     }
 }
+
+void DescriptorScriptPubKeyMan::UpdateWalletDescriptor(WalletDescriptor& descriptor)
+{
+    LOCK(cs_desc_man);
+    std::string error;
+    if (!CanUpdateToWalletDescriptor(descriptor, error)) {
+        throw std::runtime_error(std::string(__func__) + ": " + error);
+    }
+
+    m_map_pubkeys.clear();
+    m_map_script_pub_keys.clear();
+    m_max_cached_index = -1;
+    m_wallet_descriptor = descriptor;
+}
+
+bool DescriptorScriptPubKeyMan::CanUpdateToWalletDescriptor(const WalletDescriptor& descriptor, std::string& error)
+{
+    LOCK(cs_desc_man);
+    if (!HasWalletDescriptor(descriptor)) {
+        error = "can only update matching descriptor";
+        return false;
+    }
+
+    if (descriptor.range_start > m_wallet_descriptor.range_start ||
+        descriptor.range_end < m_wallet_descriptor.range_end) {
+        // Use inclusive range for error
+        error = strprintf("new range must include current range = [%d,%d]",
+                          m_wallet_descriptor.range_start,
+                          m_wallet_descriptor.range_end - 1);
+        return false;
+    }
+
+    return true;
+}

src/wallet/scriptpubkeyman.h

@@ -603,6 +603,8 @@ public:
     bool AddCryptedKey(const CKeyID& key_id, const CPubKey& pubkey, const std::vector<unsigned char>& crypted_key);
 
     bool HasWalletDescriptor(const WalletDescriptor& desc) const;
+    void UpdateWalletDescriptor(WalletDescriptor& descriptor);
+    bool CanUpdateToWalletDescriptor(const WalletDescriptor& descriptor, std::string& error);
     void AddDescriptorKey(const CKey& key, const CPubKey &pubkey);
     void WriteDescriptor();
 

src/wallet/wallet.cpp

@@ -2576,7 +2576,7 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const
 
     CAmount balance = 0;
     std::vector<COutput> vCoins;
-    AvailableCoins(vCoins, true, coinControl);
+    AvailableCoins(vCoins, coinControl);
     for (const COutput& out : vCoins) {
         if (out.fSpendable) {
             balance += out.tx->tx->vout[out.i].nValue;
@@ -2585,7 +2585,7 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const
     return balance;
 }
 
-void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount &nMinimumSumAmount, const uint64_t nMaximumCount) const
+void CWallet::AvailableCoins(std::vector<COutput> &vCoins, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount &nMinimumSumAmount, const uint64_t nMaximumCount) const
 {
     AssertLockHeld(cs_wallet);
 
@@ -2598,6 +2598,7 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const
     bool allow_used_addresses = !IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) || (coinControl && !coinControl->m_avoid_address_reuse);
     const int min_depth = {coinControl ? coinControl->m_min_depth : DEFAULT_MIN_DEPTH};
     const int max_depth = {coinControl ? coinControl->m_max_depth : DEFAULT_MAX_DEPTH};
+    const bool only_safe = {coinControl ? !coinControl->m_include_unsafe_inputs : true};
 
     std::set<uint256> trusted_parents;
     for (auto pcoin : GetSpendableTXs()) {
@@ -2618,7 +2619,7 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const
 
         bool safeTx = IsTrusted(*pcoin, trusted_parents);
 
-        if (fOnlySafe && !safeTx) {
+        if (only_safe && !safeTx) {
             continue;
         }
 
@@ -2932,8 +2933,7 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
         if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true;
 
         // Fall back to using zero confirmation change (but with as few ancestors in the mempool as
-        // possible) if we cannot fund the transaction otherwise. We never spend unconfirmed
+        // possible) if we cannot fund the transaction otherwise.
-        // outputs received from other wallets.
         if (m_spend_zero_conf_change) {
             if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true;
             if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)),
@@ -2951,6 +2951,14 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
                                    vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) {
                 return true;
             }
+            // Try with unsafe inputs if they are allowed. This may spend unconfirmed outputs
+            // received from other wallets.
+            if (coin_control.m_include_unsafe_inputs
+                && SelectCoinsMinConf(value_to_select,
+                    CoinEligibilityFilter(0 /* conf_mine */, 0 /* conf_theirs */, max_ancestors-1, max_descendants-1, true /* include_partial_groups */),
+                    vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) {
+                return true;
+            }
             // Try with unlimited ancestors/descendants. The transaction will still need to meet
             // mempool ancestor/descendant policy to be accepted to mempool and broadcasted, but
             // OutputGroups use heuristics that may overestimate ancestor/descendant counts.
@@ -3159,7 +3167,7 @@ bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::ve
 
     CCoinControl coin_control;
     coin_control.nCoinType = CoinType::ONLY_READY_TO_MIX;
-    AvailableCoins(vCoins, true, &coin_control);
+    AvailableCoins(vCoins, &coin_control);
     WalletCJLogPrint((*this), "CWallet::%s -- vCoins.size(): %d\n", __func__, vCoins.size());
 
     Shuffle(vCoins.rbegin(), vCoins.rend(), FastRandomContext());
@@ -3359,7 +3367,7 @@ bool CWallet::SelectDenominatedAmounts(CAmount nValueMax, std::set<CAmount>& set
     std::vector<COutput> vCoins;
     CCoinControl coin_control;
     coin_control.nCoinType = CoinType::ONLY_READY_TO_MIX;
-    AvailableCoins(vCoins, true, &coin_control);
+    AvailableCoins(vCoins, &coin_control);
     // larger denoms first
     std::sort(vCoins.rbegin(), vCoins.rend(), CompareByPriority());
 
@@ -3398,8 +3406,9 @@ bool CWallet::HasCollateralInputs(bool fOnlyConfirmed) const
 
     std::vector<COutput> vCoins;
     CCoinControl coin_control;
+    coin_control.m_include_unsafe_inputs = !fOnlyConfirmed;
     coin_control.nCoinType = CoinType::ONLY_COINJOIN_COLLATERAL;
-    AvailableCoins(vCoins, fOnlyConfirmed, &coin_control);
+    AvailableCoins(vCoins, &coin_control);
 
     return !vCoins.empty();
 }
@@ -3473,7 +3482,7 @@ bool CWallet::CreateTransactionInternal(
         {
             CAmount nAmountAvailable{0};
             std::vector<COutput> vAvailableCoins;
-            AvailableCoins(vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0);
+            AvailableCoins(vAvailableCoins, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0);
             CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy
             coin_selection_params.use_bnb = false; // never use BnB
 
@@ -5741,16 +5750,42 @@ void CWallet::AddActiveScriptPubKeyMan(uint256 id, bool internal)
 
 void CWallet::LoadActiveScriptPubKeyMan(uint256 id, bool internal)
 {
+    // Activating ScriptPubKeyManager for a given output and change type is incompatible with legacy wallets.
+    // Legacy wallets have only one ScriptPubKeyManager and it's active for all output and change types.
+    Assert(IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
+
     WalletLogPrintf("Setting spkMan to active: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast<int>(OutputType::LEGACY), static_cast<int>(internal));
     auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers;
+    auto& spk_mans_other = internal ? m_external_spk_managers : m_internal_spk_managers;
     auto spk_man = m_spk_managers.at(id).get();
     spk_man->SetInternal(internal);
     spk_mans = spk_man;
 
+    if (spk_mans_other == spk_man) {
+        spk_mans_other = nullptr;
+    }
+
     NotifyCanGetAddressesChanged();
 
 }
 
+void CWallet::DeactivateScriptPubKeyMan(uint256 id, bool internal)
+{
+    auto spk_man = GetScriptPubKeyMan(internal);
+    if (spk_man != nullptr && spk_man->GetID() == id) {
+        WalletLogPrintf("Deactivate spkMan: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast<int>(OutputType::LEGACY), static_cast<int>(internal));
+        WalletBatch batch(GetDatabase());
+        if (!batch.EraseActiveScriptPubKeyMan(internal)) {
+            throw std::runtime_error(std::string(__func__) + ": erasing active ScriptPubKeyMan id failed");
+        }
+
+        auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers;
+        spk_mans = nullptr;
+    }
+
+    NotifyCanGetAddressesChanged();
+}
+
 bool CWallet::IsLegacy() const
 {
     if (m_internal_spk_managers == nullptr) return false;
@@ -5779,44 +5814,26 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat
     }
 
     LOCK(cs_wallet);
-    auto new_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, desc));
+    auto spk_man = GetDescriptorScriptPubKeyMan(desc);
-
+    if (spk_man) {
-    // If we already have this descriptor, remove it from the maps but add the existing cache to desc
-    auto old_spk_man = GetDescriptorScriptPubKeyMan(desc);
-    if (old_spk_man) {
         WalletLogPrintf("Update existing descriptor: %s\n", desc.descriptor->ToString());
+        spk_man->UpdateWalletDescriptor(desc);
+    } else {
+        auto new_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, desc));
+        spk_man = new_spk_man.get();
 
-        {
+        // Save the descriptor to memory
-            LOCK(old_spk_man->cs_desc_man);
+        m_spk_managers[new_spk_man->GetID()] = std::move(new_spk_man);
-            new_spk_man->SetCache(old_spk_man->GetWalletDescriptor().cache);
-        }
-
-        // Remove from maps of active spkMans
-        auto old_spk_man_id = old_spk_man->GetID();
-        for (bool internal : {false, true}) {
-            { // only one OutputType - LEGACY
-                auto active_spk_man = GetScriptPubKeyMan(internal);
-                if (active_spk_man && active_spk_man->GetID() == old_spk_man_id) {
-                    if (internal) {
-                        m_internal_spk_managers = nullptr;
-                    } else {
-                        m_external_spk_managers = nullptr;
-                    }
-                    break;
-                }
-            }
-        }
-        m_spk_managers.erase(old_spk_man_id);
     }
 
     // Add the private keys to the descriptor
     for (const auto& entry : signing_provider.keys) {
         const CKey& key = entry.second;
-        new_spk_man->AddDescriptorKey(key, key.GetPubKey());
+        spk_man->AddDescriptorKey(key, key.GetPubKey());
     }
 
     // Top up key pool, the manager will generate new scriptPubKeys internally
-    if (!new_spk_man->TopUp()) {
+    if (!spk_man->TopUp()) {
         WalletLogPrintf("Could not top up scriptPubKeys\n");
         return nullptr;
     }
@@ -5824,7 +5841,7 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat
     // Apply the label if necessary
     // Note: we disable labels for ranged descriptors
     if (!desc.descriptor->IsRange()) {
-        auto script_pub_keys = new_spk_man->GetScriptPubKeys();
+        auto script_pub_keys = spk_man->GetScriptPubKeys();
         if (script_pub_keys.empty()) {
             WalletLogPrintf("Could not generate scriptPubKeys (cache is empty)\n");
             return nullptr;
@@ -5836,12 +5853,8 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat
         }
     }
 
-    // Save the descriptor to memory
-    auto ret = new_spk_man.get();
-    m_spk_managers[new_spk_man->GetID()] = std::move(new_spk_man);
-
     // Save the descriptor to DB
-    ret->WriteDescriptor();
+    spk_man->WriteDescriptor();
 
-    return ret;
+    return spk_man;
 }

src/wallet/wallet.h

@@ -944,7 +944,7 @@ public:
     /**
      * populate vCoins with vector of available COutputs.
      */
-    void AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe = true, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    void AvailableCoins(std::vector<COutput>& vCoins, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     /**
      * Return list of available coins and locked coins grouped by non-change output address.
@@ -1470,6 +1470,12 @@ public:
     //! @param[in] internal Whether this ScriptPubKeyMan provides change addresses
     void LoadActiveScriptPubKeyMan(uint256 id, bool internal);
 
+    //! Remove specified ScriptPubKeyMan from set of active SPK managers. Writes the change to the wallet file.
+    //! @param[in] id The unique id for the ScriptPubKeyMan
+    //! @param[in] type The OutputType this ScriptPubKeyMan provides addresses for
+    //! @param[in] internal Whether this ScriptPubKeyMan provides change addresses
+    void DeactivateScriptPubKeyMan(uint256 id, bool internal);
+
     //! Create new DescriptorScriptPubKeyMans and add them to the wallet
     void SetupDescriptorScriptPubKeyMans();
 

src/wallet/walletdb.cpp

@@ -234,6 +234,12 @@ bool WalletBatch::WriteActiveScriptPubKeyMan(const uint256& id, bool internal)
     return WriteIC(key, id);
 }
 
+bool WalletBatch::EraseActiveScriptPubKeyMan(bool internal)
+{
+    const std::string key{internal ? DBKeys::ACTIVEINTERNALSPK : DBKeys::ACTIVEEXTERNALSPK};
+    return EraseIC(key);
+}
+
 bool WalletBatch::WriteDescriptorKey(const uint256& desc_id, const CPubKey& pubkey, const CPrivKey& privkey)
 {
     // hash pubkey/privkey to accelerate wallet load

src/wallet/walletdb.h

@@ -226,6 +226,7 @@ public:
     bool EraseDestData(const std::string &address, const std::string &key);
 
     bool WriteActiveScriptPubKeyMan(const uint256& id, bool internal);
+    bool EraseActiveScriptPubKeyMan(bool internal);
 
     DBErrors LoadWallet(CWallet* pwallet);
     DBErrors FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWalletTx>& vWtx);

test/functional/feature_dip3_deterministicmns.py

@@ -24,7 +24,8 @@ class DIP3Test(BitcoinTestFramework):
         self.setup_clean_chain = True
         self.supports_cli = False
 
-        self.extra_args = ["-budgetparams=10:10:10"]
+        self.extra_args = ["-deprecatedrpc=addresses"]
+        self.extra_args += ["-budgetparams=10:10:10"]
         self.extra_args += ["-sporkkey=cP4EKFyJsHT39LDqgdcB43Y3YXjNyjb5Fuas1GQSeAtjnZWmZEQK"]
         self.extra_args += ["-dip3params=135:150"]
 

test/functional/feature_fee_estimation.py

@@ -135,9 +135,13 @@ def check_smart_estimates(node, fees_seen):
     delta = 1.0e-6  # account for rounding error
     last_feerate = float(max(fees_seen))
     all_smart_estimates = [node.estimatesmartfee(i) for i in range(1, 26)]
+    mempoolMinFee = node.getmempoolinfo()['mempoolminfee']
+    minRelaytxFee = node.getmempoolinfo()['minrelaytxfee']
     for i, e in enumerate(all_smart_estimates):  # estimate is for i+1
         feerate = float(e["feerate"])
         assert_greater_than(feerate, 0)
+        assert_greater_than_or_equal(feerate, float(mempoolMinFee))
+        assert_greater_than_or_equal(feerate, float(minRelaytxFee))
 
         if feerate + delta < min(fees_seen) or feerate - delta > max(fees_seen):
             raise AssertionError("Estimated fee (%f) out of range (%f,%f)"
@@ -281,6 +285,13 @@ class EstimateFeeTest(BitcoinTestFramework):
         self.log.info("Final estimates after emptying mempools")
         check_estimates(self.nodes[1], self.fees_per_kb)
 
+        # check that the effective feerate is greater than or equal to the mempoolminfee even for high mempoolminfee
+        self.log.info("Test fee rate estimation after restarting node with high MempoolMinFee")
+        high_val = 3*self.nodes[1].estimatesmartfee(1)['feerate']
+        self.restart_node(1, extra_args=[f'-minrelaytxfee={high_val}'])
+        check_estimates(self.nodes[1], self.fees_per_kb)
+        self.stop_node(1, expected_stderr="Warning: -minrelaytxfee is set very high! The wallet will avoid paying less than the minimum relay fee.")
+
         self.log.info("Testing that fee estimation is disabled in blocksonly.")
         self.restart_node(0, ["-blocksonly"])
         assert_raises_rpc_error(-32603, "Fee estimation disabled",

test/functional/feature_governance.py

@@ -62,14 +62,14 @@ class DashGovernanceTest (DashTestFramework):
         coinbase_outputs = self.nodes[0].getblock(self.nodes[0].getbestblockhash(), 2)["tx"][0]["vout"]
         payments_found = 0
         for txout in coinbase_outputs:
-            if txout["value"] == self.p0_amount and txout["scriptPubKey"]["addresses"][0] == self.p0_payout_address:
+            if txout["value"] == self.p0_amount and txout["scriptPubKey"]["address"] == self.p0_payout_address:
                 payments_found += 1
-            if txout["value"] == self.p1_amount and txout["scriptPubKey"]["addresses"][0] == self.p1_payout_address:
+            if txout["value"] == self.p1_amount and txout["scriptPubKey"]["address"] == self.p1_payout_address:
                 if self.p1_hash > self.p2_hash:
                     payments_found += 1
                 else:
                     assert False
-            if txout["value"] == self.p2_amount and txout["scriptPubKey"]["addresses"][0] == self.p2_payout_address:
+            if txout["value"] == self.p2_amount and txout["scriptPubKey"]["address"] == self.p2_payout_address:
                 if self.p2_hash > self.p1_hash:
                     payments_found += 1
                 else:

test/functional/p2p_filter.py

@@ -124,7 +124,7 @@ class FilterTest(BitcoinTestFramework):
         filter_peer = P2PBloomFilter()
 
         self.log.debug("Create a tx relevant to the peer before connecting")
-        filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['addresses'][0]
+        filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['address']
         txid = self.nodes[0].sendtoaddress(filter_address, 90)
 
         self.log.debug("Send a mempool msg after connecting and check that the tx is received")
@@ -136,7 +136,7 @@ class FilterTest(BitcoinTestFramework):
     def test_frelay_false(self, filter_peer):
         self.log.info("Check that a node with fRelay set to false does not receive invs until the filter is set")
         filter_peer.tx_received = False
-        filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['addresses'][0]
+        filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['address']
         self.nodes[0].sendtoaddress(filter_address, 90)
         # Sync to make sure the reason filter_peer doesn't receive the tx is not p2p delays
         filter_peer.sync_with_ping()
@@ -150,7 +150,7 @@ class FilterTest(BitcoinTestFramework):
         filter_peer.send_and_ping(filter_peer.watch_filter_init)
         # If fRelay is not already True, sending filterload sets it to True
         assert self.nodes[0].getpeerinfo()[0]['relaytxes']
-        filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['addresses'][0]
+        filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['address']
 
         self.log.info('Check that we receive merkleblock and tx if the filter matches a tx in a block')
         block_hash = self.nodes[0].generatetoaddress(1, filter_address)[0]

test/functional/rpc_addresses_deprecation.py

@@ -0,0 +1,58 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test deprecation of reqSigs and addresses RPC fields."""
+
+from io import BytesIO
+
+from test_framework.messages import CTransaction
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    hex_str_to_bytes
+)
+
+
+class AddressesDeprecationTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 2
+        self.extra_args = [[], ["-deprecatedrpc=addresses"]]
+
+    def skip_test_if_missing_module(self):
+        self.skip_if_no_wallet()
+
+    def run_test(self):
+        self.test_addresses_deprecation()
+
+    def test_addresses_deprecation(self):
+        node = self.nodes[0]
+        coin = node.listunspent().pop()
+
+        inputs = [{'txid': coin['txid'], 'vout': coin['vout']}]
+        outputs = {node.getnewaddress(): 0.99}
+        raw = node.createrawtransaction(inputs, outputs)
+        signed = node.signrawtransactionwithwallet(raw)['hex']
+
+        # This transaction is derived from test/util/data/txcreatemultisig1.json
+        tx = CTransaction()
+        tx.deserialize(BytesIO(hex_str_to_bytes(signed)))
+        tx.vout[0].scriptPubKey = hex_str_to_bytes("522102a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff39721021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d2102df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb48553ae")
+        tx_signed = node.signrawtransactionwithwallet(tx.serialize().hex())['hex']
+        txid = node.sendrawtransaction(hexstring=tx_signed, maxfeerate=0)
+
+        self.log.info("Test RPCResult scriptPubKey no longer returns the fields addresses or reqSigs by default")
+        hash = node.generateblock(output=node.getnewaddress(), transactions=[txid])['hash']
+        # Ensure both nodes have the newly generated block on disk.
+        self.sync_blocks()
+        script_pub_key = node.getblock(blockhash=hash, verbose=2)['tx'][-1]['vout'][0]['scriptPubKey']
+        assert 'addresses' not in script_pub_key and 'reqSigs' not in script_pub_key
+
+        self.log.info("Test RPCResult scriptPubKey returns the addresses field with -deprecatedrpc=addresses")
+        script_pub_key = self.nodes[1].getblock(blockhash=hash, verbose=2)['tx'][-1]['vout'][0]['scriptPubKey']
+        assert_equal(script_pub_key['addresses'], ['yb7hsErReWuYeyxH1LzXbmyU5iBdruakMi', 'yarLqM3oXZFRtuVR4SsNxqKrGxyScZhohq', 'yPfMS8LWznSCnmz1QnEnMxqHZXUrq5Lfu7'])
+        assert_equal(script_pub_key['reqSigs'], 2)
+
+
+if __name__ == "__main__":
+    AddressesDeprecationTest().main()

test/functional/rpc_createmultisig.py

@@ -152,7 +152,7 @@ class RpcCreateMultiSigTest(BitcoinTestFramework):
         txid = node0.sendtoaddress(madd, 40)
 
         tx = node0.getrawtransaction(txid, True)
-        vout = [v["n"] for v in tx["vout"] if madd in v["scriptPubKey"].get("addresses", [])]
+        vout = [v["n"] for v in tx["vout"] if madd == v["scriptPubKey"]["address"]]
         assert len(vout) == 1
         vout = vout[0]
         scriptPubKey = tx["vout"][vout]["scriptPubKey"]["hex"]

test/functional/rpc_fundrawtransaction.py

@@ -97,6 +97,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.test_address_reuse()
         self.test_option_subtract_fee_from_outputs()
         self.test_subtract_fee_with_presets()
+        self.test_include_unsafe()
 
     def test_change_position(self):
         """Ensure setting changePosition in fundraw with an exact match is handled properly."""
@@ -163,7 +164,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         totalOut = 0
         for out in dec_tx['vout']:
             totalOut += out['value']
-            address = out['scriptPubKey']['addresses'][0]
+            address = out['scriptPubKey']['address']
             if address in outputs.keys():
                 assert_equal(satoshi_round(outputs[address]), out['value'])
 
@@ -252,7 +253,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {'changeAddress': change, 'changePosition': 0})
         dec_tx  = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
         out = dec_tx['vout'][0]
-        assert_equal(change, out['scriptPubKey']['addresses'][0])
+        assert_equal(change, out['scriptPubKey']['address'])
 
     def test_coin_selection(self):
         self.log.info("Test fundrawtxn with a vin < required amount")
@@ -279,7 +280,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         matchingOuts = 0
         for i, out in enumerate(dec_tx['vout']):
             totalOut += out['value']
-            if out['scriptPubKey']['addresses'][0] in outputs:
+            if out['scriptPubKey']['address'] in outputs:
                 matchingOuts+=1
             else:
                 assert_equal(i, rawtxfund['changepos'])
@@ -310,7 +311,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         matchingOuts = 0
         for out in dec_tx['vout']:
             totalOut += out['value']
-            if out['scriptPubKey']['addresses'][0] in outputs:
+            if out['scriptPubKey']['address'] in outputs:
                 matchingOuts+=1
 
         assert_equal(matchingOuts, 1)
@@ -344,7 +345,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         matchingOuts = 0
         for out in dec_tx['vout']:
             totalOut += out['value']
-            if out['scriptPubKey']['addresses'][0] in outputs:
+            if out['scriptPubKey']['address'] in outputs:
                 matchingOuts+=1
 
         assert_equal(matchingOuts, 2)
@@ -727,7 +728,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         changeaddress = ""
         for out in res_dec['vout']:
             if out['value'] > 1.0:
-                changeaddress += out['scriptPubKey']['addresses'][0]
+                changeaddress += out['scriptPubKey']['address']
         assert changeaddress != ""
         nextaddr = self.nodes[3].getnewaddress()
         # Now the change address key should be removed from the keypool.
@@ -815,5 +816,40 @@ class RawTransactionsTest(BitcoinTestFramework):
         signedtx = self.nodes[0].signrawtransactionwithwallet(fundedtx['hex'])
         self.nodes[0].sendrawtransaction(signedtx['hex'])
 
+    def test_include_unsafe(self):
+        self.log.info("Test fundrawtxn with unsafe inputs")
+
+        self.nodes[0].createwallet("unsafe")
+        wallet = self.nodes[0].get_wallet_rpc("unsafe")
+
+        # We receive unconfirmed funds from external keys (unsafe outputs).
+        addr = wallet.getnewaddress()
+        txid1 = self.nodes[2].sendtoaddress(addr, 6)
+        txid2 = self.nodes[2].sendtoaddress(addr, 4)
+        self.sync_all()
+        vout1 = find_vout_for_address(wallet, txid1, addr)
+        vout2 = find_vout_for_address(wallet, txid2, addr)
+
+        # Unsafe inputs are ignored by default.
+        rawtx = wallet.createrawtransaction([], [{self.nodes[2].getnewaddress(): 5}])
+        assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, rawtx)
+
+        # But we can opt-in to use them for funding.
+        fundedtx = wallet.fundrawtransaction(rawtx, {"include_unsafe": True})
+        tx_dec = wallet.decoderawtransaction(fundedtx['hex'])
+        assert any([txin['txid'] == txid1 and txin['vout'] == vout1 for txin in tx_dec['vin']])
+        signedtx = wallet.signrawtransactionwithwallet(fundedtx['hex'])
+        wallet.sendrawtransaction(signedtx['hex'])
+
+        # And we can also use them once they're confirmed.
+        self.nodes[0].generate(1)
+        rawtx = wallet.createrawtransaction([], [{self.nodes[2].getnewaddress(): 3}])
+        fundedtx = wallet.fundrawtransaction(rawtx, {"include_unsafe": True})
+        tx_dec = wallet.decoderawtransaction(fundedtx['hex'])
+        assert any([txin['txid'] == txid2 and txin['vout'] == vout2 for txin in tx_dec['vin']])
+        signedtx = wallet.signrawtransactionwithwallet(fundedtx['hex'])
+        wallet.sendrawtransaction(signedtx['hex'])
+
+
 if __name__ == '__main__':
     RawTransactionsTest().main()

test/functional/rpc_generateblock.py

@@ -26,13 +26,13 @@ class GenerateBlockTest(BitcoinTestFramework):
         hash = node.generateblock(address, [])['hash']
         block = node.getblock(hash, 2)
         assert_equal(len(block['tx']), 1)
-        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['addresses'][0], address)
+        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['address'], address)
 
         self.log.info('Generate an empty block to a descriptor')
         hash = node.generateblock('addr(' + address + ')', [])['hash']
         block = node.getblock(hash, 2)
         assert_equal(len(block['tx']), 1)
-        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['addresses'][0], address)
+        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['address'], address)
 
         self.log.info('Generate an empty block to a combo descriptor with compressed pubkey')
         combo_key = '0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798'
@@ -40,7 +40,7 @@ class GenerateBlockTest(BitcoinTestFramework):
         hash = node.generateblock('combo(' + combo_key + ')', [])['hash']
         block = node.getblock(hash, 2)
         assert_equal(len(block['tx']), 1)
-        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['addresses'][0], combo_address)
+        assert_equal(block['tx'][0]['vout'][0]['scriptPubKey']['address'], combo_address)
 
         # Generate 110 blocks to spend
         node.generatetoaddress(110, address)

test/functional/rpc_getblockfrompeer.py

@@ -0,0 +1,72 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test the getblockfrompeer RPC."""
+
+from test_framework.authproxy import JSONRPCException
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_raises_rpc_error,
+)
+
+class GetBlockFromPeerTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 2
+
+    def setup_network(self):
+        self.setup_nodes()
+
+    def check_for_block(self, hash):
+        try:
+            self.nodes[0].getblock(hash)
+            return True
+        except JSONRPCException:
+            return False
+
+    def run_test(self):
+        self.log.info("Mine 4 blocks on Node 0")
+        self.nodes[0].generate(4)
+        assert_equal(self.nodes[0].getblockcount(), 204)
+
+        self.log.info("Mine competing 3 blocks on Node 1")
+        self.nodes[1].generate(3)
+        assert_equal(self.nodes[1].getblockcount(), 203)
+        short_tip = self.nodes[1].getbestblockhash()
+
+        self.log.info("Connect nodes to sync headers")
+        self.connect_nodes(0, 1)
+        self.sync_blocks()
+
+        self.log.info("Node 0 should only have the header for node 1's block 3")
+        x = next(filter(lambda x: x['hash'] == short_tip, self.nodes[0].getchaintips()))
+        assert_equal(x['status'], "headers-only")
+        assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, short_tip)
+
+        self.log.info("Fetch block from node 1")
+        peers = self.nodes[0].getpeerinfo()
+        assert_equal(len(peers), 1)
+        peer_0_peer_1_id = peers[0]["id"]
+
+        self.log.info("Arguments must be sensible")
+        assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", 0)
+
+        self.log.info("We must already have the header")
+        assert_raises_rpc_error(-1, "Block header missing", self.nodes[0].getblockfrompeer, "00" * 32, 0)
+
+        self.log.info("Non-existent peer generates error")
+        assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_0_peer_1_id + 1)
+
+        self.log.info("Successful fetch")
+        result = self.nodes[0].getblockfrompeer(short_tip, peer_0_peer_1_id)
+        self.wait_until(lambda: self.check_for_block(short_tip), timeout=1)
+        assert(not "warnings" in result)
+
+        self.log.info("Don't fetch blocks we already have")
+        result = self.nodes[0].getblockfrompeer(short_tip, peer_0_peer_1_id)
+        assert("warnings" in result)
+        assert_equal(result["warnings"], "Block already downloaded")
+
+if __name__ == '__main__':
+    GetBlockFromPeerTest().main()

test/functional/rpc_psbt.py

@@ -89,9 +89,9 @@ class PSBTTest(BitcoinTestFramework):
         p2pkh_pos = -1
         decoded = self.nodes[0].decoderawtransaction(signed_tx)
         for out in decoded['vout']:
-            if out['scriptPubKey']['addresses'][0] == p2sh:
+            if out['scriptPubKey']['address'] == p2sh:
                 p2sh_pos = out['n']
-            elif out['scriptPubKey']['addresses'][0] == p2pkh:
+            elif out['scriptPubKey']['address'] == p2pkh:
                 p2pkh_pos = out['n']
 
         # spend single key from node 1
@@ -202,6 +202,14 @@ class PSBTTest(BitcoinTestFramework):
         # We don't care about the decode result, but decoding must succeed.
         self.nodes[0].decodepsbt(double_processed_psbt["psbt"])
 
+        # Make sure unsafe inputs are included if specified
+        self.nodes[2].createwallet(wallet_name="unsafe")
+        wunsafe = self.nodes[2].get_wallet_rpc("unsafe")
+        self.nodes[0].sendtoaddress(wunsafe.getnewaddress(), 2)
+        self.sync_mempools()
+        assert_raises_rpc_error(-4, "Insufficient funds", wunsafe.walletcreatefundedpsbt, [], [{self.nodes[0].getnewaddress(): 1}])
+        wunsafe.walletcreatefundedpsbt([], [{self.nodes[0].getnewaddress(): 1}], 0, {"include_unsafe": True})
+
         # BIP 174 Test Vectors
 
         # Check that unknown values are just passed through

test/functional/test_framework/util.py

@@ -608,6 +608,6 @@ def find_vout_for_address(node, txid, addr):
     """
     tx = node.getrawtransaction(txid, True)
     for i in range(len(tx["vout"])):
-        if any([addr == a for a in tx["vout"][i]["scriptPubKey"]["addresses"]]):
+        if addr == tx["vout"][i]["scriptPubKey"]["address"]:
             return i
     raise RuntimeError("Vout not found for address: txid=%s, addr=%s" % (txid, addr))

test/functional/test_runner.py

@@ -234,6 +234,7 @@ BASE_SCRIPTS = [
     'wallet_txn_clone.py --mineblock',
     'feature_notifications.py',
     'rpc_getblockfilter.py',
+    'rpc_getblockfrompeer.py',
     'rpc_invalidateblock.py',
     'feature_txindex.py',
     'feature_utxo_set_hash.py',
@@ -344,6 +345,7 @@ BASE_SCRIPTS = [
     'feature_config_args.py',
     'feature_settings.py',
     'rpc_getdescriptorinfo.py',
+    'rpc_addresses_deprecation.py',
     'rpc_getpeerinfo_deprecation.py',
     'rpc_help.py',
     'feature_help.py',

test/functional/wallet_basic.py

@@ -631,7 +631,7 @@ class WalletTest(BitcoinTestFramework):
         destination = self.nodes[1].getnewaddress()
         txid = self.nodes[0].sendtoaddress(destination, 0.123)
         tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex'])
-        output_addresses = [vout['scriptPubKey']['addresses'][0] for vout in tx["vout"]]
+        output_addresses = [vout['scriptPubKey']['address'] for vout in tx["vout"]]
         assert len(output_addresses) > 1
         for address in output_addresses:
             ischange = self.nodes[0].getaddressinfo(address)['ischange']

test/functional/wallet_change_address.py

@@ -30,7 +30,7 @@ class WalletChangeAddressTest(BitcoinTestFramework):
     def assert_change_index(self, node, tx, index):
         change_index = None
         for vout in tx["vout"]:
-            info = node.getaddressinfo(vout["scriptPubKey"]["addresses"][0])
+            info = node.getaddressinfo(vout["scriptPubKey"]["address"])
             if (info["ismine"] and info["ischange"]):
                 change_index = int(re.findall(r'\d+', info["hdkeypath"])[-1])
                 break

test/functional/wallet_hd.py

@@ -134,7 +134,7 @@ class WalletHDTest(BitcoinTestFramework):
         keypath = ""
         for out in outs:
             if out['value'] != 1:
-                keypath = self.nodes[1].getaddressinfo(out['scriptPubKey']['addresses'][0])['hdkeypath']
+                keypath = self.nodes[1].getaddressinfo(out['scriptPubKey']['address'])['hdkeypath']
 
         assert_equal(keypath[0:13], "m/44'/1'/0'/1")
 

test/functional/wallet_importdescriptors.py

@@ -78,7 +78,6 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         # RPC importdescriptors -----------------------------------------------
 
         # # Test import fails if no descriptor present
-        key = get_generate_key()
         self.log.info("Import should fail if a descriptor is not provided")
         self.test_importdesc({"timestamp": "now"},
                              success=False,
@@ -88,10 +87,10 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         # # Test importing of a P2PKH descriptor
         key = get_generate_key()
         self.log.info("Should import a p2pkh descriptor")
-        self.test_importdesc({"desc": descsum_create("pkh(" + key.pubkey + ")"),
+        import_request = {"desc": descsum_create("pkh(" + key.pubkey + ")"),
-                              "timestamp": "now",
+                 "timestamp": "now",
-                              "label": "Descriptor import test"},
+                 "label": "Descriptor import test"}
-                             success=True)
+        self.test_importdesc(import_request, success=True)
         test_address(w1,
                      key.p2pkh_addr,
                      solvable=True,
@@ -108,11 +107,15 @@ class ImportDescriptorsTest(BitcoinTestFramework):
                      ismine=True,
                      labels=["Descriptor import test"])
 
+        self.log.info("Test can import same descriptor with public key twice")
+        self.test_importdesc(import_request, success=True)
+
+        self.log.info("Test can update descriptor label")
+        self.test_importdesc({**import_request, "label": "Updated label"}, success=True)
+        test_address(w1, key.p2pkh_addr, solvable=True, ismine=True, labels=["Updated label"])
+
         self.log.info("Internal addresses cannot have labels")
-        self.test_importdesc({"desc": descsum_create("pkh(" + key.pubkey + ")"),
+        self.test_importdesc({**import_request, "internal": True},
-                              "timestamp": "now",
-                              "internal": True,
-                              "label": "Descriptor import test"},
                              success=False,
                              error_code=-8,
                              error_message="Internal addresses should not have a label")
@@ -246,6 +249,39 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         self.test_importdesc({"desc": descsum_create(desc), "timestamp": "now", "range": [0, 1000001]},
                               success=False, error_code=-8, error_message='Range is too large')
 
+        self.log.info("Verify we can only extend descriptor's range")
+        range_request = {"desc": descsum_create(desc), "timestamp": "now", "range": [5, 10], 'active': True}
+        self.test_importdesc(range_request, wallet=wpriv, success=True)
+        assert_equal(wpriv.getwalletinfo()['keypoolsize'], 6)
+        self.test_importdesc({**range_request, "range": [0, 10]}, wallet=wpriv, success=True)
+        assert_equal(wpriv.getwalletinfo()['keypoolsize'], 11)
+        self.test_importdesc({**range_request, "range": [0, 20]}, wallet=wpriv, success=True)
+        assert_equal(wpriv.getwalletinfo()['keypoolsize'], 21)
+        # Can keep range the same
+        self.test_importdesc({**range_request, "range": [0, 20]}, wallet=wpriv, success=True)
+        assert_equal(wpriv.getwalletinfo()['keypoolsize'], 21)
+
+        self.test_importdesc({**range_request, "range": [5, 10]}, wallet=wpriv, success=False,
+                             error_code=-8, error_message='new range must include current range = [0,20]')
+        self.test_importdesc({**range_request, "range": [0, 10]}, wallet=wpriv, success=False,
+                             error_code=-8, error_message='new range must include current range = [0,20]')
+        self.test_importdesc({**range_request, "range": [5, 20]}, wallet=wpriv, success=False,
+                             error_code=-8, error_message='new range must include current range = [0,20]')
+        assert_equal(wpriv.getwalletinfo()['keypoolsize'], 21)
+
+        self.log.info("Check we can change descriptor internal flag")
+        self.test_importdesc({**range_request, "range": [0, 20], "internal": True}, wallet=wpriv, success=True)
+        assert_equal(wpriv.getwalletinfo()['keypoolsize'], 0)
+        assert_raises_rpc_error(-4, 'This wallet has no available keys', wpriv.getnewaddress, '')
+        assert_equal(wpriv.getwalletinfo()['keypoolsize_hd_internal'], 21)
+        wpriv.getrawchangeaddress()
+
+        self.test_importdesc({**range_request, "range": [0, 20], "internal": False}, wallet=wpriv, success=True)
+        assert_equal(wpriv.getwalletinfo()['keypoolsize'], 21)
+        wpriv.getnewaddress('')
+        assert_equal(wpriv.getwalletinfo()['keypoolsize_hd_internal'], 0)
+        assert_raises_rpc_error(-4, 'This wallet has no available keys', wpriv.getrawchangeaddress)
+
         # Make sure ranged imports import keys in order
         w1 = self.nodes[1].get_wallet_rpc('w1')
         self.log.info('Key ranges should be imported in order')
@@ -278,9 +314,21 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         w1.keypoolrefill()
         assert_equal(w1.getwalletinfo()['keypoolsize'], 5 )
 
+        self.log.info("Check we can change next_index")
+        # go back and forth with next_index
+        for i in [4, 0, 2, 1, 3]:
+            self.test_importdesc({'desc': descsum_create('pkh([80002067/0h/0h]' + xpub + '/*)'),
+                                  'active': True,
+                                  'range': [0, 9],
+                                  'next_index': i,
+                                  'timestamp': 'now'
+                                  },
+                                 success=True)
+            assert_equal(w1.getnewaddress(''), addresses[i])
+
         # Check active=False default
         self.log.info('Check imported descriptors are not active by default')
-        self.test_importdesc({'desc': descsum_create('pkh([12345678/0h/0h]' + xpub + '/*)'),
+        self.test_importdesc({'desc': descsum_create('pkh([12345678/1h]' + xpub + '/*)'),
                               'range' : [0, 2],
                               'timestamp': 'now',
                               'internal': True
@@ -288,6 +336,32 @@ class ImportDescriptorsTest(BitcoinTestFramework):
                              success=True)
         assert_raises_rpc_error(-4, 'This wallet has no available keys', w1.getrawchangeaddress)
 
+        self.log.info('Check can activate inactive descriptor')
+        self.test_importdesc({'desc': descsum_create('pkh([12345678]' + xpub + '/*)'),
+                              'range': [0, 5],
+                              'active': True,
+                              'timestamp': 'now',
+                              'internal': True
+                              },
+                             success=True)
+        address = w1.getrawchangeaddress()
+        assert_equal(address, "yUxX4qnzWntXhEGrYB92v7ez4EZBnUjB1y")
+
+        self.log.info('Check can deactivate active descriptor')
+        self.test_importdesc({'desc': descsum_create('pkh([12345678]' + xpub + '/*)'),
+                              'range': [0, 5],
+                              'active': False,
+                              'timestamp': 'now',
+                              'internal': True
+                              },
+                             success=True)
+        assert_raises_rpc_error(-4, 'This wallet has no available keys', w1.getrawchangeaddress)
+
+        self.log.info('Verify activation state is persistent')
+        w1.unloadwallet()
+        self.nodes[1].loadwallet('w1')
+        assert_raises_rpc_error(-4, 'This wallet has no available keys', w1.getrawchangeaddress)
+
         # # Test importing a descriptor containing a WIF private key
         wif_priv = "cTT3BvHnd51YJf8fkdr2XvZTQRRUZruWhRvRyQY1raVFg5Lvam2A"
         address = "ySWABbcNKyHUgBb1ffhpuETuis9jsdR3aq"
@@ -297,6 +371,10 @@ class ImportDescriptorsTest(BitcoinTestFramework):
                                "timestamp": "now"},
                               success=True,
                               wallet=wpriv)
+
+        self.log.info('Test can import same descriptor with private key twice')
+        self.test_importdesc({"desc": descsum_create(desc), "timestamp": "now"}, success=True, wallet=wpriv)
+
         test_address(wpriv,
                      address,
                      solvable=True,
@@ -314,14 +392,25 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         wmulti_priv = self.nodes[1].get_wallet_rpc("wmulti_priv")
         assert_equal(wmulti_priv.getwalletinfo()['keypoolsize'], 0)
 
-        self.test_importdesc({"desc":"sh(multi(2,tprv8ZgxMBicQKsPevADjDCWsa6DfhkVXicu8NQUzfibwX2MexVwW4tCec5mXdCW8kJwkzBRRmAay1KZya4WsehVvjTGVW6JLqiqd8DdZ4xSg52/84h/0h/0h/*,tprv8ZgxMBicQKsPdSNWUhDiwTScDr6JfkZuLshTRwzvZGnMSnGikV6jxpmdDkC3YRc4T3GD6Nvg9uv6hQg73RVv1EiTXDZwxVbsLugVHU8B1aq/84h/0h/0h/*,tprv8ZgxMBicQKsPeonDt8Ka2mrQmHa61hQ5FQCsvWBTpSNzBFgM58cV2EuXNAHF14VawVpznnme3SuTbA62sGriwWyKifJmXntfNeK7zeqMCj1/84h/0h/0h/*))#f5nqn4ax",
+        xprv1 = 'tprv8ZgxMBicQKsPevADjDCWsa6DfhkVXicu8NQUzfibwX2MexVwW4tCec5mXdCW8kJwkzBRRmAay1KZya4WsehVvjTGVW6JLqiqd8DdZ4xSg52'
+        acc_xpub1 = 'tpubDCJtdt5dgJpdhW4MtaVYDhG4T4tF6jcLR1PxL43q9pq1mxvXgMS9Mzw1HnXG15vxUGQJMMSqCQHMTy3F1eW5VkgVroWzchsPD5BUojrcWs8'  # /84'/0'/0'
+        chg_xpub1 = 'tpubDCXqdwWZcszwqYJSnZp8eARkxGJfHAk23KDxbztV4BbschfaTfYLTcSkSJ3TN64dRqwa1rnFUScsYormKkGqNbbPwkorQimVevXjxzUV9Gf'  # /84'/1'/0'
+        xprv2 = 'tprv8ZgxMBicQKsPdSNWUhDiwTScDr6JfkZuLshTRwzvZGnMSnGikV6jxpmdDkC3YRc4T3GD6Nvg9uv6hQg73RVv1EiTXDZwxVbsLugVHU8B1aq'
+        acc_xprv2 = 'tprv8gVCsmRAxVSxyUpsL13Y7ZEWBFPWbgS5E2MmFVNGuANrknvmmn2vWnmHvU8AwEFYzR2ji6EeZLSCLVacsYkvor3Pcb5JY5FGcevqTwYvdYx'
+        acc_xpub2 = 'tpubDDBF2BTR6s8drwrfDei8WxtckGuSm1cyoKxYY1QaKSBFbHBYQArWhHPA6eJrzZej6nfHGLSURYSLHr7GuYch8aY5n61tGqgn8b4cXrMuoPH'
+        chg_xpub2 = 'tpubDCYfZY2ceyHzYzMMVPt9MNeiqtQ2T7Uyp9QSFwYXh8Vi9iJFYXcuphJaGXfF3jUQJi5Y3GMNXvM11gaL4txzZgNGK22BFAwMXynnzv4z2Jh'
+        xprv3 = 'tprv8ZgxMBicQKsPeonDt8Ka2mrQmHa61hQ5FQCsvWBTpSNzBFgM58cV2EuXNAHF14VawVpznnme3SuTbA62sGriwWyKifJmXntfNeK7zeqMCj1'
+        acc_xpub3 = 'tpubDCsWoW1kuQB9kG5MXewHqkbjPtqPueRnXju7uM2NK7y3JYb2ajAZ9EiuZXNNuE4661RAfriBWhL8UsnAPpk8zrKKnZw1Ug7X4oHgMdZiU4E'
+        chg_xpub3 = 'tpubDC6UGqnsQStngYuGD4MKsMy7eD1Yg9NTJfPdvjdG2JE5oZ7EsSL3WHg4Gsw2pR5K39ZwJ46M1wZayhedVdQtMGaUhq5S23PH6fnENK3V1sb'
+
+        self.test_importdesc({"desc":"sh(multi(2," + xprv1 + "/84h/0h/0h/*," + xprv2 + "/84h/0h/0h/*," + xprv3 + "/84h/0h/0h/*))#f5nqn4ax",
                             "active": True,
                             "range": 1000,
                             "next_index": 0,
                             "timestamp": "now"},
                             success=True,
                             wallet=wmulti_priv)
-        self.test_importdesc({"desc":"sh(multi(2,tprv8ZgxMBicQKsPevADjDCWsa6DfhkVXicu8NQUzfibwX2MexVwW4tCec5mXdCW8kJwkzBRRmAay1KZya4WsehVvjTGVW6JLqiqd8DdZ4xSg52/84h/1h/0h/*,tprv8ZgxMBicQKsPdSNWUhDiwTScDr6JfkZuLshTRwzvZGnMSnGikV6jxpmdDkC3YRc4T3GD6Nvg9uv6hQg73RVv1EiTXDZwxVbsLugVHU8B1aq/84h/1h/0h/*,tprv8ZgxMBicQKsPeonDt8Ka2mrQmHa61hQ5FQCsvWBTpSNzBFgM58cV2EuXNAHF14VawVpznnme3SuTbA62sGriwWyKifJmXntfNeK7zeqMCj1/84h/1h/0h/*))#m4e4s5de",
+        self.test_importdesc({"desc":"sh(multi(2," + xprv1 + "/84h/1h/0h/*," + xprv2 + "/84h/1h/0h/*," + xprv3 + "/84h/1h/0h/*))#m4e4s5de",
                             "active": True,
                             "internal" : True,
                             "range": 1000,
@@ -347,14 +436,14 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         wmulti_pub = self.nodes[1].get_wallet_rpc("wmulti_pub")
         assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 0)
 
-        self.test_importdesc({"desc":"sh(multi(2,[7b2d0242/84h/0h/0h]tpubDCJtdt5dgJpdhW4MtaVYDhG4T4tF6jcLR1PxL43q9pq1mxvXgMS9Mzw1HnXG15vxUGQJMMSqCQHMTy3F1eW5VkgVroWzchsPD5BUojrcWs8/*,[59b09cd6/84h/0h/0h]tpubDDBF2BTR6s8drwrfDei8WxtckGuSm1cyoKxYY1QaKSBFbHBYQArWhHPA6eJrzZej6nfHGLSURYSLHr7GuYch8aY5n61tGqgn8b4cXrMuoPH/*,[e81a0532/84h/0h/0h]tpubDCsWoW1kuQB9kG5MXewHqkbjPtqPueRnXju7uM2NK7y3JYb2ajAZ9EiuZXNNuE4661RAfriBWhL8UsnAPpk8zrKKnZw1Ug7X4oHgMdZiU4E/*))#x75vpsak",
+        self.test_importdesc({"desc":"sh(multi(2,[7b2d0242/84h/0h/0h]" + acc_xpub1 + "/*,[59b09cd6/84h/0h/0h]" + acc_xpub2 + "/*,[e81a0532/84h/0h/0h]" + acc_xpub3 +"/*))#x75vpsak",
                             "active": True,
                             "range": 1000,
                             "next_index": 0,
                             "timestamp": "now"},
                             success=True,
                             wallet=wmulti_pub)
-        self.test_importdesc({"desc":"sh(multi(2,[7b2d0242/84h/1h/0h]tpubDCXqdwWZcszwqYJSnZp8eARkxGJfHAk23KDxbztV4BbschfaTfYLTcSkSJ3TN64dRqwa1rnFUScsYormKkGqNbbPwkorQimVevXjxzUV9Gf/*,[59b09cd6/84h/1h/0h]tpubDCYfZY2ceyHzYzMMVPt9MNeiqtQ2T7Uyp9QSFwYXh8Vi9iJFYXcuphJaGXfF3jUQJi5Y3GMNXvM11gaL4txzZgNGK22BFAwMXynnzv4z2Jh/*,[e81a0532/84h/1h/0h]tpubDC6UGqnsQStngYuGD4MKsMy7eD1Yg9NTJfPdvjdG2JE5oZ7EsSL3WHg4Gsw2pR5K39ZwJ46M1wZayhedVdQtMGaUhq5S23PH6fnENK3V1sb/*))#v0t48ucu",
+        self.test_importdesc({"desc":"sh(multi(2,[7b2d0242/84h/1h/0h]" + chg_xpub1 + "/*,[59b09cd6/84h/1h/0h]" + chg_xpub2 + "/*,[e81a0532/84h/1h/0h]" + chg_xpub3 + "/*))#v0t48ucu",
                             "active": True,
                             "internal" : True,
                             "range": 1000,
@@ -369,8 +458,15 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         change_addr = wmulti_pub.getrawchangeaddress() # uses receive 2
         assert_equal(change_addr, '91WxMwg2NHD1PwHChhbAkeCN6nQ8ikdLEx')
         assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 999)
+
+        # generate some utxos for next tests
         txid = w0.sendtoaddress(addr, 10)
         vout = find_vout_for_address(self.nodes[0], txid, addr)
+
+        addr2 = wmulti_pub.getnewaddress('')
+        txid2 = w0.sendtoaddress(addr2, 10)
+        vout2 = find_vout_for_address(self.nodes[0], txid2, addr2)
+
         self.nodes[0].generate(6)
         self.sync_all()
         assert_equal(wmulti_pub.getbalance(), wmulti_priv.getbalance())
@@ -384,14 +480,14 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         wmulti_priv1 = self.nodes[1].get_wallet_rpc("wmulti_priv1")
         res = wmulti_priv1.importdescriptors([
         {
-            "desc": descsum_create("sh(multi(2,tprv8ZgxMBicQKsPevADjDCWsa6DfhkVXicu8NQUzfibwX2MexVwW4tCec5mXdCW8kJwkzBRRmAay1KZya4WsehVvjTGVW6JLqiqd8DdZ4xSg52/84h/0h/0h/*,[59b09cd6/84h/0h/0h]tpubDDBF2BTR6s8drwrfDei8WxtckGuSm1cyoKxYY1QaKSBFbHBYQArWhHPA6eJrzZej6nfHGLSURYSLHr7GuYch8aY5n61tGqgn8b4cXrMuoPH/*,[e81a0532/84h/0h/0h]tpubDCsWoW1kuQB9kG5MXewHqkbjPtqPueRnXju7uM2NK7y3JYb2ajAZ9EiuZXNNuE4661RAfriBWhL8UsnAPpk8zrKKnZw1Ug7X4oHgMdZiU4E/*))"),
+            "desc": descsum_create("sh(multi(2," + xprv1 + "/84h/0h/0h/*,[59b09cd6/84h/0h/0h]" + acc_xpub2 + "/*,[e81a0532/84h/0h/0h]" + acc_xpub3 + "/*))"),
             "active": True,
             "range": 1000,
             "next_index": 0,
             "timestamp": "now"
         },
         {
-            "desc": descsum_create("sh(multi(2,tprv8ZgxMBicQKsPevADjDCWsa6DfhkVXicu8NQUzfibwX2MexVwW4tCec5mXdCW8kJwkzBRRmAay1KZya4WsehVvjTGVW6JLqiqd8DdZ4xSg52/84h/1h/0h/*,[59b09cd6/84h/1h/0h]tpubDCYfZY2ceyHzYzMMVPt9MNeiqtQ2T7Uyp9QSFwYXh8Vi9iJFYXcuphJaGXfF3jUQJi5Y3GMNXvM11gaL4txzZgNGK22BFAwMXynnzv4z2Jh/*,[e81a0532/84h/1h/0h]tpubDC6UGqnsQStngYuGD4MKsMy7eD1Yg9NTJfPdvjdG2JE5oZ7EsSL3WHg4Gsw2pR5K39ZwJ46M1wZayhedVdQtMGaUhq5S23PH6fnENK3V1sb/*))"),
+            "desc": descsum_create("sh(multi(2," + xprv1 + "/84h/1h/0h/*,[59b09cd6/84h/1h/0h]" + chg_xpub2 + "/*,[e81a0532/84h/1h/0h]" + chg_xpub3 + "/*))"),
             "active": True,
             "internal" : True,
             "range": 1000,
@@ -407,14 +503,14 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         wmulti_priv2 = self.nodes[1].get_wallet_rpc('wmulti_priv2')
         res = wmulti_priv2.importdescriptors([
         {
-            "desc": descsum_create("sh(multi(2,[7b2d0242/84h/0h/0h]tpubDCJtdt5dgJpdhW4MtaVYDhG4T4tF6jcLR1PxL43q9pq1mxvXgMS9Mzw1HnXG15vxUGQJMMSqCQHMTy3F1eW5VkgVroWzchsPD5BUojrcWs8/*,tprv8ZgxMBicQKsPdSNWUhDiwTScDr6JfkZuLshTRwzvZGnMSnGikV6jxpmdDkC3YRc4T3GD6Nvg9uv6hQg73RVv1EiTXDZwxVbsLugVHU8B1aq/84h/0h/0h/*,[e81a0532/84h/0h/0h]tpubDCsWoW1kuQB9kG5MXewHqkbjPtqPueRnXju7uM2NK7y3JYb2ajAZ9EiuZXNNuE4661RAfriBWhL8UsnAPpk8zrKKnZw1Ug7X4oHgMdZiU4E/*))"),
+            "desc": descsum_create("sh(multi(2,[7b2d0242/84h/0h/0h]" + acc_xpub1 + "/*," + xprv2 + "/84h/0h/0h/*,[e81a0532/84h/0h/0h]" + acc_xpub3 + "/*))"),
             "active": True,
             "range": 1000,
             "next_index": 0,
             "timestamp": "now"
         },
         {
-            "desc": descsum_create("sh(multi(2,[7b2d0242/84h/1h/0h]tpubDCXqdwWZcszwqYJSnZp8eARkxGJfHAk23KDxbztV4BbschfaTfYLTcSkSJ3TN64dRqwa1rnFUScsYormKkGqNbbPwkorQimVevXjxzUV9Gf/*,tprv8ZgxMBicQKsPdSNWUhDiwTScDr6JfkZuLshTRwzvZGnMSnGikV6jxpmdDkC3YRc4T3GD6Nvg9uv6hQg73RVv1EiTXDZwxVbsLugVHU8B1aq/84h/1h/0h/*,[e81a0532/84h/1h/0h]tpubDC6UGqnsQStngYuGD4MKsMy7eD1Yg9NTJfPdvjdG2JE5oZ7EsSL3WHg4Gsw2pR5K39ZwJ46M1wZayhedVdQtMGaUhq5S23PH6fnENK3V1sb/*))"),
+            "desc": descsum_create("sh(multi(2,[7b2d0242/84h/1h/0h]" + chg_xpub1 + "/*," + xprv2 + "/84h/1h/0h/*,[e81a0532/84h/1h/0h]" + chg_xpub3 + "/*))"),
             "active": True,
             "internal" : True,
             "range": 1000,
@@ -433,6 +529,70 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         assert_equal(tx_signed_2['complete'], True)
         self.nodes[1].sendrawtransaction(tx_signed_2['hex'])
 
+
+        self.log.info("Under P2SH, multisig are standard with up to 15 "
+                      "compressed keys")
+        self.nodes[1].createwallet(wallet_name='multi_priv_big_legacy',
+                                   blank=True, descriptors=True)
+        multi_priv_big = self.nodes[1].get_wallet_rpc('multi_priv_big_legacy')
+        xkey = "tprv8ZgxMBicQKsPeZSeYx7VXDDTs3XrTcmZQpRLbAeSQFCQGgKwR4gKpcxHaKdoTNHniv4EPDJNdzA3KxRrrBHcAgth8fU5X4oCndkkxk39iAt/*"
+        xkey_int = "tprv8ZgxMBicQKsPeZSeYx7VXDDTs3XrTcmZQpRLbAeSQFCQGgKwR4gKpcxHaKdoTNHniv4EPDJNdzA3KxRrrBHcAgth8fU5X4oCndkkxk39iAt/1/*"
+        res = multi_priv_big.importdescriptors([
+        {
+            "desc": descsum_create(f"sh(multi(15,{(xkey + ',') * 14}{xkey}))"),
+            "active": True,
+            "range": 1000,
+            "next_index": 0,
+            "timestamp": "now"
+        },
+        {
+            "desc": descsum_create(f"sh(multi(15,{(xkey_int + ',') * 14}{xkey_int}))"),
+            "active": True,
+            "internal": True,
+            "range": 1000,
+            "next_index": 0,
+            "timestamp": "now"
+        }])
+        assert_equal(res[0]['success'], True)
+        assert_equal(res[1]['success'], True)
+
+        addr = multi_priv_big.getnewaddress("")
+        w0.sendtoaddress(addr, 10)
+        self.nodes[0].generate(6)
+        self.sync_all()
+        # It is standard and would relay.
+        txid = multi_priv_big.sendtoaddress(w0.getnewaddress(), 10, "", "",
+                                            True)
+
+
+        self.log.info("Amending multisig with new private keys")
+        self.nodes[1].createwallet(wallet_name="wmulti_priv3", descriptors=True)
+        wmulti_priv3 = self.nodes[1].get_wallet_rpc("wmulti_priv3")
+        res = wmulti_priv3.importdescriptors([
+            {
+                "desc": descsum_create("sh(multi(2," + xprv1 + "/84h/0h/0h/*,[59b09cd6/84h/0h/0h]" + acc_xpub2 + "/*,[e81a0532/84h/0h/0h]" + acc_xpub3 + "/*))"),
+                "active": True,
+                "range": 1000,
+                "next_index": 0,
+                "timestamp": "now"
+            }])
+        assert_equal(res[0]['success'], True)
+        res = wmulti_priv3.importdescriptors([
+            {
+                "desc": descsum_create("sh(multi(2," + xprv1 + "/84h/0h/0h/*,[59b09cd6/84h/0h/0h]" + acc_xprv2 + "/*,[e81a0532/84h/0h/0h]" + acc_xpub3 + "/*))"),
+                "active": True,
+                "range": 1000,
+                "next_index": 0,
+                "timestamp": "now"
+            }])
+        assert_equal(res[0]['success'], True)
+
+        rawtx = self.nodes[1].createrawtransaction([{'txid': txid2, 'vout': vout2}], {w0.getnewaddress(): 9.999})
+        tx = wmulti_priv3.signrawtransactionwithwallet(rawtx)
+        assert_equal(tx['complete'], True)
+        self.nodes[1].sendrawtransaction(tx['hex'])
+
+
         self.log.info("Combo descriptors cannot be active")
         self.test_importdesc({"desc": descsum_create("combo(tpubDCJtdt5dgJpdhW4MtaVYDhG4T4tF6jcLR1PxL43q9pq1mxvXgMS9Mzw1HnXG15vxUGQJMMSqCQHMTy3F1eW5VkgVroWzchsPD5BUojrcWs8/*)"),
                               "active": True,

test/functional/wallet_send.py

@@ -31,12 +31,15 @@ class WalletSendTest(BitcoinTestFramework):
     def test_send(self, from_wallet, to_wallet=None, amount=None, data=None,
                   arg_conf_target=None, arg_estimate_mode=None,
                   conf_target=None, estimate_mode=None, add_to_wallet=None, psbt=None,
-                  inputs=None, add_inputs=None, change_address=None, change_position=None,
+                  inputs=None, add_inputs=None, include_unsafe=None, change_address=None, change_position=None,
                   include_watching=None, locktime=None, lock_unspents=None, subtract_fee_from_outputs=None,
                   expect_error=None):
         assert (amount is None) != (data is None)
 
-        from_balance_before = from_wallet.getbalance()
+        from_balance_before = from_wallet.getbalances()["mine"]["trusted"]
+        if include_unsafe:
+            from_balance_before += from_wallet.getbalances()["mine"]["untrusted_pending"]
+
         if to_wallet is None:
             assert amount is None
         else:
@@ -67,6 +70,8 @@ class WalletSendTest(BitcoinTestFramework):
             options["inputs"] = inputs
         if add_inputs is not None:
             options["add_inputs"] = add_inputs
+        if include_unsafe is not None:
+            options["include_unsafe"] = include_unsafe
         if change_address is not None:
             options["change_address"] = change_address
         if change_position is not None:
@@ -120,6 +125,10 @@ class WalletSendTest(BitcoinTestFramework):
             assert not "txid" in res
             assert "psbt" in res
 
+        from_balance = from_wallet.getbalances()["mine"]["trusted"]
+        if include_unsafe:
+            from_balance += from_wallet.getbalances()["mine"]["untrusted_pending"]
+
         if add_to_wallet and not include_watching:
             # Ensure transaction exists in the wallet:
             tx = from_wallet.gettransaction(res["txid"])
@@ -129,13 +138,13 @@ class WalletSendTest(BitcoinTestFramework):
             assert tx
             if amount:
                 if subtract_fee_from_outputs:
-                    assert_equal(from_balance_before - from_wallet.getbalance(), amount)
+                    assert_equal(from_balance_before - from_balance, amount)
                 else:
-                    assert_greater_than(from_balance_before - from_wallet.getbalance(), amount)
+                    assert_greater_than(from_balance_before - from_balance, amount)
             else:
                 assert next((out for out in tx["vout"] if out["scriptPubKey"]["asm"] == "OP_RETURN 35"), None)
         else:
-            assert_equal(from_balance_before, from_wallet.getbalance())
+            assert_equal(from_balance_before, from_balance)
 
         if to_wallet:
             self.sync_mempools()
@@ -324,10 +333,10 @@ class WalletSendTest(BitcoinTestFramework):
         assert res["complete"]
         res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=False, change_address=change_address, change_position=0)
         assert res["complete"]
-        assert_equal(self.nodes[0].decodepsbt(res["psbt"])["tx"]["vout"][0]["scriptPubKey"]["addresses"], [change_address])
+        assert_equal(self.nodes[0].decodepsbt(res["psbt"])["tx"]["vout"][0]["scriptPubKey"]["address"], change_address)
         res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, add_to_wallet=False, change_position=0)
         assert res["complete"]
-        change_address = self.nodes[0].decodepsbt(res["psbt"])["tx"]["vout"][0]["scriptPubKey"]["addresses"][0]
+        change_address = self.nodes[0].decodepsbt(res["psbt"])["tx"]["vout"][0]["scriptPubKey"]["address"]
         assert change_address[0] == "y" or change_address[0] == "8" or change_address[0] == "9"
 
         self.log.info("Set lock time...")
@@ -367,6 +376,14 @@ class WalletSendTest(BitcoinTestFramework):
         self.log.info("Subtract fee from output")
         self.test_send(from_wallet=w0, to_wallet=w1, amount=1, subtract_fee_from_outputs=[0])
 
+        self.log.info("Include unsafe inputs")
+        self.nodes[1].createwallet(wallet_name="w5")
+        w5 = self.nodes[1].get_wallet_rpc("w5")
+        self.test_send(from_wallet=w0, to_wallet=w5, amount=2)
+        self.test_send(from_wallet=w5, to_wallet=w0, amount=1, expect_error=(-4, "Insufficient funds"))
+        res = self.test_send(from_wallet=w5, to_wallet=w0, amount=1, include_unsafe=True)
+        assert res["complete"]
+
 
 if __name__ == '__main__':
     WalletSendTest().main()

test/functional/wallet_txn_clone.py

@@ -60,8 +60,8 @@ class TxnMallTest(BitcoinTestFramework):
         # Construct a clone of tx1, to be malleated
         rawtx1 = self.nodes[0].getrawtransaction(txid1, 1)
         clone_inputs = [{"txid": rawtx1["vin"][0]["txid"], "vout": rawtx1["vin"][0]["vout"], "sequence": rawtx1["vin"][0]["sequence"]}]
-        clone_outputs = {rawtx1["vout"][0]["scriptPubKey"]["addresses"][0]: rawtx1["vout"][0]["value"],
+        clone_outputs = {rawtx1["vout"][0]["scriptPubKey"]["address"]: rawtx1["vout"][0]["value"],
-                         rawtx1["vout"][1]["scriptPubKey"]["addresses"][0]: rawtx1["vout"][1]["value"]}
+                         rawtx1["vout"][1]["scriptPubKey"]["address"]: rawtx1["vout"][1]["value"]}
         clone_locktime = rawtx1["locktime"]
         clone_raw = self.nodes[0].createrawtransaction(clone_inputs, clone_outputs, clone_locktime)
 

test/functional/wallet_upgradetohd.py

@@ -58,10 +58,10 @@ class WalletUpgradeToHDTest(BitcoinTestFramework):
             outs = node.decoderawtransaction(node.gettransaction(txid)['hex'])['vout']
             for out in outs:
                 if out['value'] == 1:
-                    keypath = node.getaddressinfo(out['scriptPubKey']['addresses'][0])['hdkeypath']
+                    keypath = node.getaddressinfo(out['scriptPubKey']['address'])['hdkeypath']
                     assert_equal(keypath, "m/44'/1'/0'/0/%d" % i)
                 else:
-                    keypath = node.getaddressinfo(out['scriptPubKey']['addresses'][0])['hdkeypath']
+                    keypath = node.getaddressinfo(out['scriptPubKey']['address'])['hdkeypath']
                     assert_equal(keypath, "m/44'/1'/0'/1/%d" % i)
 
         self.bump_mocktime(1)

test/util/data/tt-delin1-out.json

@@ -194,11 +194,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 8fd139bb39ced713f231c58a4d07bf6954d1c201 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9148fd139bb39ced713f231c58a4d07bf6954d1c20188ac",
-                "reqSigs": 1,
+                "address": "XooH6vpTCuVowS9vmJowNaNGcJQ3cQT7rs",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "XooH6vpTCuVowS9vmJowNaNGcJQ3cQT7rs"
-                ]
             }
         },
         {
@@ -208,11 +205,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 6c772e9cf96371bba3da8cb733da70a2fcf20078 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9146c772e9cf96371bba3da8cb733da70a2fcf2007888ac",
-                "reqSigs": 1,
+                "address": "XkaMatRZjFxq1QbgUvdmqGzGRaPqSLvvS6",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "XkaMatRZjFxq1QbgUvdmqGzGRaPqSLvvS6"
-                ]
             }
         }
     ],

test/util/data/tt-delout1-out.json

@@ -203,11 +203,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 8fd139bb39ced713f231c58a4d07bf6954d1c201 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9148fd139bb39ced713f231c58a4d07bf6954d1c20188ac",
-                "reqSigs": 1,
+                "address": "XooH6vpTCuVowS9vmJowNaNGcJQ3cQT7rs",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "XooH6vpTCuVowS9vmJowNaNGcJQ3cQT7rs"
-                ]
             }
         }
     ],

test/util/data/tt-locktime317000-out.json

@@ -203,11 +203,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 8fd139bb39ced713f231c58a4d07bf6954d1c201 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9148fd139bb39ced713f231c58a4d07bf6954d1c20188ac",
-                "reqSigs": 1,
+                "address": "XooH6vpTCuVowS9vmJowNaNGcJQ3cQT7rs",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "XooH6vpTCuVowS9vmJowNaNGcJQ3cQT7rs"
-                ]
             }
         },
         {
@@ -217,11 +214,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 6c772e9cf96371bba3da8cb733da70a2fcf20078 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9146c772e9cf96371bba3da8cb733da70a2fcf2007888ac",
-                "reqSigs": 1,
+                "address": "XkaMatRZjFxq1QbgUvdmqGzGRaPqSLvvS6",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "XkaMatRZjFxq1QbgUvdmqGzGRaPqSLvvS6"
-                ]
             }
         }
     ],

test/util/data/txcreate1.json

@@ -41,11 +41,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9141fc11f39be1729bf973a7ab6a615ca4729d6457488ac",
-                "reqSigs": 1,
+                "address": "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw"
-                ]
             }
         },
         {
@@ -55,11 +52,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 f2d4db28cad6502226ee484ae24505c2885cb12d OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a914f2d4db28cad6502226ee484ae24505c2885cb12d88ac",
-                "reqSigs": 1,
+                "address": "XxppMBDQ6SiJrKcBrAy4WkkNdrxDBfzFdZ",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "XxppMBDQ6SiJrKcBrAy4WkkNdrxDBfzFdZ"
-                ]
             }
         }
     ],

test/util/data/txcreatedata1.json

@@ -23,11 +23,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9141fc11f39be1729bf973a7ab6a615ca4729d6457488ac",
-                "reqSigs": 1,
+                "address": "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw"
-                ]
             }
         },
         {

test/util/data/txcreatedata2.json

@@ -23,11 +23,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9141fc11f39be1729bf973a7ab6a615ca4729d6457488ac",
-                "reqSigs": 1,
+                "address": "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw"
-                ]
             }
         },
         {

test/util/data/txcreatedata_seq0.json

@@ -23,11 +23,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9141fc11f39be1729bf973a7ab6a615ca4729d6457488ac",
-                "reqSigs": 1,
+                "address": "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw"
-                ]
             }
         }
     ],

test/util/data/txcreatedata_seq1.json

@@ -32,11 +32,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 1fc11f39be1729bf973a7ab6a615ca4729d64574 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9141fc11f39be1729bf973a7ab6a615ca4729d6457488ac",
-                "reqSigs": 1,
+                "address": "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "Xdak8YsJz8tm1iHFmycfTyKeUvHgfbdpyw"
-                ]
             }
         }
     ],

test/util/data/txcreatemultisig1.json

@@ -14,13 +14,7 @@
             "scriptPubKey": {
                 "asm": "2 02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397 021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d 02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485 3 OP_CHECKMULTISIG",
                 "hex": "522102a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff39721021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d2102df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb48553ae",
-                "reqSigs": 2,
+                "type": "multisig"
-                "type": "multisig",
-                "addresses": [
-                    "XqV6rHmzCyFUKF2jSVg8ZkZ7oRhGLVxifK",
-                    "XqDjpPyN61bMZAZsVbYyvouVzgV59oSmWp",
-                    "Xe2kRBG5ZEn8T34TqvvPKwQwHEzVL9WvxH"
-                ]
             }
         }
     ],

test/util/data/txcreatemultisig2.json

@@ -14,11 +14,8 @@
             "scriptPubKey": {
                 "asm": "OP_HASH160 1c6fbaf46d64221e80cbae182c33ddf81b9294ac OP_EQUAL",
                 "hex": "a9141c6fbaf46d64221e80cbae182c33ddf81b9294ac87",
-                "reqSigs": 1,
+                "address": "7V11XGPxzBWxkiuw15a1Vgk7XT74tyYtCY",
-                "type": "scripthash",
+                "type": "scripthash"
-                "addresses": [
-                    "7V11XGPxzBWxkiuw15a1Vgk7XT74tyYtCY"
-                ]
             }
         }
     ],

test/util/data/txcreatescript2.json

@@ -14,11 +14,8 @@
             "scriptPubKey": {
                 "asm": "OP_HASH160 71ed53322d470bb96657deb786b94f97dd46fb15 OP_EQUAL",
                 "hex": "a91471ed53322d470bb96657deb786b94f97dd46fb1587",
-                "reqSigs": 1,
+                "address": "7co3R3WSW8mHKMkFK2FrzQY2fLCBWsg56D",
-                "type": "scripthash",
+                "type": "scripthash"
-                "addresses": [
-                    "7co3R3WSW8mHKMkFK2FrzQY2fLCBWsg56D"
-                ]
             }
         }
     ],

test/util/data/txcreatesignv1.json

@@ -23,11 +23,8 @@
             "scriptPubKey": {
                 "asm": "OP_DUP OP_HASH160 5834479edbbe0539b31ffd3a8f8ebadc2165ed01 OP_EQUALVERIFY OP_CHECKSIG",
                 "hex": "76a9145834479edbbe0539b31ffd3a8f8ebadc2165ed0188ac",
-                "reqSigs": 1,
+                "address": "XijDvbYpPmznwgpWD3DkdYNfGmRP2KoVSk",
-                "type": "pubkeyhash",
+                "type": "pubkeyhash"
-                "addresses": [
-                    "XijDvbYpPmznwgpWD3DkdYNfGmRP2KoVSk"
-                ]
             }
         }
     ],