Merge #11667: Add scripts to dumpwallet RPC

656fde5 Add script birthtime metadata to dump and import wallet (MeshCollider)
1bab9b2 Add script dump note to RPC help text and release notes (MeshCollider)
68c1e00 Add test for importwallet (MeshCollider)
9e1184d Add dumpwallet scripts test (MeshCollider)
ef0c730 Add scripts to importwallet RPC (MeshCollider)
b702ae8 Add CScripts to dumpwallet RPC (MeshCollider)
cdc260a Add GetCScripts to CBasicKeyStore (MeshCollider)

Pull request description:

  As discussed in https://github.com/bitcoin/bitcoin/pull/11289#issuecomment-334600457, adds the CScripts from the wallet to the `dumpwallet` RPC and then allows them to be imported with the `importwallet` RPC. Includes a basic test, and modifies the helptext of the dumpwallet RPC.

  Notes:
  - Reviewers: use `?w=1` to avoid the indentation-only change in commit `Add scripts to importwallet RPC `
  - currently the scripts are followed with `# addr=` comments just as the other keys are, unsure if this might confuse users into thinking all the scripts are for valid P2SH addresses though, but I don't think that should be an issue.
  - there are no birthtimes for scripts, so script imports don't affect rescans
  - `importwallet` imports the CScripts but I'm not sure how to approach specifying whether scripts are for P2SH addresses, BIP173 addresses, etc. whether that matters or not. Otherwise the RPC helptext might just need modification.

  Fixes #11715

Tree-SHA512: 36c55837b3a58b9d3499d4c0c2ae82153d62aa71919e751574651b63a1d2b8ecc83796db4553cc65dad9b5341c3a42ae2fcf4d62598c30af267f8e1461ba8272

3 files changed, 81 insertions, 34 deletions

diff --git a/src/keystore.cpp b/src/keystore.cpp
index 4ab089e032..a010a1244a 100644
--- a/src/keystore.cpp
+++ b/src/keystore.cpp
@@ -77,6 +77,16 @@ bool CBasicKeyStore::HaveCScript(const CScriptID& hash) const
     return mapScripts.count(hash) > 0;
 }
 
+std::set<CScriptID> CBasicKeyStore::GetCScripts() const
+{
+    LOCK(cs_KeyStore);
+    std::set<CScriptID> set_script;
+    for (const auto& mi : mapScripts) {
+        set_script.insert(mi.first);
+    }
+    return set_script;
+}
+
 bool CBasicKeyStore::GetCScript(const CScriptID &hash, CScript& redeemScriptOut) const
 {
     LOCK(cs_KeyStore);
diff --git a/src/keystore.h b/src/keystore.h
index 4e6d8e8a27..516a238241 100644
--- a/src/keystore.h
+++ b/src/keystore.h
@@ -36,6 +36,7 @@ public:
     //! Support for BIP 0013 : see https://github.com/bitcoin/bips/blob/master/bip-0013.mediawiki
     virtual bool AddCScript(const CScript& redeemScript) =0;
     virtual bool HaveCScript(const CScriptID &hash) const =0;
+    virtual std::set<CScriptID> GetCScripts() const =0;
     virtual bool GetCScript(const CScriptID &hash, CScript& redeemScriptOut) const =0;
 
     //! Support for Watch-only addresses
@@ -67,6 +68,7 @@ public:
     bool GetKey(const CKeyID &address, CKey &keyOut) const override;
     bool AddCScript(const CScript& redeemScript) override;
     bool HaveCScript(const CScriptID &hash) const override;
+    std::set<CScriptID> GetCScripts() const override;
     bool GetCScript(const CScriptID &hash, CScript& redeemScriptOut) const override;
 
     bool AddWatchOnly(const CScript &dest) override;
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index 6ca947bf1b..41179ebd4a 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -500,40 +500,57 @@ UniValue importwallet(const JSONRPCRequest& request)
         if (vstr.size() < 2)
             continue;
         CBitcoinSecret vchSecret;
-        if (!vchSecret.SetString(vstr[0]))
-            continue;
-        CKey key = vchSecret.GetKey();
-        CPubKey pubkey = key.GetPubKey();
-        assert(key.VerifyPubKey(pubkey));
-        CKeyID keyid = pubkey.GetID();
-        if (pwallet->HaveKey(keyid)) {
-            LogPrintf("Skipping import of %s (key already present)\n", EncodeDestination(keyid));
-            continue;
-        }
-        int64_t nTime = DecodeDumpTime(vstr[1]);
-        std::string strLabel;
-        bool fLabel = true;
-        for (unsigned int nStr = 2; nStr < vstr.size(); nStr++) {
-            if (boost::algorithm::starts_with(vstr[nStr], "#"))
-                break;
-            if (vstr[nStr] == "change=1")
-                fLabel = false;
-            if (vstr[nStr] == "reserve=1")
-                fLabel = false;
-            if (boost::algorithm::starts_with(vstr[nStr], "label=")) {
-                strLabel = DecodeDumpString(vstr[nStr].substr(6));
-                fLabel = true;
+        if (vchSecret.SetString(vstr[0])) {
+            CKey key = vchSecret.GetKey();
+            CPubKey pubkey = key.GetPubKey();
+            assert(key.VerifyPubKey(pubkey));
+            CKeyID keyid = pubkey.GetID();
+            if (pwallet->HaveKey(keyid)) {
+                LogPrintf("Skipping import of %s (key already present)\n", EncodeDestination(keyid));
+                continue;
             }
+            int64_t nTime = DecodeDumpTime(vstr[1]);
+            std::string strLabel;
+            bool fLabel = true;
+            for (unsigned int nStr = 2; nStr < vstr.size(); nStr++) {
+                if (boost::algorithm::starts_with(vstr[nStr], "#"))
+                    break;
+                if (vstr[nStr] == "change=1")
+                    fLabel = false;
+                if (vstr[nStr] == "reserve=1")
+                    fLabel = false;
+                if (boost::algorithm::starts_with(vstr[nStr], "label=")) {
+                    strLabel = DecodeDumpString(vstr[nStr].substr(6));
+                    fLabel = true;
+                }
+            }
+            LogPrintf("Importing %s...\n", EncodeDestination(keyid));
+            if (!pwallet->AddKeyPubKey(key, pubkey)) {
+                fGood = false;
+                continue;
+            }
+            pwallet->mapKeyMetadata[keyid].nCreateTime = nTime;
+            if (fLabel)
+                pwallet->SetAddressBook(keyid, strLabel, "receive");
+            nTimeBegin = std::min(nTimeBegin, nTime);
+        } else if(IsHex(vstr[0])) {
+           std::vector<unsigned char> vData(ParseHex(vstr[0]));
+           CScript script = CScript(vData.begin(), vData.end());
+           if (pwallet->HaveCScript(script)) {
+               LogPrintf("Skipping import of %s (script already present)\n", vstr[0]);
+               continue;
+           }
+           if(!pwallet->AddCScript(script)) {
+               LogPrintf("Error importing script %s\n", vstr[0]);
+               fGood = false;
+               continue;
+           }
+           int64_t birth_time = DecodeDumpTime(vstr[1]);
+           if (birth_time > 0) {
+               pwallet->m_script_metadata[CScriptID(script)].nCreateTime = birth_time;
+               nTimeBegin = std::min(nTimeBegin, birth_time);
+           }
         }
-        LogPrintf("Importing %s...\n", EncodeDestination(keyid));
-        if (!pwallet->AddKeyPubKey(key, pubkey)) {
-            fGood = false;
-            continue;
-        }
-        pwallet->mapKeyMetadata[keyid].nCreateTime = nTime;
-        if (fLabel)
-            pwallet->SetAddressBook(keyid, strLabel, "receive");
-        nTimeBegin = std::min(nTimeBegin, nTime);
     }
     file.close();
     pwallet->ShowProgress("", 100); // hide progress dialog in GUI
@@ -542,7 +559,7 @@ UniValue importwallet(const JSONRPCRequest& request)
     pwallet->MarkDirty();
 
     if (!fGood)
-        throw JSONRPCError(RPC_WALLET_ERROR, "Error adding some keys to wallet");
+        throw JSONRPCError(RPC_WALLET_ERROR, "Error adding some keys/scripts to wallet");
 
     return NullUniValue;
 }
@@ -601,7 +618,7 @@ UniValue dumpwallet(const JSONRPCRequest& request)
         throw std::runtime_error(
             "dumpwallet \"filename\"\n"
             "\nDumps all wallet keys in a human-readable format to a server-side file. This does not allow overwriting existing files.\n"
-            "Imported scripts are not currently included in wallet dumps, these must be backed up separately.\n"
+            "Imported scripts are included in the dumpfile, but corresponding BIP173 addresses, etc. may not be added automatically by importwallet.\n"
             "Note that if your wallet contains keys which are not derived from your HD seed (e.g. imported keys), these are not covered by\n"
             "only backing up the seed itself, and must be backed up too (e.g. ensure you back up the whole dumpfile).\n"
             "\nArguments:\n"
@@ -640,6 +657,9 @@ UniValue dumpwallet(const JSONRPCRequest& request)
     const std::map<CKeyID, int64_t>& mapKeyPool = pwallet->GetAllReserveKeys();
     pwallet->GetKeyBirthTimes(mapKeyBirth);
 
+    std::set<CScriptID> scripts = pwallet->GetCScripts();
+    // TODO: include scripts in GetKeyBirthTimes() output instead of separate
+
     // sort time/key pairs
     std::vector<std::pair<int64_t, CKeyID> > vKeyBirth;
     for (const auto& entry : mapKeyBirth) {
@@ -694,6 +714,21 @@ UniValue dumpwallet(const JSONRPCRequest& request)
         }
     }
     file << "\n";
+    for (const CScriptID &scriptid : scripts) {
+        CScript script;
+        std::string create_time = "0";
+        std::string address = EncodeDestination(scriptid);
+        // get birth times for scripts with metadata
+        auto it = pwallet->m_script_metadata.find(scriptid);
+        if (it != pwallet->m_script_metadata.end()) {
+            create_time = EncodeDumpTime(it->second.nCreateTime);
+        }
+        if(pwallet->GetCScript(scriptid, script)) {
+            file << strprintf("%s %s script=1", HexStr(script.begin(), script.end()), create_time);
+            file << strprintf(" # addr=%s\n", address);
+        }
+    }
+    file << "\n";
     file << "# End of dump\n";
     file.close();