Merge bitcoin/bitcoin#22156: Allow tr() import only when Taproot is active

laanwj · laanwj · commit b0e5fbf6fad8 · 2021-06-12T17:22:41.000+02:00
fbf485c Allow tr() import only when Taproot is active (Andrew Chow) Pull request description: To avoid issues around fund loss, only allow descriptor wallets to import `tr()` descriptors after taproot has activated. ACKs for top commit: sipa: utACK fbf485c fjahr: Code review ACK fbf485c laanwj: Code review ACK fbf485c prayank23: utACK bitcoin/bitcoin@fbf485c Tree-SHA512: 83c43376515eea523dbc89bc5a0fde53e54aec492e49a40c2a33d80fc94aac459e232ae07b024b4bd75b58078c8d090bc7a2d69541c5d3d4834d2f4cfc9c8208
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
@@ -277,6 +277,9 @@ class Chain
     //! to be prepared to handle this by ignoring notifications about unknown
     //! removed transactions and already added new transactions.
     virtual void requestMempoolTransactions(Notifications& notifications) = 0;
+
+    //! Check if Taproot has activated
+    virtual bool isTaprootActive() const = 0;
 };
 
 //! Interface to let node manage chain clients (wallets, or maybe tools for
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
@@ -679,6 +679,12 @@ class ChainImpl : public Chain
             notifications.transactionAddedToMempool(entry.GetSharedTx(), 0 /* mempool_sequence */);
         }
     }
+    bool isTaprootActive() const override
+    {
+        LOCK(::cs_main);
+        const CBlockIndex* tip = Assert(m_node.chainman)->ActiveChain().Tip();
+        return VersionBitsState(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_TAPROOT, versionbitscache) == ThresholdState::ACTIVE;
+    }
     NodeContext& m_node;
 };
 } // namespace
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
@@ -1530,6 +1530,18 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c
             }
         }
 
+        // Taproot descriptors cannot be imported if Taproot is not yet active.
+        // Check if this is a Taproot descriptor
+        CTxDestination dest;
+        ExtractDestination(scripts[0], dest);
+        if (std::holds_alternative<WitnessV1Taproot>(dest)) {
+            // Check if Taproot is active
+            if (!wallet.chain().isTaprootActive()) {
+                // Taproot is not active, raise an error
+                throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import tr() descriptor when Taproot is not active");
+            }
+        }
+
         // If private keys are enabled, check some things.
         if (!wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
            if (keys.keys.empty()) {
diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py
@@ -172,9 +172,9 @@ class WalletTaprootTest(BitcoinTestFramework):
     """Test generation and spending of P2TR address outputs."""
 
     def set_test_params(self):
-        self.num_nodes = 2
+        self.num_nodes = 3
         self.setup_clean_chain = True
-        self.extra_args = [['-keypool=100'], ['-keypool=100']]
+        self.extra_args = [['-keypool=100'], ['-keypool=100'], ["-vbparams=taproot:1:1"]]
         self.supports_cli = False
 
     def skip_test_if_missing_module(self):
@@ -230,12 +230,34 @@ def do_test_addr(self, comment, pattern, privmap, treefn, keys):
                 addr_r = self.make_addr(treefn, keys, i)
                 assert_equal(addr_g, addr_r)
 
+        # tr descriptors cannot be imported when Taproot is not active
+        result = self.privs_tr_enabled.importdescriptors([{"desc": desc, "timestamp": "now"}])
+        assert(result[0]["success"])
+        result = self.privs_tr_disabled.importdescriptors([{"desc": desc, "timestamp": "now"}])
+        assert(not result[0]["success"])
+        assert_equal(result[0]["error"]["code"], -4)
+        assert_equal(result[0]["error"]["message"], "Cannot import tr() descriptor when Taproot is not active")
+        result = self.pubs_tr_enabled.importdescriptors([{"desc": desc_pub, "timestamp": "now"}])
+        assert(result[0]["success"])
+        result = self.pubs_tr_disabled.importdescriptors([{"desc": desc_pub, "timestamp": "now"}])
+        assert(not result[0]["success"])
+        assert_equal(result[0]["error"]["code"], -4)
+        assert_equal(result[0]["error"]["message"], "Cannot import tr() descriptor when Taproot is not active")
+
     def do_test(self, comment, pattern, privmap, treefn, nkeys):
         keys = self.rand_keys(nkeys)
         self.do_test_addr(comment, pattern, privmap, treefn, keys)
 
     def run_test(self):
         self.log.info("Creating wallets...")
+        self.nodes[0].createwallet(wallet_name="privs_tr_enabled", descriptors=True, blank=True)
+        self.privs_tr_enabled = self.nodes[0].get_wallet_rpc("privs_tr_enabled")
+        self.nodes[2].createwallet(wallet_name="privs_tr_disabled", descriptors=True, blank=True)
+        self.privs_tr_disabled=self.nodes[2].get_wallet_rpc("privs_tr_disabled")
+        self.nodes[0].createwallet(wallet_name="pubs_tr_enabled", descriptors=True, blank=True, disable_private_keys=True)
+        self.pubs_tr_enabled = self.nodes[0].get_wallet_rpc("pubs_tr_enabled")
+        self.nodes[2].createwallet(wallet_name="pubs_tr_disabled", descriptors=True, blank=True, disable_private_keys=True)
+        self.pubs_tr_disabled=self.nodes[2].get_wallet_rpc("pubs_tr_disabled")
         self.nodes[0].createwallet(wallet_name="addr_gen", descriptors=True, disable_private_keys=True, blank=True)
         self.addr_gen = self.nodes[0].get_wallet_rpc("addr_gen")
 

Original file line number	Diff line number	Diff line change
`@@ -679,6 +679,12 @@ class ChainImpl : public Chain`
`679`	`679`	`notifications.transactionAddedToMempool(entry.GetSharedTx(), 0 /* mempool_sequence */);`
`680`	`680`	`}`
`681`	`681`	`}`
	`682`	`+ bool isTaprootActive() const override`
	`683`	`+ {`
	`684`	`+ LOCK(::cs_main);`
	`685`	`+ const CBlockIndex* tip = Assert(m_node.chainman)->ActiveChain().Tip();`
	`686`	`+ return VersionBitsState(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_TAPROOT, versionbitscache) == ThresholdState::ACTIVE;`
	`687`	`+ }`
`682`	`688`	`NodeContext& m_node;`
`683`	`689`	`};`
`684`	`690`	`} // namespace`