From 25c41a3f2ae24975fbcf4fef9ea3484985a45d29 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 12 Oct 2023 11:59:47 +0200 Subject: [PATCH 1/4] Merge bitcoin/bitcoin#28629: test: fix usdt undeclared function errors on mantis 4077e43bf62e5afe90d204b9ede9290ef54dee0f test: fix usdt undeclared function errors on mantis (willcl-ark) Pull request description: This is one way to fix #28600 Recently usage of undeclared functions became an error rather than a warning, in C2x. https://reviews.llvm.org/D122983?id=420290 This change has migrated into the build tools of Ubuntu 23.10 which now causes the USDT tests to fail to compile, see https://github.com/bitcoin/bitcoin/issues/28600 I think there are various potential fixes: 1. Manually declare the functions we use 2. Fix imports so that manual declarations aren't needed 3. Revert the new C2X behaviour and don't error on implicit function declarations I would have preferred solution 2, but I believe this will require changes to the upstream bcc package. Having played with the imports I can get things working in a standalone C program, using system headers, but when building the program from a python context as we do in the test it uses its own headers (bundled with the python lib) rather than the system ones, and manually importing (some) system headers results in definition mismatches. I also investigated explicitly importing required headers from the package, which use paths like `#import `, but this seems more obtuse and brittle than simply ignoring the warning. Therefore I think that until the upstream python pacakge fixes their declarations, we should fix this by setting `-Wno-error=implicit-function-declaration` for the tracing programs. cc maflcko 0xB10C ACKs for top commit: maflcko: lgtm ACK 4077e43bf62e5afe90d204b9ede9290ef54dee0f Tree-SHA512: 8368bb1155e920a95db128dc893267f8dab64f1ae53f6d63c6d9294e2e4e92bef8515e3697e9113228bedc51c0afdbc5bbcf558c119bf0eb3293dc2ced86b435 Co-authored-by: fanquake --- test/functional/interface_usdt_coinselection.py | 2 +- test/functional/interface_usdt_mempool.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/functional/interface_usdt_coinselection.py b/test/functional/interface_usdt_coinselection.py index cb1fe12a409b..29fedd664931 100755 --- a/test/functional/interface_usdt_coinselection.py +++ b/test/functional/interface_usdt_coinselection.py @@ -166,7 +166,7 @@ def run_test(self): ctx.enable_probe(probe="coin_selection:normal_create_tx_internal", fn_name="trace_normal_create_tx") ctx.enable_probe(probe="coin_selection:attempting_aps_create_tx", fn_name="trace_attempt_aps") ctx.enable_probe(probe="coin_selection:aps_create_tx_internal", fn_name="trace_aps_create_tx") - self.bpf = BPF(text=coinselection_tracepoints_program, usdt_contexts=[ctx], debug=0) + self.bpf = BPF(text=coinselection_tracepoints_program, usdt_contexts=[ctx], debug=0, cflags=["-Wno-error=implicit-function-declaration"]) self.log.info("Prepare wallets") self.generate(self.nodes[0], 101) diff --git a/test/functional/interface_usdt_mempool.py b/test/functional/interface_usdt_mempool.py index 10b244150cc6..f58af7903004 100755 --- a/test/functional/interface_usdt_mempool.py +++ b/test/functional/interface_usdt_mempool.py @@ -117,7 +117,7 @@ def added_test(self): node = self.nodes[0] ctx = USDT(pid=node.process.pid) ctx.enable_probe(probe="mempool:added", fn_name="trace_added") - bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0) + bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0, cflags=["-Wno-error=implicit-function-declaration"]) def handle_added_event(_, data, __): events.append(bpf["added_events"].event(data)) @@ -153,7 +153,7 @@ def removed_test(self): node = self.nodes[0] ctx = USDT(pid=node.process.pid) ctx.enable_probe(probe="mempool:removed", fn_name="trace_removed") - bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0) + bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0, cflags=["-Wno-error=implicit-function-declaration"]) def handle_removed_event(_, data, __): events.append(bpf["removed_events"].event(data)) @@ -201,7 +201,7 @@ def rejected_test(self): self.log.info("Hooking into mempool:rejected tracepoint...") ctx = USDT(pid=node.process.pid) ctx.enable_probe(probe="mempool:rejected", fn_name="trace_rejected") - bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0) + bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0, cflags=["-Wno-error=implicit-function-declaration"]) def handle_rejected_event(_, data, __): events.append(bpf["rejected_events"].event(data)) From ed19c19e00fc2c1a7754954af1075b1516781dcf Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 13 Oct 2023 11:07:25 +0200 Subject: [PATCH 2/4] partial Merge bitcoin/bitcoin#28639: refactor: Remove unused nchaintx from SnapshotMetadata constructor, fix test, add test BACKPORT NOTE: missing functional tests test/functional/feature_assumeutxo.py as no RPC loadtxoutset is backported yet ---- fafde92f84fb7c245bc3c1cd946a32c891861e5e test: Check snapshot file with wrong number of coins (MarcoFalke) faa90f6e7b6b8c531e1ae142a5c2f568b48502a9 refactor: Remove unused nchaintx from SnapshotMetadata constructor (MarcoFalke) Pull request description: See commit messages ACKs for top commit: Sjors: utACK fafde92f84fb7c245bc3c1cd946a32c891861e5e theStack: ACK fafde92f84fb7c245bc3c1cd946a32c891861e5e Tree-SHA512: 9ed2720b50d1c0938f30543ba143e1a4c6af3a0ff166f8b3eb452e1d99ddee6e3443a4c99f77efe94b8c3eb2feff984bf5259807ee8085e1e0e1e0d1de98227e Co-authored-by: fanquake --- src/node/utxo_snapshot.h | 3 +-- src/rpc/blockchain.cpp | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h index 8ce0e4fee0d4..18927452c096 100644 --- a/src/node/utxo_snapshot.h +++ b/src/node/utxo_snapshot.h @@ -35,8 +35,7 @@ class SnapshotMetadata SnapshotMetadata() { } SnapshotMetadata( const uint256& base_blockhash, - uint64_t coins_count, - unsigned int nchaintx) : + uint64_t coins_count) : m_base_blockhash(base_blockhash), m_coins_count(coins_count) { } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9eeae575a609..a43e28d48d1d 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -3232,7 +3232,7 @@ UniValue CreateUTXOSnapshot( tip->nHeight, tip->GetBlockHash().ToString(), fs::PathToString(path), fs::PathToString(temppath))); - SnapshotMetadata metadata{tip->GetBlockHash(), maybe_stats->coins_count, tip->nChainTx}; + SnapshotMetadata metadata{tip->GetBlockHash(), maybe_stats->coins_count}; afile << metadata; @@ -3259,9 +3259,7 @@ UniValue CreateUTXOSnapshot( result.pushKV("base_height", tip->nHeight); result.pushKV("path", path.utf8string()); result.pushKV("txoutset_hash", maybe_stats->hashSerialized.ToString()); - // Cast required because univalue doesn't have serialization specified for - // `unsigned int`, nChainTx's type. - result.pushKV("nchaintx", uint64_t{tip->nChainTx}); + result.pushKV("nchaintx", tip->nChainTx); return result; } From eefd3ace71de5a38d472ef2fb5f436b41de14e8d Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 16 Oct 2023 13:29:06 +0100 Subject: [PATCH 3/4] Merge bitcoin-core/gui#765: Fix wallet list hover crash on shutdown 8b6470a90652fcffc45b8d7998af7c8ad6251332 gui: disable top bar menu actions during shutdown (furszy) 7066e8996d0ac090535cc97cdcb54a219986460f gui: provide wallet controller context to wallet actions (furszy) Pull request description: Small follow-up to #751. Fixes another crash cause during shutdown. Which occurs when the user hovers over the wallets list. Future Note: This surely happen in other places as well, we should re-work the way we connect signals. Register lambas without any precaution can leave dangling pointers. ACKs for top commit: hebasto: ACK 8b6470a90652fcffc45b8d7998af7c8ad6251332, I've tested each commit separately on macOS Sonoma 14.0 (Apple M1). Tree-SHA512: 6fbd1bcd6717a8c1633beb9371463ed22422f929cccf9b791ee292c5364134c501e099329cf77a06b74a84c64c1c3d22539199ec49ccd74b3950036316c0dab3 Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> --- src/qt/bitcoingui.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index a6aee505b904..744424ca8b23 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -558,7 +558,7 @@ void BitcoinGUI::createActions() connect(usedSendingAddressesAction, &QAction::triggered, walletFrame, &WalletFrame::usedSendingAddresses); connect(usedReceivingAddressesAction, &QAction::triggered, walletFrame, &WalletFrame::usedReceivingAddresses); connect(openAction, &QAction::triggered, this, &BitcoinGUI::openClicked); - connect(m_open_wallet_menu, &QMenu::aboutToShow, [this] { + connect(m_open_wallet_menu, &QMenu::aboutToShow, m_wallet_controller, [this] { m_open_wallet_menu->clear(); for (const std::pair& i : m_wallet_controller->listWalletDir()) { const std::string& path = i.first; @@ -574,7 +574,7 @@ void BitcoinGUI::createActions() continue; } - connect(action, &QAction::triggered, [this, path] { + connect(action, &QAction::triggered, m_wallet_controller, [this, path] { auto activity = new OpenWalletActivity(m_wallet_controller, this); connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet, Qt::QueuedConnection); connect(activity, &OpenWalletActivity::opened, rpcConsole, &RPCConsole::setCurrentWallet, Qt::QueuedConnection); @@ -586,7 +586,7 @@ void BitcoinGUI::createActions() action->setEnabled(false); } }); - connect(m_restore_wallet_action, &QAction::triggered, [this] { + connect(m_restore_wallet_action, &QAction::triggered, m_wallet_controller, [this] { //: Name of the wallet data file format. QString name_data_file = tr("Wallet Data"); @@ -612,19 +612,19 @@ void BitcoinGUI::createActions() auto backup_file_path = fs::PathFromString(backup_file.toStdString()); activity->restore(backup_file_path, wallet_name.toStdString()); }); - connect(m_close_wallet_action, &QAction::triggered, [this] { + connect(m_close_wallet_action, &QAction::triggered, m_wallet_controller, [this] { m_wallet_controller->closeWallet(walletFrame->currentWalletModel(), this); }); - connect(m_create_wallet_action, &QAction::triggered, [this] { + connect(m_create_wallet_action, &QAction::triggered, m_wallet_controller, [this] { auto activity = new CreateWalletActivity(m_wallet_controller, this); connect(activity, &CreateWalletActivity::created, this, &BitcoinGUI::setCurrentWallet); connect(activity, &CreateWalletActivity::created, rpcConsole, &RPCConsole::setCurrentWallet); activity->create(); }); - connect(m_close_all_wallets_action, &QAction::triggered, [this] { + connect(m_close_all_wallets_action, &QAction::triggered, m_wallet_controller, [this] { m_wallet_controller->closeAllWallets(this); }); - connect(m_migrate_wallet_action, &QAction::triggered, [this] { + connect(m_migrate_wallet_action, &QAction::triggered, m_wallet_controller, [this] { auto activity = new MigrateWalletActivity(m_wallet_controller, this); connect(activity, &MigrateWalletActivity::migrated, this, &BitcoinGUI::setCurrentWallet); activity->migrate(walletFrame->currentWalletModel()); @@ -942,7 +942,8 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel, interfaces::BlockAndH } } } else { - if(trayIconMenu) + // Shutdown requested, disable menus + if (trayIconMenu) { // Disable context menu on tray icon trayIconMenu->clear(); @@ -964,6 +965,8 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel, interfaces::BlockAndH dockIconMenu->clear(); } #endif + // Disable top bar menu actions + appMenuBar->clear(); } updateCoinJoinVisibility(); From 89531800f46c9ae4b0d01efebca88347a31bf37a Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 23 Oct 2023 15:09:52 +0100 Subject: [PATCH 4/4] Merge bitcoin-core/gui#770: Revert "gui: provide wallet controller context to wallet actions" f09bfab4aff04a9cc1ea157b94bfdae19f3465b1 Revert "gui: provide wallet controller context to wallet actions" (Hennadii Stepanov) Pull request description: The commit 7066e8996d0ac090535cc97cdcb54a219986460f from https://github.com/bitcoin-core/gui/pull/765 breaks "Open Wallet", "Close Wallet" and "Close All Wallets" items in the File menu (at least on Ubuntu 23.10 + Wayland). Reverting it to avoid this regression in the 26.0 release. ACKs for top commit: furszy: ACK f09bfab4 for including it in v26. jarolrod: ACK f09bfab4aff04a9cc1ea157b94bfdae19f3465b1 Tree-SHA512: fedc621c8e9bf84a263b0c28da53225febe0267d0123830a6192297f38e40726e1613e003b634215e7d16791ba6eab52fb4baab3da9637f6660b6ae1ae98462b Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> --- src/qt/bitcoingui.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 744424ca8b23..bccb33c102de 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -558,7 +558,7 @@ void BitcoinGUI::createActions() connect(usedSendingAddressesAction, &QAction::triggered, walletFrame, &WalletFrame::usedSendingAddresses); connect(usedReceivingAddressesAction, &QAction::triggered, walletFrame, &WalletFrame::usedReceivingAddresses); connect(openAction, &QAction::triggered, this, &BitcoinGUI::openClicked); - connect(m_open_wallet_menu, &QMenu::aboutToShow, m_wallet_controller, [this] { + connect(m_open_wallet_menu, &QMenu::aboutToShow, [this] { m_open_wallet_menu->clear(); for (const std::pair& i : m_wallet_controller->listWalletDir()) { const std::string& path = i.first; @@ -574,7 +574,7 @@ void BitcoinGUI::createActions() continue; } - connect(action, &QAction::triggered, m_wallet_controller, [this, path] { + connect(action, &QAction::triggered, [this, path] { auto activity = new OpenWalletActivity(m_wallet_controller, this); connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet, Qt::QueuedConnection); connect(activity, &OpenWalletActivity::opened, rpcConsole, &RPCConsole::setCurrentWallet, Qt::QueuedConnection); @@ -586,7 +586,7 @@ void BitcoinGUI::createActions() action->setEnabled(false); } }); - connect(m_restore_wallet_action, &QAction::triggered, m_wallet_controller, [this] { + connect(m_restore_wallet_action, &QAction::triggered, [this] { //: Name of the wallet data file format. QString name_data_file = tr("Wallet Data"); @@ -612,19 +612,19 @@ void BitcoinGUI::createActions() auto backup_file_path = fs::PathFromString(backup_file.toStdString()); activity->restore(backup_file_path, wallet_name.toStdString()); }); - connect(m_close_wallet_action, &QAction::triggered, m_wallet_controller, [this] { + connect(m_close_wallet_action, &QAction::triggered, [this] { m_wallet_controller->closeWallet(walletFrame->currentWalletModel(), this); }); - connect(m_create_wallet_action, &QAction::triggered, m_wallet_controller, [this] { + connect(m_create_wallet_action, &QAction::triggered, [this] { auto activity = new CreateWalletActivity(m_wallet_controller, this); connect(activity, &CreateWalletActivity::created, this, &BitcoinGUI::setCurrentWallet); connect(activity, &CreateWalletActivity::created, rpcConsole, &RPCConsole::setCurrentWallet); activity->create(); }); - connect(m_close_all_wallets_action, &QAction::triggered, m_wallet_controller, [this] { + connect(m_close_all_wallets_action, &QAction::triggered, [this] { m_wallet_controller->closeAllWallets(this); }); - connect(m_migrate_wallet_action, &QAction::triggered, m_wallet_controller, [this] { + connect(m_migrate_wallet_action, &QAction::triggered, [this] { auto activity = new MigrateWalletActivity(m_wallet_controller, this); connect(activity, &MigrateWalletActivity::migrated, this, &BitcoinGUI::setCurrentWallet); activity->migrate(walletFrame->currentWalletModel());