Merge pull request #3683

bdc83e8 [Qt] ensure payment request network matches client network (Philip Kaufmann)

src/qt/paymentserver.cpp

@@ -178,6 +178,9 @@ void PaymentServer::LoadRootCAs(X509_STORE* _store)
 // and the items in savedPaymentRequest will be handled
 // when uiReady() is called.
 //
+// Warning: ipcSendCommandLine() is called early in init,
+// so don't use "emit message()", but "QMessageBox::"!
+//
 bool PaymentServer::ipcParseCommandLine(int argc, char* argv[])
 {
     for (int i = 1; i < argc; i++)
@@ -411,7 +414,15 @@ void PaymentServer::handleURIOrFile(const QString& s)
         {
             SendCoinsRecipient recipient;
             if (GUIUtil::parseBitcoinURI(s, &recipient))
-                emit receivedPaymentRequest(recipient);
+            {
+                CBitcoinAddress address(recipient.address.toStdString());
+                if (!address.IsValid()) {
+                    emit message(tr("URI handling"), tr("Invalid payment address %1").arg(recipient.address),
+                        CClientUIInterface::MSG_ERROR);
+                }
+                else
+                    emit receivedPaymentRequest(recipient);
+            }
             else
                 emit message(tr("URI handling"),
                     tr("URI can not be parsed! This can be caused by an invalid Bitcoin address or malformed URI parameters."),
@@ -425,12 +436,14 @@ void PaymentServer::handleURIOrFile(const QString& s)
     {
         PaymentRequestPlus request;
         SendCoinsRecipient recipient;
-        if (readPaymentRequest(s, request) && processPaymentRequest(request, recipient))
+        if (!readPaymentRequest(s, request))
-            emit receivedPaymentRequest(recipient);
+        {
-        else
             emit message(tr("Payment request file handling"),
-                tr("Payment request file can not be read or processed! This can be caused by an invalid payment request file."),
+                tr("Payment request file can not be read! This can be caused by an invalid payment request file."),
                 CClientUIInterface::ICON_WARNING);
+        }
+        else if (processPaymentRequest(request, recipient))
+            emit receivedPaymentRequest(recipient);
 
         return;
     }
@@ -482,6 +495,35 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins
     if (!optionsModel)
         return false;
 
+    if (request.IsInitialized()) {
+        const payments::PaymentDetails& details = request.getDetails();
+
+        // Payment request network matches client network?
+        if ((details.network() == "main" && TestNet()) ||
+            (details.network() == "test" && !TestNet()))
+        {
+            emit message(tr("Payment request rejected"), tr("Payment request network doesn't match client network."),
+                CClientUIInterface::MSG_ERROR);
+
+            return false;
+        }
+
+        // Expired payment request?
+        if (details.has_expires() && (int64_t)details.expires() < GetTime())
+        {
+            emit message(tr("Payment request rejected"), tr("Payment request has expired."),
+                CClientUIInterface::MSG_ERROR);
+
+            return false;
+        }
+    }
+    else {
+        emit message(tr("Payment request error"), tr("Payment request is not initialized."),
+            CClientUIInterface::MSG_ERROR);
+
+        return false;
+    }
+
     recipient.paymentRequest = request;
     recipient.message = GUIUtil::HtmlEscape(request.getDetails().memo());
 
@@ -497,11 +539,11 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins
             // Append destination address
             addresses.append(QString::fromStdString(CBitcoinAddress(dest).ToString()));
         }
-        else if (!recipient.authenticatedMerchant.isEmpty()){
+        else if (!recipient.authenticatedMerchant.isEmpty()) {
             // Insecure payments to custom bitcoin addresses are not supported
             // (there is no good way to tell the user where they are paying in a way
             // they'd have a chance of understanding).
-            emit message(tr("Payment request error"),
+            emit message(tr("Payment request rejected"),
                 tr("Unverified payment requests to custom payment scripts are unsupported."),
                 CClientUIInterface::MSG_ERROR);
             return false;
@@ -510,11 +552,10 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins
         // Extract and check amounts
         CTxOut txOut(sendingTo.second, sendingTo.first);
         if (txOut.IsDust(CTransaction::nMinRelayTxFee)) {
-            QString msg = tr("Requested payment amount of %1 is too small (considered dust).")
+            emit message(tr("Payment request error"), tr("Requested payment amount of %1 is too small (considered dust).")
-                .arg(BitcoinUnits::formatWithUnit(optionsModel->getDisplayUnit(), sendingTo.second));
+                .arg(BitcoinUnits::formatWithUnit(optionsModel->getDisplayUnit(), sendingTo.second)),
+                CClientUIInterface::MSG_ERROR);
 
-            qDebug() << "PaymentServer::processPaymentRequest : " << msg;
-            emit message(tr("Payment request error"), msg, CClientUIInterface::MSG_ERROR);
             return false;
         }
 
@@ -581,8 +622,8 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, SendCoinsRecipient recipien
             refund_to->set_script(&s[0], s.size());
         }
         else {
-            // This should never happen, because sending coins should have just unlocked the wallet
+            // This should never happen, because sending coins should have
-            // and refilled the keypool
+            // just unlocked the wallet and refilled the keypool.
             qDebug() << "PaymentServer::fetchPaymentACK : Error getting refund key, refund_to not set";
         }
     }
@@ -594,7 +635,7 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, SendCoinsRecipient recipien
         netManager->post(netRequest, serData);
     }
     else {
-        // This should never happen, either:
+        // This should never happen, either.
         qDebug() << "PaymentServer::fetchPaymentACK : Error serializing payment message";
     }
 }
@@ -620,17 +661,15 @@ void PaymentServer::netRequestFinished(QNetworkReply* reply)
     {
         PaymentRequestPlus request;
         SendCoinsRecipient recipient;
-        if (request.parse(data) && processPaymentRequest(request, recipient))
+        if (!request.parse(data))
         {
-            emit receivedPaymentRequest(recipient);
+            qDebug() << "PaymentServer::netRequestFinished : Error parsing payment request";
-        }
-        else
-        {
-            qDebug() << "PaymentServer::netRequestFinished : Error processing payment request";
             emit message(tr("Payment request error"),
-                tr("Payment request can not be parsed or processed!"),
+                tr("Payment request can not be parsed!"),
                 CClientUIInterface::MSG_ERROR);
         }
+        else if (processPaymentRequest(request, recipient))
+            emit receivedPaymentRequest(recipient);
 
         return;
     }

src/qt/sendcoinsdialog.cpp

@@ -377,26 +377,8 @@ void SendCoinsDialog::pasteEntry(const SendCoinsRecipient &rv)
 
 bool SendCoinsDialog::handlePaymentRequest(const SendCoinsRecipient &rv)
 {
-    QString strSendCoins = tr("Send Coins");
+    // Just paste the entry, all pre-checks
-    if (rv.paymentRequest.IsInitialized()) {
+    // are done in paymentserver.cpp.
-        // Expired payment request?
-        const payments::PaymentDetails& details = rv.paymentRequest.getDetails();
-        if (details.has_expires() && (int64_t)details.expires() < GetTime())
-        {
-            emit message(strSendCoins, tr("Payment request expired"),
-                CClientUIInterface::MSG_WARNING);
-            return false;
-        }
-    }
-    else {
-        CBitcoinAddress address(rv.address.toStdString());
-        if (!address.IsValid()) {
-            emit message(strSendCoins, tr("Invalid payment address %1").arg(rv.address),
-                CClientUIInterface::MSG_WARNING);
-            return false;
-        }
-    }
-
     pasteEntry(rv);
     return true;
 }