Merge branch 'opt_wallet_segwit2' into timebomb_knots

Author: luke-jr

src/dummywallet.cpp

@@ -45,6 +45,7 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const
         "-wallet=<path>",
         "-walletbroadcast",
         "-walletdir=<dir>",
+        "-walletimplicitsegwit",
         "-walletnotify=<cmd>",
         "-walletrbf",
         "-dblogsize=<n>",

src/outputtype.cpp

@@ -72,7 +72,7 @@ std::vector<CTxDestination> GetAllDestinationsForKey(const CPubKey& key)
 {
     PKHash keyid(key);
     CTxDestination p2pkh{keyid};
-    if (key.IsCompressed()) {
+    if (key.IsCompressed() && g_implicit_segwit) {
         CTxDestination segwit = WitnessV0KeyHash(keyid);
         CTxDestination p2sh = ScriptHash(GetScriptForDestination(segwit));
         return Vector(std::move(p2pkh), std::move(p2sh), std::move(segwit));

src/script/descriptor.cpp

@@ -823,8 +823,11 @@ class PKDescriptor final : public DescriptorImpl
 private:
     const bool m_xonly;
 protected:
-    std::vector<CScript> MakeScripts(const std::vector<CPubKey>& keys, Span<const CScript>, FlatSigningProvider&) const override
+    std::vector<CScript> MakeScripts(const std::vector<CPubKey>& keys, Span<const CScript>, FlatSigningProvider& out) const override
     {
+        CKeyID id = keys[0].GetID();
+        out.pubkeys.emplace(id, keys[0]);
+
         if (m_xonly) {
             CScript script = CScript() << ToByteVector(XOnlyPubKey(keys[0])) << OP_CHECKSIG;
             return Vector(std::move(script));

src/script/signingprovider.cpp

@@ -9,6 +9,8 @@
 
 #include <logging.h>
 
+bool g_implicit_segwit = true;
+
 const SigningProvider& DUMMY_SIGNING_PROVIDER = SigningProvider();
 
 template<typename M, typename K, typename V>
@@ -102,7 +104,7 @@ void FillableSigningProvider::ImplicitlyLearnRelatedKeyScripts(const CPubKey& pu
     // "Implicitly" refers to fact that scripts are derived automatically from
     // existing keys, and are present in memory, even without being explicitly
     // loaded (e.g. from a file).
-    if (pubkey.IsCompressed()) {
+    if (pubkey.IsCompressed() && g_implicit_segwit) {
         CScript script = GetScriptForDestination(WitnessV0KeyHash(key_id));
         // This does not use AddCScript, as it may be overridden.
         CScriptID id(script);

src/script/signingprovider.h

@@ -14,6 +14,10 @@
 #include <script/script.h>
 #include <sync.h>
 
+static const bool DEFAULT_WALLET_IMPLICIT_SEGWIT = false;
+
+extern bool g_implicit_segwit;
+
 struct ShortestVectorFirstComparator
 {
     bool operator()(const std::vector<unsigned char>& a, const std::vector<unsigned char>& b) const

src/wallet/init.cpp

@@ -14,6 +14,7 @@
 #include <node/context.h>
 #include <node/interface_ui.h>
 #include <outputtype.h>
+#include <script/signingprovider.h>
 #include <univalue.h>
 #include <util/check.h>
 #include <util/moneystr.h>
@@ -76,6 +77,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
     argsman.AddArg("-wallet=<path>", "Specify wallet path to load at startup. Can be used multiple times to load multiple wallets. Path is to a directory containing wallet data and log files. If the path is not absolute, it is interpreted relative to <walletdir>. This only loads existing wallets and does not create new ones. For backwards compatibility this also accepts names of existing top-level data files in <walletdir>.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET);
     argsman.AddArg("-walletbroadcast",  strprintf("Make the wallet broadcast transactions (default: %u)", DEFAULT_WALLETBROADCAST), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-walletdir=<dir>", "Specify directory to hold wallets (default: <datadir>/wallets if it exists, otherwise <datadir>)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET);
+    argsman.AddArg("-walletimplicitsegwit", strprintf("Support segwit when restoring wallet backups and importing keys (default: %u)", DEFAULT_WALLET_IMPLICIT_SEGWIT), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
 #if HAVE_SYSTEM
     argsman.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
 #endif
@@ -119,6 +121,13 @@ bool WalletInit::ParameterInteraction() const
         LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__);
     }
 
+    g_implicit_segwit = gArgs.GetBoolArg("-walletimplicitsegwit", DEFAULT_WALLET_IMPLICIT_SEGWIT);
+    if (!g_implicit_segwit) {
+        if (gArgs.SoftSetArg("-addresstype", "legacy")) {
+            LogPrintf("%s: parameter interaction: -walletimplicitsegwit=%u -> setting -addresstype=legacy\n", __func__, g_implicit_segwit);
+        }
+    }
+
     return true;
 }
 

src/wallet/rpc/addresses.cpp

@@ -531,6 +531,7 @@ RPCHelpMan getaddressinfo()
                         {RPCResult::Type::STR, "address", "The bitcoin address validated."},
                         {RPCResult::Type::STR_HEX, "scriptPubKey", "The hex-encoded output script generated by the address."},
                         {RPCResult::Type::BOOL, "ismine", "If the address is yours."},
+                        {RPCResult::Type::BOOL, "isactive", "If the key is in the active keypool (always equal to \"ismine\" in descriptor wallets)."},
                         {RPCResult::Type::BOOL, "iswatchonly", "If the address is watchonly."},
                         {RPCResult::Type::BOOL, "solvable", "If we know how to spend coins sent to this address, ignoring the possible lack of private keys."},
                         {RPCResult::Type::STR, "desc", /*optional=*/true, "A descriptor for spending coins sent to this address (only when solvable)."},
@@ -601,6 +602,7 @@ RPCHelpMan getaddressinfo()
 
     isminetype mine = pwallet->IsMine(dest);
     ret.pushKV("ismine", bool(mine & ISMINE_SPENDABLE));
+    ret.pushKV("isactive", pwallet->IsDestinationActive(dest));
 
     if (provider) {
         auto inferred = InferDescriptor(scriptPubKey, *provider);

src/wallet/scriptpubkeyman.cpp

@@ -402,6 +402,23 @@ std::vector<WalletDestination> LegacyScriptPubKeyMan::MarkUnusedAddresses(const
     return result;
 }
 
+bool LegacyDataSPKM::IsKeyActive(const CScript& script) const
+{
+    LOCK(cs_KeyStore);
+
+    if (!IsMine(script)) return false; // Not in the keystore at all
+
+    for (const auto& key_id : GetAffectedKeys(script, *this)) {
+        const auto it = mapKeyMetadata.find(key_id);
+        if (it == mapKeyMetadata.end()) return false; // This key must be really old
+
+        if (!it->second.hd_seed_id.IsNull() && it->second.hd_seed_id == m_hd_chain.seed_id) return true;
+    }
+
+    // Imported or dumped for a new keypool
+    return false;
+}
+
 void LegacyScriptPubKeyMan::UpgradeKeyMetadata()
 {
     LOCK(cs_KeyStore);
@@ -1505,6 +1522,7 @@ void LegacyScriptPubKeyMan::LearnRelatedScripts(const CPubKey& key, OutputType t
 
 void LegacyScriptPubKeyMan::LearnAllRelatedScripts(const CPubKey& key)
 {
+    if (!g_implicit_segwit) return;
     // OutputType::P2SH_SEGWIT always adds all necessary scripts for all types.
     LearnRelatedScripts(key, OutputType::P2SH_SEGWIT);
 }

src/wallet/scriptpubkeyman.h

@@ -205,6 +205,9 @@ class ScriptPubKeyMan
      */
     virtual std::vector<WalletDestination> MarkUnusedAddresses(const CScript& script) { return {}; }
 
+    /* Determines if address is derived from active key manager */
+    virtual bool IsKeyActive(const CScript& script) const = 0;
+
     /** Sets up the key generation stuff, i.e. generates new HD seeds and sets them as active.
       * Returns false if already setup or setup fails, true if setup is successful
       * Set force=true to make it re-setup if already setup, used for upgrades
@@ -313,6 +316,7 @@ class LegacyDataSPKM : public ScriptPubKeyMan, public FillableSigningProvider
 
     // ScriptPubKeyMan overrides
     bool CheckDecryptionKey(const CKeyingMaterial& master_key) override;
+    [[nodiscard]] bool IsKeyActive(const CScript& script) const override;
     std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override;
     std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script) const override;
     uint256 GetID() const override { return uint256::ONE; }
@@ -650,6 +654,8 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
 
     std::vector<WalletDestination> MarkUnusedAddresses(const CScript& script) override;
 
+    [[nodiscard]] bool IsKeyActive(const CScript& script) const override { return IsMine(script); }
+
     bool IsHDEnabled() const override;
 
     //! Setup descriptors based on the given CExtkey

src/wallet/test/scriptpubkeyman_tests.cpp

@@ -40,5 +40,174 @@ BOOST_AUTO_TEST_CASE(CanProvide)
     BOOST_CHECK(keyman.CanProvide(p2sh_script, data));
 }
 
+static void legacy_IsKeyActive(const node::NodeContext& node, bool implicit_segwit)
+{
+    const bool save_g_implicit_segwit{g_implicit_segwit};
+    g_implicit_segwit = implicit_segwit;
+    CWallet wallet(node.chain.get(), "", CreateMockableWalletDatabase());
+    {
+        LOCK(wallet.cs_wallet);
+        wallet.SetMinVersion(FEATURE_LATEST);
+        wallet.m_keypool_size = 10;
+    }
+    LegacyScriptPubKeyMan& spkm = *wallet.GetOrCreateLegacyScriptPubKeyMan();
+
+    // Start off empty
+    BOOST_CHECK(spkm.GetScriptPubKeys().empty());
+
+    // Generate 20 keypool keys (10 internal, 10 external)
+    {
+        LOCK(wallet.cs_wallet);
+        spkm.SetupGeneration();
+    }
+
+    // 4 scripts per keypool key (P2PK, P2PKH, P2WPKH, P2SH-P2WPKH)
+    // Plus 4 scripts for the seed key
+    // (If !implicit_segwit, P2WPKH and P2SH-P2WPKH are not generated.)
+    auto scripts1 = spkm.GetScriptPubKeys();
+    BOOST_CHECK_EQUAL(scripts1.size(), implicit_segwit ? 84 : 42);
+
+    // All keys are active
+    for (const CScript& script : scripts1) {
+        BOOST_CHECK(spkm.IsKeyActive(script));
+    }
+
+    // Requesting single from spkm should not deactivate key
+    CTxDestination dest1;
+    {
+        LOCK(wallet.cs_wallet);
+        auto result = spkm.GetNewDestination(OutputType::BECH32);
+        dest1 = result.value();
+    }
+    CScript script = GetScriptForDestination(dest1);
+    BOOST_CHECK(spkm.IsKeyActive(script));
+
+    // Key pool size did not change
+    // (If !implicit_segwit, the two segwit addresses are added back.)
+    auto scripts2 = spkm.GetScriptPubKeys();
+    BOOST_CHECK_EQUAL(scripts2.size(), implicit_segwit ? 84 : 44);
+
+    // Use key that is not the next key
+    // (i.e. address gap in wallet recovery)
+    {
+        LOCK(wallet.cs_wallet);
+        LOCK(spkm.cs_KeyStore);
+        auto keys = spkm.MarkReserveKeysAsUsed(5);
+        BOOST_CHECK_EQUAL(keys.size(), 4); // Because we already used one with GetNewDestination
+    }
+
+    // Key pool size did not change
+    auto scripts3 = spkm.GetScriptPubKeys();
+    BOOST_CHECK_EQUAL(scripts3.size(), implicit_segwit ? 84 : 44);
+
+    // All keys are still active
+    for (const CScript& script : scripts3) {
+        BOOST_CHECK(spkm.IsKeyActive(script));
+    }
+
+    // When user encrypts wallet for the first time,
+    // all existing keys are removed from active keypool
+    {
+        LOCK(wallet.cs_wallet);
+        // called by EncryptWallet()
+        spkm.SetupGeneration(true);
+    }
+
+    // 20 new keys were added
+    auto scripts4 = spkm.GetScriptPubKeys();
+    BOOST_CHECK_EQUAL(scripts4.size(), (implicit_segwit ? 84 : 43) * 2);
+
+    // All 10 original keys are now inactive
+    for (const CScript& script : scripts3) {
+        BOOST_CHECK(!spkm.IsKeyActive(script));
+    }
+    g_implicit_segwit = save_g_implicit_segwit;
+}
+
+BOOST_AUTO_TEST_CASE(Legacy_IsKeyActive)
+{
+    legacy_IsKeyActive(m_node, /*implicit_segwit=*/true);
+}
+
+BOOST_AUTO_TEST_CASE(Legacy_IsKeyActive_no_implicit_segwit)
+{
+    legacy_IsKeyActive(m_node, /*implicit_segwit=*/false);
+}
+
+BOOST_AUTO_TEST_CASE(Descriptor_IsKeyActive)
+{
+    CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
+    {
+        LOCK(wallet.cs_wallet);
+        wallet.LoadMinVersion(FEATURE_LATEST);
+        wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+        wallet.m_keypool_size = 10;
+        wallet.SetupDescriptorScriptPubKeyMans();
+    }
+    DescriptorScriptPubKeyMan* spkm = dynamic_cast<DescriptorScriptPubKeyMan*>(wallet.GetScriptPubKeyMan(OutputType::BECH32, /*internal=*/false));
+
+    // Start off with 10 pre-generated keys, 1 script each
+    auto scripts1 = spkm->GetScriptPubKeys();
+    BOOST_CHECK_EQUAL(scripts1.size(), 10);
+
+    // All keys are active
+    for (const CScript& script : scripts1) {
+        BOOST_CHECK(spkm->IsKeyActive(script));
+    }
+
+    // Requesting single key from spkm should not deactivate key
+    auto dest1 = spkm->GetNewDestination(OutputType::BECH32);
+    CScript script = GetScriptForDestination(dest1.value());
+    BOOST_CHECK(spkm->IsKeyActive(script));
+
+    // Key pool size did not change
+    auto scripts2 = spkm->GetScriptPubKeys();
+    BOOST_CHECK_EQUAL(scripts2.size(), 10);
+
+    // Use key that is not the next key
+    // (i.e. address gap in wallet recovery)
+    {
+        LOCK(spkm->cs_desc_man);
+        WalletDescriptor descriptor = spkm->GetWalletDescriptor();
+        FlatSigningProvider provider;
+        std::vector<CScript> scripts3;
+        descriptor.descriptor->ExpandFromCache(/*pos=*/5, descriptor.cache, scripts3, provider);
+
+        BOOST_CHECK_EQUAL(scripts3.size(), 1);
+        spkm->MarkUnusedAddresses(scripts3.front());
+    }
+
+    // Key pool size increased to replace used keys
+    auto scripts4 = spkm->GetScriptPubKeys();
+    BOOST_CHECK_EQUAL(scripts4.size(), 16);
+
+    // All keys are still active
+    for (const CScript& script : scripts4) {
+        BOOST_CHECK(spkm->IsKeyActive(script));
+    }
+
+    // When user encrypts wallet for the first time,
+    // all existing keys are removed from active keypool
+    {
+        LOCK(wallet.cs_wallet);
+        // called by EncryptWallet()
+        wallet.SetupDescriptorScriptPubKeyMans();
+    }
+
+    // This SPKM is not affected
+    for (const CScript& script : scripts4) {
+        BOOST_CHECK(spkm->IsKeyActive(script));
+    }
+
+    // ...but at the wallet level all the keys from that SPKM are deactivated
+    int num_script_keys_not_found = 0;
+    for (const CScript& script : scripts4) {
+        if (!wallet.IsDestinationActive(WitnessV0ScriptHash(script))) {
+            ++num_script_keys_not_found;
+        }
+    }
+    BOOST_CHECK_EQUAL(num_script_keys_not_found, 16);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet