diff options
33 files changed, 570 insertions, 343 deletions
diff --git a/doc/release-notes-pr13381.md b/doc/release-notes-pr13381.md new file mode 100644 index 0000000000..75faad9906 --- /dev/null +++ b/doc/release-notes-pr13381.md @@ -0,0 +1,29 @@ +RPC importprivkey: new label behavior +------------------------------------- + +Previously, `importprivkey` automatically added the default empty label +("") to all addresses associated with the imported private key. Now it +defaults to using any existing label for those addresses. For example: + +- Old behavior: you import a watch-only address with the label "cold + wallet". Later, you import the corresponding private key using the + default settings. The address's label is changed from "cold wallet" + to "". + +- New behavior: you import a watch-only address with the label "cold + wallet". Later, you import the corresponding private key using the + default settings. The address's label remains "cold wallet". + +In both the previous and current case, if you directly specify a label +during the import, that label will override whatever previous label the +addresses may have had. Also in both cases, if none of the addresses +previously had a label, they will still receive the default empty label +(""). Examples: + +- You import a watch-only address with the label "temporary". Later you + import the corresponding private key with the label "final". The + address's label will be changed to "final". + +- You use the default settings to import a private key for an address that + was not previously in the wallet. Its addresses will receive the default + empty label (""). diff --git a/share/qt/Info.plist.in b/share/qt/Info.plist.in index 0c0335a1e8..abe605efd0 100644 --- a/share/qt/Info.plist.in +++ b/share/qt/Info.plist.in @@ -97,9 +97,6 @@ <key>NSHighResolutionCapable</key> <string>True</string> - <key>LSAppNapIsDisabled</key> - <string>True</string> - <key>NSRequiresAquaSystemAppearance</key> <string>True</string> diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index e3eb01304f..445849e3d8 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -162,7 +162,8 @@ QT_MOC_CPP = \ BITCOIN_MM = \ qt/macdockiconhandler.mm \ - qt/macnotificationhandler.mm + qt/macnotificationhandler.mm \ + qt/macos_appnap.mm QT_MOC = \ qt/bitcoin.moc \ @@ -205,6 +206,7 @@ BITCOIN_QT_H = \ qt/intro.h \ qt/macdockiconhandler.h \ qt/macnotificationhandler.h \ + qt/macos_appnap.h \ qt/modaloverlay.h \ qt/networkstyle.h \ qt/notificator.h \ diff --git a/src/prevector.h b/src/prevector.h index 6ddb6f321f..aa77573746 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -10,6 +10,7 @@ #include <stdint.h> #include <string.h> +#include <algorithm> #include <cstddef> #include <iterator> #include <type_traits> @@ -198,22 +199,11 @@ private: const T* item_ptr(difference_type pos) const { return is_direct() ? direct_ptr(pos) : indirect_ptr(pos); } void fill(T* dst, ptrdiff_t count) { - if (IS_TRIVIALLY_CONSTRUCTIBLE<T>::value) { - // The most common use of prevector is where T=unsigned char. For - // trivially constructible types, we can use memset() to avoid - // looping. - ::memset(dst, 0, count * sizeof(T)); - } else { - for (auto i = 0; i < count; ++i) { - new(static_cast<void*>(dst + i)) T(); - } - } + std::fill_n(dst, count, T{}); } void fill(T* dst, ptrdiff_t count, const T& value) { - for (auto i = 0; i < count; ++i) { - new(static_cast<void*>(dst + i)) T(value); - } + std::fill_n(dst, count, value); } template<typename InputIterator> diff --git a/src/qt/bitcoinamountfield.cpp b/src/qt/bitcoinamountfield.cpp index b68f3a439b..558fcf50ba 100644 --- a/src/qt/bitcoinamountfield.cpp +++ b/src/qt/bitcoinamountfield.cpp @@ -23,9 +23,7 @@ class AmountSpinBox: public QAbstractSpinBox public: explicit AmountSpinBox(QWidget *parent): - QAbstractSpinBox(parent), - currentUnit(BitcoinUnits::BTC), - singleStep(100000) // satoshis + QAbstractSpinBox(parent) { setAlignment(Qt::AlignRight); @@ -44,10 +42,19 @@ public: void fixup(QString &input) const { - bool valid = false; - CAmount val = parse(input, &valid); - if(valid) - { + bool valid; + CAmount val; + + if (input.isEmpty() && !m_allow_empty) { + valid = true; + val = m_min_amount; + } else { + valid = false; + val = parse(input, &valid); + } + + if (valid) { + val = qBound(m_min_amount, val, m_max_amount); input = BitcoinUnits::format(currentUnit, val, false, BitcoinUnits::separatorAlways); lineEdit()->setText(input); } @@ -64,12 +71,27 @@ public: Q_EMIT valueChanged(); } + void SetAllowEmpty(bool allow) + { + m_allow_empty = allow; + } + + void SetMinValue(const CAmount& value) + { + m_min_amount = value; + } + + void SetMaxValue(const CAmount& value) + { + m_max_amount = value; + } + void stepBy(int steps) { bool valid = false; CAmount val = value(&valid); val = val + steps * singleStep; - val = qMin(qMax(val, CAmount(0)), BitcoinUnits::maxMoney()); + val = qBound(m_min_amount, val, m_max_amount); setValue(val); } @@ -125,9 +147,12 @@ public: } private: - int currentUnit; - CAmount singleStep; + int currentUnit{BitcoinUnits::BTC}; + CAmount singleStep{CAmount(100000)}; // satoshis mutable QSize cachedMinimumSizeHint; + bool m_allow_empty{true}; + CAmount m_min_amount{CAmount(0)}; + CAmount m_max_amount{BitcoinUnits::maxMoney()}; /** * Parse a string into a number of base monetary units and @@ -174,11 +199,10 @@ protected: StepEnabled rv = 0; bool valid = false; CAmount val = value(&valid); - if(valid) - { - if(val > 0) + if (valid) { + if (val > m_min_amount) rv |= StepDownEnabled; - if(val < BitcoinUnits::maxMoney()) + if (val < m_max_amount) rv |= StepUpEnabled; } return rv; @@ -275,6 +299,21 @@ void BitcoinAmountField::setValue(const CAmount& value) amount->setValue(value); } +void BitcoinAmountField::SetAllowEmpty(bool allow) +{ + amount->SetAllowEmpty(allow); +} + +void BitcoinAmountField::SetMinValue(const CAmount& value) +{ + amount->SetMinValue(value); +} + +void BitcoinAmountField::SetMaxValue(const CAmount& value) +{ + amount->SetMaxValue(value); +} + void BitcoinAmountField::setReadOnly(bool fReadOnly) { amount->setReadOnly(fReadOnly); diff --git a/src/qt/bitcoinamountfield.h b/src/qt/bitcoinamountfield.h index f93579c492..650481e30d 100644 --- a/src/qt/bitcoinamountfield.h +++ b/src/qt/bitcoinamountfield.h @@ -31,6 +31,15 @@ public: CAmount value(bool *value=0) const; void setValue(const CAmount& value); + /** If allow empty is set to false the field will be set to the minimum allowed value if left empty. **/ + void SetAllowEmpty(bool allow); + + /** Set the minimum value in satoshis **/ + void SetMinValue(const CAmount& value); + + /** Set the maximum value in satoshis **/ + void SetMaxValue(const CAmount& value); + /** Set single step in satoshis **/ void setSingleStep(const CAmount& step); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 072334ebb0..ef82351551 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -92,12 +92,8 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty windowTitle += tr("Node"); } windowTitle += " " + networkStyle->getTitleAddText(); -#ifndef Q_OS_MAC QApplication::setWindowIcon(networkStyle->getTrayAndWindowIcon()); setWindowIcon(networkStyle->getTrayAndWindowIcon()); -#else - MacDockIconHandler::instance()->setIcon(networkStyle->getAppIcon()); -#endif setWindowTitle(windowTitle); rpcConsole = new RPCConsole(node, _platformStyle, 0); @@ -131,7 +127,9 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty createToolBars(); // Create system tray icon and notification - createTrayIcon(networkStyle); + if (QSystemTrayIcon::isSystemTrayAvailable()) { + createTrayIcon(networkStyle); + } // Create status bar statusBar(); @@ -211,6 +209,10 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty connect(progressBar, &GUIUtil::ClickableProgressBar::clicked, this, &BitcoinGUI::showModalOverlay); } #endif + +#ifdef Q_OS_MAC + m_app_nap_inhibitor = new CAppNapInhibitor; +#endif } BitcoinGUI::~BitcoinGUI() @@ -223,6 +225,7 @@ BitcoinGUI::~BitcoinGUI() if(trayIcon) // Hide tray icon, as deleting will let it linger until quit (on Ubuntu) trayIcon->hide(); #ifdef Q_OS_MAC + delete m_app_nap_inhibitor; delete appMenuBar; MacDockIconHandler::cleanup(); #endif @@ -273,17 +276,17 @@ void BitcoinGUI::createActions() #ifdef ENABLE_WALLET // These showNormalIfMinimized are needed because Send Coins and Receive Coins // can be triggered from the tray menu, and need to show the GUI to be useful. - connect(overviewAction, &QAction::triggered, this, static_cast<void (BitcoinGUI::*)()>(&BitcoinGUI::showNormalIfMinimized)); + connect(overviewAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(overviewAction, &QAction::triggered, this, &BitcoinGUI::gotoOverviewPage); - connect(sendCoinsAction, &QAction::triggered, this, static_cast<void (BitcoinGUI::*)()>(&BitcoinGUI::showNormalIfMinimized)); + connect(sendCoinsAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(sendCoinsAction, &QAction::triggered, [this]{ gotoSendCoinsPage(); }); - connect(sendCoinsMenuAction, &QAction::triggered, this, static_cast<void (BitcoinGUI::*)()>(&BitcoinGUI::showNormalIfMinimized)); + connect(sendCoinsMenuAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(sendCoinsMenuAction, &QAction::triggered, [this]{ gotoSendCoinsPage(); }); - connect(receiveCoinsAction, &QAction::triggered, this, static_cast<void (BitcoinGUI::*)()>(&BitcoinGUI::showNormalIfMinimized)); + connect(receiveCoinsAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(receiveCoinsAction, &QAction::triggered, this, &BitcoinGUI::gotoReceiveCoinsPage); - connect(receiveCoinsMenuAction, &QAction::triggered, this, static_cast<void (BitcoinGUI::*)()>(&BitcoinGUI::showNormalIfMinimized)); + connect(receiveCoinsMenuAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(receiveCoinsMenuAction, &QAction::triggered, this, &BitcoinGUI::gotoReceiveCoinsPage); - connect(historyAction, &QAction::triggered, this, static_cast<void (BitcoinGUI::*)()>(&BitcoinGUI::showNormalIfMinimized)); + connect(historyAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(historyAction, &QAction::triggered, this, &BitcoinGUI::gotoHistoryPage); #endif // ENABLE_WALLET @@ -350,7 +353,9 @@ void BitcoinGUI::createActions() connect(encryptWalletAction, &QAction::triggered, walletFrame, &WalletFrame::encryptWallet); connect(backupWalletAction, &QAction::triggered, walletFrame, &WalletFrame::backupWallet); connect(changePassphraseAction, &QAction::triggered, walletFrame, &WalletFrame::changePassphrase); + connect(signMessageAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(signMessageAction, &QAction::triggered, [this]{ gotoSignMessageTab(); }); + connect(verifyMessageAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(verifyMessageAction, &QAction::triggered, [this]{ gotoVerifyMessageTab(); }); connect(usedSendingAddressesAction, &QAction::triggered, walletFrame, &WalletFrame::usedSendingAddresses); connect(usedReceivingAddressesAction, &QAction::triggered, walletFrame, &WalletFrame::usedReceivingAddresses); @@ -585,6 +590,8 @@ void BitcoinGUI::setWalletActionsEnabled(bool enabled) void BitcoinGUI::createTrayIcon(const NetworkStyle *networkStyle) { + assert(QSystemTrayIcon::isSystemTrayAvailable()); + #ifndef Q_OS_MAC trayIcon = new QSystemTrayIcon(this); QString toolTip = tr("%1 client").arg(tr(PACKAGE_NAME)) + " " + networkStyle->getTitleAddText(); @@ -599,7 +606,7 @@ void BitcoinGUI::createTrayIcon(const NetworkStyle *networkStyle) void BitcoinGUI::createTrayIconMenu() { #ifndef Q_OS_MAC - // return if trayIcon is unset (only on non-Mac OSes) + // return if trayIcon is unset (only on non-macOSes) if (!trayIcon) return; @@ -608,15 +615,17 @@ void BitcoinGUI::createTrayIconMenu() connect(trayIcon, &QSystemTrayIcon::activated, this, &BitcoinGUI::trayIconActivated); #else - // Note: On Mac, the dock icon is used to provide the tray's functionality. + // Note: On macOS, the Dock icon is used to provide the tray's functionality. MacDockIconHandler *dockIconHandler = MacDockIconHandler::instance(); - dockIconHandler->setMainWindow(static_cast<QMainWindow*>(this)); - trayIconMenu = dockIconHandler->dockMenu(); + connect(dockIconHandler, &MacDockIconHandler::dockIconClicked, this, &BitcoinGUI::macosDockIconActivated); + + trayIconMenu = new QMenu(this); + trayIconMenu->setAsDockMenu(); #endif - // Configuration of the tray icon (or dock icon) icon menu + // Configuration of the tray icon (or Dock icon) menu #ifndef Q_OS_MAC - // Note: On Mac, the dock icon's menu already has show / hide action. + // Note: On macOS, the Dock icon's menu already has Show / Hide action. trayIconMenu->addAction(toggleHideAction); trayIconMenu->addSeparator(); #endif @@ -630,7 +639,7 @@ void BitcoinGUI::createTrayIconMenu() trayIconMenu->addAction(openRPCConsoleAction); } trayIconMenu->addAction(optionsAction); -#ifndef Q_OS_MAC // This is built-in on Mac +#ifndef Q_OS_MAC // This is built-in on macOS trayIconMenu->addSeparator(); trayIconMenu->addAction(quitAction); #endif @@ -645,6 +654,12 @@ void BitcoinGUI::trayIconActivated(QSystemTrayIcon::ActivationReason reason) toggleHidden(); } } +#else +void BitcoinGUI::macosDockIconActivated() +{ + show(); + activateWindow(); +} #endif void BitcoinGUI::optionsClicked() @@ -663,10 +678,7 @@ void BitcoinGUI::aboutClicked() void BitcoinGUI::showDebugWindow() { - rpcConsole->showNormal(); - rpcConsole->show(); - rpcConsole->raise(); - rpcConsole->activateWindow(); + GUIUtil::bringToFront(rpcConsole); } void BitcoinGUI::showDebugWindowActivateConsole() @@ -786,6 +798,11 @@ void BitcoinGUI::openOptionsDialogWithTab(OptionsDialog::Tab tab) void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool header) { +// Disabling macOS App Nap on initial sync, disk and reindex operations. +#ifdef Q_OS_MAC + (m_node.isInitialBlockDownload() || m_node.getReindex() || m_node.getImporting()) ? m_app_nap_inhibitor->disableAppNap() : m_app_nap_inhibitor->enableAppNap(); +#endif + if (modalOverlay) { if (header) @@ -1148,24 +1165,11 @@ void BitcoinGUI::showNormalIfMinimized(bool fToggleHidden) if(!clientModel) return; - // activateWindow() (sometimes) helps with keyboard focus on Windows - if (isHidden()) - { - show(); - activateWindow(); - } - else if (isMinimized()) - { - showNormal(); - activateWindow(); - } - else if (GUIUtil::isObscured(this)) - { - raise(); - activateWindow(); - } - else if(fToggleHidden) + if (!isHidden() && !isMinimized() && !GUIUtil::isObscured(this) && fToggleHidden) { hide(); + } else { + GUIUtil::bringToFront(this); + } } void BitcoinGUI::toggleHidden() diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index dcaca10557..e8b857c17c 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -20,6 +20,10 @@ #include <QPoint> #include <QSystemTrayIcon> +#ifdef Q_OS_MAC +#include <qt/macos_appnap.h> +#endif + #include <memory> class ClientModel; @@ -143,6 +147,10 @@ private: HelpMessageDialog* helpMessageDialog = nullptr; ModalOverlay* modalOverlay = nullptr; +#ifdef Q_OS_MAC + CAppNapInhibitor* m_app_nap_inhibitor = nullptr; +#endif + /** Keep track of previous number of blocks, to detect progress */ int prevBlocks = 0; int spinnerFrame = 0; @@ -260,6 +268,9 @@ public Q_SLOTS: #ifndef Q_OS_MAC /** Handle tray icon clicked */ void trayIconActivated(QSystemTrayIcon::ActivationReason reason); +#else + /** Handle macOS Dock icon clicked */ + void macosDockIconActivated(); #endif /** Show window if hidden, unminimize when minimized, rise when obscured or show if hidden and fToggleHidden is true */ diff --git a/src/qt/forms/sendcoinsdialog.ui b/src/qt/forms/sendcoinsdialog.ui index 6b31ddea90..386d559281 100644 --- a/src/qt/forms/sendcoinsdialog.ui +++ b/src/qt/forms/sendcoinsdialog.ui @@ -878,28 +878,15 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satoshis p <item> <layout class="QHBoxLayout" name="horizontalLayoutFee8"> <item> - <widget class="QCheckBox" name="checkBoxMinimumFee"> - <property name="toolTip"> - <string>Paying only the minimum fee is just fine as long as there is less transaction volume than space in the blocks. But be aware that this can end up in a never confirming transaction once there is more demand for bitcoin transactions than the network can process.</string> - </property> - <property name="text"> - <string/> - </property> - </widget> - </item> - <item> - <widget class="QLabel" name="labelMinFeeWarning"> + <widget class="QLabel" name="labelCustomFeeWarning"> <property name="enabled"> <bool>true</bool> </property> <property name="toolTip"> - <string>Paying only the minimum fee is just fine as long as there is less transaction volume than space in the blocks. But be aware that this can end up in a never confirming transaction once there is more demand for bitcoin transactions than the network can process.</string> + <string>When there is less transaction volume than space in the blocks, miners as well as relaying nodes may enforce a minimum fee. Paying only this minimum fee is just fine, but be aware that this can result in a never confirming transaction once there is more demand for bitcoin transactions than the network can process.</string> </property> <property name="text"> - <string>(read the tooltip)</string> - </property> - <property name="margin"> - <number>5</number> + <string>A too low fee might result in a never confirming transaction (read the tooltip)</string> </property> </widget> </item> @@ -992,9 +979,6 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satoshis p <property name="text"> <string/> </property> - <property name="margin"> - <number>2</number> - </property> </widget> </item> <item> @@ -1009,9 +993,6 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satoshis p <property name="text"> <string>(Smart fee not initialized yet. This usually takes a few blocks...)</string> </property> - <property name="margin"> - <number>2</number> - </property> </widget> </item> <item> @@ -1038,24 +1019,8 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satoshis p <property name="text"> <string>Confirmation time target:</string> </property> - <property name="margin"> - <number>2</number> - </property> </widget> </item> - <item> - <spacer name="verticalSpacer_3"> - <property name="orientation"> - <enum>Qt::Vertical</enum> - </property> - <property name="sizeHint" stdset="0"> - <size> - <width>1</width> - <height>1</height> - </size> - </property> - </spacer> - </item> </layout> </item> <item> diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index a68140ccf9..b1cd2f77d0 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -60,6 +60,14 @@ #include <QFontDatabase> #endif +#if defined(Q_OS_MAC) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + +#include <objc/objc-runtime.h> +#include <CoreServices/CoreServices.h> +#endif + namespace GUIUtil { QString dateTimeStr(const QDateTime &date) @@ -353,6 +361,27 @@ bool isObscured(QWidget *w) && checkPoint(QPoint(w->width() / 2, w->height() / 2), w)); } +void bringToFront(QWidget* w) +{ +#ifdef Q_OS_MAC + // Force application activation on macOS. With Qt 5.4 this is required when + // an action in the dock menu is triggered. + id app = objc_msgSend((id) objc_getClass("NSApplication"), sel_registerName("sharedApplication")); + objc_msgSend(app, sel_registerName("activateIgnoringOtherApps:"), YES); +#endif + + if (w) { + // activateWindow() (sometimes) helps with keyboard focus on Windows + if (w->isMinimized()) { + w->showNormal(); + } else { + w->show(); + } + w->activateWindow(); + w->raise(); + } +} + void openDebugLogfile() { fs::path pathDebug = GetDataDir() / "debug.log"; @@ -663,13 +692,8 @@ bool SetStartOnSystemStartup(bool fAutoStart) #elif defined(Q_OS_MAC) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" // based on: https://github.com/Mozketo/LaunchAtLoginController/blob/master/LaunchAtLoginController.m -#include <CoreFoundation/CoreFoundation.h> -#include <CoreServices/CoreServices.h> - LSSharedFileListItemRef findStartupItemInList(LSSharedFileListRef list, CFURLRef findUrl); LSSharedFileListItemRef findStartupItemInList(LSSharedFileListRef list, CFURLRef findUrl) { diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 011827e134..f1d0aa48ef 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -115,6 +115,9 @@ namespace GUIUtil // Determine whether a widget is hidden behind other windows bool isObscured(QWidget *w); + // Activate, show and raise the widget + void bringToFront(QWidget* w); + // Open debug.log void openDebugLogfile(); diff --git a/src/qt/macdockiconhandler.h b/src/qt/macdockiconhandler.h index 1c28593d4a..ff867e21a7 100644 --- a/src/qt/macdockiconhandler.h +++ b/src/qt/macdockiconhandler.h @@ -1,44 +1,27 @@ -// Copyright (c) 2011-2015 The Bitcoin Core developers +// Copyright (c) 2011-2018 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. #ifndef BITCOIN_QT_MACDOCKICONHANDLER_H #define BITCOIN_QT_MACDOCKICONHANDLER_H -#include <QMainWindow> #include <QObject> -QT_BEGIN_NAMESPACE -class QIcon; -class QMenu; -class QWidget; -QT_END_NAMESPACE - -/** Macintosh-specific dock icon handler. +/** macOS-specific Dock icon handler. */ class MacDockIconHandler : public QObject { Q_OBJECT public: - ~MacDockIconHandler(); - - QMenu *dockMenu(); - void setIcon(const QIcon &icon); - void setMainWindow(QMainWindow *window); static MacDockIconHandler *instance(); static void cleanup(); - void handleDockIconClickEvent(); Q_SIGNALS: void dockIconClicked(); private: MacDockIconHandler(); - - QWidget *m_dummyWidget; - QMenu *m_dockMenu; - QMainWindow *mainWindow; }; #endif // BITCOIN_QT_MACDOCKICONHANDLER_H diff --git a/src/qt/macdockiconhandler.mm b/src/qt/macdockiconhandler.mm index b9ad191da7..102adce6c5 100644 --- a/src/qt/macdockiconhandler.mm +++ b/src/qt/macdockiconhandler.mm @@ -1,107 +1,36 @@ -// Copyright (c) 2011-2013 The Bitcoin Core developers +// Copyright (c) 2011-2018 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "macdockiconhandler.h" -#include <QImageWriter> -#include <QMenu> -#include <QBuffer> -#include <QWidget> - #undef slots -#include <Cocoa/Cocoa.h> #include <objc/objc.h> #include <objc/message.h> static MacDockIconHandler *s_instance = nullptr; -bool dockClickHandler(id self,SEL _cmd,...) { +bool dockClickHandler(id self, SEL _cmd, ...) { Q_UNUSED(self) Q_UNUSED(_cmd) - s_instance->handleDockIconClickEvent(); + Q_EMIT s_instance->dockIconClicked(); - // Return NO (false) to suppress the default OS X actions + // Return NO (false) to suppress the default macOS actions return false; } void setupDockClickHandler() { - Class cls = objc_getClass("NSApplication"); - id appInst = objc_msgSend((id)cls, sel_registerName("sharedApplication")); - - if (appInst != nullptr) { - id delegate = objc_msgSend(appInst, sel_registerName("delegate")); - Class delClass = (Class)objc_msgSend(delegate, sel_registerName("class")); - SEL shouldHandle = sel_registerName("applicationShouldHandleReopen:hasVisibleWindows:"); - if (class_getInstanceMethod(delClass, shouldHandle)) - class_replaceMethod(delClass, shouldHandle, (IMP)dockClickHandler, "B@:"); - else - class_addMethod(delClass, shouldHandle, (IMP)dockClickHandler,"B@:"); - } + id app = objc_msgSend((id)objc_getClass("NSApplication"), sel_registerName("sharedApplication")); + id delegate = objc_msgSend(app, sel_registerName("delegate")); + Class delClass = (Class)objc_msgSend(delegate, sel_registerName("class")); + SEL shouldHandle = sel_registerName("applicationShouldHandleReopen:hasVisibleWindows:"); + class_replaceMethod(delClass, shouldHandle, (IMP)dockClickHandler, "B@:"); } - MacDockIconHandler::MacDockIconHandler() : QObject() { - NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; - setupDockClickHandler(); - this->m_dummyWidget = new QWidget(); - this->m_dockMenu = new QMenu(this->m_dummyWidget); - this->setMainWindow(nullptr); -#if QT_VERSION >= 0x050200 - this->m_dockMenu->setAsDockMenu(); -#endif - [pool release]; -} - -void MacDockIconHandler::setMainWindow(QMainWindow *window) { - this->mainWindow = window; -} - -MacDockIconHandler::~MacDockIconHandler() -{ - delete this->m_dummyWidget; - this->setMainWindow(nullptr); -} - -QMenu *MacDockIconHandler::dockMenu() -{ - return this->m_dockMenu; -} - -void MacDockIconHandler::setIcon(const QIcon &icon) -{ - NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; - NSImage *image = nil; - if (icon.isNull()) - image = [[NSImage imageNamed:@"NSApplicationIcon"] retain]; - else { - // generate NSImage from QIcon and use this as dock icon. - QSize size = icon.actualSize(QSize(128, 128)); - QPixmap pixmap = icon.pixmap(size); - - // Write image into a R/W buffer from raw pixmap, then save the image. - QBuffer notificationBuffer; - if (!pixmap.isNull() && notificationBuffer.open(QIODevice::ReadWrite)) { - QImageWriter writer(¬ificationBuffer, "PNG"); - if (writer.write(pixmap.toImage())) { - NSData* macImgData = [NSData dataWithBytes:notificationBuffer.buffer().data() - length:notificationBuffer.buffer().size()]; - image = [[NSImage alloc] initWithData:macImgData]; - } - } - - if(!image) { - // if testnet image could not be created, load std. app icon - image = [[NSImage imageNamed:@"NSApplicationIcon"] retain]; - } - } - - [NSApp setApplicationIconImage:image]; - [image release]; - [pool release]; } MacDockIconHandler *MacDockIconHandler::instance() @@ -115,14 +44,3 @@ void MacDockIconHandler::cleanup() { delete s_instance; } - -void MacDockIconHandler::handleDockIconClickEvent() -{ - if (this->mainWindow) - { - this->mainWindow->activateWindow(); - this->mainWindow->show(); - } - - Q_EMIT this->dockIconClicked(); -} diff --git a/src/qt/macos_appnap.h b/src/qt/macos_appnap.h new file mode 100644 index 0000000000..8c2cd840b0 --- /dev/null +++ b/src/qt/macos_appnap.h @@ -0,0 +1,24 @@ +// Copyright (c) 2011-2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_QT_MACOS_APPNAP_H +#define BITCOIN_QT_MACOS_APPNAP_H + +#include <memory> + +class CAppNapInhibitor final +{ +public: + explicit CAppNapInhibitor(); + ~CAppNapInhibitor(); + + void disableAppNap(); + void enableAppNap(); + +private: + class CAppNapImpl; + std::unique_ptr<CAppNapImpl> impl; +}; + +#endif // BITCOIN_QT_MACOS_APPNAP_H diff --git a/src/qt/macos_appnap.mm b/src/qt/macos_appnap.mm new file mode 100644 index 0000000000..22a88782ab --- /dev/null +++ b/src/qt/macos_appnap.mm @@ -0,0 +1,71 @@ +// Copyright (c) 2011-2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "macos_appnap.h" + +#include <AvailabilityMacros.h> +#include <Foundation/NSProcessInfo.h> +#include <Foundation/Foundation.h> + +class CAppNapInhibitor::CAppNapImpl +{ +public: + ~CAppNapImpl() + { + if(activityId) + enableAppNap(); + } + + void disableAppNap() + { + if (!activityId) + { + @autoreleasepool { + const NSActivityOptions activityOptions = + NSActivityUserInitiatedAllowingIdleSystemSleep & + ~(NSActivitySuddenTerminationDisabled | + NSActivityAutomaticTerminationDisabled); + + id processInfo = [NSProcessInfo processInfo]; + if ([processInfo respondsToSelector:@selector(beginActivityWithOptions:reason:)]) + { + activityId = [processInfo beginActivityWithOptions: activityOptions reason:@"Temporarily disable App Nap for bitcoin-qt."]; + [activityId retain]; + } + } + } + } + + void enableAppNap() + { + if(activityId) + { + @autoreleasepool { + id processInfo = [NSProcessInfo processInfo]; + if ([processInfo respondsToSelector:@selector(endActivity:)]) + [processInfo endActivity:activityId]; + + [activityId release]; + activityId = nil; + } + } + } + +private: + NSObject* activityId; +}; + +CAppNapInhibitor::CAppNapInhibitor() : impl(new CAppNapImpl()) {} + +CAppNapInhibitor::~CAppNapInhibitor() = default; + +void CAppNapInhibitor::disableAppNap() +{ + impl->disableAppNap(); +} + +void CAppNapInhibitor::enableAppNap() +{ + impl->enableAppNap(); +} diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index b51322394f..c9871f6c66 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -23,6 +23,7 @@ #include <QIntValidator> #include <QLocale> #include <QMessageBox> +#include <QSystemTrayIcon> #include <QTimer> OptionsDialog::OptionsDialog(QWidget *parent, bool enableWallet) : @@ -126,6 +127,13 @@ OptionsDialog::OptionsDialog(QWidget *parent, bool enableWallet) : connect(ui->proxyIpTor, &QValidatedLineEdit::validationDidChange, this, &OptionsDialog::updateProxyValidationState); connect(ui->proxyPort, &QLineEdit::textChanged, this, &OptionsDialog::updateProxyValidationState); connect(ui->proxyPortTor, &QLineEdit::textChanged, this, &OptionsDialog::updateProxyValidationState); + + if (!QSystemTrayIcon::isSystemTrayAvailable()) { + ui->hideTrayIcon->setChecked(true); + ui->hideTrayIcon->setEnabled(false); + ui->minimizeToTray->setChecked(false); + ui->minimizeToTray->setEnabled(false); + } } OptionsDialog::~OptionsDialog() @@ -211,8 +219,10 @@ void OptionsDialog::setMapper() /* Window */ #ifndef Q_OS_MAC - mapper->addMapping(ui->hideTrayIcon, OptionsModel::HideTrayIcon); - mapper->addMapping(ui->minimizeToTray, OptionsModel::MinimizeToTray); + if (QSystemTrayIcon::isSystemTrayAvailable()) { + mapper->addMapping(ui->hideTrayIcon, OptionsModel::HideTrayIcon); + mapper->addMapping(ui->minimizeToTray, OptionsModel::MinimizeToTray); + } mapper->addMapping(ui->minimizeOnClose, OptionsModel::MinimizeOnClose); #endif diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 858128f9f9..65db0280b7 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -119,13 +119,11 @@ SendCoinsDialog::SendCoinsDialog(const PlatformStyle *_platformStyle, QWidget *p settings.setValue("nSmartFeeSliderPosition", 0); if (!settings.contains("nTransactionFee")) settings.setValue("nTransactionFee", (qint64)DEFAULT_PAY_TX_FEE); - if (!settings.contains("fPayOnlyMinFee")) - settings.setValue("fPayOnlyMinFee", false); ui->groupFee->setId(ui->radioSmartFee, 0); ui->groupFee->setId(ui->radioCustomFee, 1); ui->groupFee->button((int)std::max(0, std::min(1, settings.value("nFeeRadio").toInt())))->setChecked(true); + ui->customFee->SetAllowEmpty(false); ui->customFee->setValue(settings.value("nTransactionFee").toLongLong()); - ui->checkBoxMinimumFee->setChecked(settings.value("fPayOnlyMinFee").toBool()); minimizeFeeSection(settings.value("fFeeSectionMinimized").toBool()); } @@ -174,14 +172,15 @@ void SendCoinsDialog::setModel(WalletModel *_model) connect(ui->groupFee, static_cast<void (QButtonGroup::*)(int)>(&QButtonGroup::buttonClicked), this, &SendCoinsDialog::updateFeeSectionControls); connect(ui->groupFee, static_cast<void (QButtonGroup::*)(int)>(&QButtonGroup::buttonClicked), this, &SendCoinsDialog::coinControlUpdateLabels); connect(ui->customFee, &BitcoinAmountField::valueChanged, this, &SendCoinsDialog::coinControlUpdateLabels); - connect(ui->checkBoxMinimumFee, &QCheckBox::stateChanged, this, &SendCoinsDialog::setMinimumFee); - connect(ui->checkBoxMinimumFee, &QCheckBox::stateChanged, this, &SendCoinsDialog::updateFeeSectionControls); - connect(ui->checkBoxMinimumFee, &QCheckBox::stateChanged, this, &SendCoinsDialog::coinControlUpdateLabels); connect(ui->optInRBF, &QCheckBox::stateChanged, this, &SendCoinsDialog::updateSmartFeeLabel); connect(ui->optInRBF, &QCheckBox::stateChanged, this, &SendCoinsDialog::coinControlUpdateLabels); - ui->customFee->setSingleStep(model->wallet().getRequiredFee(1000)); + CAmount requiredFee = model->wallet().getRequiredFee(1000); + ui->customFee->SetMinValue(requiredFee); + if (ui->customFee->value() < requiredFee) { + ui->customFee->setValue(requiredFee); + } + ui->customFee->setSingleStep(requiredFee); updateFeeSectionControls(); - updateMinFeeLabel(); updateSmartFeeLabel(); // set default rbf checkbox state @@ -210,7 +209,6 @@ SendCoinsDialog::~SendCoinsDialog() settings.setValue("nFeeRadio", ui->groupFee->checkedId()); settings.setValue("nConfTarget", getConfTargetForIndex(ui->confTargetSelector->currentIndex())); settings.setValue("nTransactionFee", (qint64)ui->customFee->value()); - settings.setValue("fPayOnlyMinFee", ui->checkBoxMinimumFee->isChecked()); delete ui; } @@ -542,7 +540,6 @@ void SendCoinsDialog::updateDisplayUnit() { setBalance(model->wallet().getBalances()); ui->customFee->setDisplayUnit(model->getOptionsModel()->getDisplayUnit()); - updateMinFeeLabel(); updateSmartFeeLabel(); } @@ -642,11 +639,6 @@ void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry) } } -void SendCoinsDialog::setMinimumFee() -{ - ui->customFee->setValue(model->wallet().getRequiredFee(1000)); -} - void SendCoinsDialog::updateFeeSectionControls() { ui->confTargetSelector ->setEnabled(ui->radioSmartFee->isChecked()); @@ -654,10 +646,9 @@ void SendCoinsDialog::updateFeeSectionControls() ui->labelSmartFee2 ->setEnabled(ui->radioSmartFee->isChecked()); ui->labelSmartFee3 ->setEnabled(ui->radioSmartFee->isChecked()); ui->labelFeeEstimation ->setEnabled(ui->radioSmartFee->isChecked()); - ui->checkBoxMinimumFee ->setEnabled(ui->radioCustomFee->isChecked()); - ui->labelMinFeeWarning ->setEnabled(ui->radioCustomFee->isChecked()); - ui->labelCustomPerKilobyte ->setEnabled(ui->radioCustomFee->isChecked() && !ui->checkBoxMinimumFee->isChecked()); - ui->customFee ->setEnabled(ui->radioCustomFee->isChecked() && !ui->checkBoxMinimumFee->isChecked()); + ui->labelCustomFeeWarning ->setEnabled(ui->radioCustomFee->isChecked()); + ui->labelCustomPerKilobyte ->setEnabled(ui->radioCustomFee->isChecked()); + ui->customFee ->setEnabled(ui->radioCustomFee->isChecked()); } void SendCoinsDialog::updateFeeMinimizedLabel() @@ -672,14 +663,6 @@ void SendCoinsDialog::updateFeeMinimizedLabel() } } -void SendCoinsDialog::updateMinFeeLabel() -{ - if (model && model->getOptionsModel()) - ui->checkBoxMinimumFee->setText(tr("Pay only the required fee of %1").arg( - BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), model->wallet().getRequiredFee(1000)) + "/kB") - ); -} - void SendCoinsDialog::updateCoinControlState(CCoinControl& ctrl) { if (ui->radioCustomFee->isChecked()) { diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h index 7009855f17..e1ebc77d59 100644 --- a/src/qt/sendcoinsdialog.h +++ b/src/qt/sendcoinsdialog.h @@ -92,9 +92,7 @@ private Q_SLOTS: void coinControlClipboardBytes(); void coinControlClipboardLowOutput(); void coinControlClipboardChange(); - void setMinimumFee(); void updateFeeSectionControls(); - void updateMinFeeLabel(); void updateSmartFeeLabel(); Q_SIGNALS: diff --git a/src/qt/test/util.h b/src/qt/test/util.h index 5363c94547..377f07dcba 100644 --- a/src/qt/test/util.h +++ b/src/qt/test/util.h @@ -1,6 +1,8 @@ #ifndef BITCOIN_QT_TEST_UTIL_H #define BITCOIN_QT_TEST_UTIL_H +#include <QString> + /** * Press "Ok" button in message box dialog. * diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 053e951921..a619992344 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -292,9 +292,7 @@ void WalletView::usedSendingAddresses() if(!walletModel) return; - usedSendingAddressesPage->show(); - usedSendingAddressesPage->raise(); - usedSendingAddressesPage->activateWindow(); + GUIUtil::bringToFront(usedSendingAddressesPage); } void WalletView::usedReceivingAddresses() @@ -302,9 +300,7 @@ void WalletView::usedReceivingAddresses() if(!walletModel) return; - usedReceivingAddressesPage->show(); - usedReceivingAddressesPage->raise(); - usedReceivingAddressesPage->activateWindow(); + GUIUtil::bringToFront(usedReceivingAddressesPage); } void WalletView::showProgress(const QString &title, int nProgress) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 51b4e44aed..c27d5d142b 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1538,12 +1538,13 @@ UniValue finalizepsbt(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } - // Get all of the previous transactions + // Finalize input signatures -- in case we have partial signatures that add up to a complete + // signature, but have not combined them yet (e.g. because the combiner that created this + // PartiallySignedTransaction did not understand them), this will combine them into a final + // script. bool complete = true; for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { - PSBTInput& input = psbtx.inputs.at(i); - - complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, *psbtx.tx, input, i, 1); + complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, SIGHASH_ALL); } UniValue result(UniValue::VOBJ); @@ -1556,10 +1557,10 @@ UniValue finalizepsbt(const JSONRPCRequest& request) mtx.vin[i].scriptWitness = psbtx.inputs[i].final_script_witness; } ssTx << mtx; - result.pushKV("hex", HexStr(ssTx.begin(), ssTx.end())); + result.pushKV("hex", HexStr(ssTx.str())); } else { ssTx << psbtx; - result.pushKV("psbt", EncodeBase64((unsigned char*)ssTx.data(), ssTx.size())); + result.pushKV("psbt", EncodeBase64(ssTx.str())); } result.pushKV("complete", complete); @@ -1671,7 +1672,7 @@ UniValue converttopsbt(const JSONRPCRequest& request) // Remove all scriptSigs and scriptWitnesses from inputs for (CTxIn& input : tx.vin) { - if ((!input.scriptSig.empty() || !input.scriptWitness.IsNull()) && (request.params[1].isNull() || (!request.params[1].isNull() && request.params[1].get_bool()))) { + if ((!input.scriptSig.empty() || !input.scriptWitness.IsNull()) && !permitsigdata) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Inputs must not have scriptSigs and scriptWitnesses"); } input.scriptSig.clear(); diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 89cc7c808c..2795dc96d3 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -123,7 +123,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator case TX_PUBKEYHASH: { CKeyID keyID = CKeyID(uint160(vSolutions[0])); CPubKey pubkey; - GetPubKey(provider, sigdata, keyID, pubkey); + if (!GetPubKey(provider, sigdata, keyID, pubkey)) return false; if (!CreateSig(creator, sigdata, provider, sig, pubkey, scriptPubKey, sigversion)) return false; ret.push_back(std::move(sig)); ret.push_back(ToByteVector(pubkey)); @@ -239,10 +239,17 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato return sigdata.complete; } -bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, int index, int sighash) +bool PSBTInputSigned(PSBTInput& input) { - // if this input has a final scriptsig or scriptwitness, don't do anything with it - if (!input.final_script_sig.empty() || !input.final_script_witness.IsNull()) { + return !input.final_script_sig.empty() || !input.final_script_witness.IsNull(); +} + +bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash) +{ + PSBTInput& input = psbt.inputs.at(index); + const CMutableTransaction& tx = *psbt.tx; + + if (PSBTInputSigned(input)) { return true; } @@ -253,15 +260,19 @@ bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& t // Get UTXO bool require_witness_sig = false; CTxOut utxo; + + // Verify input sanity, which checks that at most one of witness or non-witness utxos is provided. + if (!input.IsSane()) { + return false; + } + if (input.non_witness_utxo) { // If we're taking our information from a non-witness UTXO, verify that it matches the prevout. - if (input.non_witness_utxo->GetHash() != tx.vin[index].prevout.hash) return false; - // If both witness and non-witness UTXO are provided, verify that they match. This check shouldn't - // matter, as the PSBT deserializer enforces only one of both is provided, and the only way both - // can be present is when they're added simultaneously by FillPSBT (in which case they always match). - // Still, check in order to not rely on callers to enforce this. - if (!input.witness_utxo.IsNull() && input.non_witness_utxo->vout[tx.vin[index].prevout.n] != input.witness_utxo) return false; - utxo = input.non_witness_utxo->vout[tx.vin[index].prevout.n]; + COutPoint prevout = tx.vin[index].prevout; + if (input.non_witness_utxo->GetHash() != prevout.hash) { + return false; + } + utxo = input.non_witness_utxo->vout[prevout.n]; } else if (!input.witness_utxo.IsNull()) { utxo = input.witness_utxo; // When we're taking our information from a witness UTXO, we can't verify it is actually data from @@ -280,18 +291,10 @@ bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& t if (require_witness_sig && !sigdata.witness) return false; input.FromSignatureData(sigdata); + // If we have a witness signature, use the smaller witness UTXO. if (sigdata.witness) { - assert(!utxo.IsNull()); input.witness_utxo = utxo; - } - - // If both UTXO types are present, drop the unnecessary one. - if (input.non_witness_utxo && !input.witness_utxo.IsNull()) { - if (sigdata.witness) { - input.non_witness_utxo = nullptr; - } else { - input.witness_utxo.SetNull(); - } + input.non_witness_utxo = nullptr; } return sig_complete; @@ -513,6 +516,12 @@ bool IsSolvable(const SigningProvider& provider, const CScript& script) return false; } +PartiallySignedTransaction::PartiallySignedTransaction(const CTransaction& tx) : tx(tx) +{ + inputs.resize(tx.vin.size()); + outputs.resize(tx.vout.size()); +} + bool PartiallySignedTransaction::IsNull() const { return !tx && inputs.empty() && outputs.empty() && unknown.empty(); diff --git a/src/script/sign.h b/src/script/sign.h index d47aada17d..a478f49789 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -206,6 +206,9 @@ template<typename Stream> void SerializeHDKeypaths(Stream& s, const std::map<CPubKey, KeyOriginInfo>& hd_keypaths, uint8_t type) { for (auto keypath_pair : hd_keypaths) { + if (!keypath_pair.first.IsValid()) { + throw std::ios_base::failure("Invalid CPubKey being serialized"); + } SerializeToVector(s, type, MakeSpan(keypath_pair.first)); WriteCompactSize(s, (keypath_pair.second.path.size() + 1) * sizeof(uint32_t)); s << keypath_pair.second.fingerprint; @@ -566,6 +569,7 @@ struct PartiallySignedTransaction bool IsSane() const; PartiallySignedTransaction() {} PartiallySignedTransaction(const PartiallySignedTransaction& psbt_in) : tx(psbt_in.tx), inputs(psbt_in.inputs), outputs(psbt_in.outputs), unknown(psbt_in.unknown) {} + explicit PartiallySignedTransaction(const CTransaction& tx); // Only checks if they refer to the same transaction friend bool operator==(const PartiallySignedTransaction& a, const PartiallySignedTransaction &b) @@ -729,8 +733,11 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType); bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType); +/** Checks whether a PSBTInput is already signed. */ +bool PSBTInputSigned(PSBTInput& input); + /** Signs a PSBTInput, verifying that all provided data matches what is being signed. */ -bool SignPSBTInput(const SigningProvider& provider, const CMutableTransaction& tx, PSBTInput& input, int index, int sighash = SIGHASH_ALL); +bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash = SIGHASH_ALL); /** Extract signature data from a transaction input, and insert it. */ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn, const CTxOut& txout); diff --git a/src/util/system.cpp b/src/util/system.cpp index 4f5dd2d6e9..f6f36c2238 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -826,8 +826,10 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect std::string::size_type pos; int linenr = 1; while (std::getline(stream, str)) { + bool used_hash = false; if ((pos = str.find('#')) != std::string::npos) { str = str.substr(0, pos); + used_hash = true; } const static std::string pattern = " \t\r\n"; str = TrimString(str, pattern); @@ -840,6 +842,10 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect } else if ((pos = str.find('=')) != std::string::npos) { std::string name = prefix + TrimString(str.substr(0, pos), pattern); std::string value = TrimString(str.substr(pos + 1), pattern); + if (used_hash && name == "rpcpassword") { + error = strprintf("parse error on line %i, using # in rpcpassword can be ambiguous and should be avoided", linenr); + return false; + } options.emplace_back(name, value); } else { error = strprintf("parse error on line %i: %s", linenr, str); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e1e4fc51fe..7dbe0c8462 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -113,7 +113,7 @@ UniValue importprivkey(const JSONRPCRequest& request) "Hint: use importmulti to import more than one private key.\n" "\nArguments:\n" "1. \"privkey\" (string, required) The private key (see dumpprivkey)\n" - "2. \"label\" (string, optional, default=\"\") An optional label\n" + "2. \"label\" (string, optional, default=current label if address exists, otherwise \"\") An optional label\n" "3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" @@ -163,9 +163,14 @@ UniValue importprivkey(const JSONRPCRequest& request) CKeyID vchAddress = pubkey.GetID(); { pwallet->MarkDirty(); - // We don't know which corresponding address will be used; label them all + + // We don't know which corresponding address will be used; + // label all new addresses, and label existing addresses if a + // label was passed. for (const auto& dest : GetAllDestinationsForKey(pubkey)) { - pwallet->SetAddressBook(dest, strLabel, "receive"); + if (!request.params[1].isNull() || pwallet->mapAddressBook.count(dest) == 0) { + pwallet->SetAddressBook(dest, strLabel, "receive"); + } } // Don't throw error in case a key is already there diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 5a89448e02..bfac990639 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3772,24 +3772,34 @@ void AddKeypathToMap(const CWallet* pwallet, const CKeyID& keyID, std::map<CPubK hd_keypaths.emplace(vchPubKey, std::move(info)); } -bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const CTransaction* txConst, int sighash_type, bool sign, bool bip32derivs) +bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs) { LOCK(pwallet->cs_wallet); // Get all of the previous transactions bool complete = true; - for (unsigned int i = 0; i < txConst->vin.size(); ++i) { - const CTxIn& txin = txConst->vin[i]; + for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + const CTxIn& txin = psbtx.tx->vin[i]; PSBTInput& input = psbtx.inputs.at(i); - // If we don't know about this input, skip it and let someone else deal with it - const uint256& txhash = txin.prevout.hash; - const auto it = pwallet->mapWallet.find(txhash); - if (it != pwallet->mapWallet.end()) { - const CWalletTx& wtx = it->second; - CTxOut utxo = wtx.tx->vout[txin.prevout.n]; - // Update both UTXOs from the wallet. - input.non_witness_utxo = wtx.tx; - input.witness_utxo = utxo; + if (PSBTInputSigned(input)) { + continue; + } + + // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness. + if (!input.IsSane()) { + throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "PSBT input is not sane."); + } + + // If we have no utxo, grab it from the wallet. + if (!input.non_witness_utxo && input.witness_utxo.IsNull()) { + const uint256& txhash = txin.prevout.hash; + const auto it = pwallet->mapWallet.find(txhash); + if (it != pwallet->mapWallet.end()) { + const CWalletTx& wtx = it->second; + // We only need the non_witness_utxo, which is a superset of the witness_utxo. + // The signing code will switch to the smaller witness_utxo if this is ok. + input.non_witness_utxo = wtx.tx; + } } // Get the Sighash type @@ -3797,12 +3807,12 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const C throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Specified Sighash and sighash in PSBT do not match."); } - complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, !bip32derivs), *psbtx.tx, input, i, sighash_type); + complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, !bip32derivs), psbtx, i, sighash_type); } // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change - for (unsigned int i = 0; i < txConst->vout.size(); ++i) { - const CTxOut& out = txConst->vout.at(i); + for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { + const CTxOut& out = psbtx.tx->vout.at(i); PSBTOutput& psbt_out = psbtx.outputs.at(i); // Fill a SignatureData with output info @@ -3867,19 +3877,15 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) // Get the sighash type int nHashType = ParseSighashString(request.params[2]); - // Use CTransaction for the constant parts of the - // transaction to avoid rehashing. - const CTransaction txConst(*psbtx.tx); - // Fill transaction with our data and also sign bool sign = request.params[1].isNull() ? true : request.params[1].get_bool(); bool bip32derivs = request.params[3].isNull() ? false : request.params[3].get_bool(); - bool complete = FillPSBT(pwallet, psbtx, &txConst, nHashType, sign, bip32derivs); + bool complete = FillPSBT(pwallet, psbtx, nHashType, sign, bip32derivs); UniValue result(UniValue::VOBJ); CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); ssTx << psbtx; - result.pushKV("psbt", EncodeBase64((unsigned char*)ssTx.data(), ssTx.size())); + result.pushKV("psbt", EncodeBase64(ssTx.str())); result.pushKV("complete", complete); return result; @@ -3971,29 +3977,18 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]); // Make a blank psbt - PartiallySignedTransaction psbtx; - psbtx.tx = rawTx; - for (unsigned int i = 0; i < rawTx.vin.size(); ++i) { - psbtx.inputs.push_back(PSBTInput()); - } - for (unsigned int i = 0; i < rawTx.vout.size(); ++i) { - psbtx.outputs.push_back(PSBTOutput()); - } - - // Use CTransaction for the constant parts of the - // transaction to avoid rehashing. - const CTransaction txConst(*psbtx.tx); + PartiallySignedTransaction psbtx(rawTx); // Fill transaction with out data but don't sign bool bip32derivs = request.params[4].isNull() ? false : request.params[4].get_bool(); - FillPSBT(pwallet, psbtx, &txConst, 1, false, bip32derivs); + FillPSBT(pwallet, psbtx, 1, false, bip32derivs); // Serialize the PSBT CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); ssTx << psbtx; UniValue result(UniValue::VOBJ); - result.pushKV("psbt", EncodeBase64((unsigned char*)ssTx.data(), ssTx.size())); + result.pushKV("psbt", EncodeBase64(ssTx.str())); result.pushKV("fee", ValueFromAmount(fee)); result.pushKV("changepos", change_position); return result; diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index 9b9a159b86..abd7750874 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -30,5 +30,5 @@ bool EnsureWalletIsAvailable(CWallet *, bool avoidException); UniValue getaddressinfo(const JSONRPCRequest& request); UniValue signrawtransactionwithwallet(const JSONRPCRequest& request); -bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const CTransaction* txConst, int sighash_type = 1, bool sign = true, bool bip32derivs = false); +bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false); #endif //BITCOIN_WALLET_RPCWALLET_H diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index cb1ad25461..9918eeb89f 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -59,12 +59,8 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) CDataStream ssData(ParseHex("70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f000000000000000000"), SER_NETWORK, PROTOCOL_VERSION); ssData >> psbtx; - // Use CTransaction for the constant parts of the - // transaction to avoid rehashing. - const CTransaction txConst(*psbtx.tx); - // Fill transaction with our data - FillPSBT(&m_wallet, psbtx, &txConst, 1, false, true); + FillPSBT(&m_wallet, psbtx, SIGHASH_ALL, false, true); // Get the final tx CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 492772d5e3..88a9aadc7b 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -30,6 +30,10 @@ class ConfArgsTest(BitcoinTestFramework): self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 1: nono, if you intended to specify a negated option, use nono=1 instead') with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: + conf.write('server=1\nrpcuser=someuser\nrpcpassword=some#pass') + self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 3, using # in rpcpassword can be ambiguous and should be avoided') + + with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: conf.write('') # clear def run_test(self): diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index fca910bf64..04d9bb65a6 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -146,6 +146,9 @@ class PSBTTest(BitcoinTestFramework): # Make sure that a psbt with signatures cannot be converted signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx['hex']) assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].converttopsbt, signedtx['hex']) + assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].converttopsbt, signedtx['hex'], False) + # Unless we allow it to convert and strip signatures + self.nodes[0].converttopsbt(signedtx['hex'], True) # Explicitly allow converting non-empty txs new_psbt = self.nodes[0].converttopsbt(rawtx['hex']) @@ -207,6 +210,13 @@ class PSBTTest(BitcoinTestFramework): assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE assert_equal(decoded_psbt["tx"]["locktime"], 0) + # Regression test for 14473 (mishandling of already-signed witness transaction): + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}]) + complete_psbt = self.nodes[0].walletprocesspsbt(psbtx_info["psbt"]) + double_processed_psbt = self.nodes[0].walletprocesspsbt(complete_psbt["psbt"]) + assert_equal(complete_psbt, double_processed_psbt) + # We don't care about the decode result, but decoding must succeed. + self.nodes[0].decodepsbt(double_processed_psbt["psbt"]) # BIP 174 Test Vectors @@ -269,6 +279,10 @@ class PSBTTest(BitcoinTestFramework): self.test_utxo_conversion() + # Test that psbts with p2pkh outputs are created properly + p2pkh = self.nodes[0].getnewaddress(address_type='legacy') + psbt = self.nodes[1].walletcreatefundedpsbt([], [{p2pkh : 1}], 0, {"includeWatching" : True}, True) + self.nodes[0].decodepsbt(psbt['psbt']) if __name__ == '__main__': PSBTTest().main() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index ffff81e070..9dcc0e6d0e 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -118,22 +118,19 @@ class TestNode(): def get_mem_rss(self): """Get the memory usage (RSS) per `ps`. - If process is stopped or `ps` is unavailable, return None. + Returns None if `ps` is unavailable. """ - if not (self.running and self.process): - self.log.warning("Couldn't get memory usage; process isn't running.") - return None + assert self.running try: return int(subprocess.check_output( - "ps h -o rss {}".format(self.process.pid), - shell=True, stderr=subprocess.DEVNULL).strip()) + ["ps", "h", "-o", "rss", "{}".format(self.process.pid)], + stderr=subprocess.DEVNULL).split()[-1]) - # Catching `Exception` broadly to avoid failing on platforms where ps - # isn't installed or doesn't work as expected, e.g. OpenBSD. + # Avoid failing on platforms where ps isn't installed. # # We could later use something like `psutils` to work across platforms. - except Exception: + except (FileNotFoundError, subprocess.SubprocessError): self.log.exception("Unable to get memory usage") return None @@ -308,7 +305,7 @@ class TestNode(): self.log.warning("Unable to detect memory usage (RSS) - skipping memory check.") return - perc_increase_memory_usage = 1 - (float(before_memory_usage) / after_memory_usage) + perc_increase_memory_usage = (after_memory_usage / before_memory_usage) - 1 if perc_increase_memory_usage > perc_increase_allowed: self._raise_assertion_error( diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 90b333f45a..ad53788b06 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -155,6 +155,7 @@ BASE_SCRIPTS = [ 'feature_nulldummy.py', 'mempool_accept.py', 'wallet_import_rescan.py', + 'wallet_import_with_label.py', 'rpc_bind.py --ipv4', 'rpc_bind.py --ipv6', 'rpc_bind.py --nonloopback', diff --git a/test/functional/wallet_import_with_label.py b/test/functional/wallet_import_with_label.py new file mode 100755 index 0000000000..95acaa752e --- /dev/null +++ b/test/functional/wallet_import_with_label.py @@ -0,0 +1,134 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test the behavior of RPC importprivkey on set and unset labels of +addresses. + +It tests different cases in which an address is imported with importaddress +with or without a label and then its private key is imported with importprivkey +with and without a label. +""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +class ImportWithLabel(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.setup_clean_chain = True + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + """Main test logic""" + + self.log.info( + "Test importaddress with label and importprivkey without label." + ) + self.log.info("Import a watch-only address with a label.") + address = self.nodes[0].getnewaddress() + label = "Test Label" + self.nodes[1].importaddress(address, label) + address_assert = self.nodes[1].getaddressinfo(address) + + assert_equal(address_assert["iswatchonly"], True) + assert_equal(address_assert["ismine"], False) + assert_equal(address_assert["label"], label) + + self.log.info( + "Import the watch-only address's private key without a " + "label and the address should keep its label." + ) + priv_key = self.nodes[0].dumpprivkey(address) + self.nodes[1].importprivkey(priv_key) + + assert_equal(label, self.nodes[1].getaddressinfo(address)["label"]) + + self.log.info( + "Test importaddress without label and importprivkey with label." + ) + self.log.info("Import a watch-only address without a label.") + address2 = self.nodes[0].getnewaddress() + self.nodes[1].importaddress(address2) + address_assert2 = self.nodes[1].getaddressinfo(address2) + + assert_equal(address_assert2["iswatchonly"], True) + assert_equal(address_assert2["ismine"], False) + assert_equal(address_assert2["label"], "") + + self.log.info( + "Import the watch-only address's private key with a " + "label and the address should have its label updated." + ) + priv_key2 = self.nodes[0].dumpprivkey(address2) + label2 = "Test Label 2" + self.nodes[1].importprivkey(priv_key2, label2) + + assert_equal(label2, self.nodes[1].getaddressinfo(address2)["label"]) + + self.log.info("Test importaddress with label and importprivkey with label.") + self.log.info("Import a watch-only address with a label.") + address3 = self.nodes[0].getnewaddress() + label3_addr = "Test Label 3 for importaddress" + self.nodes[1].importaddress(address3, label3_addr) + address_assert3 = self.nodes[1].getaddressinfo(address3) + + assert_equal(address_assert3["iswatchonly"], True) + assert_equal(address_assert3["ismine"], False) + assert_equal(address_assert3["label"], label3_addr) + + self.log.info( + "Import the watch-only address's private key with a " + "label and the address should have its label updated." + ) + priv_key3 = self.nodes[0].dumpprivkey(address3) + label3_priv = "Test Label 3 for importprivkey" + self.nodes[1].importprivkey(priv_key3, label3_priv) + + assert_equal(label3_priv, self.nodes[1].getaddressinfo(address3)["label"]) + + self.log.info( + "Test importprivkey won't label new dests with the same " + "label as others labeled dests for the same key." + ) + self.log.info("Import a watch-only legacy address with a label.") + address4 = self.nodes[0].getnewaddress() + label4_addr = "Test Label 4 for importaddress" + self.nodes[1].importaddress(address4, label4_addr) + address_assert4 = self.nodes[1].getaddressinfo(address4) + + assert_equal(address_assert4["iswatchonly"], True) + assert_equal(address_assert4["ismine"], False) + assert_equal(address_assert4["label"], label4_addr) + + self.log.info("Asserts address has no embedded field with dests.") + + assert_equal(address_assert4.get("embedded"), None) + + self.log.info( + "Import the watch-only address's private key without a " + "label and new destinations for the key should have an " + "empty label while the 'old' destination should keep " + "its label." + ) + priv_key4 = self.nodes[0].dumpprivkey(address4) + self.nodes[1].importprivkey(priv_key4) + address_assert4 = self.nodes[1].getaddressinfo(address4) + + assert address_assert4.get("embedded") + + bcaddress_assert = self.nodes[1].getaddressinfo( + address_assert4["embedded"]["address"] + ) + + assert_equal(address_assert4["label"], label4_addr) + assert_equal(bcaddress_assert["label"], "") + + self.stop_nodes() + + +if __name__ == "__main__": + ImportWithLabel().main() |