diff options
30 files changed, 955 insertions, 169 deletions
diff --git a/ci/lint/04_install.sh b/ci/lint/04_install.sh index 47cb8864c2..6b12c53f2a 100755 --- a/ci/lint/04_install.sh +++ b/ci/lint/04_install.sh @@ -46,7 +46,7 @@ if [ ! -d "${LINT_RUNNER_PATH}" ]; then fi ${CI_RETRY_EXE} pip3 install \ - codespell==2.2.5 \ + codespell==2.2.6 \ flake8==6.1.0 \ lief==0.13.2 \ mypy==1.4.1 \ diff --git a/contrib/guix/patches/glibc-2.27-fcommon.patch b/contrib/guix/patches/glibc-2.27-fcommon.patch index 817aa85bb9..f8d14837fc 100644 --- a/contrib/guix/patches/glibc-2.27-fcommon.patch +++ b/contrib/guix/patches/glibc-2.27-fcommon.patch @@ -5,7 +5,7 @@ Date: Fri May 6 11:03:04 2022 +0100 build: use -fcommon to retain legacy behaviour with GCC 10 GCC 10 started using -fno-common by default, which causes issues with - the powerpc builds using gibc 2.27. A patch was commited to glibc to fix + the powerpc builds using gibc 2.27. A patch was committed to glibc to fix the issue, 18363b4f010da9ba459b13310b113ac0647c2fcc but is non-trvial to backport, and was broken in at least one way, see the followup in commit 7650321ce037302bfc2f026aa19e0213b8d02fe6. diff --git a/depends/packages/expat.mk b/depends/packages/expat.mk index 2db283ef3c..2ec660109c 100644 --- a/depends/packages/expat.mk +++ b/depends/packages/expat.mk @@ -6,7 +6,7 @@ $(package)_sha256_hash=f79b8f904b749e3e0d20afeadecf8249c55b2e32d4ebb089ae378df47 # -D_DEFAULT_SOURCE defines __USE_MISC, which exposes additional # definitions in endian.h, which are required for a working -# endianess check in configure when building with -flto. +# endianness check in configure when building with -flto. define $(package)_set_vars $(package)_config_opts=--disable-shared --without-docbook --without-tests --without-examples $(package)_config_opts += --disable-dependency-tracking --enable-option-checking diff --git a/src/cuckoocache.h b/src/cuckoocache.h index cb0b362143..df320ed465 100644 --- a/src/cuckoocache.h +++ b/src/cuckoocache.h @@ -359,7 +359,7 @@ public: * @param bytes the approximate number of bytes to use for this data * structure * @returns A pair of the maximum number of elements storable (see setup() - * documentation for more detail) and the approxmiate total size of these + * documentation for more detail) and the approximate total size of these * elements in bytes or std::nullopt if the size requested is too large. */ std::optional<std::pair<uint32_t, size_t>> setup_bytes(size_t bytes) @@ -223,6 +223,12 @@ struct CExtKey { a.key == b.key; } + CExtKey() = default; + CExtKey(const CExtPubKey& xpub, const CKey& key_in) : nDepth(xpub.nDepth), nChild(xpub.nChild), chaincode(xpub.chaincode), key(key_in) + { + std::copy(xpub.vchFingerprint, xpub.vchFingerprint + sizeof(xpub.vchFingerprint), vchFingerprint); + } + void Encode(unsigned char code[BIP32_EXTKEY_SIZE]) const; void Decode(const unsigned char code[BIP32_EXTKEY_SIZE]); [[nodiscard]] bool Derive(CExtKey& out, unsigned int nChild) const; diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index a2c6990657..84c3352b9d 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -190,9 +190,7 @@ std::optional<std::pair<DiagramCheckError, std::string>> ImprovesFeerateDiagram( CAmount replacement_fees, int64_t replacement_vsize) { - // Require that the replacement strictly improve the mempool's feerate diagram. - std::vector<FeeFrac> old_diagram, new_diagram; - + // Require that the replacement strictly improves the mempool's feerate diagram. const auto diagram_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; if (!diagram_results.has_value()) { diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index eb05f33b42..b8dc148eae 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -277,6 +277,11 @@ static const CRPCConvertParam vRPCConvertParams[] = { "logging", 1, "exclude" }, { "disconnectnode", 1, "nodeid" }, { "upgradewallet", 0, "version" }, + { "gethdkeys", 0, "active_only" }, + { "gethdkeys", 0, "options" }, + { "gethdkeys", 0, "private" }, + { "createwalletdescriptor", 1, "options" }, + { "createwalletdescriptor", 1, "internal" }, // Echo with conversion (For testing only) { "echojson", 0, "arg0" }, { "echojson", 1, "arg1" }, diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index c6bc5f8f1d..a0e755afac 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -212,6 +212,11 @@ public: /** Derive a private key, if private data is available in arg. */ virtual bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const = 0; + + /** Return the non-extended public key for this PubkeyProvider, if it has one. */ + virtual std::optional<CPubKey> GetRootPubKey() const = 0; + /** Return the extended public key for this PubkeyProvider, if it has one. */ + virtual std::optional<CExtPubKey> GetRootExtPubKey() const = 0; }; class OriginPubkeyProvider final : public PubkeyProvider @@ -265,6 +270,14 @@ public: { return m_provider->GetPrivKey(pos, arg, key); } + std::optional<CPubKey> GetRootPubKey() const override + { + return m_provider->GetRootPubKey(); + } + std::optional<CExtPubKey> GetRootExtPubKey() const override + { + return m_provider->GetRootExtPubKey(); + } }; /** An object representing a parsed constant public key in a descriptor. */ @@ -310,6 +323,14 @@ public: { return arg.GetKey(m_pubkey.GetID(), key); } + std::optional<CPubKey> GetRootPubKey() const override + { + return m_pubkey; + } + std::optional<CExtPubKey> GetRootExtPubKey() const override + { + return std::nullopt; + } }; enum class DeriveType { @@ -525,6 +546,14 @@ public: key = extkey.key; return true; } + std::optional<CPubKey> GetRootPubKey() const override + { + return std::nullopt; + } + std::optional<CExtPubKey> GetRootExtPubKey() const override + { + return m_root_extkey; + } }; /** Base class for all Descriptor implementations. */ @@ -720,6 +749,19 @@ public: std::optional<int64_t> MaxSatisfactionWeight(bool) const override { return {}; } std::optional<int64_t> MaxSatisfactionElems() const override { return {}; } + + void GetPubKeys(std::set<CPubKey>& pubkeys, std::set<CExtPubKey>& ext_pubs) const override + { + for (const auto& p : m_pubkey_args) { + std::optional<CPubKey> pub = p->GetRootPubKey(); + if (pub) pubkeys.insert(*pub); + std::optional<CExtPubKey> ext_pub = p->GetRootExtPubKey(); + if (ext_pub) ext_pubs.insert(*ext_pub); + } + for (const auto& arg : m_subdescriptor_args) { + arg->GetPubKeys(pubkeys, ext_pubs); + } + } }; /** A parsed addr(A) descriptor. */ diff --git a/src/script/descriptor.h b/src/script/descriptor.h index caa5d1608d..e78a775330 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -158,6 +158,13 @@ struct Descriptor { /** Get the maximum size number of stack elements for satisfying this descriptor. */ virtual std::optional<int64_t> MaxSatisfactionElems() const = 0; + + /** Return all (extended) public keys for this descriptor, including any from subdescriptors. + * + * @param[out] pubkeys Any public keys + * @param[out] ext_pubs Any extended public keys + */ + virtual void GetPubKeys(std::set<CPubKey>& pubkeys, std::set<CExtPubKey>& ext_pubs) const = 0; }; /** Parse a `descriptor` string. Included private keys are put in `out`. diff --git a/src/script/miniscript.h b/src/script/miniscript.h index 76b952350b..f635fa7340 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -1617,7 +1617,7 @@ public: //! Produce a witness for this script, if possible and given the information available in the context. //! The non-malleable satisfaction is guaranteed to be valid if it exists, and ValidSatisfaction() //! is true. If IsSane() holds, this satisfaction is guaranteed to succeed in case the node's - //! conditions are satisfied (private keys and hash preimages available, locktimes satsified). + //! conditions are satisfied (private keys and hash preimages available, locktimes satisfied). template<typename Ctx> Availability Satisfy(const Ctx& ctx, std::vector<std::vector<unsigned char>>& stack, bool nonmalleable = true) const { auto ret = ProduceInput(ctx); diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 42008c6ad9..754aff4e70 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -127,6 +127,10 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) } mempool_txs.emplace_back(*child); pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back())); + + if (fuzzed_data_provider.ConsumeBool()) { + pool.PrioritiseTransaction(mempool_txs.back().GetHash().ToUint256(), fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(-100000, 100000)); + } } // Pick some transactions at random to be the direct conflicts @@ -174,5 +178,16 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) // If internals report error, wrapper should too auto err_tuple{ImprovesFeerateDiagram(pool, direct_conflicts, all_conflicts, replacement_fees, replacement_vsize)}; - if (!calc_results.has_value()) assert(err_tuple.has_value()); + if (!calc_results.has_value()) { + assert(err_tuple.value().first == DiagramCheckError::UNCALCULABLE); + } else { + // Diagram check succeeded + if (!err_tuple.has_value()) { + // New diagram's final fee should always match or exceed old diagram's + assert(calc_results->first.back().fee <= calc_results->second.back().fee); + } else if (calc_results->first.back().fee > calc_results->second.back().fee) { + // Or it failed, and if old diagram had higher fees, it should be a failure + assert(err_tuple.value().first == DiagramCheckError::FAILURE); + } + } } diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index 961fd62fe4..ed33969710 100644 --- a/src/test/rbf_tests.cpp +++ b/src/test/rbf_tests.cpp @@ -37,6 +37,37 @@ static inline CTransactionRef make_tx(const std::vector<CTransactionRef>& inputs return MakeTransactionRef(tx); } +// Make two child transactions from parent (which must have at least 2 outputs). +// Each tx will have the same outputs, using the amounts specified in output_values. +static inline std::pair<CTransactionRef, CTransactionRef> make_two_siblings(const CTransactionRef parent, + const std::vector<CAmount>& output_values) +{ + assert(parent->vout.size() >= 2); + + // First tx takes first parent output + CMutableTransaction tx1 = CMutableTransaction(); + tx1.vin.resize(1); + tx1.vout.resize(output_values.size()); + + tx1.vin[0].prevout.hash = parent->GetHash(); + tx1.vin[0].prevout.n = 0; + // Add a witness so wtxid != txid + CScriptWitness witness; + witness.stack.emplace_back(10); + tx1.vin[0].scriptWitness = witness; + + for (size_t i = 0; i < output_values.size(); ++i) { + tx1.vout[i].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx1.vout[i].nValue = output_values[i]; + } + + // Second tx takes second parent output + CMutableTransaction tx2 = tx1; + tx2.vin[0].prevout.n = 1; + + return std::make_pair(MakeTransactionRef(tx1), MakeTransactionRef(tx2)); +} + static CTransactionRef add_descendants(const CTransactionRef& tx, int32_t num_descendants, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs) { @@ -67,6 +98,20 @@ static CTransactionRef add_descendant_to_parents(const std::vector<CTransactionR return child_tx; } +// Makes two children for a single parent +static std::pair<CTransactionRef, CTransactionRef> add_children_to_parent(const CTransactionRef parent, CTxMemPool& pool) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs) +{ + AssertLockHeld(::cs_main); + AssertLockHeld(pool.cs); + TestMemPoolEntryHelper entry; + // Assumes this isn't already spent in mempool + auto children_tx = make_two_siblings(/*parent=*/parent, /*output_values=*/{50 * CENT}); + pool.addUnchecked(entry.FromTx(children_tx.first)); + pool.addUnchecked(entry.FromTx(children_tx.second)); + return children_tx; +} + BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup) { CTxMemPool& pool = *Assert(m_node.mempool); @@ -116,6 +161,10 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup) const auto tx12 = make_tx(/*inputs=*/ {m_coinbase_txns[8]}, /*output_values=*/ {995 * CENT}); pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx12)); + // Will make two children of this single parent + const auto tx13 = make_tx(/*inputs=*/ {m_coinbase_txns[9]}, /*output_values=*/ {995 * CENT, 995 * CENT}); + pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx13)); + const auto entry1_normal = pool.GetIter(tx1->GetHash()).value(); const auto entry2_normal = pool.GetIter(tx2->GetHash()).value(); const auto entry3_low = pool.GetIter(tx3->GetHash()).value(); @@ -128,6 +177,7 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup) const auto entry10_unchained = pool.GetIter(tx10->GetHash()).value(); const auto entry11_unchained = pool.GetIter(tx11->GetHash()).value(); const auto entry12_unchained = pool.GetIter(tx12->GetHash()).value(); + const auto entry13_unchained = pool.GetIter(tx13->GetHash()).value(); BOOST_CHECK_EQUAL(entry1_normal->GetFee(), normal_fee); BOOST_CHECK_EQUAL(entry2_normal->GetFee(), normal_fee); @@ -261,7 +311,7 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup) // Tests for CheckConflictTopology // Tx4 has 23 descendants - BOOST_CHECK(pool.CheckConflictTopology(set_34_cpfp).has_value()); + BOOST_CHECK_EQUAL(pool.CheckConflictTopology(set_34_cpfp).value(), strprintf("%s has 23 descendants, max 1 allowed", entry4_high->GetSharedTx()->GetHash().ToString())); // No descendants yet BOOST_CHECK(pool.CheckConflictTopology({entry9_unchained}) == std::nullopt); @@ -292,6 +342,14 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup) BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry11_unchained}).value(), strprintf("%s is not the only parent of child %s", entry11_unchained->GetSharedTx()->GetHash().ToString(), entry_two_parent_child->GetSharedTx()->GetHash().ToString())); BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry12_unchained}).value(), strprintf("%s is not the only parent of child %s", entry12_unchained->GetSharedTx()->GetHash().ToString(), entry_two_parent_child->GetSharedTx()->GetHash().ToString())); BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry_two_parent_child}).value(), strprintf("%s has 2 ancestors, max 1 allowed", entry_two_parent_child->GetSharedTx()->GetHash().ToString())); + + // Single parent with two children, we will conflict with the siblings directly only + const auto two_siblings = add_children_to_parent(tx13, pool); + const auto entry_sibling_1 = pool.GetIter(two_siblings.first->GetHash()).value(); + const auto entry_sibling_2 = pool.GetIter(two_siblings.second->GetHash()).value(); + BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry_sibling_1}).value(), strprintf("%s is not the only child of parent %s", entry_sibling_1->GetSharedTx()->GetHash().ToString(), entry13_unchained->GetSharedTx()->GetHash().ToString())); + BOOST_CHECK_EQUAL(pool.CheckConflictTopology({entry_sibling_2}).value(), strprintf("%s is not the only child of parent %s", entry_sibling_2->GetSharedTx()->GetHash().ToString(), entry13_unchained->GetSharedTx()->GetHash().ToString())); + } BOOST_FIXTURE_TEST_CASE(improves_feerate, TestChain100Setup) @@ -327,6 +385,17 @@ BOOST_FIXTURE_TEST_CASE(improves_feerate, TestChain100Setup) // With one more satoshi it does BOOST_CHECK(ImprovesFeerateDiagram(pool, {entry1}, {entry1, entry2}, tx1_fee + tx2_fee + 1, tx1_size + tx2_size) == std::nullopt); + // With prioritisation of in-mempool conflicts, it affects the results of the comparison using the same args as just above + pool.PrioritiseTransaction(entry1->GetSharedTx()->GetHash(), /*nFeeDelta=*/1); + const auto res2 = ImprovesFeerateDiagram(pool, {entry1}, {entry1, entry2}, tx1_fee + tx2_fee + 1, tx1_size + tx2_size); + BOOST_CHECK(res2.has_value()); + BOOST_CHECK(res2.value().first == DiagramCheckError::FAILURE); + BOOST_CHECK(res2.value().second == "insufficient feerate: does not improve feerate diagram"); + pool.PrioritiseTransaction(entry1->GetSharedTx()->GetHash(), /*nFeeDelta=*/-1); + + // With one less vB it does + BOOST_CHECK(ImprovesFeerateDiagram(pool, {entry1}, {entry1, entry2}, tx1_fee + tx2_fee, tx1_size + tx2_size - 1) == std::nullopt); + // Adding a grandchild makes the cluster size 3, which is uncalculable const auto tx3 = make_tx(/*inputs=*/ {tx2}, /*output_values=*/ {995 * CENT}); pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx3)); @@ -347,38 +416,33 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const CAmount normal_fee{CENT/10}; const CAmount high_fee{CENT}; - // low -> high -> medium fee transactions that would result in two chunks together + // low -> high -> medium fee transactions that would result in two chunks together since they + // are all same size const auto low_tx = make_tx(/*inputs=*/ {m_coinbase_txns[0]}, /*output_values=*/ {10 * COIN}); pool.addUnchecked(entry.Fee(low_fee).FromTx(low_tx)); const auto entry_low = pool.GetIter(low_tx->GetHash()).value(); const auto low_size = entry_low->GetTxSize(); - std::vector<FeeFrac> old_diagram, new_diagram; - // Replacement of size 1 - const auto replace_one{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})}; - BOOST_CHECK(replace_one.has_value()); - old_diagram = replace_one->first; - new_diagram = replace_one->second; - BOOST_CHECK(old_diagram.size() == 2); - BOOST_CHECK(new_diagram.size() == 2); - BOOST_CHECK(old_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(old_diagram[1] == FeeFrac(low_fee, low_size)); - BOOST_CHECK(new_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(new_diagram[1] == FeeFrac(0, 1)); + { + const auto replace_one{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})}; + BOOST_CHECK(replace_one.has_value()); + std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)}; + BOOST_CHECK(replace_one->first == expected_old_diagram); + std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(0, 1)}; + BOOST_CHECK(replace_one->second == expected_new_diagram); + } // Non-zero replacement fee/size - const auto replace_one_fee{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})}; - BOOST_CHECK(replace_one_fee.has_value()); - old_diagram = replace_one_fee->first; - new_diagram = replace_one_fee->second; - BOOST_CHECK(old_diagram.size() == 2); - BOOST_CHECK(new_diagram.size() == 2); - BOOST_CHECK(old_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(old_diagram[1] == FeeFrac(low_fee, low_size)); - BOOST_CHECK(new_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(new_diagram[1] == FeeFrac(high_fee, low_size)); + { + const auto replace_one_fee{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})}; + BOOST_CHECK(replace_one_fee.has_value()); + std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)}; + BOOST_CHECK(replace_one_fee->first == expected_old_diagram); + std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)}; + BOOST_CHECK(replace_one_fee->second == expected_new_diagram); + } // Add a second transaction to the cluster that will make a single chunk, to be evicted in the RBF const auto high_tx = make_tx(/*inputs=*/ {low_tx}, /*output_values=*/ {995 * CENT}); @@ -386,29 +450,24 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto entry_high = pool.GetIter(high_tx->GetHash()).value(); const auto high_size = entry_high->GetTxSize(); - const auto replace_single_chunk{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})}; - BOOST_CHECK(replace_single_chunk.has_value()); - old_diagram = replace_single_chunk->first; - new_diagram = replace_single_chunk->second; - BOOST_CHECK(old_diagram.size() == 2); - BOOST_CHECK(new_diagram.size() == 2); - BOOST_CHECK(old_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(old_diagram[1] == FeeFrac(low_fee + high_fee, low_size + high_size)); - BOOST_CHECK(new_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(new_diagram[1] == FeeFrac(high_fee, low_size)); + { + const auto replace_single_chunk{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})}; + BOOST_CHECK(replace_single_chunk.has_value()); + std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)}; + BOOST_CHECK(replace_single_chunk->first == expected_old_diagram); + std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)}; + BOOST_CHECK(replace_single_chunk->second == expected_new_diagram); + } // Conflict with the 2nd tx, resulting in new diagram with three entries - const auto replace_cpfp_child{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})}; - BOOST_CHECK(replace_cpfp_child.has_value()); - old_diagram = replace_cpfp_child->first; - new_diagram = replace_cpfp_child->second; - BOOST_CHECK(old_diagram.size() == 2); - BOOST_CHECK(new_diagram.size() == 3); - BOOST_CHECK(old_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(old_diagram[1] == FeeFrac(low_fee + high_fee, low_size + high_size)); - BOOST_CHECK(new_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(new_diagram[1] == FeeFrac(high_fee, low_size)); - BOOST_CHECK(new_diagram[2] == FeeFrac(low_fee + high_fee, low_size + low_size)); + { + const auto replace_cpfp_child{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})}; + BOOST_CHECK(replace_cpfp_child.has_value()); + std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)}; + BOOST_CHECK(replace_cpfp_child->first == expected_old_diagram); + std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size), FeeFrac(low_fee + high_fee, low_size + low_size)}; + BOOST_CHECK(replace_cpfp_child->second == expected_new_diagram); + } // third transaction causes the topology check to fail const auto normal_tx = make_tx(/*inputs=*/ {high_tx}, /*output_values=*/ {995 * CENT}); @@ -416,11 +475,11 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto entry_normal = pool.GetIter(normal_tx->GetHash()).value(); const auto normal_size = entry_normal->GetTxSize(); - const auto replace_too_large{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})}; - BOOST_CHECK(!replace_too_large.has_value()); - BOOST_CHECK_EQUAL(util::ErrorString(replace_too_large).original, strprintf("%s has 2 descendants, max 1 allowed", low_tx->GetHash().GetHex())); - old_diagram.clear(); - new_diagram.clear(); + { + const auto replace_too_large{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})}; + BOOST_CHECK(!replace_too_large.has_value()); + BOOST_CHECK_EQUAL(util::ErrorString(replace_too_large).original, strprintf("%s has 2 descendants, max 1 allowed", low_tx->GetHash().GetHex())); + } // Make a size 2 cluster that is itself two chunks; evict both txns const auto high_tx_2 = make_tx(/*inputs=*/ {m_coinbase_txns[1]}, /*output_values=*/ {10 * COIN}); @@ -433,19 +492,16 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto entry_low_2 = pool.GetIter(low_tx_2->GetHash()).value(); const auto low_size_2 = entry_low_2->GetTxSize(); - const auto replace_two_chunks_single_cluster{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})}; - BOOST_CHECK(replace_two_chunks_single_cluster.has_value()); - old_diagram = replace_two_chunks_single_cluster->first; - new_diagram = replace_two_chunks_single_cluster->second; - BOOST_CHECK(old_diagram.size() == 3); - BOOST_CHECK(new_diagram.size() == 2); - BOOST_CHECK(old_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(old_diagram[1] == FeeFrac(high_fee, high_size_2)); - BOOST_CHECK(old_diagram[2] == FeeFrac(low_fee + high_fee, low_size_2 + high_size_2)); - BOOST_CHECK(new_diagram[0] == FeeFrac(0, 0)); - BOOST_CHECK(new_diagram[1] == FeeFrac(high_fee, low_size_2)); - - // You can have more than two direct conflicts if the there are multiple effected clusters, all of size 2 or less + { + const auto replace_two_chunks_single_cluster{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})}; + BOOST_CHECK(replace_two_chunks_single_cluster.has_value()); + std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(high_fee, high_size_2), FeeFrac(low_fee + high_fee, low_size_2 + high_size_2)}; + BOOST_CHECK(replace_two_chunks_single_cluster->first == expected_old_diagram); + std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size_2)}; + BOOST_CHECK(replace_two_chunks_single_cluster->second == expected_new_diagram); + } + + // You can have more than two direct conflicts if the there are multiple affected clusters, all of size 2 or less const auto conflict_1 = make_tx(/*inputs=*/ {m_coinbase_txns[2]}, /*output_values=*/ {10 * COIN}); pool.addUnchecked(entry.Fee(low_fee).FromTx(conflict_1)); const auto conflict_1_entry = pool.GetIter(conflict_1->GetHash()).value(); @@ -458,40 +514,37 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) pool.addUnchecked(entry.Fee(low_fee).FromTx(conflict_3)); const auto conflict_3_entry = pool.GetIter(conflict_3->GetHash()).value(); - const auto replace_multiple_clusters{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})}; - - BOOST_CHECK(replace_multiple_clusters.has_value()); - old_diagram = replace_multiple_clusters->first; - new_diagram = replace_multiple_clusters->second; - BOOST_CHECK(old_diagram.size() == 4); - BOOST_CHECK(new_diagram.size() == 2); + { + const auto replace_multiple_clusters{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})}; + BOOST_CHECK(replace_multiple_clusters.has_value()); + BOOST_CHECK(replace_multiple_clusters->first.size() == 4); + BOOST_CHECK(replace_multiple_clusters->second.size() == 2); + } - // Add a child transaction to conflict_1 and make it cluster size 2, still one chunk due to same feerate + // Add a child transaction to conflict_1 and make it cluster size 2, two chunks due to same feerate const auto conflict_1_child = make_tx(/*inputs=*/{conflict_1}, /*output_values=*/ {995 * CENT}); pool.addUnchecked(entry.Fee(low_fee).FromTx(conflict_1_child)); const auto conflict_1_child_entry = pool.GetIter(conflict_1_child->GetHash()).value(); - const auto replace_multiple_clusters_2{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})}; + { + const auto replace_multiple_clusters_2{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})}; - BOOST_CHECK(replace_multiple_clusters_2.has_value()); - old_diagram = replace_multiple_clusters_2->first; - new_diagram = replace_multiple_clusters_2->second; - BOOST_CHECK(old_diagram.size() == 4); - BOOST_CHECK(new_diagram.size() == 2); - old_diagram.clear(); - new_diagram.clear(); + BOOST_CHECK(replace_multiple_clusters_2.has_value()); + BOOST_CHECK(replace_multiple_clusters_2->first.size() == 5); + BOOST_CHECK(replace_multiple_clusters_2->second.size() == 2); + } // Add another descendant to conflict_1, making the cluster size > 2 should fail at this point. const auto conflict_1_grand_child = make_tx(/*inputs=*/{conflict_1_child}, /*output_values=*/ {995 * CENT}); pool.addUnchecked(entry.Fee(high_fee).FromTx(conflict_1_grand_child)); const auto conflict_1_grand_child_entry = pool.GetIter(conflict_1_child->GetHash()).value(); - const auto replace_cluster_size_3{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})}; + { + const auto replace_cluster_size_3{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})}; - BOOST_CHECK(!replace_cluster_size_3.has_value()); - BOOST_CHECK_EQUAL(util::ErrorString(replace_cluster_size_3).original, strprintf("%s has 2 descendants, max 1 allowed", conflict_1->GetHash().GetHex())); - BOOST_CHECK(old_diagram.empty()); - BOOST_CHECK(new_diagram.empty()); + BOOST_CHECK(!replace_cluster_size_3.has_value()); + BOOST_CHECK_EQUAL(util::ErrorString(replace_cluster_size_3).original, strprintf("%s has 2 descendants, max 1 allowed", conflict_1->GetHash().GetHex())); + } } BOOST_AUTO_TEST_CASE(feerate_diagram_utilities) @@ -503,18 +556,21 @@ BOOST_AUTO_TEST_CASE(feerate_diagram_utilities) std::vector<FeeFrac> new_diagram{{FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1050, 400}}}; BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); + BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); // Incomparable diagrams old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; new_diagram = {FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1000, 400}}; BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); + BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); // Strictly better but smaller size. old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 300}}; BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); + BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); // New diagram is strictly better due to the first chunk, even though // second chunk contributes no fees @@ -522,24 +578,28 @@ BOOST_AUTO_TEST_CASE(feerate_diagram_utilities) new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 100}, FeeFrac{1100, 200}}; BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); + BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); // Feerate of first new chunk is better with, but second chunk is worse old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{999, 350}, FeeFrac{1150, 1000}}; BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); + BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); // If we make the second chunk slightly better, the new diagram now wins. old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{1000, 350}, FeeFrac{1150, 500}}; BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); + BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); // Identical diagrams, cannot be strictly better old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; BOOST_CHECK(std::is_eq(CompareFeerateDiagram(old_diagram, new_diagram))); + BOOST_CHECK(std::is_eq(CompareFeerateDiagram(new_diagram, old_diagram))); // Same aggregate fee, but different total size (trigger single tail fee check step) old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}}; @@ -547,8 +607,6 @@ BOOST_AUTO_TEST_CASE(feerate_diagram_utilities) // No change in evaluation when tail check needed. BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram))); - - // Padding works on either argument BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram))); // Trigger multiple tail fee check steps @@ -561,6 +619,7 @@ BOOST_AUTO_TEST_CASE(feerate_diagram_utilities) // Multiple tail fee check steps, unordered result new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}, FeeFrac{1051, 403}}; BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); + BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4047ceda3c..82eec6241f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1294,9 +1294,10 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool:: // direct_conflicts that is at its own fee/size, along with the replacement // tx/package at its own fee/size - // old diagram will consist of each element of all_conflicts either at - // its own feerate (followed by any descendant at its own feerate) or as a - // single chunk at its descendant's ancestor feerate. + // old diagram will consist of the ancestors and descendants of each element of + // all_conflicts. every such transaction will either be at its own feerate (followed + // by any descendant at its own feerate), or as a single chunk at the descendant's + // ancestor feerate. std::vector<FeeFrac> old_chunks; // Step 1: build the old diagram. @@ -1317,7 +1318,7 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool:: // We'll add chunks for either the ancestor by itself and this tx // by itself, or for a combined package. FeeFrac package{txiter->GetModFeesWithAncestors(), static_cast<int32_t>(txiter->GetSizeWithAncestors())}; - if (individual > package) { + if (individual >> package) { // The individual feerate is higher than the package, and // therefore higher than the parent's fee. Chunk these // together. diff --git a/src/util/feefrac.cpp b/src/util/feefrac.cpp index a271fe585e..68fb836936 100644 --- a/src/util/feefrac.cpp +++ b/src/util/feefrac.cpp @@ -53,7 +53,6 @@ std::partial_ordering CompareFeerateDiagram(Span<const FeeFrac> dia0, Span<const const FeeFrac& point_p = next_point(unproc_side); const FeeFrac& point_a = prev_point(!unproc_side); - // Slope of AP can be negative, unlike AB const auto slope_ap = point_p - point_a; Assume(slope_ap.size > 0); std::weak_ordering cmp = std::weak_ordering::equivalent; @@ -77,10 +76,12 @@ std::partial_ordering CompareFeerateDiagram(Span<const FeeFrac> dia0, Span<const if (std::is_gt(cmp)) better_somewhere[unproc_side] = true; if (std::is_lt(cmp)) better_somewhere[!unproc_side] = true; ++next_index[unproc_side]; + + // If both diagrams are better somewhere, they are incomparable. + if (better_somewhere[0] && better_somewhere[1]) return std::partial_ordering::unordered; + } while(true); - // If both diagrams are better somewhere, they are incomparable. - if (better_somewhere[0] && better_somewhere[1]) return std::partial_ordering::unordered; // Otherwise compare the better_somewhere values. return better_somewhere[0] <=> better_somewhere[1]; } diff --git a/src/util/feefrac.h b/src/util/feefrac.h index 48bd287c7c..7102f85f88 100644 --- a/src/util/feefrac.h +++ b/src/util/feefrac.h @@ -31,7 +31,7 @@ * A FeeFrac is considered "better" if it sorts after another, by this ordering. All standard * comparison operators (<=>, ==, !=, >, <, >=, <=) respect this ordering. * - * The CompareFeeFrac, and >> and << operators only compare feerate and treat equal feerate but + * The FeeRateCompare, and >> and << operators only compare feerate and treat equal feerate but * different size as equivalent. The empty FeeFrac is neither lower or higher in feerate than any * other. */ diff --git a/src/validation.cpp b/src/validation.cpp index 5c585438d1..8a78f2106d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5784,7 +5784,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( CBlockIndex* index = nullptr; // Don't make any modifications to the genesis block since it shouldn't be - // neccessary, and since the genesis block doesn't have normal flags like + // necessary, and since the genesis block doesn't have normal flags like // BLOCK_VALID_SCRIPTS set. constexpr int AFTER_GENESIS_START{1}; diff --git a/src/validation.h b/src/validation.h index de81058033..36e28b9745 100644 --- a/src/validation.h +++ b/src/validation.h @@ -275,7 +275,7 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra * Validate (and maybe submit) a package to the mempool. See doc/policy/packages.md for full details * on package validation rules. * @param[in] test_accept When true, run validation checks but don't submit to mempool. -* @param[in] client_maxfeerate If exceeded by an individual transaction, rest of (sub)package evalution is aborted. +* @param[in] client_maxfeerate If exceeded by an individual transaction, rest of (sub)package evaluation is aborted. * Only for sanity checks against local submission of transactions. * @returns a PackageMempoolAcceptResult which includes a MempoolAcceptResult for each transaction. * If a transaction fails, validation will exit early and some results may be missing. It is also diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 6a8ce954fb..a684d4e191 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -817,6 +817,217 @@ static RPCHelpMan migratewallet() }; } +RPCHelpMan gethdkeys() +{ + return RPCHelpMan{ + "gethdkeys", + "\nList all BIP 32 HD keys in the wallet and which descriptors use them.\n", + { + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { + {"active_only", RPCArg::Type::BOOL, RPCArg::Default{false}, "Show the keys for only active descriptors"}, + {"private", RPCArg::Type::BOOL, RPCArg::Default{false}, "Show private keys"} + }}, + }, + RPCResult{RPCResult::Type::ARR, "", "", { + { + {RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR, "xpub", "The extended public key"}, + {RPCResult::Type::BOOL, "has_private", "Whether the wallet has the private key for this xpub"}, + {RPCResult::Type::STR, "xprv", /*optional=*/true, "The extended private key if \"private\" is true"}, + {RPCResult::Type::ARR, "descriptors", "Array of descriptor objects that use this HD key", + { + {RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR, "desc", "Descriptor string representation"}, + {RPCResult::Type::BOOL, "active", "Whether this descriptor is currently used to generate new addresses"}, + }}, + }}, + }}, + } + }}, + RPCExamples{ + HelpExampleCli("gethdkeys", "") + HelpExampleRpc("gethdkeys", "") + + HelpExampleCliNamed("gethdkeys", {{"active_only", "true"}, {"private", "true"}}) + HelpExampleRpcNamed("gethdkeys", {{"active_only", "true"}, {"private", "true"}}) + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + const std::shared_ptr<const CWallet> wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return UniValue::VNULL; + + if (!wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "gethdkeys is not available for non-descriptor wallets"); + } + + LOCK(wallet->cs_wallet); + + UniValue options{request.params[0].isNull() ? UniValue::VOBJ : request.params[0]}; + const bool active_only{options.exists("active_only") ? options["active_only"].get_bool() : false}; + const bool priv{options.exists("private") ? options["private"].get_bool() : false}; + if (priv) { + EnsureWalletIsUnlocked(*wallet); + } + + + std::set<ScriptPubKeyMan*> spkms; + if (active_only) { + spkms = wallet->GetActiveScriptPubKeyMans(); + } else { + spkms = wallet->GetAllScriptPubKeyMans(); + } + + std::map<CExtPubKey, std::set<std::tuple<std::string, bool, bool>>> wallet_xpubs; + std::map<CExtPubKey, CExtKey> wallet_xprvs; + for (auto* spkm : spkms) { + auto* desc_spkm{dynamic_cast<DescriptorScriptPubKeyMan*>(spkm)}; + CHECK_NONFATAL(desc_spkm); + LOCK(desc_spkm->cs_desc_man); + WalletDescriptor w_desc = desc_spkm->GetWalletDescriptor(); + + // Retrieve the pubkeys from the descriptor + std::set<CPubKey> desc_pubkeys; + std::set<CExtPubKey> desc_xpubs; + w_desc.descriptor->GetPubKeys(desc_pubkeys, desc_xpubs); + for (const CExtPubKey& xpub : desc_xpubs) { + std::string desc_str; + bool ok = desc_spkm->GetDescriptorString(desc_str, false); + CHECK_NONFATAL(ok); + wallet_xpubs[xpub].emplace(desc_str, wallet->IsActiveScriptPubKeyMan(*spkm), desc_spkm->HasPrivKey(xpub.pubkey.GetID())); + if (std::optional<CKey> key = priv ? desc_spkm->GetKey(xpub.pubkey.GetID()) : std::nullopt) { + wallet_xprvs[xpub] = CExtKey(xpub, *key); + } + } + } + + UniValue response(UniValue::VARR); + for (const auto& [xpub, descs] : wallet_xpubs) { + bool has_xprv = false; + UniValue descriptors(UniValue::VARR); + for (const auto& [desc, active, has_priv] : descs) { + UniValue d(UniValue::VOBJ); + d.pushKV("desc", desc); + d.pushKV("active", active); + has_xprv |= has_priv; + + descriptors.push_back(std::move(d)); + } + UniValue xpub_info(UniValue::VOBJ); + xpub_info.pushKV("xpub", EncodeExtPubKey(xpub)); + xpub_info.pushKV("has_private", has_xprv); + if (priv) { + xpub_info.pushKV("xprv", EncodeExtKey(wallet_xprvs.at(xpub))); + } + xpub_info.pushKV("descriptors", std::move(descriptors)); + + response.push_back(std::move(xpub_info)); + } + + return response; + }, + }; +} + +static RPCHelpMan createwalletdescriptor() +{ + return RPCHelpMan{"createwalletdescriptor", + "Creates the wallet's descriptor for the given address type. " + "The address type must be one that the wallet does not already have a descriptor for." + + HELP_REQUIRING_PASSPHRASE, + { + {"type", RPCArg::Type::STR, RPCArg::Optional::NO, "The address type the descriptor will produce. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."}, + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { + {"internal", RPCArg::Type::BOOL, RPCArg::DefaultHint{"Both external and internal will be generated unless this parameter is specified"}, "Whether to only make one descriptor that is internal (if parameter is true) or external (if parameter is false)"}, + {"hdkey", RPCArg::Type::STR, RPCArg::DefaultHint{"The HD key used by all other active descriptors"}, "The HD key that the wallet knows the private key of, listed using 'gethdkeys', to use for this descriptor's key"}, + }}, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::ARR, "descs", "The public descriptors that were added to the wallet", + {{RPCResult::Type::STR, "", ""}} + } + }, + }, + RPCExamples{ + HelpExampleCli("createwalletdescriptor", "bech32m") + + HelpExampleRpc("createwalletdescriptor", "bech32m") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return UniValue::VNULL; + + // Make sure wallet is a descriptor wallet + if (!pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "createwalletdescriptor is not available for non-descriptor wallets"); + } + + std::optional<OutputType> output_type = ParseOutputType(request.params[0].get_str()); + if (!output_type) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str())); + } + + UniValue options{request.params[1].isNull() ? UniValue::VOBJ : request.params[1]}; + UniValue internal_only{options["internal"]}; + UniValue hdkey{options["hdkey"]}; + + std::vector<bool> internals; + if (internal_only.isNull()) { + internals.push_back(false); + internals.push_back(true); + } else { + internals.push_back(internal_only.get_bool()); + } + + LOCK(pwallet->cs_wallet); + EnsureWalletIsUnlocked(*pwallet); + + CExtPubKey xpub; + if (hdkey.isNull()) { + std::set<CExtPubKey> active_xpubs = pwallet->GetActiveHDPubKeys(); + if (active_xpubs.size() != 1) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Unable to determine which HD key to use from active descriptors. Please specify with 'hdkey'"); + } + xpub = *active_xpubs.begin(); + } else { + xpub = DecodeExtPubKey(hdkey.get_str()); + if (!xpub.pubkey.IsValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Unable to parse HD key. Please provide a valid xpub"); + } + } + + std::optional<CKey> key = pwallet->GetKey(xpub.pubkey.GetID()); + if (!key) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Private key for %s is not known", EncodeExtPubKey(xpub))); + } + CExtKey active_hdkey(xpub, *key); + + std::vector<std::reference_wrapper<DescriptorScriptPubKeyMan>> spkms; + WalletBatch batch{pwallet->GetDatabase()}; + for (bool internal : internals) { + WalletDescriptor w_desc = GenerateWalletDescriptor(xpub, *output_type, internal); + uint256 w_id = DescriptorID(*w_desc.descriptor); + if (!pwallet->GetScriptPubKeyMan(w_id)) { + spkms.emplace_back(pwallet->SetupDescriptorScriptPubKeyMan(batch, active_hdkey, *output_type, internal)); + } + } + if (spkms.empty()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Descriptor already exists"); + } + + // Fetch each descspkm from the wallet in order to get the descriptor strings + UniValue descs{UniValue::VARR}; + for (const auto& spkm : spkms) { + std::string desc_str; + bool ok = spkm.get().GetDescriptorString(desc_str, false); + CHECK_NONFATAL(ok); + descs.push_back(desc_str); + } + UniValue out{UniValue::VOBJ}; + out.pushKV("descs", std::move(descs)); + return out; + } + }; +} + // addresses RPCHelpMan getaddressinfo(); RPCHelpMan getnewaddress(); @@ -900,6 +1111,7 @@ Span<const CRPCCommand> GetWalletRPCCommands() {"wallet", &bumpfee}, {"wallet", &psbtbumpfee}, {"wallet", &createwallet}, + {"wallet", &createwalletdescriptor}, {"wallet", &restorewallet}, {"wallet", &dumpprivkey}, {"wallet", &dumpwallet}, @@ -907,6 +1119,7 @@ Span<const CRPCCommand> GetWalletRPCCommands() {"wallet", &getaddressesbylabel}, {"wallet", &getaddressinfo}, {"wallet", &getbalance}, + {"wallet", &gethdkeys}, {"wallet", &getnewaddress}, {"wallet", &getrawchangeaddress}, {"wallet", &getreceivedbyaddress}, diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index e10a17f003..59171f6db7 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -11,6 +11,7 @@ #include <script/sign.h> #include <script/solver.h> #include <util/bip32.h> +#include <util/check.h> #include <util/strencodings.h> #include <util/string.h> #include <util/time.h> @@ -2143,6 +2144,36 @@ std::map<CKeyID, CKey> DescriptorScriptPubKeyMan::GetKeys() const return m_map_keys; } +bool DescriptorScriptPubKeyMan::HasPrivKey(const CKeyID& keyid) const +{ + AssertLockHeld(cs_desc_man); + return m_map_keys.contains(keyid) || m_map_crypted_keys.contains(keyid); +} + +std::optional<CKey> DescriptorScriptPubKeyMan::GetKey(const CKeyID& keyid) const +{ + AssertLockHeld(cs_desc_man); + if (m_storage.HasEncryptionKeys() && !m_storage.IsLocked()) { + const auto& it = m_map_crypted_keys.find(keyid); + if (it == m_map_crypted_keys.end()) { + return std::nullopt; + } + const std::vector<unsigned char>& crypted_secret = it->second.second; + CKey key; + if (!Assume(m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptKey(encryption_key, crypted_secret, it->second.first, key); + }))) { + return std::nullopt; + } + return key; + } + const auto& it = m_map_keys.find(keyid); + if (it == m_map_keys.end()) { + return std::nullopt; + } + return it->second; +} + bool DescriptorScriptPubKeyMan::TopUp(unsigned int size) { WalletBatch batch(m_storage.GetDatabase()); @@ -2296,55 +2327,7 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(WalletBatch& batch, co return false; } - int64_t creation_time = GetTime(); - - std::string xpub = EncodeExtPubKey(master_key.Neuter()); - - // Build descriptor string - std::string desc_prefix; - std::string desc_suffix = "/*)"; - switch (addr_type) { - case OutputType::LEGACY: { - desc_prefix = "pkh(" + xpub + "/44h"; - break; - } - case OutputType::P2SH_SEGWIT: { - desc_prefix = "sh(wpkh(" + xpub + "/49h"; - desc_suffix += ")"; - break; - } - case OutputType::BECH32: { - desc_prefix = "wpkh(" + xpub + "/84h"; - break; - } - case OutputType::BECH32M: { - desc_prefix = "tr(" + xpub + "/86h"; - break; - } - case OutputType::UNKNOWN: { - // We should never have a DescriptorScriptPubKeyMan for an UNKNOWN OutputType, - // so if we get to this point something is wrong - assert(false); - } - } // no default case, so the compiler can warn about missing cases - assert(!desc_prefix.empty()); - - // Mainnet derives at 0', testnet and regtest derive at 1' - if (Params().IsTestChain()) { - desc_prefix += "/1h"; - } else { - desc_prefix += "/0h"; - } - - std::string internal_path = internal ? "/1" : "/0"; - std::string desc_str = desc_prefix + "/0h" + internal_path + desc_suffix; - - // Make the descriptor - FlatSigningProvider keys; - std::string error; - std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, error, false); - WalletDescriptor w_desc(std::move(desc), creation_time, 0, 0, 0); - m_wallet_descriptor = w_desc; + m_wallet_descriptor = GenerateWalletDescriptor(master_key.Neuter(), addr_type, internal); // Store the master private key, and descriptor if (!AddDescriptorKeyWithDB(batch, master_key.key, master_key.key.GetPubKey())) { diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 2d83ae556f..4575881d96 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -633,6 +633,9 @@ public: bool SetupDescriptorGeneration(WalletBatch& batch, const CExtKey& master_key, OutputType addr_type, bool internal); bool HavePrivateKeys() const override; + bool HasPrivKey(const CKeyID& keyid) const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); + //! Retrieve the particular key if it is available. Returns nullopt if the key is not in the wallet, or if the wallet is locked. + std::optional<CKey> GetKey(const CKeyID& keyid) const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); std::optional<int64_t> GetOldestKeyPoolTime() const override; unsigned int GetKeyPoolSize() const override; @@ -669,7 +672,7 @@ public: std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys(int32_t minimum_index) const; int32_t GetEndRange() const; - bool GetDescriptorString(std::string& out, const bool priv) const; + [[nodiscard]] bool GetDescriptorString(std::string& out, const bool priv) const; void UpgradeDescriptorCache(); }; diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index 3dba2231f0..2e43eda582 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -34,6 +34,7 @@ public: std::optional<int64_t> ScriptSize() const override { return {}; } std::optional<int64_t> MaxSatisfactionWeight(bool) const override { return {}; } std::optional<int64_t> MaxSatisfactionElems() const override { return {}; } + void GetPubKeys(std::set<CPubKey>& pubkeys, std::set<CExtPubKey>& ext_pubs) const override {} }; BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 591c5eca6e..96c4397504 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3497,6 +3497,17 @@ std::set<ScriptPubKeyMan*> CWallet::GetActiveScriptPubKeyMans() const return spk_mans; } +bool CWallet::IsActiveScriptPubKeyMan(const ScriptPubKeyMan& spkm) const +{ + for (const auto& [_, ext_spkm] : m_external_spk_managers) { + if (ext_spkm == &spkm) return true; + } + for (const auto& [_, int_spkm] : m_internal_spk_managers) { + if (int_spkm == &spkm) return true; + } + return false; +} + std::set<ScriptPubKeyMan*> CWallet::GetAllScriptPubKeyMans() const { std::set<ScriptPubKeyMan*> spk_mans; @@ -3651,6 +3662,26 @@ DescriptorScriptPubKeyMan& CWallet::LoadDescriptorScriptPubKeyMan(uint256 id, Wa return *spk_manager; } +DescriptorScriptPubKeyMan& CWallet::SetupDescriptorScriptPubKeyMan(WalletBatch& batch, const CExtKey& master_key, const OutputType& output_type, bool internal) +{ + AssertLockHeld(cs_wallet); + auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, m_keypool_size)); + if (IsCrypted()) { + if (IsLocked()) { + throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors"); + } + if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, &batch)) { + throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors"); + } + } + spk_manager->SetupDescriptorGeneration(batch, master_key, output_type, internal); + DescriptorScriptPubKeyMan* out = spk_manager.get(); + uint256 id = spk_manager->GetID(); + AddScriptPubKeyMan(id, std::move(spk_manager)); + AddActiveScriptPubKeyManWithDb(batch, id, output_type, internal); + return *out; +} + void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) { AssertLockHeld(cs_wallet); @@ -3661,19 +3692,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) for (bool internal : {false, true}) { for (OutputType t : OUTPUT_TYPES) { - auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, m_keypool_size)); - if (IsCrypted()) { - if (IsLocked()) { - throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors"); - } - if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, &batch)) { - throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors"); - } - } - spk_manager->SetupDescriptorGeneration(batch, master_key, t, internal); - uint256 id = spk_manager->GetID(); - AddScriptPubKeyMan(id, std::move(spk_manager)); - AddActiveScriptPubKeyManWithDb(batch, id, t, internal); + SetupDescriptorScriptPubKeyMan(batch, master_key, t, internal); } } @@ -4501,4 +4520,40 @@ void CWallet::TopUpCallback(const std::set<CScript>& spks, ScriptPubKeyMan* spkm // Update scriptPubKey cache CacheNewScriptPubKeys(spks, spkm); } + +std::set<CExtPubKey> CWallet::GetActiveHDPubKeys() const +{ + AssertLockHeld(cs_wallet); + + Assert(IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); + + std::set<CExtPubKey> active_xpubs; + for (const auto& spkm : GetActiveScriptPubKeyMans()) { + const DescriptorScriptPubKeyMan* desc_spkm = dynamic_cast<DescriptorScriptPubKeyMan*>(spkm); + assert(desc_spkm); + LOCK(desc_spkm->cs_desc_man); + WalletDescriptor w_desc = desc_spkm->GetWalletDescriptor(); + + std::set<CPubKey> desc_pubkeys; + std::set<CExtPubKey> desc_xpubs; + w_desc.descriptor->GetPubKeys(desc_pubkeys, desc_xpubs); + active_xpubs.merge(std::move(desc_xpubs)); + } + return active_xpubs; +} + +std::optional<CKey> CWallet::GetKey(const CKeyID& keyid) const +{ + Assert(IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); + + for (const auto& spkm : GetAllScriptPubKeyMans()) { + const DescriptorScriptPubKeyMan* desc_spkm = dynamic_cast<DescriptorScriptPubKeyMan*>(spkm); + assert(desc_spkm); + LOCK(desc_spkm->cs_desc_man); + if (std::optional<CKey> key = desc_spkm->GetKey(keyid)) { + return key; + } + } + return std::nullopt; +} } // namespace wallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index fdfe0a8b65..b49b5a7d0d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -938,6 +938,7 @@ public: //! Returns all unique ScriptPubKeyMans in m_internal_spk_managers and m_external_spk_managers std::set<ScriptPubKeyMan*> GetActiveScriptPubKeyMans() const; + bool IsActiveScriptPubKeyMan(const ScriptPubKeyMan& spkm) const; //! Returns all unique ScriptPubKeyMans std::set<ScriptPubKeyMan*> GetAllScriptPubKeyMans() const; @@ -1013,6 +1014,8 @@ public: //! @param[in] internal Whether this ScriptPubKeyMan provides change addresses void DeactivateScriptPubKeyMan(uint256 id, OutputType type, bool internal); + //! Create new DescriptorScriptPubKeyMan and add it to the wallet + DescriptorScriptPubKeyMan& SetupDescriptorScriptPubKeyMan(WalletBatch& batch, const CExtKey& master_key, const OutputType& output_type, bool internal) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Create new DescriptorScriptPubKeyMans and add them to the wallet void SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetupDescriptorScriptPubKeyMans() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -1049,6 +1052,13 @@ public: void CacheNewScriptPubKeys(const std::set<CScript>& spks, ScriptPubKeyMan* spkm); void TopUpCallback(const std::set<CScript>& spks, ScriptPubKeyMan* spkm) override; + + //! Retrieve the xpubs in use by the active descriptors + std::set<CExtPubKey> GetActiveHDPubKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + + //! Find the private key for the given key id from the wallet's descriptors, if available + //! Returns nullopt when no descriptor has the key or if the wallet is locked. + std::optional<CKey> GetKey(const CKeyID& keyid) const; }; /** diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index fdd5bc36d8..0de2617d45 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -4,7 +4,9 @@ #include <wallet/walletutil.h> +#include <chainparams.h> #include <common/args.h> +#include <key_io.h> #include <logging.h> namespace wallet { @@ -43,4 +45,58 @@ WalletFeature GetClosestWalletFeature(int version) } return static_cast<WalletFeature>(0); } + +WalletDescriptor GenerateWalletDescriptor(const CExtPubKey& master_key, const OutputType& addr_type, bool internal) +{ + int64_t creation_time = GetTime(); + + std::string xpub = EncodeExtPubKey(master_key); + + // Build descriptor string + std::string desc_prefix; + std::string desc_suffix = "/*)"; + switch (addr_type) { + case OutputType::LEGACY: { + desc_prefix = "pkh(" + xpub + "/44h"; + break; + } + case OutputType::P2SH_SEGWIT: { + desc_prefix = "sh(wpkh(" + xpub + "/49h"; + desc_suffix += ")"; + break; + } + case OutputType::BECH32: { + desc_prefix = "wpkh(" + xpub + "/84h"; + break; + } + case OutputType::BECH32M: { + desc_prefix = "tr(" + xpub + "/86h"; + break; + } + case OutputType::UNKNOWN: { + // We should never have a DescriptorScriptPubKeyMan for an UNKNOWN OutputType, + // so if we get to this point something is wrong + assert(false); + } + } // no default case, so the compiler can warn about missing cases + assert(!desc_prefix.empty()); + + // Mainnet derives at 0', testnet and regtest derive at 1' + if (Params().IsTestChain()) { + desc_prefix += "/1h"; + } else { + desc_prefix += "/0h"; + } + + std::string internal_path = internal ? "/1" : "/0"; + std::string desc_str = desc_prefix + "/0h" + internal_path + desc_suffix; + + // Make the descriptor + FlatSigningProvider keys; + std::string error; + std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, error, false); + WalletDescriptor w_desc(std::move(desc), creation_time, 0, 0, 0); + return w_desc; +} + } // namespace wallet diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h index 7ad3ffe9e4..38926c1eb8 100644 --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -114,6 +114,8 @@ public: WalletDescriptor() {} WalletDescriptor(std::shared_ptr<Descriptor> descriptor, uint64_t creation_time, int32_t range_start, int32_t range_end, int32_t next_index) : descriptor(descriptor), id(DescriptorID(*descriptor)), creation_time(creation_time), range_start(range_start), range_end(range_end), next_index(next_index) { } }; + +WalletDescriptor GenerateWalletDescriptor(const CExtPubKey& master_key, const OutputType& output_type, bool internal); } // namespace wallet #endif // BITCOIN_WALLET_WALLETUTIL_H diff --git a/test/functional/p2p_block_sync.py b/test/functional/p2p_block_sync.py index d821edc1b1..6c7f08364e 100755 --- a/test/functional/p2p_block_sync.py +++ b/test/functional/p2p_block_sync.py @@ -22,7 +22,7 @@ class BlockSyncTest(BitcoinTestFramework): # node0 -> node1 -> node2 # So node1 has both an inbound and outbound peer. # In our test, we will mine a block on node0, and ensure that it makes - # to to both node1 and node2. + # to both node1 and node2. self.connect_nodes(0, 1) self.connect_nodes(1, 2) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 1408854e02..3f6e47d410 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -181,6 +181,8 @@ BASE_SCRIPTS = [ 'wallet_keypool_topup.py --legacy-wallet', 'wallet_keypool_topup.py --descriptors', 'wallet_fast_rescan.py --descriptors', + 'wallet_gethdkeys.py --descriptors', + 'wallet_createwalletdescriptor.py --descriptors', 'interface_zmq.py', 'rpc_invalid_address_message.py', 'rpc_validateaddress.py', diff --git a/test/functional/wallet_backwards_compatibility.py b/test/functional/wallet_backwards_compatibility.py index 4d6e6024c5..ab008a40cd 100755 --- a/test/functional/wallet_backwards_compatibility.py +++ b/test/functional/wallet_backwards_compatibility.py @@ -355,6 +355,25 @@ class BackwardsCompatibilityTest(BitcoinTestFramework): down_wallet_name = f"re_down_{node.version}" down_backup_path = os.path.join(self.options.tmpdir, f"{down_wallet_name}.dat") wallet.backupwallet(down_backup_path) + + # Check that taproot descriptors can be added to 0.21 wallets + # This must be done after the backup is created so that 0.21 can still load + # the backup + if self.options.descriptors and self.major_version_equals(node, 21): + assert_raises_rpc_error(-12, "No bech32m addresses available", wallet.getnewaddress, address_type="bech32m") + xpubs = wallet.gethdkeys(active_only=True) + assert_equal(len(xpubs), 1) + assert_equal(len(xpubs[0]["descriptors"]), 6) + wallet.createwalletdescriptor("bech32m") + xpubs = wallet.gethdkeys(active_only=True) + assert_equal(len(xpubs), 1) + assert_equal(len(xpubs[0]["descriptors"]), 8) + tr_descs = [desc["desc"] for desc in xpubs[0]["descriptors"] if desc["desc"].startswith("tr(")] + assert_equal(len(tr_descs), 2) + for desc in tr_descs: + assert info["hdmasterfingerprint"] in desc + wallet.getnewaddress(address_type="bech32m") + wallet.unloadwallet() # Check that no automatic upgrade broke the downgrading the wallet diff --git a/test/functional/wallet_createwalletdescriptor.py b/test/functional/wallet_createwalletdescriptor.py new file mode 100755 index 0000000000..18e1703da3 --- /dev/null +++ b/test/functional/wallet_createwalletdescriptor.py @@ -0,0 +1,123 @@ +#!/usr/bin/env python3 +# Copyright (c) 2023 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 wallet createwalletdescriptor RPC.""" + +from test_framework.descriptors import descsum_create +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) +from test_framework.wallet_util import WalletUnlock + + +class WalletCreateDescriptorTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=True, legacy=False) + + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + self.test_basic() + self.test_imported_other_keys() + self.test_encrypted() + + def test_basic(self): + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.nodes[0].createwallet("blank", blank=True) + wallet = self.nodes[0].get_wallet_rpc("blank") + + xpub_info = def_wallet.gethdkeys(private=True) + xpub = xpub_info[0]["xpub"] + xprv = xpub_info[0]["xprv"] + expected_descs = [] + for desc in def_wallet.listdescriptors()["descriptors"]: + if desc["desc"].startswith("wpkh("): + expected_descs.append(desc["desc"]) + + assert_raises_rpc_error(-5, "Unable to determine which HD key to use from active descriptors. Please specify with 'hdkey'", wallet.createwalletdescriptor, "bech32") + assert_raises_rpc_error(-5, f"Private key for {xpub} is not known", wallet.createwalletdescriptor, type="bech32", hdkey=xpub) + + self.log.info("Test createwalletdescriptor after importing active descriptor to blank wallet") + # Import one active descriptor + assert_equal(wallet.importdescriptors([{"desc": descsum_create(f"pkh({xprv}/44h/2h/0h/0/0/*)"), "timestamp": "now", "active": True}])[0]["success"], True) + assert_equal(len(wallet.listdescriptors()["descriptors"]), 1) + assert_equal(len(wallet.gethdkeys()), 1) + + new_descs = wallet.createwalletdescriptor("bech32")["descs"] + assert_equal(len(new_descs), 2) + assert_equal(len(wallet.gethdkeys()), 1) + assert_equal(new_descs, expected_descs) + + self.log.info("Test descriptor creation options") + old_descs = set([(d["desc"], d["active"], d["internal"]) for d in wallet.listdescriptors(private=True)["descriptors"]]) + wallet.createwalletdescriptor(type="bech32m", internal=False) + curr_descs = set([(d["desc"], d["active"], d["internal"]) for d in wallet.listdescriptors(private=True)["descriptors"]]) + new_descs = list(curr_descs - old_descs) + assert_equal(len(new_descs), 1) + assert_equal(len(wallet.gethdkeys()), 1) + assert_equal(new_descs[0][0], descsum_create(f"tr({xprv}/86h/1h/0h/0/*)")) + assert_equal(new_descs[0][1], True) + assert_equal(new_descs[0][2], False) + + old_descs = curr_descs + wallet.createwalletdescriptor(type="bech32m", internal=True) + curr_descs = set([(d["desc"], d["active"], d["internal"]) for d in wallet.listdescriptors(private=True)["descriptors"]]) + new_descs = list(curr_descs - old_descs) + assert_equal(len(new_descs), 1) + assert_equal(len(wallet.gethdkeys()), 1) + assert_equal(new_descs[0][0], descsum_create(f"tr({xprv}/86h/1h/0h/1/*)")) + assert_equal(new_descs[0][1], True) + assert_equal(new_descs[0][2], True) + + def test_imported_other_keys(self): + self.log.info("Test createwalletdescriptor with multiple keys in active descriptors") + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.nodes[0].createwallet("multiple_keys") + wallet = self.nodes[0].get_wallet_rpc("multiple_keys") + + wallet_xpub = wallet.gethdkeys()[0]["xpub"] + + xpub_info = def_wallet.gethdkeys(private=True) + xpub = xpub_info[0]["xpub"] + xprv = xpub_info[0]["xprv"] + + assert_equal(wallet.importdescriptors([{"desc": descsum_create(f"wpkh({xprv}/0/0/*)"), "timestamp": "now", "active": True}])[0]["success"], True) + assert_equal(len(wallet.gethdkeys()), 2) + + assert_raises_rpc_error(-5, "Unable to determine which HD key to use from active descriptors. Please specify with 'hdkey'", wallet.createwalletdescriptor, "bech32") + assert_raises_rpc_error(-4, "Descriptor already exists", wallet.createwalletdescriptor, type="bech32m", hdkey=wallet_xpub) + assert_raises_rpc_error(-5, "Unable to parse HD key. Please provide a valid xpub", wallet.createwalletdescriptor, type="bech32m", hdkey=xprv) + + # Able to replace tr() descriptor with other hd key + wallet.createwalletdescriptor(type="bech32m", hdkey=xpub) + + def test_encrypted(self): + self.log.info("Test createwalletdescriptor with encrypted wallets") + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.nodes[0].createwallet("encrypted", blank=True, passphrase="pass") + wallet = self.nodes[0].get_wallet_rpc("encrypted") + + xpub_info = def_wallet.gethdkeys(private=True) + xprv = xpub_info[0]["xprv"] + + with WalletUnlock(wallet, "pass"): + assert_equal(wallet.importdescriptors([{"desc": descsum_create(f"wpkh({xprv}/0/0/*)"), "timestamp": "now", "active": True}])[0]["success"], True) + assert_equal(len(wallet.gethdkeys()), 1) + + assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", wallet.createwalletdescriptor, type="bech32m") + + with WalletUnlock(wallet, "pass"): + wallet.createwalletdescriptor(type="bech32m") + + + +if __name__ == '__main__': + WalletCreateDescriptorTest().main() diff --git a/test/functional/wallet_gethdkeys.py b/test/functional/wallet_gethdkeys.py new file mode 100755 index 0000000000..f09b8c875a --- /dev/null +++ b/test/functional/wallet_gethdkeys.py @@ -0,0 +1,185 @@ +#!/usr/bin/env python3 +# Copyright (c) 2023 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 wallet gethdkeys RPC.""" + +from test_framework.descriptors import descsum_create +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) +from test_framework.wallet_util import WalletUnlock + + +class WalletGetHDKeyTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, descriptors=True, legacy=False) + + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + self.test_basic_gethdkeys() + self.test_ranged_imports() + self.test_lone_key_imports() + self.test_ranged_multisig() + self.test_mixed_multisig() + + def test_basic_gethdkeys(self): + self.log.info("Test gethdkeys basics") + self.nodes[0].createwallet("basic") + wallet = self.nodes[0].get_wallet_rpc("basic") + xpub_info = wallet.gethdkeys() + assert_equal(len(xpub_info), 1) + assert_equal(xpub_info[0]["has_private"], True) + + assert "xprv" not in xpub_info[0] + xpub = xpub_info[0]["xpub"] + + xpub_info = wallet.gethdkeys(private=True) + xprv = xpub_info[0]["xprv"] + assert_equal(xpub_info[0]["xpub"], xpub) + assert_equal(xpub_info[0]["has_private"], True) + + descs = wallet.listdescriptors(True) + for desc in descs["descriptors"]: + assert xprv in desc["desc"] + + self.log.info("HD pubkey can be retrieved from encrypted wallets") + prev_xprv = xprv + wallet.encryptwallet("pass") + # HD key is rotated on encryption, there should now be 2 HD keys + assert_equal(len(wallet.gethdkeys()), 2) + # New key is active, should be able to get only that one and its descriptors + xpub_info = wallet.gethdkeys(active_only=True) + assert_equal(len(xpub_info), 1) + assert xpub_info[0]["xpub"] != xpub + assert "xprv" not in xpub_info[0] + assert_equal(xpub_info[0]["has_private"], True) + + self.log.info("HD privkey can be retrieved from encrypted wallets") + assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first", wallet.gethdkeys, private=True) + with WalletUnlock(wallet, "pass"): + xpub_info = wallet.gethdkeys(active_only=True, private=True)[0] + assert xpub_info["xprv"] != xprv + for desc in wallet.listdescriptors(True)["descriptors"]: + if desc["active"]: + # After encrypting, HD key was rotated and should appear in all active descriptors + assert xpub_info["xprv"] in desc["desc"] + else: + # Inactive descriptors should have the previous HD key + assert prev_xprv in desc["desc"] + + def test_ranged_imports(self): + self.log.info("Keys of imported ranged descriptors appear in gethdkeys") + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.nodes[0].createwallet("imports") + wallet = self.nodes[0].get_wallet_rpc("imports") + + xpub_info = wallet.gethdkeys() + assert_equal(len(xpub_info), 1) + active_xpub = xpub_info[0]["xpub"] + + import_xpub = def_wallet.gethdkeys(active_only=True)[0]["xpub"] + desc_import = def_wallet.listdescriptors(True)["descriptors"] + for desc in desc_import: + desc["active"] = False + wallet.importdescriptors(desc_import) + assert_equal(wallet.gethdkeys(active_only=True), xpub_info) + + xpub_info = wallet.gethdkeys() + assert_equal(len(xpub_info), 2) + for x in xpub_info: + if x["xpub"] == active_xpub: + for desc in x["descriptors"]: + assert_equal(desc["active"], True) + elif x["xpub"] == import_xpub: + for desc in x["descriptors"]: + assert_equal(desc["active"], False) + else: + assert False + + + def test_lone_key_imports(self): + self.log.info("Non-HD keys do not appear in gethdkeys") + self.nodes[0].createwallet("lonekey", blank=True) + wallet = self.nodes[0].get_wallet_rpc("lonekey") + + assert_equal(wallet.gethdkeys(), []) + wallet.importdescriptors([{"desc": descsum_create("wpkh(cTe1f5rdT8A8DFgVWTjyPwACsDPJM9ff4QngFxUixCSvvbg1x6sh)"), "timestamp": "now"}]) + assert_equal(wallet.gethdkeys(), []) + + self.log.info("HD keys of non-ranged descriptors should appear in gethdkeys") + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + xpub_info = def_wallet.gethdkeys(private=True) + xpub = xpub_info[0]["xpub"] + xprv = xpub_info[0]["xprv"] + prv_desc = descsum_create(f"wpkh({xprv})") + pub_desc = descsum_create(f"wpkh({xpub})") + assert_equal(wallet.importdescriptors([{"desc": prv_desc, "timestamp": "now"}])[0]["success"], True) + xpub_info = wallet.gethdkeys() + assert_equal(len(xpub_info), 1) + assert_equal(xpub_info[0]["xpub"], xpub) + assert_equal(len(xpub_info[0]["descriptors"]), 1) + assert_equal(xpub_info[0]["descriptors"][0]["desc"], pub_desc) + assert_equal(xpub_info[0]["descriptors"][0]["active"], False) + + def test_ranged_multisig(self): + self.log.info("HD keys of a multisig appear in gethdkeys") + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.nodes[0].createwallet("ranged_multisig") + wallet = self.nodes[0].get_wallet_rpc("ranged_multisig") + + xpub1 = wallet.gethdkeys()[0]["xpub"] + xprv1 = wallet.gethdkeys(private=True)[0]["xprv"] + xpub2 = def_wallet.gethdkeys()[0]["xpub"] + + prv_multi_desc = descsum_create(f"wsh(multi(2,{xprv1}/*,{xpub2}/*))") + pub_multi_desc = descsum_create(f"wsh(multi(2,{xpub1}/*,{xpub2}/*))") + assert_equal(wallet.importdescriptors([{"desc": prv_multi_desc, "timestamp": "now"}])[0]["success"], True) + + xpub_info = wallet.gethdkeys() + assert_equal(len(xpub_info), 2) + for x in xpub_info: + if x["xpub"] == xpub1: + found_desc = next((d for d in xpub_info[0]["descriptors"] if d["desc"] == pub_multi_desc), None) + assert found_desc is not None + assert_equal(found_desc["active"], False) + elif x["xpub"] == xpub2: + assert_equal(len(x["descriptors"]), 1) + assert_equal(x["descriptors"][0]["desc"], pub_multi_desc) + assert_equal(x["descriptors"][0]["active"], False) + else: + assert False + + def test_mixed_multisig(self): + self.log.info("Non-HD keys of a multisig do not appear in gethdkeys") + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.nodes[0].createwallet("single_multisig") + wallet = self.nodes[0].get_wallet_rpc("single_multisig") + + xpub = wallet.gethdkeys()[0]["xpub"] + xprv = wallet.gethdkeys(private=True)[0]["xprv"] + pub = def_wallet.getaddressinfo(def_wallet.getnewaddress())["pubkey"] + + prv_multi_desc = descsum_create(f"wsh(multi(2,{xprv},{pub}))") + pub_multi_desc = descsum_create(f"wsh(multi(2,{xpub},{pub}))") + import_res = wallet.importdescriptors([{"desc": prv_multi_desc, "timestamp": "now"}]) + assert_equal(import_res[0]["success"], True) + + xpub_info = wallet.gethdkeys() + assert_equal(len(xpub_info), 1) + assert_equal(xpub_info[0]["xpub"], xpub) + found_desc = next((d for d in xpub_info[0]["descriptors"] if d["desc"] == pub_multi_desc), None) + assert found_desc is not None + assert_equal(found_desc["active"], False) + + +if __name__ == '__main__': + WalletGetHDKeyTest().main() |