From be62903c417293f6217e124669e62fd2172a18f1 Mon Sep 17 00:00:00 2001
From: Pieter Wuille <pieter.wuille@gmail.com>
Date: Tue, 12 Feb 2019 18:56:53 -0800
Subject: [PATCH] Make descriptor checksums mandatory in deriveaddresses and
 importmulti

---
 src/rpc/misc.cpp                       |  4 +--
 src/wallet/rpcdump.cpp                 |  2 +-
 test/functional/rpc_deriveaddresses.py | 28 +++++++++--------
 test/functional/wallet_importmulti.py  | 42 +++++++++++++++++---------
 4 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index bfb86aa12b..999a307e2b 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -207,7 +207,7 @@ UniValue deriveaddresses(const JSONRPCRequest& request)
             },
             RPCExamples{
                 "First three native segwit receive addresses\n" +
-                HelpExampleCli("deriveaddresses", "\"wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)\" 0 2")
+                HelpExampleCli("deriveaddresses", "\"wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#trd0mf0l\" 0 2")
             }}.ToString()
         );
     }
@@ -233,7 +233,7 @@ UniValue deriveaddresses(const JSONRPCRequest& request)
     }
 
     FlatSigningProvider provider;
-    auto desc = Parse(desc_str, provider);
+    auto desc = Parse(desc_str, provider, /* require_checksum = */ true);
     if (!desc) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor"));
     }
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index 930274f8a1..66a52c1270 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -1117,7 +1117,7 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
 
     const std::string& descriptor = data["desc"].get_str();
     FlatSigningProvider keys;
-    auto parsed_desc = Parse(descriptor, keys);
+    auto parsed_desc = Parse(descriptor, keys, /* require_checksum = */ true);
     if (!parsed_desc) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Descriptor is invalid");
     }
diff --git a/test/functional/rpc_deriveaddresses.py b/test/functional/rpc_deriveaddresses.py
index 2cc5bc974b..c8b58cfc74 100755
--- a/test/functional/rpc_deriveaddresses.py
+++ b/test/functional/rpc_deriveaddresses.py
@@ -4,6 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the deriveaddresses rpc call."""
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.descriptors import descsum_create
 from test_framework.util import assert_equal, assert_raises_rpc_error
 
 class DeriveaddressesTest(BitcoinTestFramework):
@@ -14,36 +15,37 @@ class DeriveaddressesTest(BitcoinTestFramework):
     def run_test(self):
         assert_raises_rpc_error(-5, "Invalid descriptor", self.nodes[0].deriveaddresses, "a")
 
-        descriptor = "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)"
+        descriptor = "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)#t6wfjs64"
         address = "bcrt1qjqmxmkpmxt80xz4y3746zgt0q3u3ferr34acd5"
-
         assert_equal(self.nodes[0].deriveaddresses(descriptor), [address])
 
-        descriptor_pubkey = "wpkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/0)"
-        address = "bcrt1qjqmxmkpmxt80xz4y3746zgt0q3u3ferr34acd5"
+        descriptor = descriptor[:-9]
+        assert_raises_rpc_error(-5, "Invalid descriptor", self.nodes[0].deriveaddresses, descriptor)
 
+        descriptor_pubkey = "wpkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/0)#s9ga3alw"
+        address = "bcrt1qjqmxmkpmxt80xz4y3746zgt0q3u3ferr34acd5"
         assert_equal(self.nodes[0].deriveaddresses(descriptor_pubkey), [address])
 
-        ranged_descriptor = "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"
+        ranged_descriptor = "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)#kft60nuy"
         assert_equal(self.nodes[0].deriveaddresses(ranged_descriptor, 0, 2), [address, "bcrt1qhku5rq7jz8ulufe2y6fkcpnlvpsta7rq4442dy", "bcrt1qpgptk2gvshyl0s9lqshsmx932l9ccsv265tvaq"])
 
-        assert_raises_rpc_error(-8, "Range should not be specified for an un-ranged descriptor", self.nodes[0].deriveaddresses, "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)", 0, 2)
+        assert_raises_rpc_error(-8, "Range should not be specified for an un-ranged descriptor", self.nodes[0].deriveaddresses, descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)"), 0, 2)
 
-        assert_raises_rpc_error(-8, "Range must be specified for a ranged descriptor", self.nodes[0].deriveaddresses, "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)")
+        assert_raises_rpc_error(-8, "Range must be specified for a ranged descriptor", self.nodes[0].deriveaddresses, descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"))
 
-        assert_raises_rpc_error(-8, "Missing range end parameter", self.nodes[0].deriveaddresses, "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)", 0)
+        assert_raises_rpc_error(-8, "Missing range end parameter", self.nodes[0].deriveaddresses, descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), 0)
 
-        assert_raises_rpc_error(-8, "Range end should be equal to or greater than begin", self.nodes[0].deriveaddresses, "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)", 2, 0)
+        assert_raises_rpc_error(-8, "Range end should be equal to or greater than begin", self.nodes[0].deriveaddresses, descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), 2, 0)
 
-        assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].deriveaddresses, "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)", -1, 0)
+        assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].deriveaddresses, descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), -1, 0)
 
-        combo_descriptor = "combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)"
+        combo_descriptor = descsum_create("combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)")
         assert_equal(self.nodes[0].deriveaddresses(combo_descriptor), ["mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", "mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", address, "2NDvEwGfpEqJWfybzpKPHF2XH3jwoQV3D7x"])
 
-        hardened_without_privkey_descriptor = "wpkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1'/1/0)"
+        hardened_without_privkey_descriptor = descsum_create("wpkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1'/1/0)")
         assert_raises_rpc_error(-5, "Cannot derive script without private keys", self.nodes[0].deriveaddresses, hardened_without_privkey_descriptor)
 
-        bare_multisig_descriptor = "multi(1, tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/0, tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/1)"
+        bare_multisig_descriptor = descsum_create("multi(1,tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/0,tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/1)")
         assert_raises_rpc_error(-5, "Descriptor does not have a corresponding address", self.nodes[0].deriveaddresses, bare_multisig_descriptor)
 
 if __name__ == '__main__':
diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py
index 46e3ab77c8..8e25aa7337 100755
--- a/test/functional/wallet_importmulti.py
+++ b/test/functional/wallet_importmulti.py
@@ -20,6 +20,7 @@ from test_framework.script import (
     OP_NOP,
 )
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.descriptors import descsum_create
 from test_framework.util import (
     assert_equal,
     assert_greater_than,
@@ -545,11 +546,22 @@ class ImportMultiTest(BitcoinTestFramework):
 
         # Test importing of a P2SH-P2WPKH address via descriptor + private key
         key = get_key(self.nodes[0])
-        self.log.info("Should import a p2sh-p2wpkh address from descriptor and private key")
+        self.log.info("Should not import a p2sh-p2wpkh address from descriptor without checksum and private key")
         self.test_importmulti({"desc": "sh(wpkh(" + key.pubkey + "))",
                                "timestamp": "now",
                                "label": "Descriptor import test",
                                "keys": [key.privkey]},
+                              success=False,
+                              error_code=-5,
+                              error_message="Descriptor is invalid")
+
+        # Test importing of a P2SH-P2WPKH address via descriptor + private key
+        key = get_key(self.nodes[0])
+        self.log.info("Should import a p2sh-p2wpkh address from descriptor and private key")
+        self.test_importmulti({"desc": descsum_create("sh(wpkh(" + key.pubkey + "))"),
+                               "timestamp": "now",
+                               "label": "Descriptor import test",
+                               "keys": [key.privkey]},
                               success=True)
         test_address(self.nodes[1],
                      key.p2sh_p2wpkh_addr,
@@ -562,7 +574,7 @@ class ImportMultiTest(BitcoinTestFramework):
         addresses = ["2N7yv4p8G8yEaPddJxY41kPihnWvs39qCMf", "2MsHxyb2JS3pAySeNUsJ7mNnurtpeenDzLA"] # hdkeypath=m/0'/0'/0' and 1'
         desc = "sh(wpkh(" + xpriv + "/0'/0'/*'" + "))"
         self.log.info("Ranged descriptor import should fail without a specified range")
-        self.test_importmulti({"desc": desc,
+        self.test_importmulti({"desc": descsum_create(desc),
                                "timestamp": "now"},
                               success=False,
                               error_code=-8,
@@ -570,7 +582,7 @@ class ImportMultiTest(BitcoinTestFramework):
 
         # Test importing of a ranged descriptor without keys
         self.log.info("Should import the ranged descriptor with specified range as solvable")
-        self.test_importmulti({"desc": desc,
+        self.test_importmulti({"desc": descsum_create(desc),
                                "timestamp": "now",
                                "range": {"end": 1}},
                               success=True,
@@ -583,7 +595,7 @@ class ImportMultiTest(BitcoinTestFramework):
         # Test importing of a P2PKH address via descriptor
         key = get_key(self.nodes[0])
         self.log.info("Should import a p2pkh address from descriptor")
-        self.test_importmulti({"desc": "pkh(" + key.pubkey + ")",
+        self.test_importmulti({"desc": descsum_create("pkh(" + key.pubkey + ")"),
                                "timestamp": "now",
                                "label": "Descriptor import test"},
                               True,
@@ -597,7 +609,7 @@ class ImportMultiTest(BitcoinTestFramework):
         # Test import fails if both desc and scriptPubKey are provided
         key = get_key(self.nodes[0])
         self.log.info("Import should fail if both scriptPubKey and desc are provided")
-        self.test_importmulti({"desc": "pkh(" + key.pubkey + ")",
+        self.test_importmulti({"desc": descsum_create("pkh(" + key.pubkey + ")"),
                                "scriptPubKey": {"address": key.p2pkh_addr},
                                "timestamp": "now"},
                               success=False,
@@ -616,7 +628,7 @@ class ImportMultiTest(BitcoinTestFramework):
         key1 = get_key(self.nodes[0])
         key2 = get_key(self.nodes[0])
         self.log.info("Should import a 1-of-2 bare multisig from descriptor")
-        self.test_importmulti({"desc": "multi(1," + key1.pubkey + "," + key2.pubkey + ")",
+        self.test_importmulti({"desc": descsum_create("multi(1," + key1.pubkey + "," + key2.pubkey + ")"),
                                "timestamp": "now"},
                               success=True)
         self.log.info("Should not treat individual keys from the imported bare multisig as watchonly")
@@ -635,7 +647,7 @@ class ImportMultiTest(BitcoinTestFramework):
         pub_fpr = info['hdmasterfingerprint']
         result = self.nodes[0].importmulti(
             [{
-                'desc' : "wpkh([" + pub_fpr + pub_keypath[1:] +"]" + pub + ")",
+                'desc' : descsum_create("wpkh([" + pub_fpr + pub_keypath[1:] +"]" + pub + ")"),
                 "timestamp": "now",
             }]
         )
@@ -653,7 +665,7 @@ class ImportMultiTest(BitcoinTestFramework):
         priv_fpr = info['hdmasterfingerprint']
         result = self.nodes[0].importmulti(
             [{
-                'desc' : "wpkh([" + priv_fpr + priv_keypath[1:] + "]" + priv + ")",
+                'desc' : descsum_create("wpkh([" + priv_fpr + priv_keypath[1:] + "]" + priv + ")"),
                 "timestamp": "now",
             }]
         )
@@ -701,12 +713,12 @@ class ImportMultiTest(BitcoinTestFramework):
         pub2 = self.nodes[0].getaddressinfo(addr2)['pubkey']
         result = wrpc.importmulti(
             [{
-                'desc': 'wpkh(' + pub1 + ')',
+                'desc': descsum_create('wpkh(' + pub1 + ')'),
                 'keypool': True,
                 "timestamp": "now",
             },
             {
-                'desc': 'wpkh(' + pub2 + ')',
+                'desc': descsum_create('wpkh(' + pub2 + ')'),
                 'keypool': True,
                 "timestamp": "now",
             }]
@@ -727,13 +739,13 @@ class ImportMultiTest(BitcoinTestFramework):
         pub2 = self.nodes[0].getaddressinfo(addr2)['pubkey']
         result = wrpc.importmulti(
             [{
-                'desc': 'wpkh(' + pub1 + ')',
+                'desc': descsum_create('wpkh(' + pub1 + ')'),
                 'keypool': True,
                 'internal': True,
                 "timestamp": "now",
             },
             {
-                'desc': 'wpkh(' + pub2 + ')',
+                'desc': descsum_create('wpkh(' + pub2 + ')'),
                 'keypool': True,
                 'internal': True,
                 "timestamp": "now",
@@ -755,7 +767,7 @@ class ImportMultiTest(BitcoinTestFramework):
         pub2 = self.nodes[0].getaddressinfo(addr2)['pubkey']
         result = wrpc.importmulti(
             [{
-                'desc': 'wsh(multi(2,' + pub1 + ',' + pub2 + '))',
+                'desc': descsum_create('wsh(multi(2,' + pub1 + ',' + pub2 + '))'),
                 'keypool': True,
                 "timestamp": "now",
             }]
@@ -769,7 +781,7 @@ class ImportMultiTest(BitcoinTestFramework):
         assert wrpc.getwalletinfo()['private_keys_enabled']
         result = wrpc.importmulti(
             [{
-                'desc': 'wpkh(' + pub1 + ')',
+                'desc': descsum_create('wpkh(' + pub1 + ')'),
                 'keypool': True,
                 "timestamp": "now",
             }]
@@ -792,7 +804,7 @@ class ImportMultiTest(BitcoinTestFramework):
         ]
         result = wrpc.importmulti(
             [{
-                'desc': 'wpkh([80002067/0h/0h]' + xpub + '/*)',
+                'desc': descsum_create('wpkh([80002067/0h/0h]' + xpub + '/*)'),
                 'keypool': True,
                 'timestamp': 'now',
                 'range' : {'start': 0, 'end': 4}