highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-06-26 08:51:55 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge #17300: LegacyScriptPubKeyMan code cleanups

Browse Source 
53fe0b70adeffe4cb94e6fa18a9abbdf674a2cd0 Fix missing strFailReason in CreateTransaction (Russell Yanofsky)
4b28a05f080de8acefaaa74f1204829995611d9e Fix misplaced AssertLockHeld (Russell Yanofsky)
2632b1f12466f970b667b42f462a6503df88e128 doc: Clarify WalletStorage / Wallet relation (Russell Yanofsky)
628d11b2ba0f1ba715c2358373e603a56d9f69ff Add back mistakenly removed AssertLockHeld (Russell Yanofsky)
52cf68f7ffa1dd3a6850606a75c32b77b1308c4b Refactor: Add GetLegacyScriptPubKeyMan helper (Russell Yanofsky)

Pull request description:

  This PR implements suggested code cleanups from https://github.com/bitcoin/bitcoin/pull/17260 review comments

ACKs for top commit:
  Sjors:
    ACK 53fe0b70adeffe4cb94e6fa18a9abbdf674a2cd0
  achow101:
    ACK 53fe0b70adeffe4cb94e6fa18a9abbdf674a2cd0
  MarcoFalke:
    ACK 53fe0b70adeffe4cb94e6fa18a9abbdf674a2cd0

Tree-SHA512: a577b96cb21a9aa7185d7d900e4db0665c302adcd12097957b9d8e838a8548c7de8f901bcb83e7c46d231b841221345c9264f5e29ed069f3d9236896430f959b
This commit is contained in:
MarcoFalke 2019-10-31 14:40:33 -04:00
commit 100fa0a62a
No known key found for this signature in database
GPG Key ID: D2EA4850E7528B25
4 changed files with 35 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
src/wallet/rpcdump.cpp
View File

@ -87,6 +87,15 @@ static void RescanWallet(CWallet& wallet, const WalletRescanReserver& reserver,
} }
} }
static LegacyScriptPubKeyMan& GetLegacyScriptPubKeyMan(CWallet& wallet)
{
LegacyScriptPubKeyMan* spk_man = wallet.GetLegacyScriptPubKeyMan();
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}
return *spk_man;
}
UniValue importprivkey(const JSONRPCRequest& request) UniValue importprivkey(const JSONRPCRequest& request)
{ {
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
@ -125,10 +134,7 @@ UniValue importprivkey(const JSONRPCRequest& request)
throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled"); throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled");
} }
LegacyScriptPubKeyMan* spk_man = pwallet->GetLegacyScriptPubKeyMan(); LegacyScriptPubKeyMan& spk_man = GetLegacyScriptPubKeyMan(*wallet);
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}
WalletRescanReserver reserver(pwallet); WalletRescanReserver reserver(pwallet);
bool fRescan = true; bool fRescan = true;
@ -256,10 +262,7 @@ UniValue importaddress(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
LegacyScriptPubKeyMan* spk_man = pwallet->GetLegacyScriptPubKeyMan(); LegacyScriptPubKeyMan& spk_man = GetLegacyScriptPubKeyMan(*pwallet);
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}
std::string strLabel; std::string strLabel;
if (!request.params[1].isNull()) if (!request.params[1].isNull())
@ -462,10 +465,7 @@ UniValue importpubkey(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
LegacyScriptPubKeyMan* spk_man = pwallet->GetLegacyScriptPubKeyMan(); LegacyScriptPubKeyMan& spk_man = GetLegacyScriptPubKeyMan(*wallet);
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}
std::string strLabel; std::string strLabel;
if (!request.params[1].isNull()) if (!request.params[1].isNull())
@ -549,10 +549,7 @@ UniValue importwallet(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
LegacyScriptPubKeyMan* spk_man = pwallet->GetLegacyScriptPubKeyMan(); LegacyScriptPubKeyMan& spk_man = GetLegacyScriptPubKeyMan(*wallet);
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}
if (pwallet->chain().havePruned()) { if (pwallet->chain().havePruned()) {
// Exit early and print an error. // Exit early and print an error.
@ -711,10 +708,7 @@ UniValue dumpprivkey(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
LegacyScriptPubKeyMan* spk_man = pwallet->GetLegacyScriptPubKeyMan(); LegacyScriptPubKeyMan& spk_man = GetLegacyScriptPubKeyMan(*wallet);
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}
auto locked_chain = pwallet->chain().lock(); auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
@ -726,12 +720,12 @@ UniValue dumpprivkey(const JSONRPCRequest& request)
if (!IsValidDestination(dest)) { if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address"); throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address");
} }
auto keyid = GetKeyForDestination(*spk_man, dest); auto keyid = GetKeyForDestination(spk_man, dest);
if (keyid.IsNull()) { if (keyid.IsNull()) {
throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to a key"); throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to a key");
} }
CKey vchSecret; CKey vchSecret;
if (!spk_man->GetKey(keyid, vchSecret)) { if (!spk_man.GetKey(keyid, vchSecret)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known"); throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known");
} }
return EncodeSecret(vchSecret); return EncodeSecret(vchSecret);
@ -765,14 +759,11 @@ UniValue dumpwallet(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
LegacyScriptPubKeyMan* spk_man = pwallet->GetLegacyScriptPubKeyMan(); LegacyScriptPubKeyMan& spk_man = GetLegacyScriptPubKeyMan(*wallet);
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}
auto locked_chain = pwallet->chain().lock(); auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
AssertLockHeld(spk_man->cs_wallet); AssertLockHeld(spk_man.cs_wallet);
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
@ -794,10 +785,10 @@ UniValue dumpwallet(const JSONRPCRequest& request)
throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file");
std::map<CKeyID, int64_t> mapKeyBirth; std::map<CKeyID, int64_t> mapKeyBirth;
const std::map<CKeyID, int64_t>& mapKeyPool = spk_man->GetAllReserveKeys(); const std::map<CKeyID, int64_t>& mapKeyPool = spk_man.GetAllReserveKeys();
pwallet->GetKeyBirthTimes(*locked_chain, mapKeyBirth); pwallet->GetKeyBirthTimes(*locked_chain, mapKeyBirth);
std::set<CScriptID> scripts = spk_man->GetCScripts(); std::set<CScriptID> scripts = spk_man.GetCScripts();
// sort time/key pairs // sort time/key pairs
std::vector<std::pair<int64_t, CKeyID> > vKeyBirth; std::vector<std::pair<int64_t, CKeyID> > vKeyBirth;
@ -816,11 +807,11 @@ UniValue dumpwallet(const JSONRPCRequest& request)
file << "\n"; file << "\n";
// add the base58check encoded extended master if the wallet uses HD // add the base58check encoded extended master if the wallet uses HD
CKeyID seed_id = spk_man->GetHDChain().seed_id; CKeyID seed_id = spk_man.GetHDChain().seed_id;
if (!seed_id.IsNull()) if (!seed_id.IsNull())
{ {
CKey seed; CKey seed;
if (spk_man->GetKey(seed_id, seed)) { if (spk_man.GetKey(seed_id, seed)) {
CExtKey masterKey; CExtKey masterKey;
masterKey.SetSeed(seed.begin(), seed.size()); masterKey.SetSeed(seed.begin(), seed.size());
@ -833,20 +824,20 @@ UniValue dumpwallet(const JSONRPCRequest& request)
std::string strAddr; std::string strAddr;
std::string strLabel; std::string strLabel;
CKey key; CKey key;
if (spk_man->GetKey(keyid, key)) { if (spk_man.GetKey(keyid, key)) {
file << strprintf("%s %s ", EncodeSecret(key), strTime); file << strprintf("%s %s ", EncodeSecret(key), strTime);
if (GetWalletAddressesForKey(spk_man, pwallet, keyid, strAddr, strLabel)) { if (GetWalletAddressesForKey(&spk_man, pwallet, keyid, strAddr, strLabel)) {
file << strprintf("label=%s", strLabel); file << strprintf("label=%s", strLabel);
} else if (keyid == seed_id) { } else if (keyid == seed_id) {
file << "hdseed=1"; file << "hdseed=1";
} else if (mapKeyPool.count(keyid)) { } else if (mapKeyPool.count(keyid)) {
file << "reserve=1"; file << "reserve=1";
} else if (spk_man->mapKeyMetadata[keyid].hdKeypath == "s") { } else if (spk_man.mapKeyMetadata[keyid].hdKeypath == "s") {
file << "inactivehdseed=1"; file << "inactivehdseed=1";
} else { } else {
file << "change=1"; file << "change=1";
} }
file << strprintf(" # addr=%s%s\n", strAddr, (spk_man->mapKeyMetadata[keyid].has_key_origin ? " hdkeypath="+WriteHDKeypath(spk_man->mapKeyMetadata[keyid].key_origin.path) : "")); file << strprintf(" # addr=%s%s\n", strAddr, (spk_man.mapKeyMetadata[keyid].has_key_origin ? " hdkeypath="+WriteHDKeypath(spk_man.mapKeyMetadata[keyid].key_origin.path) : ""));
} }
} }
file << "\n"; file << "\n";
@ -855,11 +846,11 @@ UniValue dumpwallet(const JSONRPCRequest& request)
std::string create_time = "0"; std::string create_time = "0";
std::string address = EncodeDestination(ScriptHash(scriptid)); std::string address = EncodeDestination(ScriptHash(scriptid));
// get birth times for scripts with metadata // get birth times for scripts with metadata
auto it = spk_man->m_script_metadata.find(scriptid); auto it = spk_man.m_script_metadata.find(scriptid);
if (it != spk_man->m_script_metadata.end()) { if (it != spk_man.m_script_metadata.end()) {
create_time = FormatISO8601DateTime(it->second.nCreateTime); create_time = FormatISO8601DateTime(it->second.nCreateTime);
} }
if(spk_man->GetCScript(scriptid, script)) { if(spk_man.GetCScript(scriptid, script)) {
file << strprintf("%s %s script=1", HexStr(script.begin(), script.end()), create_time); file << strprintf("%s %s script=1", HexStr(script.begin(), script.end()), create_time);
file << strprintf(" # addr=%s\n", address); file << strprintf(" # addr=%s\n", address);
} }
@ -1355,10 +1346,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
RPCTypeCheck(mainRequest.params, {UniValue::VARR, UniValue::VOBJ}); RPCTypeCheck(mainRequest.params, {UniValue::VARR, UniValue::VOBJ});
LegacyScriptPubKeyMan* spk_man = pwallet->GetLegacyScriptPubKeyMan(); LegacyScriptPubKeyMan& spk_man = GetLegacyScriptPubKeyMan(*wallet);
if (!spk_man) {
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
}
const UniValue& requests = mainRequest.params[0]; const UniValue& requests = mainRequest.params[0];

2
src/wallet/scriptpubkeyman.cpp
View File

@ -386,6 +386,8 @@ bool LegacyScriptPubKeyMan::AddKeyPubKey(const CKey& secret, const CPubKey &pubk
bool LegacyScriptPubKeyMan::AddKeyPubKeyWithDB(WalletBatch& batch, const CKey& secret, const CPubKey& pubkey) bool LegacyScriptPubKeyMan::AddKeyPubKeyWithDB(WalletBatch& batch, const CKey& secret, const CPubKey& pubkey)
{ {
AssertLockHeld(cs_wallet);
// Make sure we aren't adding private keys to private key disabled wallets // Make sure we aren't adding private keys to private key disabled wallets
assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));

2
src/wallet/scriptpubkeyman.h
View File

@ -20,7 +20,7 @@ enum class OutputType;
// It provides access to things that are part of the entire wallet and not specific to a ScriptPubKeyMan such as // It provides access to things that are part of the entire wallet and not specific to a ScriptPubKeyMan such as
// wallet flags, wallet version, encryption keys, encryption status, and the database itself. This allows a // wallet flags, wallet version, encryption keys, encryption status, and the database itself. This allows a
// ScriptPubKeyMan to have callbacks into CWallet without causing a circular dependency. // ScriptPubKeyMan to have callbacks into CWallet without causing a circular dependency.
// WalletStorage should be the same for all ScriptPubKeyMans. // WalletStorage should be the same for all ScriptPubKeyMans of a wallet.
class WalletStorage class WalletStorage
{ {
public: public:

8
src/wallet/wallet.cpp
View File

@ -249,8 +249,8 @@ const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const
void CWallet::UpgradeKeyMetadata() void CWallet::UpgradeKeyMetadata()
{ {
AssertLockHeld(m_spk_man->cs_wallet);
if (m_spk_man) { if (m_spk_man) {
AssertLockHeld(m_spk_man->cs_wallet);
m_spk_man->UpgradeKeyMetadata(); m_spk_man->UpgradeKeyMetadata();
} }
} }
@ -2783,11 +2783,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
SignatureData sigdata; SignatureData sigdata;
const SigningProvider* provider = GetSigningProvider(); const SigningProvider* provider = GetSigningProvider();
if (!provider) { if (!provider || !ProduceSignature(*provider, MutableTransactionSignatureCreator(&txNew, nIn, coin.txout.nValue, SIGHASH_ALL), scriptPubKey, sigdata))
return false;
}
if (!ProduceSignature(*provider, MutableTransactionSignatureCreator(&txNew, nIn, coin.txout.nValue, SIGHASH_ALL), scriptPubKey, sigdata))
{ {
strFailReason = _("Signing transaction failed").translated; strFailReason = _("Signing transaction failed").translated;
return false; return false;