From 33d9be11d0e70cebefe1c9bdb006b494a0b37c53 Mon Sep 17 00:00:00 2001
From: Oghenovo Usiwoma <37949128+Eunovo@users.noreply.github.com>
Date: Sun, 4 Aug 2024 09:12:21 +0100
Subject: [PATCH] Allow ext keys (#3)

* rpc/deriveaddress: Add support for sp desc

* spscriptpubkeyman: Move labelled destination generation to seperate methods

* rpc/getnewaddress: Add silent-payment to addess_type list

* descriptor: Allow extended keys in sp()

* Fix scan key storage issues in watch only wallet

- Stop adding scan key to singingprovider private key map, this will prevent scan key from being counted among private keys
- Stop storing scan keys to db, scan keys are already saved in descriptor

* spscriptpubkeyman: Fix MarkUnusedAddresses SegFault

DescriptorScriptPubKeyMan::MarkUnusedAddresses tries to expand the descriptor and read the resulting scripts
This does not work for sp descriptors as no scripts are returned
This leads to a SegFault

* test: Add E2E test for sp descriptor
---
 src/addresstype.h                             |  15 ++
 src/rpc/output_script.cpp                     |   4 +
 src/script/descriptor.cpp                     | 129 +++++++++++++-----
 src/silentpaymentkey.h                        |  12 ++
 src/test/descriptor_tests.cpp                 |  68 +++++++++
 src/wallet/rpc/addresses.cpp                  |   2 +-
 src/wallet/rpc/backup.cpp                     |   2 +-
 src/wallet/scriptpubkeyman.cpp                |  50 +++++--
 src/wallet/scriptpubkeyman.h                  |   5 +
 src/wallet/wallet.cpp                         |  13 ++
 src/wallet/walletutil.cpp                     |   4 +-
 test/functional/wallet_miniscript.py          | 112 +++++++++------
 .../wallet_silentpayments_receiving.py        |   8 +-
 13 files changed, 335 insertions(+), 89 deletions(-)

diff --git a/src/addresstype.h b/src/addresstype.h
index 446593b90a6765..2af91f4dc8cb23 100644
--- a/src/addresstype.h
+++ b/src/addresstype.h
@@ -8,6 +8,7 @@
 #include <attributes.h>
 #include <pubkey.h>
 #include <script/script.h>
+#include <silentpaymentkey.h>
 #include <uint256.h>
 #include <util/hash_type.h>
 
@@ -121,6 +122,20 @@ struct V0SilentPaymentDestination
     CPubKey m_scan_pubkey;
     CPubKey m_spend_pubkey;
 
+    V0SilentPaymentDestination() = default;
+
+    V0SilentPaymentDestination(const CPubKey& scan_pubkey, const CPubKey& spend_pubkey) : m_scan_pubkey(scan_pubkey), m_spend_pubkey(spend_pubkey) {};
+
+    V0SilentPaymentDestination(const SpPubKey& sppubkey) {
+        m_scan_pubkey = sppubkey.scanKey.GetPubKey();
+        m_spend_pubkey = sppubkey.spendKey;
+    }
+
+    V0SilentPaymentDestination(const SpKey& spkey) {
+        m_scan_pubkey = spkey.scanKey.GetPubKey();
+        m_spend_pubkey = spkey.spendKey.GetPubKey();
+    }
+
     friend bool operator==(const V0SilentPaymentDestination& a, const V0SilentPaymentDestination& b) {
         if (a.m_scan_pubkey != b.m_scan_pubkey) return false;
         if (a.m_spend_pubkey != b.m_spend_pubkey) return false;
diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp
index 65a9be2762fc8e..5aaa08cd809520 100644
--- a/src/rpc/output_script.cpp
+++ b/src/rpc/output_script.cpp
@@ -285,6 +285,10 @@ static RPCHelpMan deriveaddresses()
 
                     addresses.push_back(EncodeDestination(dest));
                 }
+
+                for (const auto& sppubkey_pair: provider.sppubkeys) {
+                    addresses.push_back(EncodeDestination(V0SilentPaymentDestination(sppubkey_pair.second)));
+                }
             }
 
             // This should not be possible, but an assert seems overkill:
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index cde6762da9e7ea..e5af61be595e34 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -638,6 +638,10 @@ class SilentPubkeyProvider final : public PubkeyProvider
         std::copy(keyid.begin(), keyid.begin() + sizeof(info.fingerprint), info.fingerprint);
         pubkey = m_sppk.scanKey.GetPubKey();
     }
+    void SetSpendPubKey(const CPubKey pubkey)
+    {
+        m_sppk.spendKey = pubkey;
+    }
 };
 
 /** Base class for all Descriptor implementations. */
@@ -830,11 +834,20 @@ class DescriptorImpl : public Descriptor
         for (const auto& p : m_pubkey_args) {
             try {
                 auto sppubkeyprovider = dynamic_cast<SilentPubkeyProvider&>(*p);
-                SpKey key;
-                sppubkeyprovider.GetSpKey(provider, key);
-                out.spkeys.emplace(key.Neuter().GetID(), key);
-                out.keys.emplace(key.spendKey.GetPubKey().GetID(), key.spendKey);
-                out.keys.emplace(key.scanKey.GetPubKey().GetID(), key.scanKey);
+                {
+                    SpKey key;
+                    if (sppubkeyprovider.GetSpKey(provider, key)) {
+                        out.spkeys.emplace(key.Neuter().GetID(), key);
+                        out.keys.emplace(key.spendKey.GetPubKey().GetID(), key.spendKey);
+                        out.keys.emplace(key.scanKey.GetPubKey().GetID(), key.scanKey);
+                    }
+                }
+                {
+                    SpPubKey key;
+                    if (sppubkeyprovider.GetSpPubKey(key)) {
+                        out.keys.emplace(key.scanKey.GetPubKey().GetID(), key.scanKey);
+                    }
+                }
                 continue;
             } catch (const std::bad_cast&) {}
 
@@ -1428,7 +1441,9 @@ enum class ParseScriptContext {
     P2WPKH,  //!< Inside wpkh() (no script, pubkey only)
     P2WSH,   //!< Inside wsh() (script becomes v0 witness script)
     P2TR,    //!< Inside tr() (either internal key, or BIP342 script leaf)
-    SP,      //!< Inside sp() (spkeys are only valid under sp())
+    SP_ONLY,      //!< Argument inside sp(<spkey>) variant (spkeys are only valid under sp())
+    SP_SCAN,      //!< First argument inside sp()
+    SP_SPEND,     //!< Second argument inside sp()
 };
 
 /**
@@ -1484,6 +1499,10 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(uint32_t key_exp_index, const S
                 error = "Hybrid public keys are not allowed";
                 return nullptr;
             }
+            if (pubkey.IsValid() && ctx == ParseScriptContext::SP_SCAN) {
+                error = "Scan key must be a private key or extended private key";
+                return nullptr;
+            }
             if (pubkey.IsFullyValid()) {
                 if (permit_uncompressed || pubkey.IsCompressed()) {
                     return std::make_unique<ConstPubkeyProvider>(key_exp_index, pubkey, false);
@@ -1520,24 +1539,17 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(uint32_t key_exp_index, const S
     CExtKey extkey = DecodeExtKey(str);
     CExtPubKey extpubkey = DecodeExtPubKey(str);
 
-    if (ctx == ParseScriptContext::SP) {
-        if (extkey.key.IsValid() || extpubkey.pubkey.IsValid()) {
-            error = "extended keys are not allowed";
-            return nullptr;
-        }
-
+    if (ctx == ParseScriptContext::SP_ONLY) {
         if (spkey.IsValid()) {
-            out.keys.emplace(spkey.scanKey.GetPubKey().GetID(), spkey.scanKey);
             out.keys.emplace(spkey.spendKey.GetPubKey().GetID(), spkey.spendKey);
             return std::make_unique<SilentPubkeyProvider>(key_exp_index, spkey.Neuter());
         }
 
         if (sppubkey.IsValid()) {
-            out.keys.emplace(sppubkey.scanKey.GetPubKey().GetID(), sppubkey.scanKey);
             return std::make_unique<SilentPubkeyProvider>(key_exp_index, sppubkey);
         }
 
-        error = "provided key is not a valid silent payment key";
+        error = strprintf("key '%s' is not a valid sp key", str);
         return nullptr;
     }
 
@@ -1556,10 +1568,57 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(uint32_t key_exp_index, const S
         type = DeriveType::HARDENED;
     }
     if (!ParseKeyPath(split, path, apostrophe, error)) return nullptr;
+
+    if (ctx == ParseScriptContext::SP_SCAN || ctx == ParseScriptContext::SP_SPEND) {
+        CExtKey derivedKey;
+        CExtPubKey derivedPubKey;
+        for (const auto& p : path) {
+            derivedKey = extkey;
+            derivedPubKey = extpubkey;
+
+            if (derivedKey.key.IsValid()) {
+                if (!derivedKey.Derive(derivedKey, p)) {
+                    error = "Failed to derive key";
+                    return nullptr;
+                }
+            }
+            if (derivedPubKey.pubkey.IsValid()) {
+                if (!derivedPubKey.Derive(derivedPubKey, p)) {
+                    error = "Failed to derive key";
+                    return nullptr;
+                }
+            }
+        }
+
+        if (ctx == ParseScriptContext::SP_SCAN) {
+            if (!extkey.key.IsValid()) {
+                error = "Scan key must be a private key or extended private key";
+                return nullptr;
+            }
+
+            // Derive sp scan key from extkey
+            CKey scan_key = derivedKey.key.IsValid() ? derivedKey.key : extkey.key;
+            return std::make_unique<SilentPubkeyProvider>(key_exp_index, SpPubKey(scan_key));
+        }
+
+        if (extkey.key.IsValid()) {
+            // Derive sp spend key from extkey
+            CKey spend_key = derivedKey.key.IsValid() ? derivedKey.key : extkey.key;
+            out.keys.emplace(spend_key.GetPubKey().GetID(), spend_key);
+            return std::make_unique<ConstPubkeyProvider>(key_exp_index, spend_key.GetPubKey(), false);
+        }
+        if (extpubkey.pubkey.IsValid()) {
+            // Derive sp spend key from extpubkey
+            CPubKey spend_key = derivedPubKey.pubkey.IsValid() ? derivedPubKey.pubkey : extpubkey.pubkey;
+            return std::make_unique<ConstPubkeyProvider>(key_exp_index, spend_key, false);
+        }
+    }
+
     if (extkey.key.IsValid()) {
         extpubkey = extkey.Neuter();
         out.keys.emplace(extpubkey.pubkey.GetID(), extkey.key);
     }
+
     return std::make_unique<BIP32PubkeyProvider>(key_exp_index, extpubkey, std::move(path), type, apostrophe);
 }
 
@@ -1744,39 +1803,41 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
 
     if (ctx == ParseScriptContext::TOP && Func("sp", expr)) {
         auto arg1 = Expr(expr);
-        auto firstKey = ParsePubkey(key_exp_index, arg1, ParseScriptContext::SP, out, error);
-        if (!firstKey) {
+        if (!Const(",", expr)) {
+            auto sppKey = ParsePubkey(key_exp_index, arg1, ParseScriptContext::SP_ONLY, out, error);
+            if (!sppKey) {
+                error = strprintf("sp(): %s", error);
+                return nullptr;
+            }
+            ++key_exp_index;
+            return std::make_unique<SpDescriptor>(std::move(sppKey));
+        }
+        auto arg2 = Expr(expr);
+
+        auto scanKey = ParsePubkey(key_exp_index, arg1, ParseScriptContext::SP_SCAN, out, error);
+        if (!scanKey) {
             error = strprintf("sp(): %s", error);
             return nullptr;
         }
         ++key_exp_index;
-        if (!Const(",", expr)) {
-            return std::make_unique<SpDescriptor>(std::move(firstKey));
-        }
-        auto arg2 = Expr(expr);
-        auto spendKey = ParsePubkey(key_exp_index, arg2, ParseScriptContext::SP, out, error);
+
+        auto spendKey = ParsePubkey(key_exp_index, arg2, ParseScriptContext::SP_SPEND, out, error);
         if (!spendKey) {
             error = strprintf("sp(): %s", error);
             return nullptr;
         }
-        auto scanPubKey = firstKey->GetRootPubKey();
         auto spendPubKey = spendKey->GetRootPubKey();
-        if (!scanPubKey.has_value()) {
-            error = "sp(): could not get scan pubkey";
-            return nullptr;
-        }
         if (!spendPubKey.has_value()) {
             error = "sp(): could not get spend pubkey";
             return nullptr;
         }
-        auto it = out.keys.find(scanPubKey->GetID());
-        if (it == out.keys.end()) {
-            error = "sp(): requires the scan priv key";
-            return nullptr;
-        }
-        auto sppk = std::make_unique<SilentPubkeyProvider>(key_exp_index, SpPubKey(it->second, *spendPubKey));
+
+        SilentPubkeyProvider* sppubKey = dynamic_cast<SilentPubkeyProvider*>(scanKey.get());
+        assert(sppubKey != nullptr);
+        sppubKey->SetSpendPubKey(*spendPubKey);
+
         ++key_exp_index;
-        return std::make_unique<SpDescriptor>(std::move(sppk));
+        return std::make_unique<SpDescriptor>(std::move(scanKey));
     } else if (Func("sp", expr)) {
         error = "Can only have sp() at top level";
         return nullptr;
diff --git a/src/silentpaymentkey.h b/src/silentpaymentkey.h
index c5d854b941a4db..028ca32808da45 100644
--- a/src/silentpaymentkey.h
+++ b/src/silentpaymentkey.h
@@ -22,6 +22,18 @@ struct SpPubKey {
 
     SpPubKey() = default;
 
+    /**
+     * Creates an incomplete SpPubKey.
+     */
+    SpPubKey(CKey scan_key) : scanKey(scan_key)
+    {
+      memset(version, 0, sizeof(version));
+      memset(vchFingerprint, 0, sizeof(vchFingerprint));
+      maximumNumberOfLabels = 0;
+      CPubKey dummySpendPubKey;
+      spendKey = dummySpendPubKey;
+    }
+
     SpPubKey(CKey scan_key, CPubKey spend_key) : scanKey(scan_key), spendKey(spend_key)
     {
       memset(version, 0, sizeof(version));
diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp
index e6821dd321884f..a736c55745b100 100644
--- a/src/test/descriptor_tests.cpp
+++ b/src/test/descriptor_tests.cpp
@@ -434,6 +434,27 @@ void CheckInferDescriptor(const std::string& script_hex, const std::string& expe
     BOOST_CHECK_EQUAL(desc->ToString(), expected_desc + "#" + checksum);
 }
 
+void CheckSilentPayments(const std::string& desc, const std::string& expected_private_string, const std::string& expected_public_string, const std::string& expected_norm_string, int flags = 0)
+{
+    FlatSigningProvider keys;
+    std::string error;
+    auto parsed_desc = Parse(desc, keys, error, false);
+    BOOST_CHECK_MESSAGE(parsed_desc, error);
+
+    if (~flags & MISSING_PRIVKEYS) {
+        std::string private_string;
+        BOOST_CHECK(parsed_desc->ToPrivateString(keys, private_string));
+        BOOST_CHECK_MESSAGE(EqualDescriptor(private_string, expected_private_string), "Private: " + private_string + " Expected: " + expected_private_string);
+    }
+
+    std::string public_string = parsed_desc->ToString();
+    std::string norm_string;
+    BOOST_CHECK(parsed_desc->ToNormalizedString(keys, norm_string));
+
+    BOOST_CHECK_MESSAGE(EqualDescriptor(public_string, expected_public_string), "Public: " + public_string + " Expected: " + expected_public_string);
+    BOOST_CHECK_MESSAGE(EqualDescriptor(norm_string, expected_norm_string), "Normalized: " + norm_string + " Expected: " + expected_norm_string);
+}
+
 }
 
 BOOST_FIXTURE_TEST_SUITE(descriptor_tests, BasicTestingSetup)
@@ -657,6 +678,53 @@ BOOST_AUTO_TEST_CASE(descriptor_test)
     CheckInferDescriptor("76a914a31725c74421fadc50d35520ab8751ed120af80588ac", "pkh(04c56fe4a92d401bcbf1b3dfbe4ac3dac5602ca155a3681497f02c1b9a733b92d704e2da6ec4162e4846af9236ef4171069ac8b7f8234a8405b6cadd96f34f5a31)", {}, {{"04c56fe4a92d401bcbf1b3dfbe4ac3dac5602ca155a3681497f02c1b9a733b92d704e2da6ec4162e4846af9236ef4171069ac8b7f8234a8405b6cadd96f34f5a31", ""}});
     // Infer pk() from p2pk with uncompressed key
     CheckInferDescriptor("4104032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220ac", "pk(04032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220)", {}, {{"04032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220", ""}});
+
+    // Silent Payments
+    // Check that /* uses default derivation path for SP
+    CheckSilentPayments("sp(xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi/*,xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi/*)",
+        "sp(spprv1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5qw3uewwg77eaq9rth6er3vj0yutvs5xyup0ndsrg2ffwgheppkkdg3tdld9)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5pnngmqzvcpt976aaqlhevn5qkv2y7sk42j0mpd7yzsut507jwgtssknx4ck)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5pnngmqzvcpt976aaqlhevn5qkv2y7sk42j0mpd7yzsut507jwgtssknx4ck)");
+
+    // Check that no path uses default derivation path for SP
+    CheckSilentPayments("sp(xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi,xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi)",
+        "sp(spprv1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5qw3uewwg77eaq9rth6er3vj0yutvs5xyup0ndsrg2ffwgheppkkdg3tdld9)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5pnngmqzvcpt976aaqlhevn5qkv2y7sk42j0mpd7yzsut507jwgtssknx4ck)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5pnngmqzvcpt976aaqlhevn5qkv2y7sk42j0mpd7yzsut507jwgtssknx4ck)");
+
+    // Check that provided path is used instead of the default derivation path for SP
+    CheckSilentPayments("sp(xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi,xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi/0h)",
+        "sp(spprv1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5qwmvhpf70wwlfxmkfmfm8dargka4qgec2fkmxcpvr3tgkezxs2l6sf4t658)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5p457zxv2j2yzn9ha42hxhf3fkqdz5pc5hykqevp765qrrsdn7vc4skd7hrr)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5p457zxv2j2yzn9ha42hxhf3fkqdz5pc5hykqevp765qrrsdn7vc4skd7hrr)");
+
+    CheckSilentPayments("sp(xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi/0h,xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi)",
+        "sp(spprv1qqqqqqqqqqqqqq8dkts5l8h805ndmya5ank735tw6syvu9ymdnvqkpc45tv3rg90agqw3uewwg77eaq9rth6er3vj0yutvs5xyup0ndsrg2ffwgheppkkdgnemptj)",
+        "sp(sppub1qqqqqqqqqqqqqq8dkts5l8h805ndmya5ank735tw6syvu9ymdnvqkpc45tv3rg90agpnngmqzvcpt976aaqlhevn5qkv2y7sk42j0mpd7yzsut507jwgtss5pst7p)",
+        "sp(sppub1qqqqqqqqqqqqqq8dkts5l8h805ndmya5ank735tw6syvu9ymdnvqkpc45tv3rg90agpnngmqzvcpt976aaqlhevn5qkv2y7sk42j0mpd7yzsut507jwgtss5pst7p)");
+
+    // Check that xpubs are accepted for spend key
+    CheckSilentPayments("sp(xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi,xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5pnngmqzvcpt976aaqlhevn5qkv2y7sk42j0mpd7yzsut507jwgtssknx4ck)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5pnngmqzvcpt976aaqlhevn5qkv2y7sk42j0mpd7yzsut507jwgtssknx4ck)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5pnngmqzvcpt976aaqlhevn5qkv2y7sk42j0mpd7yzsut507jwgtssknx4ck)",
+        MISSING_PRIVKEYS);
+
+    CheckSilentPayments("sp(xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi,xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8/0)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5p8cjcfl7ucts5c4ln7tqfjvm9ledmcpdyq4s55kz6rmssl903az0qr6947z)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5p8cjcfl7ucts5c4ln7tqfjvm9ledmcpdyq4s55kz6rmssl903az0qr6947z)",
+        "sp(sppub1qqqqqqqqqqqqqq8g7vh8y00v7sz34mav3ckf8jw9kg2rzwqhekcp59y5hytussmtx5p8cjcfl7ucts5c4ln7tqfjvm9ledmcpdyq4s55kz6rmssl903az0qr6947z)",
+        MISSING_PRIVKEYS);
+
+    CheckUnparsable("sp(xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi)",
+        "sp(xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8)",
+        "sp(): key 'xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8' is not a valid sp key");
+    CheckUnparsable("sp(xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8,xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi)",
+        "sp(xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8,xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi)",
+        "sp(): Scan key must be a private key or extended private key");
+    CheckUnparsable("sp(spprv1qqqqqqqqqqqqqqqcw78khf9nvvgng9a0vsnzgz9hc29vqta5gwhwls59y60grpjpnvqryr9c920g3tru2cs3nazwqjdapvhx249ph95zkp3scsg957vzqag8ml0eq,xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi)",
+        "sp(sppub1qqqqqqqqqqqqqqqcw78khf9nvvgng9a0vsnzgz9hc29vqta5gwhwls59y60grpjpnvper07qpyg2fg4r32cfll4wytqcf9km7zzjaqd0aw69y2tdw0g2y2cklv80y,xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8)",
+        "sp(): key 'sppub1qqqqqqqqqqqqqqqcw78khf9nvvgng9a0vsnzgz9hc29vqta5gwhwls59y60grpjpnvper07qpyg2fg4r32cfll4wytqcf9km7zzjaqd0aw69y2tdw0g2y2cklv80y' is not valid");
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index b5bfe21960139d..03663227ad78b3 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -26,7 +26,7 @@ RPCHelpMan getnewaddress()
                 "so payments received with the address will be associated with 'label'.\n",
                 {
                     {"label", RPCArg::Type::STR, RPCArg::Default{""}, "The label name for the address to be linked to. It can also be set to the empty string \"\" to represent the default label. The label does not need to exist, it will be created if there is no label by the given name."},
-                    {"address_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -addresstype"}, "The address type to use. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."},
+                    {"address_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -addresstype"}, "The address type to use. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", \"bech32m\" and \"silent-payment\"."},
                 },
                 RPCResult{
                     RPCResult::Type::STR, "address", "The new bitcoin address"
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 8cddb8b0990485..8287eb0e7278e6 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -1483,7 +1483,7 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c
         }
 
         // Active descriptors must be ranged
-        if (active && !parsed_desc->IsRange()) {
+        if (active && !parsed_desc->IsRange() && parsed_desc->GetOutputType() != OutputType::SILENT_PAYMENT) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Active descriptors must be ranged");
         }
 
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 1c514fe5a5cde0..1601c0b04826f3 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -2868,17 +2868,41 @@ util::Result<CTxDestination> SilentPaymentDescriptorScriptPubKeyMan::GetNewDesti
     if (!sppubkey.has_value()) {
         throw std::runtime_error(std::string(__func__) + ": descriptor expansion failed");
     }
+
+    V0SilentPaymentDestination main_dest;
+    main_dest.m_scan_pubkey = sppubkey->scanKey.GetPubKey();
+    main_dest.m_spend_pubkey = sppubkey->spendKey;
+    CTxDestination dest{main_dest};
+    return dest;
+}
+
+V0SilentPaymentDestination SilentPaymentDescriptorScriptPubKeyMan::GetLabelledDestination(uint64_t index)
+{
+    AssertLockHeld(cs_desc_man);
+
+    auto sppubkey = GetSpPubKeyFrom(m_wallet_descriptor.descriptor);
+    if (!sppubkey.has_value()) {
+        throw std::runtime_error(std::string(__func__) + ": descriptor expansion failed");
+    }
+
     V0SilentPaymentDestination main_dest;
     main_dest.m_scan_pubkey = sppubkey->scanKey.GetPubKey();
     main_dest.m_spend_pubkey = sppubkey->spendKey;
-    auto label_data = BIP352::CreateLabelTweak(sppubkey->scanKey, m_wallet_descriptor.next_index);
+    // Generate label tweak
+    auto label_data = BIP352::CreateLabelTweak(sppubkey->scanKey, index);
     m_map_label_tweaks.insert(label_data);
     V0SilentPaymentDestination label_dest = BIP352::GenerateSilentPaymentLabelledAddress(main_dest, label_data.second);
-    CTxDestination dest{label_dest};
+    return label_dest;
+}
 
+util::Result<CTxDestination> SilentPaymentDescriptorScriptPubKeyMan::GetNewLabelledDestination(uint64_t& index)
+{
+    LOCK(cs_desc_man);
+    auto dest = GetLabelledDestination(m_wallet_descriptor.next_index);
+    index = m_wallet_descriptor.next_index; // Return the index for this destination
     m_wallet_descriptor.next_index++;
     WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor);
-    return dest;
+    return CTxDestination{dest};
 }
 
 isminetype SilentPaymentDescriptorScriptPubKeyMan::IsMine(const CScript& script) const
@@ -2923,13 +2947,9 @@ isminetype SilentPaymentDescriptorScriptPubKeyMan::IsMine(std::vector<XOnlyPubKe
 util::Result<CTxDestination> SilentPaymentDescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool)
 {
     LOCK(cs_desc_man);
-
-    int64_t index_cache = m_wallet_descriptor.next_index;
-    m_wallet_descriptor.next_index = 0; // Set to zero for change
-    auto op_dest = GetNewDestination(type);
-    index = 0;
-    m_wallet_descriptor.next_index = index_cache; // Restore next_index
-    return op_dest;
+    // Destination at index 0 is the change destination
+    auto op_dest = GetLabelledDestination(0);
+    return CTxDestination{op_dest};
 }
 
 bool SilentPaymentDescriptorScriptPubKeyMan::TopUp(const uint256& tweak)
@@ -3004,4 +3024,14 @@ void SilentPaymentDescriptorScriptPubKeyMan::AddTweak(const uint256& tweak)
     m_map_spk_tweaks.emplace(spk, tweak);
 }
 
+std::vector<WalletDestination> SilentPaymentDescriptorScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)
+{
+    // The wallet normally maintains a cache of unused destinations/addresses
+    // MarkUnusedAddresses is supposed to return the destinations for the given scriptPubKey
+    // so the wallet can mark these destinations as used in it's AddresBook
+    // This concept does not apply Silent Payments as there is no cache of unused destinations/addresses
+    // Still, we must subclass this because the original method tries to expand the descriptor which will result in an error for SP descriptors
+    return {};
+}
+
 } // namespace wallet
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index a15b6394be9658..f600f1251fb0f4 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -25,6 +25,7 @@
 
 #include <boost/signals2/signal.hpp>
 
+#include <cstdint>
 #include <functional>
 #include <optional>
 #include <unordered_map>
@@ -726,6 +727,8 @@ class SilentPaymentDescriptorScriptPubKeyMan : public DescriptorScriptPubKeyMan
         {}
 
     util::Result<CTxDestination> GetNewDestination(const OutputType type) override;
+    util::Result<CTxDestination> GetNewLabelledDestination(uint64_t& index);
+    V0SilentPaymentDestination GetLabelledDestination(uint64_t index) EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man);
     util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) override;
     bool CanGetAddresses(bool internal) const override { return true; }
 
@@ -741,6 +744,8 @@ class SilentPaymentDescriptorScriptPubKeyMan : public DescriptorScriptPubKeyMan
 
     //! Add tweak into m_map_spk_tweaks without saving to DB
     void AddTweak(const uint256& tweak);
+
+    std::vector<WalletDestination> MarkUnusedAddresses(const CScript& script) override;
 };
 
 /** struct containing information needed for migrating legacy wallets to descriptor wallets */
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 210f6c9b9df412..87e7c239f49411 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -74,6 +74,7 @@
 #include <algorithm>
 #include <cassert>
 #include <condition_variable>
+#include <cstdint>
 #include <exception>
 #include <optional>
 #include <stdexcept>
@@ -2610,6 +2611,18 @@ util::Result<CTxDestination> CWallet::GetNewDestination(const OutputType type, c
         return util::Error{strprintf(_("Error: No %s addresses available."), FormatOutputType(type))};
     }
 
+    if (type == OutputType::SILENT_PAYMENT && !label.empty()){
+        SilentPaymentDescriptorScriptPubKeyMan* sp_spk_man = dynamic_cast<SilentPaymentDescriptorScriptPubKeyMan*>(spk_man);
+        assert(sp_spk_man);
+        uint64_t index;
+        auto op_dest = sp_spk_man->GetNewLabelledDestination(index);
+        if (op_dest) {
+            SetAddressBook(*op_dest, label, AddressPurpose::RECEIVE);
+        }
+
+        return op_dest;
+    }
+
     auto op_dest = spk_man->GetNewDestination(type);
     if (op_dest) {
         SetAddressBook(*op_dest, label, AddressPurpose::RECEIVE);
diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp
index 27369564a818f6..c4cd72d9b05306 100644
--- a/src/wallet/walletutil.cpp
+++ b/src/wallet/walletutil.cpp
@@ -97,9 +97,9 @@ WalletDescriptor GenerateWalletDescriptor(const CExtKey& master_key, const Outpu
             }
         }
 
-        // Add the derived keys
+        // We only need to save the spend key
+        // The scan key is preserved in the descriptor
         out_keys.emplace_back(spend_key.key, spend_key.key.GetPubKey());
-        out_keys.emplace_back(scan_key.key, scan_key.key.GetPubKey());
 
         SpPubKey sppub_key(scan_key.key, spend_key.key.GetPubKey());
         std::string sppub = EncodeSpPubKey(sppub_key);
diff --git a/test/functional/wallet_miniscript.py b/test/functional/wallet_miniscript.py
index 67e128390200ca..df25238d2ff95c 100755
--- a/test/functional/wallet_miniscript.py
+++ b/test/functional/wallet_miniscript.py
@@ -2,7 +2,7 @@
 # Copyright (c) 2022 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 Miniscript descriptors integration in the wallet."""
+"""Test Miniscript descriptors integration in the self.ms_wo_wallet."""
 
 from test_framework.descriptors import descsum_create
 from test_framework.psbt import PSBT, PSBT_IN_SHA256
@@ -55,6 +55,8 @@
     f"tr(4d54bb9928a0683b7e383de72943b214b0716f58aa54c7ba6bcea2328bc9c768,{{{{{P2WSH_MINISCRIPTS[0]},{P2WSH_MINISCRIPTS[1]}}},{P2WSH_MINISCRIPTS[2].replace('multi', 'multi_a')}}})",
     # A Taproot with all above scripts in its tree.
     f"tr(4d54bb9928a0683b7e383de72943b214b0716f58aa54c7ba6bcea2328bc9c768,{{{{{P2WSH_MINISCRIPTS[0]},{P2WSH_MINISCRIPTS[1]}}},{{{P2WSH_MINISCRIPTS[2].replace('multi', 'multi_a')},{P2WSH_MINISCRIPTS[3]}}}}})",
+    f"sp({TPRVS[0]},{TPUBS[0]})",
+    "sp(sprtpub1qqqqqqqqqqqqqqz2cezqye6220rn3xzpxu5lxdh8l89x6mvvz25vx699mlz5tss635p7y6txdrl97smz23cmxylj0wmtygzulgc9ynpctsj8g3zxp9eycgcpvngw9)",
 ]
 
 DESCS_PRIV = [
@@ -199,6 +201,14 @@
         "sigs_count": 2,
         "stack_size": 8,
     },
+    # Silent Payments
+    {
+        "desc": f"sp({TPRVS[0]},{TPRVS[0]})",
+        "sequence": None,
+        "locktime": None,
+        "sigs_count": 1,
+        "stack_size": 1,
+    },
 ]
 
 
@@ -217,28 +227,39 @@ def skip_test_if_missing_module(self):
     def watchonly_test(self, desc):
         self.log.info(f"Importing descriptor '{desc}'")
         desc = descsum_create(f"{desc}")
-        assert self.ms_wo_wallet.importdescriptors(
-            [
-                {
-                    "desc": desc,
-                    "active": True,
-                    "range": 2,
-                    "next_index": 0,
-                    "timestamp": "now",
-                }
-            ]
-        )[0]["success"]
+        range = None if desc.startswith("sp(") else 2
+
+        import_desc_data = {
+            "desc": desc,
+            "active": True,
+            "next_index": 0,
+            "timestamp": "now",
+        }
+        if range is not None:
+            import_desc_data["range"] = range
+        assert self.ms_wo_wallet.importdescriptors([import_desc_data])[0]["success"]
 
         self.log.info("Testing we derive new addresses for it")
-        addr_type = "bech32m" if desc.startswith("tr(") else "bech32"
-        assert_equal(
-            self.ms_wo_wallet.getnewaddress(address_type=addr_type),
-            self.funder.deriveaddresses(desc, 0)[0],
-        )
-        assert_equal(
-            self.ms_wo_wallet.getnewaddress(address_type=addr_type),
-            self.funder.deriveaddresses(desc, 1)[1],
-        )
+        addr_type = "bech32"
+        if desc.startswith("tr("):
+            addr_type = "bech32m"
+        elif desc.startswith("sp("):
+            addr_type = "silent-payment"
+
+        if range is not None:
+            assert_equal(
+                self.ms_wo_wallet.getnewaddress(address_type=addr_type),
+                self.funder.deriveaddresses(desc, 0)[0],
+            )
+            assert_equal(
+                self.ms_wo_wallet.getnewaddress(address_type=addr_type),
+                self.funder.deriveaddresses(desc, 1)[1],
+            )
+        else:
+            assert_equal(
+                self.ms_wo_wallet.getnewaddress(address_type=addr_type),
+                self.funder.deriveaddresses(desc)[0],
+            )
 
         self.log.info("Testing we detect funds sent to one of them")
         addr = self.ms_wo_wallet.getnewaddress()
@@ -254,27 +275,34 @@ def signing_test(
     ):
         self.log.info(f"Importing private Miniscript descriptor '{desc}'")
         is_taproot = desc.startswith("tr(")
+        is_silent_payments = desc.startswith("sp(")
         desc = descsum_create(desc)
-        res = self.ms_sig_wallet.importdescriptors(
-            [
-                {
-                    "desc": desc,
-                    "active": True,
-                    "range": 0,
-                    "next_index": 0,
-                    "timestamp": "now",
-                }
-            ]
-        )
+
+        range = None if desc.startswith("sp(") else 2
+
+        import_desc_data = {
+            "desc": desc,
+            "active": True,
+            "next_index": 0,
+            "timestamp": "now",
+        }
+        if range is not None:
+            import_desc_data["range"] = range
+        res = self.ms_sig_wallet.importdescriptors([import_desc_data,])
         assert res[0]["success"], res
 
         self.log.info("Generating an address for it and testing it detects funds")
-        addr_type = "bech32m" if is_taproot else "bech32"
+        addr_type = "bech32m"
+        if is_silent_payments:
+            addr_type = "silent-payment"
+        elif is_taproot == False:
+            addr_type = "bech32"
         addr = self.ms_sig_wallet.getnewaddress(address_type=addr_type)
         txid = self.funder.sendtoaddress(addr, 0.01)
         self.wait_until(lambda: txid in self.funder.getrawmempool())
         self.funder.generatetoaddress(1, self.funder.getnewaddress())
-        utxo = self.ms_sig_wallet.listunspent(addresses=[addr])[0]
+        utxos = self.ms_sig_wallet.listunspent()
+        utxo = [utxo for utxo in utxos if utxo["txid"] == txid][0]
         assert txid == utxo["txid"] and utxo["solvable"]
 
         self.log.info("Creating a transaction spending these funds")
@@ -302,8 +330,16 @@ def signing_test(
             psbt = psbt.to_base64()
         res = self.ms_sig_wallet.walletprocesspsbt(psbt=psbt, finalize=False)
         psbtin = self.nodes[0].rpc.decodepsbt(res["psbt"])["inputs"][0]
-        sigs_field_name = "taproot_script_path_sigs" if is_taproot else "partial_signatures"
-        assert len(psbtin[sigs_field_name]) == sigs_count
+
+        actual_sig_count = 0
+        if is_taproot:
+            actual_sig_count = len(psbtin["taproot_script_path_sigs"])
+        elif is_silent_payments:
+            actual_sig_count = 1
+        else:
+            actual_sig_count = len(psbtin["partial_signatures"])
+        assert actual_sig_count == sigs_count
+
         res = self.ms_sig_wallet.finalizepsbt(res["psbt"])
         assert res["complete"] == (stack_size is not None)
 
@@ -326,10 +362,10 @@ def run_test(self):
         self.log.info("Making a descriptor wallet")
         self.funder = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
         self.nodes[0].createwallet(
-            wallet_name="ms_wo", descriptors=True, disable_private_keys=True
+            wallet_name="ms_wo", descriptors=True, disable_private_keys=True, silent_payment=True
         )
         self.ms_wo_wallet = self.nodes[0].get_wallet_rpc("ms_wo")
-        self.nodes[0].createwallet(wallet_name="ms_sig", descriptors=True)
+        self.nodes[0].createwallet(wallet_name="ms_sig", descriptors=True, silent_payment=True)
         self.ms_sig_wallet = self.nodes[0].get_wallet_rpc("ms_sig")
 
         # Sanity check we wouldn't let an insane Miniscript descriptor in
diff --git a/test/functional/wallet_silentpayments_receiving.py b/test/functional/wallet_silentpayments_receiving.py
index db7457d2d3d62c..c3b5eee99f6a7c 100755
--- a/test/functional/wallet_silentpayments_receiving.py
+++ b/test/functional/wallet_silentpayments_receiving.py
@@ -83,6 +83,8 @@ def test_basic(self):
         wallet = self.nodes[0].get_wallet_rpc("basic")
 
         addr = wallet.getnewaddress(address_type="silent-payment")
+        addr_again = wallet.getnewaddress(address_type="silent-payment")
+        assert addr == addr_again
         txid = self.def_wallet.sendtoaddress(addr, 10)
         self.generate(self.nodes[0], 1)
 
@@ -90,9 +92,9 @@ def test_basic(self):
         wallet.gettransaction(txid)
 
         self.log.info("Test getnewaddress returns new labelled address")
-        new_addr = wallet.getnewaddress(address_type="silent-payment")
-        assert new_addr != addr
-        txid = self.def_wallet.sendtoaddress(new_addr, 10)
+        labeled_addr = wallet.getnewaddress(address_type="silent-payment", label="foo")
+        assert labeled_addr != addr
+        txid = self.def_wallet.sendtoaddress(labeled_addr, 10)
         self.generate(self.nodes[0], 1)
 
         assert_equal(wallet.getbalance(), 20)