diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index eff6e3442ef5..a0b973ddeefa 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -819,12 +819,12 @@ std::unique_ptr ParseScript(uint32_t key_exp_index, Span(std::move(pubkey)); } if (Func("pkh", expr)) { - auto pubkey = ParsePubkey(key_exp_index, expr, ctx != ParseScriptContext::P2SH, out, error); + auto pubkey = ParsePubkey(key_exp_index, expr, true, out, error); if (!pubkey) return nullptr; return std::make_unique(std::move(pubkey)); } @@ -851,7 +851,7 @@ std::unique_ptr ParseScript(uint32_t key_exp_index, SpanGetSize() + 1; providers.emplace_back(std::move(pk)); diff --git a/src/script/signingprovider.cpp b/src/script/signingprovider.cpp index fd5231519bde..7ce92a546336 100644 --- a/src/script/signingprovider.cpp +++ b/src/script/signingprovider.cpp @@ -150,3 +150,13 @@ bool FillableSigningProvider::GetCScript(const CScriptID &hash, CScript& redeemS } return false; } + +CKeyID GetKeyForDestination(const SigningProvider& store, const CTxDestination& dest) +{ + // Only supports destinations which map to single public keys, i.e. P2PKH + const PKHash *pkhash = std::get_if(&dest); + + if (pkhash != nullptr) return ToKeyID(*pkhash); + + return CKeyID(); +} diff --git a/src/script/signingprovider.h b/src/script/signingprovider.h index 468dbb4a0674..8ab2793a5b5d 100644 --- a/src/script/signingprovider.h +++ b/src/script/signingprovider.h @@ -131,4 +131,7 @@ class FillableSigningProvider : public SigningProvider virtual bool GetCScript(const CScriptID &hash, CScript& redeemScriptOut) const override; }; +/** Return the CKeyID of the key involved in a script (if there is a unique one). */ +CKeyID GetKeyForDestination(const SigningProvider& store, const CTxDestination& dest); + #endif // BITCOIN_SCRIPT_SIGNINGPROVIDER_H diff --git a/src/test/fuzz/key_io.cpp b/src/test/fuzz/key_io.cpp index 181789a2505b..30f4160c4fde 100644 --- a/src/test/fuzz/key_io.cpp +++ b/src/test/fuzz/key_io.cpp @@ -40,7 +40,7 @@ FUZZ_TARGET_INIT(key_io, initialize_key_io) const CTxDestination tx_destination = DecodeDestination(random_string); (void)DescribeAddress(tx_destination); - // (void)GetKeyForDestination(/* store */ {}, tx_destination); + (void)GetKeyForDestination(/* store */ {}, tx_destination); (void)GetScriptForDestination(tx_destination); (void)IsValidDestination(tx_destination); diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index cf0899cdc7d9..2ab7f72c5cef 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -12,6 +12,12 @@ BOOST_FIXTURE_TEST_SUITE(script_standard_tests, BasicTestingSetup) +BOOST_AUTO_TEST_CASE(dest_default_is_no_dest) +{ + CTxDestination dest; + BOOST_CHECK(!IsValidDestination(dest)); +} + BOOST_AUTO_TEST_CASE(script_standard_Solver_success) { CKey keys[3]; diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 0ff6074e6317..bdc8a81e9be5 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -302,7 +302,6 @@ UniValue importprunedfunds(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input."); } uint256 hashTx = tx.GetHash(); - CWalletTx wtx(pwallet, MakeTransactionRef(std::move(tx))); CDataStream ssMB(ParseHexV(request.params[1], "proof"), SER_NETWORK, PROTOCOL_VERSION); CMerkleBlock merkleBlock; @@ -329,10 +328,10 @@ UniValue importprunedfunds(const JSONRPCRequest& request) unsigned int txnIndex = vIndex[it - vMatch.begin()]; CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, merkleBlock.header.GetHash(), txnIndex); - wtx.m_confirm = confirm; - if (pwallet->IsMine(*wtx.tx)) { - pwallet->AddToWallet(wtx, false); + CTransactionRef tx_ref = MakeTransactionRef(tx); + if (pwallet->IsMine(*tx_ref)) { + pwallet->AddToWallet(std::move(tx_ref), confirm); return NullUniValue; } @@ -908,15 +907,19 @@ UniValue dumpwallet(const JSONRPCRequest& request) }, }.Check(request); - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; - LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*wallet); + CWallet& wallet = *pwallet; + LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(wallet); - LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + wallet.BlockUntilSyncedToCurrentChain(); - EnsureWalletIsUnlocked(pwallet); + LOCK2(wallet.cs_wallet, spk_man.cs_KeyStore); + + EnsureWalletIsUnlocked(&wallet); fs::path filepath = request.params[0].get_str(); filepath = fs::absolute(filepath); @@ -937,7 +940,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) std::map mapKeyBirth; const std::map& mapKeyPool = spk_man.GetAllReserveKeys(); - pwallet->GetKeyBirthTimes(mapKeyBirth); + wallet.GetKeyBirthTimes(mapKeyBirth); std::set scripts = spk_man.GetCScripts(); @@ -952,16 +955,16 @@ UniValue dumpwallet(const JSONRPCRequest& request) // produce output file << strprintf("# Wallet dump created by Dash Core %s\n", CLIENT_BUILD); file << strprintf("# * Created on %s\n", FormatISO8601DateTime(GetTime())); - file << strprintf("# * Best block at time of backup was %i (%s),\n", pwallet->GetLastBlockHeight(), pwallet->GetLastBlockHash().ToString()); + file << strprintf("# * Best block at time of backup was %i (%s),\n", wallet.GetLastBlockHeight(), wallet.GetLastBlockHash().ToString()); int64_t block_time = 0; - CHECK_NONFATAL(pwallet->chain().findBlock(pwallet->GetLastBlockHash(), FoundBlock().time(block_time))); + CHECK_NONFATAL(wallet.chain().findBlock(wallet.GetLastBlockHash(), FoundBlock().time(block_time))); file << strprintf("# mined on %s\n", FormatISO8601DateTime(block_time)); file << "\n"; UniValue obj(UniValue::VOBJ); obj.pushKV("dashcoreversion", CLIENT_BUILD); - obj.pushKV("lastblockheight", pwallet->GetLastBlockHeight()); - obj.pushKV("lastblockhash", pwallet->GetLastBlockHash().ToString()); + obj.pushKV("lastblockheight", wallet.GetLastBlockHeight()); + obj.pushKV("lastblockhash", wallet.GetLastBlockHash().ToString()); obj.pushKV("lastblocktime", block_time); // add the base58check encoded extended master if the wallet uses HD @@ -1012,7 +1015,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) CKey key; if (spk_man.GetKey(keyid, key)) { file << strprintf("%s %s ", EncodeSecret(key), strTime); - const auto* address_book_entry = pwallet->FindAddressBookEntry(pkhash); + const auto* address_book_entry = wallet.FindAddressBookEntry(pkhash); if (address_book_entry) { file << strprintf("label=%s", EncodeDumpString(address_book_entry->GetLabel())); } else if (mapKeyPool.count(keyid)) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 752098cbf57c..41b756df1c88 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -258,10 +258,6 @@ UniValue getnewaddress(const JSONRPCRequest& request) if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); - LegacyScriptPubKeyMan* spk_man = pwallet->GetLegacyScriptPubKeyMan(); - if (!spk_man) { - throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command"); - } LOCK(pwallet->cs_wallet); if (!pwallet->CanGetAddresses()) { @@ -1632,7 +1628,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) UniValue transactions(UniValue::VARR); for (const std::pair& pairWtx : pwallet->mapWallet) { - CWalletTx tx = pairWtx.second; + const CWalletTx& tx = pairWtx.second; if (depth == -1 || abs(tx.GetDepthInMainChain()) < depth) { ListTransactions(pwallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); @@ -2560,9 +2556,9 @@ static UniValue getwalletinfo(const JSONRPCRequest& request) if (spk_man) { obj.pushKV("timefirstkey", spk_man->GetTimeFirstKey()); obj.pushKV("keypoololdest", spk_man->GetOldestKeyPoolTime()); - obj.pushKV("keypoolsize", (int64_t)spk_man->KeypoolCountExternalKeys()); - obj.pushKV("keypoolsize_hd_internal", (int64_t)(spk_man->KeypoolCountInternalKeys())); } + obj.pushKV("keypoolsize", (int64_t)pwallet->KeypoolCountExternalKeys()); + obj.pushKV("keypoolsize_hd_internal", (int64_t)(pwallet->KeypoolCountInternalKeys())); obj.pushKV("keys_left", pwallet->nKeysLeftSinceAutoBackup); if (pwallet->IsCrypted()) obj.pushKV("unlocked_until", pwallet->nRelockTime); @@ -3833,12 +3829,12 @@ UniValue getaddressinfo(const JSONRPCRequest& request) ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan(scriptPubKey); if (spk_man) { - const PKHash *pkhash = std::get_if(&dest); if (const CKeyMetadata* meta = spk_man->GetMetadata(dest)) { ret.pushKV("timestamp", meta->nCreateTime); CHDChain hdChainCurrent; LegacyScriptPubKeyMan* legacy_spk_man = pwallet->GetLegacyScriptPubKeyMan(); if (legacy_spk_man != nullptr) { + const PKHash *pkhash = std::get_if(&dest); if (pkhash && legacy_spk_man->HaveHDKey(ToKeyID(*pkhash), hdChainCurrent)) { ret.pushKV("hdchainid", hdChainCurrent.GetID().GetHex()); } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 7260aae0b733..78a2dbc2bc09 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -23,7 +23,7 @@ bool LegacyScriptPubKeyMan::GetNewDestination(CTxDestination& dest, std::string& // Generate a new key that is added to wallet CPubKey new_key; if (!GetKeyFromPool(new_key, false)) { - error = "Error: Keypool ran out, please call keypoolrefill first"; + error = _("Error: Keypool ran out, please call keypoolrefill first").translated; return false; } //LearnRelatedScripts(new_key); @@ -749,9 +749,9 @@ const CKeyMetadata* LegacyScriptPubKeyMan::GetMetadata(const CTxDestination& des { LOCK(cs_KeyStore); - const PKHash *pkhash = std::get_if(&dest); - if (pkhash != nullptr && !ToKeyID(*pkhash).IsNull()) { - auto it = mapKeyMetadata.find(ToKeyID(*pkhash)); + CKeyID key_id = GetKeyForDestination(*this, dest); + if (!key_id.IsNull()) { + auto it = mapKeyMetadata.find(key_id); if (it != mapKeyMetadata.end()) { return &it->second; } diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 8cbf8dbdfd17..80db47319b36 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -25,8 +25,6 @@ BOOST_FIXTURE_TEST_SUITE(coinselector_tests, WalletTestingSetup) // we repeat those tests this many times and only complain if all iterations of the test fail #define RANDOM_REPEATS 5 -std::vector> wtxn; - typedef std::set CoinSet; static std::vector vCoins; @@ -76,16 +74,14 @@ static void add_coin(CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bo // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() tx.vin.resize(1); } - std::unique_ptr wtx = std::make_unique(&wallet, MakeTransactionRef(std::move(tx))); + CWalletTx* wtx = wallet.AddToWallet(MakeTransactionRef(std::move(tx)), /* confirm= */ {}); if (fIsFromMe) { wtx->m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1); wtx->m_is_cache_empty = false; } - COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); + COutput output(wtx, nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); vCoins.push_back(output); - wallet.AddToWallet(*wtx.get()); - wtxn.emplace_back(std::move(wtx)); } static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false) { @@ -95,7 +91,6 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa static void empty_wallet(void) { vCoins.clear(); - wtxn.clear(); balance = 0; } diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index e425be6dbcd5..9680da50708c 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -23,14 +23,12 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) CDataStream s_prev_tx1(ParseHex("0200000000010158e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd7501000000171600145f275f436b09a8cc9a2eb2a2f528485c68a56323feffffff02d8231f1b0100000017a914aed962d6654f9a2b36608eb9d64d2b260db4f1118700c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88702483045022100a22edcc6e5bc511af4cc4ae0de0fcd75c7e04d8c1c3a8aa9d820ed4b967384ec02200642963597b9b1bc22c75e9f3e117284a962188bf5e8a74c895089046a20ad770121035509a48eb623e10aace8bfd0212fdb8a8e5af3c94b0b133b95e114cab89e4f7965000000"), SER_NETWORK, PROTOCOL_VERSION); CTransactionRef prev_tx1; s_prev_tx1 >> prev_tx1; - CWalletTx prev_wtx1(&m_wallet, prev_tx1); - m_wallet.mapWallet.emplace(prev_wtx1.GetHash(), std::move(prev_wtx1)); + m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx1->GetHash()), std::forward_as_tuple(&m_wallet, prev_tx1)); CDataStream s_prev_tx2(ParseHex("0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f618765000000"), SER_NETWORK, PROTOCOL_VERSION); CTransactionRef prev_tx2; s_prev_tx2 >> prev_tx2; - CWalletTx prev_wtx2(&m_wallet, prev_tx2); - m_wallet.mapWallet.emplace(prev_wtx2.GetHash(), std::move(prev_wtx2)); + m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx2->GetHash()), std::forward_as_tuple(&m_wallet, prev_tx2)); // Add scripts CScript rs1; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index dc72bee749f1..dbe1350c0153 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -264,17 +264,21 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Import key into wallet and call dumpwallet to create backup file. { std::shared_ptr wallet = std::make_shared(m_node.chain.get(), "", CreateDummyWalletDatabase()); - auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); - LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); - spk_man->mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; - spk_man->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); + { + auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); + LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); + spk_man->mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; + spk_man->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); + AddWallet(wallet); + wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); + } CoreContext context{m_node}; JSONRPCRequest request(context); + request.params.setArray(); request.params.push_back(backup_file); - AddWallet(wallet); - wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); + ::dumpwallet(request); RemoveWallet(wallet, std::nullopt); } @@ -415,6 +419,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime) { CMutableTransaction tx; + CWalletTx::Confirmation confirm; tx.nLockTime = lockTime; SetMockTime(mockTime); CBlockIndex* block = nullptr; @@ -426,23 +431,15 @@ static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lock block = inserted.first->second; block->nTime = blockTime; block->phashBlock = &hash; + confirm = {CWalletTx::Status::CONFIRMED, block->nHeight, hash, 0}; } - CWalletTx wtx(&wallet, MakeTransactionRef(tx)); - LOCK(wallet.cs_wallet); // If transaction is already in map, to avoid inconsistencies, unconfirmation // is needed before confirm again with different block. - std::map::iterator it = wallet.mapWallet.find(wtx.GetHash()); - if (it != wallet.mapWallet.end()) { + return wallet.AddToWallet(MakeTransactionRef(tx), confirm, [&](CWalletTx& wtx, bool /* new_tx */) { wtx.setUnconfirmed(); - wallet.AddToWallet(wtx); - } - if (block) { - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block->nHeight, block->GetBlockHash(), 0); - wtx.m_confirm = confirm; - } - wallet.AddToWallet(wtx); - return wallet.mapWallet.at(wtx.GetHash()).nTimeSmart; + return true; + })->nTimeSmart; } // Simple test to verify assignment of CWalletTx::nSmartTime value. Could be diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 69a62b07a169..7cfc2e2763ac 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -122,6 +122,7 @@ bool AddWallet(const std::shared_ptr& wallet) assert(::masternodeSync != nullptr && ::coinJoinClientManagers != nullptr); ::coinJoinClientManagers->Add(*wallet); g_wallet_init_interface.InitCoinJoinSettings(*::coinJoinClientManagers); + wallet->NotifyCanGetAddressesChanged(); return true; } @@ -649,9 +650,11 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) // must get current HD chain before EncryptKeys CHDChain hdChainCurrent; - // GetHDChain exist only at legacy.... how to validate it? just do static cast? we don't have any other type yet so may be ok temporary! - if (auto spk_man = GetLegacyScriptPubKeyMan()) { - spk_man->GetHDChain(hdChainCurrent); + for (const auto& spk_man_pair : m_spk_managers) { + auto spk_man = spk_man_pair.second.get(); + LegacyScriptPubKeyMan *spk_man_legacy = dynamic_cast(spk_man); + if (spk_man_legacy != nullptr) spk_man_legacy->GetHDChain(hdChainCurrent); + if (!spk_man->Encrypt(_vMasterKey, encrypted_batch)) { encrypted_batch->TxnAbort(); delete encrypted_batch; @@ -661,10 +664,10 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) assert(false); } if (!hdChainCurrent.IsNull()) { - assert(spk_man->EncryptHDChain(_vMasterKey)); + assert(spk_man_legacy->EncryptHDChain(_vMasterKey)); CHDChain hdChainCrypted; - assert(spk_man->GetHDChain(hdChainCrypted)); + assert(spk_man_legacy->GetHDChain(hdChainCrypted)); DBG( tfm::format(std::cout, "EncryptWallet -- current seed: '%s'\n", HexStr(hdChainCurrent.GetSeed())); @@ -675,7 +678,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) assert(hdChainCurrent.GetID() == hdChainCrypted.GetID()); assert(hdChainCurrent.GetSeedHash() != hdChainCrypted.GetSeedHash()); - assert(spk_man->SetCryptedHDChain(*encrypted_batch, hdChainCrypted, false)); + assert(spk_man_legacy->SetCryptedHDChain(*encrypted_batch, hdChainCrypted, false)); } } @@ -846,19 +849,19 @@ bool CWallet::IsSpentKey(const uint256& hash, unsigned int n) const return false; } -bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) +CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose) { LOCK(cs_wallet); WalletBatch batch(GetDatabase(), fFlushOnClose); - uint256 hash = wtxIn.GetHash(); + uint256 hash = tx->GetHash(); if (IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { // Mark used destinations std::set tx_destinations; - for (const CTxIn& txin : wtxIn.tx->vin) { + for (const CTxIn& txin : tx->vin) { const COutPoint& op = txin.prevout; SetSpentKeyState(batch, op.hash, op.n, true, tx_destinations); } @@ -867,11 +870,12 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) } // Inserts only if not already there, returns tx inserted or tx found - std::pair::iterator, bool> ret = mapWallet.insert(std::make_pair(hash, wtxIn)); + auto ret = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(this, tx)); CWalletTx& wtx = (*ret.first).second; - wtx.BindWallet(this); bool fInsertedNew = ret.second; + bool fUpdated = update_wtx && update_wtx(wtx, fInsertedNew); if (fInsertedNew) { + wtx.m_confirm = confirm; wtx.nTimeReceived = GetTime(); wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); @@ -889,24 +893,18 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) } } - bool fUpdated = false; if (!fInsertedNew) { - if (wtxIn.m_confirm.status != wtx.m_confirm.status) { - wtx.m_confirm.status = wtxIn.m_confirm.status; - wtx.m_confirm.nIndex = wtxIn.m_confirm.nIndex; - wtx.m_confirm.hashBlock = wtxIn.m_confirm.hashBlock; - wtx.m_confirm.block_height = wtxIn.m_confirm.block_height; + if (confirm.status != wtx.m_confirm.status) { + wtx.m_confirm.status = confirm.status; + wtx.m_confirm.nIndex = confirm.nIndex; + wtx.m_confirm.hashBlock = confirm.hashBlock; + wtx.m_confirm.block_height = confirm.block_height; fUpdated = true; } else { - assert(wtx.m_confirm.nIndex == wtxIn.m_confirm.nIndex); - assert(wtx.m_confirm.hashBlock == wtxIn.m_confirm.hashBlock); - assert(wtx.m_confirm.block_height == wtxIn.m_confirm.block_height); - } - if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe) - { - wtx.fFromMe = wtxIn.fFromMe; - fUpdated = true; + assert(wtx.m_confirm.nIndex == confirm.nIndex); + assert(wtx.m_confirm.hashBlock == confirm.hashBlock); + assert(wtx.m_confirm.block_height == confirm.block_height); } auto mnList = deterministicMNManager->GetListAtChainTip(); @@ -922,12 +920,12 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) } //// debug print - WalletLogPrintf("AddToWallet %s %s%s\n", wtxIn.GetHash().ToString(), (fInsertedNew ? "new" : ""), (fUpdated ? "update" : "")); + WalletLogPrintf("AddToWallet %s %s%s\n", hash.ToString(), (fInsertedNew ? "new" : ""), (fUpdated ? "update" : "")); // Write to disk if (fInsertedNew || fUpdated) if (!batch.WriteTx(wtx)) - return false; + return nullptr; // Break debit/credit balance caches: wtx.MarkDirty(); @@ -941,7 +939,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) if (!strCmd.empty()) { - ReplaceAll(strCmd, "%s", wtxIn.GetHash().GetHex()); + ReplaceAll(strCmd, "%s", hash.GetHex()); #ifndef WIN32 // Substituting the wallet name isn't currently supported on windows // because windows shell escaping has not been implemented yet: @@ -958,36 +956,37 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) fAnonymizableTallyCached = false; fAnonymizableTallyCachedNonDenom = false; - return true; + return &wtx; } -void CWallet::LoadToWallet(CWalletTx& wtxIn) +bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) { + const auto& ins = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(this, nullptr)); + CWalletTx& wtx = ins.first->second; + if (!fill_wtx(wtx, ins.second)) { + return false; + } // If wallet doesn't have a chain (e.g dash-wallet), don't bother to update txn. if (HaveChain()) { bool active; int height; - if (chain().findBlock(wtxIn.m_confirm.hashBlock, FoundBlock().inActiveChain(active).height(height)) && active) { + if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().inActiveChain(active).height(height)) && active) { // Update cached block height variable since it not stored in the // serialized transaction. - wtxIn.m_confirm.block_height = height; - } else if (wtxIn.isConflicted() || wtxIn.isConfirmed()) { + wtx.m_confirm.block_height = height; + } else if (wtx.isConflicted() || wtx.isConfirmed()) { // If tx block (or conflicting block) was reorged out of chain // while the wallet was shutdown, change tx status to UNCONFIRMED // and reset block height, hash, and index. ABANDONED tx don't have // associated blocks and don't need to be updated. The case where a // transaction was reorged out while online and then reconfirmed // while offline is covered by the rescan logic. - wtxIn.setUnconfirmed(); - wtxIn.m_confirm.hashBlock = uint256(); - wtxIn.m_confirm.block_height = 0; - wtxIn.m_confirm.nIndex = 0; + wtx.setUnconfirmed(); + wtx.m_confirm.hashBlock = uint256(); + wtx.m_confirm.block_height = 0; + wtx.m_confirm.nIndex = 0; } } - uint256 hash = wtxIn.GetHash(); - const auto& ins = mapWallet.emplace(hash, wtxIn); - CWalletTx& wtx = ins.first->second; - wtx.BindWallet(this); if (/* insertion took place */ ins.second) { wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); } @@ -1001,6 +1000,7 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn) } } } + return true; } bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool fUpdate) @@ -1046,13 +1046,9 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co } } - CWalletTx wtx(this, ptx); - // Block disconnection override an abandoned tx as unconfirmed // which means user may have to call abandontransaction again - wtx.m_confirm = confirm; - - return AddToWallet(wtx, false); + return AddToWallet(MakeTransactionRef(tx), confirm, /* update_wtx= */ nullptr, /* fFlushOnClose= */ false); } } return false; @@ -3459,20 +3455,17 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac // rediscover unknown transactions that were written with keys of ours to recover // post-backup change. - // Reserve a new key pair from key pool - if (!CanGetAddresses(true)) { - error = _("Can't generate a change-address key. No keys in the internal keypool and can't generate any keys."); - return false; - } + // Reserve a new key pair from key pool. If it fails, provide a dummy + // destination in case we don't need change. CTxDestination dest; - bool ret = reservedest.GetReservedDestination(dest, true); - if (!ret) - { - error = _("Keypool ran out, please call keypoolrefill first"); - return false; + if (!reservedest.GetReservedDestination(dest, true)) { + error = _("Transaction needs a change address, but we can't generate it. Please call keypoolrefill first."); } - scriptChange = GetScriptForDestination(dest); + // A valid destination implies a change script (and + // vice-versa). An empty change script will abort later, if the + // change keypool ran out, but change is required. + CHECK_NONFATAL(IsValidDestination(dest) != scriptChange.empty()); } nFeeRet = 0; @@ -3722,6 +3715,11 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac error = _("Exceeded max tries."); return false; } + + // Give up if change keypool ran out and change is required + if (scriptChange.empty() && nChangePosInOut != -1) { + return false; + } } // Make sure change position was updated one way or another @@ -3767,32 +3765,34 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm) { LOCK(cs_wallet); + WalletLogPrintf("CommitTransaction:\n%s", tx->ToString()); /* Continued */ - CWalletTx wtxNew(this, std::move(tx)); - wtxNew.mapValue = std::move(mapValue); - wtxNew.vOrderForm = std::move(orderForm); - wtxNew.fTimeReceivedIsTxTime = true; - wtxNew.fFromMe = true; - - WalletLogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString()); /* Continued */ // Add tx to wallet, because if it has change it's also ours, // otherwise just for transaction history. - AddToWallet(wtxNew); + AddToWallet(tx, {}, [&](CWalletTx& wtx, bool new_tx) { + CHECK_NONFATAL(wtx.mapValue.empty()); + CHECK_NONFATAL(wtx.vOrderForm.empty()); + wtx.mapValue = std::move(mapValue); + wtx.vOrderForm = std::move(orderForm); + wtx.fTimeReceivedIsTxTime = true; + wtx.fFromMe = true; + return true; + }); // Notify that old coins are spent std::set updated_hahes; - for (const CTxIn& txin : wtxNew.tx->vin){ + for (const CTxIn& txin : tx->vin){ // notify only once if(updated_hahes.find(txin.prevout.hash) != updated_hahes.end()) continue; CWalletTx &coin = mapWallet.at(txin.prevout.hash); - coin.BindWallet(this); + coin.MarkDirty(); NotifyTransactionChanged(this, txin.prevout.hash, CT_UPDATED); updated_hahes.insert(txin.prevout.hash); } // Get the inserted-CWalletTx from mapWallet so that the // fInMempool flag is cached properly - CWalletTx& wtx = mapWallet.at(wtxNew.GetHash()); + CWalletTx& wtx = mapWallet.at(tx->GetHash()); if (!fBroadcastTransactions) { // Don't submit tx to the mempool @@ -3900,7 +3900,6 @@ DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vector::max(); + for (const auto& spk_man_pair : m_spk_managers) { + oldestKey = std::min(oldestKey, spk_man_pair.second->GetOldestKeyPoolTime()); + } + + return oldestKey; +} + void CWallet::MarkDestinationsDirty(const std::set& destinations) { for (auto& entry : mapWallet) { CWalletTx& wtx = entry.second; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e70c4b60507d..bda3989e0198 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -355,15 +355,15 @@ class CWalletTx mutable bool fInMempool; mutable CAmount nChangeCached; - CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) - : tx(std::move(arg)) + CWalletTx(const CWallet* wallet, CTransactionRef arg) + : pwallet(wallet), + tx(std::move(arg)) { - Init(pwalletIn); + Init(); } - void Init(const CWallet* pwalletIn) + void Init() { - pwallet = pwalletIn; mapValue.clear(); vOrderForm.clear(); fTimeReceivedIsTxTime = false; @@ -431,7 +431,7 @@ class CWalletTx template void Unserialize(Stream& s) { - Init(nullptr); + Init(); std::vector dummy_vector1; //!< Used to be vMerkleBranch std::vector dummy_vector2; //!< Used to be vtxPrev @@ -483,12 +483,6 @@ class CWalletTx m_is_cache_empty = true; } - void BindWallet(CWallet *pwalletIn) - { - pwallet = pwalletIn; - MarkDirty(); - } - const CWallet* GetWallet() const { return pwallet; @@ -585,6 +579,12 @@ class CWalletTx const uint256& GetHash() const { return tx->GetHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } bool IsImmatureCoinBase() const; + + // Disable copying of CWalletTx objects to prevent bugs where instances get + // copied in and out of the mapWallet map, and fields are updated in the + // wrong copy. + CWalletTx(CWalletTx const &) = delete; + void operator=(CWalletTx const &x) = delete; }; struct WalletTxHasher @@ -984,8 +984,17 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati DBErrors ReorderTransactions(); void MarkDirty(); - bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); - void LoadToWallet(CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + + //! Callback for updating transaction metadata in mapWallet. + //! + //! @param wtx - reference to mapWallet transaction to update + //! @param new_tx - true if wtx is newly inserted, false if it previously existed + //! + //! @return true if wtx is changed and needs to be saved to disk, otherwise false + using UpdateWalletTxFn = std::function; + + CWalletTx* AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true); + bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) override; void blockConnected(const CBlock& block, int height) override; void blockDisconnected(const CBlock& block, int height) override; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 9b922201bb3f..023a4b94a556 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -264,36 +264,43 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, } else if (strType == DBKeys::TX) { uint256 hash; ssKey >> hash; - CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); - ssValue >> wtx; - if (wtx.GetHash() != hash) - return false; + // LoadToWallet call below creates a new CWalletTx that fill_wtx + // callback fills with transaction metadata. + auto fill_wtx = [&](CWalletTx& wtx, bool new_tx) { + assert(new_tx); + ssValue >> wtx; + if (wtx.GetHash() != hash) + return false; - // Undo serialize changes in 31600 - if (31404 <= wtx.fTimeReceivedIsTxTime && wtx.fTimeReceivedIsTxTime <= 31703) - { - if (!ssValue.empty()) - { - char fTmp; - char fUnused; - std::string unused_string; - ssValue >> fTmp >> fUnused >> unused_string; - strErr = strprintf("LoadWallet() upgrading tx ver=%d %d %s", - wtx.fTimeReceivedIsTxTime, fTmp, hash.ToString()); - wtx.fTimeReceivedIsTxTime = fTmp; - } - else + // Undo serialize changes in 31600 + if (31404 <= wtx.fTimeReceivedIsTxTime && wtx.fTimeReceivedIsTxTime <= 31703) { - strErr = strprintf("LoadWallet() repairing tx ver=%d %s", wtx.fTimeReceivedIsTxTime, hash.ToString()); - wtx.fTimeReceivedIsTxTime = 0; + if (!ssValue.empty()) + { + char fTmp; + char fUnused; + std::string unused_string; + ssValue >> fTmp >> fUnused >> unused_string; + strErr = strprintf("LoadWallet() upgrading tx ver=%d %d %s", + wtx.fTimeReceivedIsTxTime, fTmp, hash.ToString()); + wtx.fTimeReceivedIsTxTime = fTmp; + } + else + { + strErr = strprintf("LoadWallet() repairing tx ver=%d %s", wtx.fTimeReceivedIsTxTime, hash.ToString()); + wtx.fTimeReceivedIsTxTime = 0; + } + wss.vWalletUpgrade.push_back(hash); } - wss.vWalletUpgrade.push_back(hash); - } - if (wtx.nOrderPos == -1) - wss.fAnyUnordered = true; + if (wtx.nOrderPos == -1) + wss.fAnyUnordered = true; - pwallet->LoadToWallet(wtx); + return true; + }; + if (!pwallet->LoadToWallet(hash, fill_wtx)) { + return false; + } } else if (strType == DBKeys::WATCHS) { wss.nWatchKeys++; CScript script; @@ -665,7 +672,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) return result; } -DBErrors WalletBatch::FindWalletTx(std::vector& vTxHash, std::vector& vWtx) +DBErrors WalletBatch::FindWalletTx(std::vector& vTxHash, std::list& vWtx) { DBErrors result = DBErrors::LOAD_OK; @@ -703,12 +710,9 @@ DBErrors WalletBatch::FindWalletTx(std::vector& vTxHash, std::vector> hash; - - CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); - ssValue >> wtx; - vTxHash.push_back(hash); - vWtx.push_back(wtx); + vWtx.emplace_back(nullptr /* wallet */, nullptr /* tx */); + ssValue >> vWtx.back(); } } } catch (...) { @@ -723,7 +727,7 @@ DBErrors WalletBatch::ZapSelectTx(std::vector& vTxHashIn, std::vector vTxHash; - std::vector vWtx; + std::list vWtx; DBErrors err = FindWalletTx(vTxHash, vWtx); if (err != DBErrors::LOAD_OK) { return err; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 1ebfedfb8d29..b7863fc66060 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -208,7 +208,7 @@ class WalletBatch bool EraseDestData(const std::string &address, const std::string &key); DBErrors LoadWallet(CWallet* pwallet); - DBErrors FindWalletTx(std::vector& vTxHash, std::vector& vWtx); + DBErrors FindWalletTx(std::vector& vTxHash, std::list& vWtx); DBErrors ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut); /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */ static bool IsKeyType(const std::string& strType); diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 4b95562ad8e4..d61a561c5c06 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -492,12 +492,20 @@ def test_locked_wallet(self): # Drain the keypool. self.nodes[1].getnewaddress() - inputs = [] - outputs = {self.nodes[0].getnewaddress():1.1} + inputs = self.nodes[1].listunspent() + # Deduce fee to produce a changeless transaction + # bnb doesn't work same way as in bitcoin, so, `value` is also calculated by different way + value = sum(input_it["amount"] for input_it in inputs) - Decimal("0.00002200") + outputs = {self.nodes[0].getnewaddress():value} rawtx = self.nodes[1].createrawtransaction(inputs, outputs) + # fund a transaction that does not require a new key for the change output + self.nodes[1].fundrawtransaction(rawtx) + # fund a transaction that requires a new key for the change output # creating the key must be impossible because the wallet is locked - assert_raises_rpc_error(-4, "Keypool ran out, please call keypoolrefill first", self.nodes[1].fundrawtransaction, rawtx) + outputs = {self.nodes[0].getnewaddress():1.1} + rawtx = self.nodes[1].createrawtransaction(inputs, outputs) + assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it. Please call keypoolrefill first.", self.nodes[1].fundrawtransaction, rawtx) # Refill the keypool. self.nodes[1].walletpassphrase("test", 100) diff --git a/test/functional/wallet_keypool_hd.py b/test/functional/wallet_keypool_hd.py index d22effdeafb6..0e45cdb52a09 100755 --- a/test/functional/wallet_keypool_hd.py +++ b/test/functional/wallet_keypool_hd.py @@ -8,6 +8,7 @@ # Add python-bitcoinrpc to module search path: import time +from decimal import Decimal from test_framework.authproxy import JSONRPCException from test_framework.test_framework import BitcoinTestFramework @@ -16,7 +17,6 @@ class KeyPoolTest(BitcoinTestFramework): def set_test_params(self): - self.setup_clean_chain = True self.num_nodes = 1 self.extra_args = [['-usehd=1']] @@ -101,5 +101,51 @@ def run_test(self): assert_equal(wi['keypoolsize_hd_internal'], 100) assert_equal(wi['keypoolsize'], 100) + # create a blank wallet + nodes[0].createwallet(wallet_name='w2', blank=True) + w2 = nodes[0].get_wallet_rpc('w2') + + # refer to initial wallet as w1 + w1 = nodes[0].get_wallet_rpc(self.default_wallet_name) + + # import private key and fund it + address = addr.pop() + privkey = w1.dumpprivkey(address) + res = w2.importmulti([{'scriptPubKey': {'address': address}, 'keys': [privkey], 'timestamp': 'now'}]) + assert_equal(res[0]['success'], True) + w1.walletpassphrase('test', 100) + + res = w1.sendtoaddress(address=address, amount=0.00010000) + nodes[0].generate(1) + destination = addr.pop() + + # Using a fee rate (10 sat / byte) well above the minimum relay rate + # creating a 5,000 sat transaction with change should not be possible + assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it. Please call keypoolrefill first.", w2.walletcreatefundedpsbt, inputs=[], outputs=[{addr.pop(): 0.00005000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010}) + + # creating a 10,000 sat transaction without change, with a manual input, should still be possible + res = w2.walletcreatefundedpsbt(inputs=w2.listunspent(), outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010}) + assert_equal("psbt" in res, True) + + # creating a 10,000 sat transaction without change should still be possible + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010}) + assert_equal("psbt" in res, True) + # should work without subtractFeeFromOutputs if the exact fee is subtracted from the amount + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008900}], options={"feeRate": 0.000010}) + assert_equal("psbt" in res, True) + + # dust change should be removed + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008800}], options={"feeRate": 0.000010}) + assert_equal("psbt" in res, True) + + # create a transaction without change at the maximum fee rate, such that the output is still spendable: + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.00008824}) + assert_equal("psbt" in res, True) + assert_equal(res["fee"], Decimal("0.00001685")) + + # creating a 10,000 sat transaction with a manual change address should be possible + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010, "changeAddress": addr.pop()}) + assert_equal("psbt" in res, True) + if __name__ == '__main__': KeyPoolTest().main() diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 5ec99a4233e2..cb0dde6625eb 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -75,9 +75,8 @@ def wallet_file(name): # create symlink to verify wallet directory path can be referenced # through symlink - if os.name != 'nt': - os.mkdir(wallet_dir('w7')) - os.symlink('w7', wallet_dir('w7_symlink')) + os.mkdir(wallet_dir('w7')) + os.symlink('w7', wallet_dir('w7_symlink')) # rename wallet.dat to make sure plain wallet file paths (as opposed to # directory paths) can be loaded @@ -109,8 +108,6 @@ def wallet_file(name): to_load.append('w8') wallet_names = to_create + to_load # Wallet names loaded in the wallet in_wallet_dir += to_load # The loaded wallets are also in the wallet dir - if os.name == 'nt': - wallet_names.remove('w7_symlink') self.start_node(0) for wallet_name in to_create: @@ -146,9 +143,8 @@ def wallet_file(name): self.nodes[0].assert_start_raises_init_error(['-wallet=w8', '-wallet=w8_copy'], exp_stderr, match=ErrorMatch.PARTIAL_REGEX) # should not initialize if wallet file is a symlink - if os.name != 'nt': - os.symlink('w8', wallet_dir('w8_symlink')) - self.nodes[0].assert_start_raises_init_error(['-wallet=w8_symlink'], r'Error: Invalid -wallet path \'w8_symlink\'\. .*', match=ErrorMatch.FULL_REGEX) + os.symlink('w8', wallet_dir('w8_symlink')) + self.nodes[0].assert_start_raises_init_error(['-wallet=w8_symlink'], r'Error: Invalid -wallet path \'w8_symlink\'\. .*', match=ErrorMatch.FULL_REGEX) # should not initialize if the specified walletdir does not exist self.nodes[0].assert_start_raises_init_error(['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist') @@ -300,8 +296,7 @@ def wallet_file(name): assert_raises_rpc_error(-4, "BerkeleyDatabase: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy') # Fail to load if wallet file is a symlink - if os.name != 'nt': - assert_raises_rpc_error(-4, "Wallet file verification failed. Invalid -wallet path 'w8_symlink'", self.nodes[0].loadwallet, 'w8_symlink') + assert_raises_rpc_error(-4, "Wallet file verification failed. Invalid -wallet path 'w8_symlink'", self.nodes[0].loadwallet, 'w8_symlink') self.log.info("Test dynamic wallet creation.")