diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp index 013f56342e..a874fab0f7 100644 --- a/src/bench/rpc_mempool.cpp +++ b/src/bench/rpc_mempool.cpp @@ -12,7 +12,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, fee, /* time */ 0, /* height */ 1, /* spendsCoinbase */ false, /* sigOps */ 1, lp)); + pool.addUnchecked(CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*spends_coinbase=*/false, /*sigops*/1, lp)); } static void RpcMempool(benchmark::Bench& bench) diff --git a/src/net.cpp b/src/net.cpp index 76723a4fd5..e1c86e2396 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -4789,7 +4789,7 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) size_t nMessageSize = msg.data.size(); LogPrint(BCLog::NET, "sending %s (%d bytes) peer=%d\n", msg.m_type, nMessageSize, pnode->GetId()); if (gArgs.GetBoolArg("-capturemessages", false)) { - CaptureMessage(pnode->addr, msg.m_type, msg.data, /* incoming */ false); + CaptureMessage(pnode->addr, msg.m_type, msg.data, /*is_incoming=*/false); } TRACE6(net, outbound_message, diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 02b81c1f51..4ea95ac0ca 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5094,7 +5094,7 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic& interrupt ); if (gArgs.GetBoolArg("-capturemessages", false)) { - CaptureMessage(pfrom->addr, msg.m_type, MakeUCharSpan(msg.m_recv), /* incoming */ true); + CaptureMessage(pfrom->addr, msg.m_type, MakeUCharSpan(msg.m_recv), /* is_incoming */ true); } msg.SetVersion(pfrom->GetCommonVersion()); diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index 78d8331eb3..34ea983441 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -74,7 +74,7 @@ int main(int argc, char* argv[]) #if defined(WIN32) if (getenv("QT_QPA_PLATFORM") == nullptr) _putenv_s("QT_QPA_PLATFORM", "minimal"); #else - setenv("QT_QPA_PLATFORM", "minimal", /* overwrite */ 0); + setenv("QT_QPA_PLATFORM", "minimal", 0 /* overwrite */); #endif BitcoinApplication app; diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index d8c60b9e93..2133999b9b 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -198,11 +198,12 @@ WalletControllerActivity::~WalletControllerActivity() delete m_progress_dialog; } -void WalletControllerActivity::showProgressDialog(const QString& label_text) +void WalletControllerActivity::showProgressDialog(const QString& title_text, const QString& label_text) { assert(!m_progress_dialog); m_progress_dialog = new QProgressDialog(m_parent_widget); + m_progress_dialog->setWindowTitle(title_text); m_progress_dialog->setLabelText(label_text); m_progress_dialog->setRange(0, 0); m_progress_dialog->setCancelButton(nullptr); @@ -251,7 +252,12 @@ void CreateWalletActivity::askPassphrase() void CreateWalletActivity::createWallet() { - showProgressDialog(tr("Creating Wallet %1…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped())); + showProgressDialog( + //: Title of window indicating the progress of creation of a new wallet. + tr("Create Wallet"), + /*: Descriptive text of the create wallet progress window which indicates + to the user which wallet is currently being created. */ + tr("Creating Wallet %1…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped())); std::string name = m_create_wallet_dialog->walletName().toStdString(); uint64_t flags = 0; @@ -334,7 +340,12 @@ void OpenWalletActivity::open(const std::string& path) { QString name = path.empty() ? QString("["+tr("default wallet")+"]") : QString::fromStdString(path); - showProgressDialog(tr("Opening Wallet %1…").arg(name.toHtmlEscaped())); + showProgressDialog( + //: Title of window indicating the progress of opening of a wallet. + tr("Open Wallet"), + /*: Descriptive text of the open wallet progress window which indicates + to the user which wallet is currently being opened. */ + tr("Opening Wallet %1…").arg(name.toHtmlEscaped())); QTimer::singleShot(0, worker(), [this, path] { std::unique_ptr wallet = node().walletLoader().loadWallet(path, m_error_message, m_warning_message); diff --git a/src/qt/walletcontroller.h b/src/qt/walletcontroller.h index 07c781791f..f65455e7c8 100644 --- a/src/qt/walletcontroller.h +++ b/src/qt/walletcontroller.h @@ -98,7 +98,7 @@ protected: interfaces::Node& node() const { return m_wallet_controller->m_node; } QObject* worker() const { return m_wallet_controller->m_activity_worker; } - void showProgressDialog(const QString& label_text); + void showProgressDialog(const QString& title_text, const QString& label_text); void destroyProgressDialog(); WalletController* const m_wallet_controller; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 57f22f4bd7..4160636467 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1378,7 +1378,7 @@ CoinStatsHashType ParseHashType(const std::string& hash_type_input) } else if (hash_type_input == "none") { return CoinStatsHashType::NONE; } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s is not a valid hash_type", hash_type_input)); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("'%s' is not a valid hash_type", hash_type_input)); } } @@ -2524,7 +2524,7 @@ static RPCHelpMan getblockstats() for (const std::string& stat : stats) { const UniValue& value = ret_all[stat]; if (value.isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid selected statistic %s", stat)); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid selected statistic '%s'", stat)); } ret.pushKV(stat, value); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 50956048da..6b7e46dc33 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1060,7 +1060,7 @@ static RPCHelpMan submitblock() bool new_block; auto sc = std::make_shared(block.GetHash()); RegisterSharedValidationInterface(sc); - bool accepted = chainman.ProcessNewBlock(Params(), blockptr, /* fForceProcessing */ true, /* fNewBlock */ &new_block); + bool accepted = chainman.ProcessNewBlock(Params(), blockptr, /*force_processing=*/true, /*new_block=*/&new_block); UnregisterSharedValidationInterface(sc); if (!new_block && accepted) { return "duplicate"; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index d3c37f7345..208529414e 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1849,7 +1849,7 @@ static RPCHelpMan utxoupdatepsbt() } } // We don't actually need private keys further on; hide them as a precaution. - HidingSigningProvider public_provider(&provider, /* nosign */ true, /* nobip32derivs */ false); + HidingSigningProvider public_provider(&provider, /*hide_secret=*/true, /*hide_origin=*/false); // Fetch previous transactions (inputs): CCoinsView viewDummy; @@ -1881,7 +1881,7 @@ static RPCHelpMan utxoupdatepsbt() // Update script/keypath information using descriptor data. // Note that SignPSBTInput does a lot more than just constructing ECDSA signatures // we don't actually care about those here, in fact. - SignPSBTInput(public_provider, psbtx, i, /* sighash_type */ SIGHASH_ALL); + SignPSBTInput(public_provider, psbtx, i, /*sighash=*/ SIGHASH_ALL); } // Update script/keypath information using descriptor data. diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 3b02da632f..3741a1b9a9 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -255,7 +255,7 @@ CPubKey AddrToPubKey(const FillableSigningProvider& keystore, const std::string& } const PKHash *pkhash = std::get_if(&dest); if (!pkhash) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("%s does not refer to a key", addr_in)); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' does not refer to a key", addr_in)); } CPubKey vchPubKey; if (!keystore.GetPubKey(ToKeyID(*pkhash), vchPubKey)) { diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 30046a3052..9cebbf5b8c 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -329,7 +329,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) /*nKeyedNetGroupIn=*/1, /*nLocalHostNonceIn=*/1, CAddress(), - /*pszDest=*/"", + /*addrNameIn=*/"", ConnectionType::INBOUND, /*inbound_onion=*/false}; nodes[1]->SetCommonVersion(PROTOCOL_VERSION); @@ -360,7 +360,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) /*nKeyedNetGroupIn=*/1, /*nLocalHostNonceIn=*/1, CAddress(), - /*pszDest=*/"", + /*addrNameIn=*/"", ConnectionType::OUTBOUND_FULL_RELAY, /*inbound_onion=*/false}; nodes[2]->SetCommonVersion(PROTOCOL_VERSION); diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index cbdffe29d8..c689b52cc6 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -93,7 +93,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, con const auto info_all = tx_pool.infoAll(); if (!info_all.empty()) { const auto& tx_to_remove = *PickValue(fuzzed_data_provider, info_all).tx; - WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, /* dummy */ MemPoolRemovalReason::BLOCK)); + WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, MemPoolRemovalReason::BLOCK /* dummy */)); std::vector all_txids; tx_pool.queryHashes(all_txids); assert(all_txids.size() < info_all.size()); diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 352421a815..e532da6289 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -221,7 +221,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) { bool ignored; auto ProcessBlock = [&](std::shared_ptr block) -> bool { - return Assert(m_node.chainman)->ProcessNewBlock(Params(), block, /* fForceProcessing */ true, /* fNewBlock */ &ignored); + return Assert(m_node.chainman)->ProcessNewBlock(Params(), block, /*force_processing=*/true, /*new_block=*/&ignored); }; // Process all mined blocks diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 9042bd25bc..af8b041ad2 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -324,6 +324,7 @@ public: std::map m_descriptor_caches; std::map, CKey> m_descriptor_keys; std::map, std::pair>> m_descriptor_crypt_keys; + bool tx_corrupt{false}; CWalletScanState() { } @@ -358,7 +359,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, // 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); + if(!new_tx) { + // There's some corruption here since the tx we just tried to load was already in the wallet. + // We don't consider this type of corruption critical, and can fix it by removing tx data and + // rescanning. + wss.tx_corrupt = true; + return false; + } ssValue >> wtx; if (wtx.GetHash() != hash) return false; @@ -797,6 +804,11 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) } else if (strType == DBKeys::FLAGS) { // reading the wallet flags can only fail if unknown flags are present result = DBErrors::TOO_NEW; + } else if (wss.tx_corrupt) { + pwallet->WalletLogPrintf("Error: Corrupt transaction found. This can be fixed by removing transactions from wallet and rescanning.\n"); + // Set tx_corrupt back to false so that the error is only printed once (per corrupt tx) + wss.tx_corrupt = false; + result = DBErrors::CORRUPT; } else { // Leave other errors alone, if we try to fix them we might make things worse. fNoncriticalErrors = true; // ... but do warn the user there is something wrong. diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 9547f44624..28deda6908 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -300,7 +300,7 @@ class BlockchainTest(BitcoinTestFramework): assert 'muhash' not in r # Unknown hash_type raises an error - assert_raises_rpc_error(-8, "foohash is not a valid hash_type", node.gettxoutsetinfo, "foohash") + assert_raises_rpc_error(-8, "'foo hash' is not a valid hash_type", node.gettxoutsetinfo, "foo hash") def _test_getblockheader(self): self.log.info("Test getblockheader") diff --git a/test/functional/rpc_getblockstats.py b/test/functional/rpc_getblockstats.py index ebcf4e2ebb..19295b70e5 100755 --- a/test/functional/rpc_getblockstats.py +++ b/test/functional/rpc_getblockstats.py @@ -143,17 +143,17 @@ class GetblockstatsTest(BitcoinTestFramework): inv_sel_stat = 'asdfghjkl' inv_stats = [ [inv_sel_stat], - ['minfee' , inv_sel_stat], + ['minfee', inv_sel_stat], [inv_sel_stat, 'minfee'], ['minfee', inv_sel_stat, 'maxfee'], ] for inv_stat in inv_stats: - assert_raises_rpc_error(-8, 'Invalid selected statistic %s' % inv_sel_stat, + assert_raises_rpc_error(-8, f"Invalid selected statistic '{inv_sel_stat}'", self.nodes[0].getblockstats, hash_or_height=1, stats=inv_stat) # Make sure we aren't always returning inv_sel_stat as the culprit stat - assert_raises_rpc_error(-8, 'Invalid selected statistic aaa%s' % inv_sel_stat, - self.nodes[0].getblockstats, hash_or_height=1, stats=['minfee' , 'aaa%s' % inv_sel_stat]) + assert_raises_rpc_error(-8, f"Invalid selected statistic 'aaa{inv_sel_stat}'", + self.nodes[0].getblockstats, hash_or_height=1, stats=['minfee', f'aaa{inv_sel_stat}']) # Mainchain's genesis block shouldn't be found on regtest assert_raises_rpc_error(-5, 'Block not found', self.nodes[0].getblockstats, hash_or_height='000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f')