[Qt] prepare paymentservertests for new unit tests #5642

Diapolo commented at 6:49 PM on January 11, 2015: none

The intention of this pull is to be able to extend the existing unit-tests for payment requests. This is achieved by adding a second PaymentRequest Test CA (serial number f0:da:97:e4:38:d7:64:16) into paymentrequestdata.h. The CA was generated by my using "ca_in_a_box" (by @gavinandresen), which I've partly re-worked to natively run on Windows. Also the code in paymentservertests was reworked to be able to directly load payment request raw data from paymentrequestdata.h into an usable PaymentRequestPlus object. This is used to skip normal client code flow, to be able to explicitly test and verify payment request validity.

As a first step I integrated a new unit test, which is able to verify that the "client network matches the payment request network" rule is working as intended.

Diapolo commented at 2:38 PM on January 12, 2015: none

@gavinandresen @laanwj Travis should pass now and I'm also able to extend unit-tests after this is merged. Can you review and/or comment this? I know there are quite a few open payment request related pulls, but hey you wanted me to to something more valuable than just cleanups ^^.

gavinandresen commented at 3:08 PM on January 12, 2015: contributor

Untested ACK. You should edit the first comment in this pull request to describe what the pull request does.

Diapolo commented at 6:48 PM on January 12, 2015: none

@gavinandresen Thanks, I updated the initial comment. Would be great if we could get that in soon :). @laanwj Mind also giving an ACK here?

[Qt] prepare paymentservertests for new unit tests

- add a second PaymentRequest Test CA certificate to paymentrequestdata.h
  (serial number f0:da:97:e4:38:d7:64:16) as caCert2_BASE64
- rename existing Test CA certificate to caCert1_BASE64
- rename existing payment request data to know they belong to
  caCert1_BASE64
- update comments to reflect the changes and add a missing comment to one
  of the payment requests

080da96c7c

in src/qt/paymentserver.cpp:None in a5babb7f84 outdated

 743 | @@ -745,3 +744,16 @@ void PaymentServer::handlePaymentACK(const QString& paymentACKMsg)
 744 |      // currently we don't futher process or store the paymentACK message
 745 |      emit message(tr("Payment acknowledged"), paymentACKMsg, CClientUIInterface::ICON_INFORMATION | CClientUIInterface::MODAL);
 746 |  }
 747 | +
 748 | +bool PaymentServer::verifyNetwork(const payments::PaymentDetails& requestDetails)
 749 | +{
 750 | +    const QString requestNetwork = QString::fromStdString(requestDetails.network());

laanwj commented at 8:06 AM on January 14, 2015:

Why change these to QString just to compare? std::strings compare in the same way.

Diapolo commented at 8:26 AM on January 14, 2015:

@laanwj The logging only works when converted to QString ;).

Diapolo commented at 10:16 AM on January 14, 2015:

@laanwj If you want I can change this into:

bool fVerified = requestDetails.network() == Params().NetworkIDString();
if (!fVerified) {
    qWarning() << QString("PaymentServer::%1: Payment request network \"%2\" doesn't match client network \"%3\".")
        .arg(__func__)
        .arg(QString::fromStdString(requestDetails.network()))
        .arg(QString::fromStdString(Params().NetworkIDString()));
}
return fVerified;

laanwj commented at 11:56 AM on January 14, 2015:

Yes. That's better, just convert as needed.

laanwj commented at 11:56 AM on January 14, 2015: member

ACK, more tests is good.

[Qt] add payment request unit test for non matching networks

- verify that payment request network matches client network
- add static verifyNetwork() function to PaymentServer to be able to use
  the same validation code in GUI and unit-testing code

17005bc0fc

Diapolo commented at 12:17 PM on January 14, 2015: none

@laanwj Great, that counts 2 ACKs now. The last rebase just changed PaymentServer::verifyNetwork() to only convert networks as needed.

I'll be happy to add aditional testing for payment requests :).

laanwj merged this on Jan 14, 2015

laanwj closed this on Jan 14, 2015

laanwj referenced this in commit 30a5b5fa7a on Jan 14, 2015

Diapolo deleted the branch on Jan 14, 2015

DrahtBot locked this on Sep 8, 2021