From cdc260afd530165a3167e049b630bc177a1398f9 Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Sat, 11 Nov 2017 20:49:27 +1300 Subject: [PATCH 1/7] Add GetCScripts to CBasicKeyStore --- src/keystore.cpp | 10 ++++++++++ src/keystore.h | 2 ++ 2 files changed, 12 insertions(+) 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 CBasicKeyStore::GetCScripts() const +{ + LOCK(cs_KeyStore); + std::set 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 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 GetCScripts() const override; bool GetCScript(const CScriptID &hash, CScript& redeemScriptOut) const override; bool AddWatchOnly(const CScript &dest) override; From b702ae812c88fb6ccc4b5163383d96997004c3c8 Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Sat, 11 Nov 2017 21:21:12 +1300 Subject: [PATCH 2/7] Add CScripts to dumpwallet RPC --- src/wallet/rpcdump.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 6ca947bf1b..29f5a0da94 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -640,6 +640,8 @@ UniValue dumpwallet(const JSONRPCRequest& request) const std::map& mapKeyPool = pwallet->GetAllReserveKeys(); pwallet->GetKeyBirthTimes(mapKeyBirth); + std::set scripts = pwallet->GetCScripts(); + // sort time/key pairs std::vector > vKeyBirth; for (const auto& entry : mapKeyBirth) { @@ -694,6 +696,15 @@ UniValue dumpwallet(const JSONRPCRequest& request) } } file << "\n"; + for (const CScriptID &scriptid : scripts) { + CScript script; + std::string address = EncodeDestination(scriptid); + if(pwallet->GetCScript(scriptid, script)) { + file << strprintf("%s 0 script=1", HexStr(script.begin(), script.end())); + file << strprintf(" # addr=%s\n", address); + } + } + file << "\n"; file << "# End of dump\n"; file.close(); From ef0c73022061cae00f9e978b04f3fd0cce8d627d Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Sun, 12 Nov 2017 16:28:46 +1300 Subject: [PATCH 3/7] Add scripts to importwallet RPC --- src/wallet/rpcdump.cpp | 78 ++++++++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 29f5a0da94..93f92f5153 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -500,40 +500,52 @@ 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 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; + } } - 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 +554,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; } From 9e1184dd54d4b2a1d2ae590207ee5beec0d15b38 Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Sun, 12 Nov 2017 16:48:10 +1300 Subject: [PATCH 4/7] Add dumpwallet scripts test --- test/functional/wallet-dump.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/test/functional/wallet-dump.py b/test/functional/wallet-dump.py index 47de8777a6..ab10e96db0 100755 --- a/test/functional/wallet-dump.py +++ b/test/functional/wallet-dump.py @@ -10,13 +10,14 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import (assert_equal, assert_raises_rpc_error) -def read_dump(file_name, addrs, hd_master_addr_old): +def read_dump(file_name, addrs, script_addrs, hd_master_addr_old): """ Read the given dump, count the addrs that match, count change and reserve. Also check that the old hd_master is inactive """ with open(file_name, encoding='utf8') as inputfile: found_addr = 0 + found_script_addr = 0 found_addr_chg = 0 found_addr_rsv = 0 hd_master_addr_ret = None @@ -38,6 +39,9 @@ def read_dump(file_name, addrs, hd_master_addr_old): # ensure we have generated a new hd master key assert(hd_master_addr_old != addr) hd_master_addr_ret = addr + elif keytype == "script=1": + # scripts don't have keypaths + keypath = None else: keypath = addr_keypath.rstrip().split("hdkeypath=")[1] @@ -52,7 +56,14 @@ def read_dump(file_name, addrs, hd_master_addr_old): elif keytype == "reserve=1": found_addr_rsv += 1 break - return found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_ret + + # count scripts + for script_addr in script_addrs: + if script_addr == addr.rstrip() and keytype == "script=1": + found_script_addr += 1 + break + + return found_addr, found_script_addr, found_addr_chg, found_addr_rsv, hd_master_addr_ret class WalletDumpTest(BitcoinTestFramework): @@ -81,13 +92,19 @@ class WalletDumpTest(BitcoinTestFramework): # Should be a no-op: self.nodes[0].keypoolrefill() + # Test scripts dump by adding a P2SH witness and a 1-of-1 multisig address + witness_addr = self.nodes[0].addwitnessaddress(addrs[0]["address"], True) + multisig_addr = self.nodes[0].addmultisigaddress(1, [addrs[1]["address"]]) + script_addrs = [witness_addr, multisig_addr] + # dump unencrypted wallet result = self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.unencrypted.dump") assert_equal(result['filename'], os.path.abspath(tmpdir + "/node0/wallet.unencrypted.dump")) - found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_unenc = \ - read_dump(tmpdir + "/node0/wallet.unencrypted.dump", addrs, None) + found_addr, found_script_addr, found_addr_chg, found_addr_rsv, hd_master_addr_unenc = \ + read_dump(tmpdir + "/node0/wallet.unencrypted.dump", addrs, script_addrs, None) assert_equal(found_addr, test_addr_count) # all keys must be in the dump + assert_equal(found_script_addr, 2) # all scripts must be in the dump assert_equal(found_addr_chg, 50) # 50 blocks where mined assert_equal(found_addr_rsv, 90*2) # 90 keys plus 100% internal keys @@ -99,9 +116,10 @@ class WalletDumpTest(BitcoinTestFramework): self.nodes[0].keypoolrefill() self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.encrypted.dump") - found_addr, found_addr_chg, found_addr_rsv, _ = \ - read_dump(tmpdir + "/node0/wallet.encrypted.dump", addrs, hd_master_addr_unenc) + found_addr, found_script_addr, found_addr_chg, found_addr_rsv, _ = \ + read_dump(tmpdir + "/node0/wallet.encrypted.dump", addrs, script_addrs, hd_master_addr_unenc) assert_equal(found_addr, test_addr_count) + assert_equal(found_script_addr, 2) assert_equal(found_addr_chg, 90*2 + 50) # old reserve keys are marked as change now assert_equal(found_addr_rsv, 90*2) From 68c1e00a002dd2c5982105a6fae59eac2d2ce97b Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Sun, 12 Nov 2017 19:01:15 +1300 Subject: [PATCH 5/7] Add test for importwallet --- test/functional/wallet-dump.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/functional/wallet-dump.py b/test/functional/wallet-dump.py index ab10e96db0..85812f9acc 100755 --- a/test/functional/wallet-dump.py +++ b/test/functional/wallet-dump.py @@ -126,5 +126,19 @@ class WalletDumpTest(BitcoinTestFramework): # Overwriting should fail assert_raises_rpc_error(-8, "already exists", self.nodes[0].dumpwallet, tmpdir + "/node0/wallet.unencrypted.dump") + # Restart node with new wallet, and test importwallet + self.stop_node(0) + self.start_node(0, ['-wallet=w2']) + + # Make sure the address is not IsMine before import + result = self.nodes[0].validateaddress(multisig_addr) + assert(result['ismine'] == False) + + self.nodes[0].importwallet(os.path.abspath(tmpdir + "/node0/wallet.unencrypted.dump")) + + # Now check IsMine is true + result = self.nodes[0].validateaddress(multisig_addr) + assert(result['ismine'] == True) + if __name__ == '__main__': WalletDumpTest().main () From 1bab9b23af95986f9452d468257cc34d2c5017b2 Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Sun, 12 Nov 2017 17:11:26 +1300 Subject: [PATCH 6/7] Add script dump note to RPC help text and release notes --- doc/release-notes.md | 4 ++++ src/wallet/rpcdump.cpp | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index 78caddc8f0..989e8d636d 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -100,6 +100,10 @@ The `share/rpcuser/rpcuser.py` script was renamed to `share/rpcauth/rpcauth.py`. used to create `rpcauth` credentials for a JSON-RPC user. +- `dumpwallet` now includes hex-encoded scripts from the wallet in the dumpfile, and + `importwallet` now imports these scripts, but corresponding addresses may not be added + correctly or a manual rescan may be required to find relevant transactions. + Credits ======= diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 93f92f5153..92c3bd080c 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -613,7 +613,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" From 656fde53a3a0d88a1e3c1aef7ae99083e4b06a7d Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Wed, 20 Dec 2017 19:01:05 +1300 Subject: [PATCH 7/7] Add script birthtime metadata to dump and import wallet --- src/wallet/rpcdump.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 92c3bd080c..41179ebd4a 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -545,6 +545,11 @@ UniValue importwallet(const JSONRPCRequest& request) 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); + } } } file.close(); @@ -653,6 +658,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) pwallet->GetKeyBirthTimes(mapKeyBirth); std::set scripts = pwallet->GetCScripts(); + // TODO: include scripts in GetKeyBirthTimes() output instead of separate // sort time/key pairs std::vector > vKeyBirth; @@ -710,9 +716,15 @@ 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 0 script=1", HexStr(script.begin(), script.end())); + file << strprintf("%s %s script=1", HexStr(script.begin(), script.end()), create_time); file << strprintf(" # addr=%s\n", address); } }