Merge 12146 via opt_wallet_segwit2

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/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>
@@ -112,7 +114,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 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/scriptpubkeyman.cpp

@@ -1573,6 +1573,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/test/scriptpubkeyman_tests.cpp

@@ -40,9 +40,11 @@ BOOST_AUTO_TEST_CASE(CanProvide)
     BOOST_CHECK(keyman.CanProvide(p2sh_script, data));
 }
 
-BOOST_AUTO_TEST_CASE(Legacy_IsKeyActive)
+static void legacy_IsKeyActive(const node::NodeContext& node, bool implicit_segwit)
 {
-    CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
+    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);
@@ -61,8 +63,9 @@ BOOST_AUTO_TEST_CASE(Legacy_IsKeyActive)
 
     // 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(), 84);
+    BOOST_CHECK_EQUAL(scripts1.size(), implicit_segwit ? 84 : 42);
 
     // All keys are active
     for (const CScript& script : scripts1) {
@@ -80,8 +83,9 @@ BOOST_AUTO_TEST_CASE(Legacy_IsKeyActive)
     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(), 84);
+    BOOST_CHECK_EQUAL(scripts2.size(), implicit_segwit ? 84 : 44);
 
     // Use key that is not the next key
     // (i.e. address gap in wallet recovery)
@@ -94,7 +98,7 @@ BOOST_AUTO_TEST_CASE(Legacy_IsKeyActive)
 
     // Key pool size did not change
     auto scripts3 = spkm.GetScriptPubKeys();
-    BOOST_CHECK_EQUAL(scripts3.size(), 84);
+    BOOST_CHECK_EQUAL(scripts3.size(), implicit_segwit ? 84 : 44);
 
     // All keys are still active
     for (const CScript& script : scripts3) {
@@ -111,12 +115,23 @@ BOOST_AUTO_TEST_CASE(Legacy_IsKeyActive)
 
     // 20 new keys were added
     auto scripts4 = spkm.GetScriptPubKeys();
-    BOOST_CHECK_EQUAL(scripts4.size(), 84 * 2);
+    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)

test/functional/test_framework/test_node.py

@@ -129,6 +129,9 @@ class TestNode():
                          "--gen-suppressions=all", "--exit-on-first-error=yes",
                          "--error-exitcode=1", "--quiet"] + self.args
 
+        if self.version is None:
+            self.args.append("-walletimplicitsegwit")
+
         if self.version_is_at_least(190000):
             self.args.append("-logthreadnames")
         if self.version_is_at_least(219900):

test/functional/wallet_implicitsegwit.py

@@ -38,6 +38,18 @@ def check_implicit_transactions(implicit_keys, implicit_node):
             b_address = key_to_address(pubkey, b)
             assert ('receive', b_address) in tuple((tx['category'], tx['address']) for tx in txs)
 
+def check_explicit_transactions(explicit_keys, explicit_node):
+    # The explicit segwit node doesn't allow conversion from legacy to segwit
+    txs = explicit_node.listtransactions(None, 99999)
+    for a in address_types:
+        pubkey = explicit_keys[a]
+        for b in address_types:
+            b_address = key_to_address(pubkey, b)
+            if a == 'legacy' and a != b:
+                assert(('receive', b_address) not in tuple((tx['category'], tx['address']) for tx in txs))
+            else:
+                assert(('receive', b_address) in tuple((tx['category'], tx['address']) for tx in txs))
+
 class ImplicitSegwitTest(BitcoinTestFramework):
     def add_options(self, parser):
         self.add_wallet_options(parser, descriptors=False)
@@ -45,6 +57,14 @@ class ImplicitSegwitTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
         self.supports_cli = False
+        self.extra_args = [
+            [
+                "-walletimplicitsegwit=1",
+            ],
+            [
+                "-walletimplicitsegwit=0",
+            ],
+        ]
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
@@ -52,17 +72,20 @@ class ImplicitSegwitTest(BitcoinTestFramework):
     def run_test(self):
         self.log.info("Manipulating addresses and sending transactions to all variations")
         implicit_keys = send_a_to_b(self.nodes[0], self.nodes[1])
+        explicit_keys = send_a_to_b(self.nodes[1], self.nodes[0])
 
         self.sync_all()
 
         self.log.info("Checking that transactions show up correctly without a restart")
         check_implicit_transactions(implicit_keys, self.nodes[0])
+        check_explicit_transactions(explicit_keys, self.nodes[1])
 
         self.log.info("Checking that transactions still show up correctly after a restart")
         self.restart_node(0)
         self.restart_node(1)
 
         check_implicit_transactions(implicit_keys, self.nodes[0])
+        check_explicit_transactions(explicit_keys, self.nodes[1])
 
 if __name__ == '__main__':
     ImplicitSegwitTest(__file__).main()