[Qt] Rework of payment request UI (mainly for insecure pr)#3145
[Qt] Rework of payment request UI (mainly for insecure pr)#3145laanwj merged 4 commits intobitcoin:masterfrom Diapolo:payment-request-GUI
Conversation
|
Haven't tested yet, but screenshots look good. |
|
You might want to write "verified" vs "unverified", it's a bit closer to the truth. |
|
Yay! Thanks for tackling this! |
|
@mikehearn Agreed, thanks. |
|
Updated:
|
There was a problem hiding this comment.
Can you be sure that the new SendCoinsRecipient is always the first one? (index 0)
Nit: It may be better to first build a SendCoinsRecipient and append it to recipients at the end of the function. This also avoids a lot of repeated recipients[0].
There was a problem hiding this comment.
I locally changed all QList usages with processPaymentRequest() to a single SendCoinsEntry, as we don't have multiple SendCoinsEntries with payment requests (as that is one of the things which this pull is changing), so yes I can be sure (as long as there are no subtle bugs left then).
Your nit should be fixed by the next push to this branch :).
|
I've tested a bit, works great. Next thing we need to deal with is how payment request-generated transactions are shown in the transactions list. We ought to show the name of the merchant and the memo instead of bare addresses/labels. |
|
Thanks for testing, I'm with you, we should rework how pr are shown in the tx list. But that should be part of another pull request then. My roadmap with this is to re-add the delete button for pr's and then re-check the issue for what is left to be done here, before this needs testing by some other devs. |
|
Eh yes, I certainly am not proposing that you add that to this pull req, it was just a general remark :) And indeed a delete button would be useful, currently the only way to get rid of payment requests in the send list is the Clear button. |
|
@laanwj A further thing to consider is our baloon pop-up when a transaction is detected. I mainly use 3 addresses I sent coins to when testing a payment request. This is really ugly with the pop-up, as we only show 1 a time, but the 3 are practically created right in a row... any idea here? |
|
It should really show only one popup per transaction, not per output. For the transaction list some kind of grouping makes sense too. Right now we always generate one record per output. Grouping on transaction level would be too coarse, but it should ideally show paid payment requests (which can involve multiple consecutive outputs) as one row and not multiple. |
- this shows insecure (unsecured) payment requests in a new yellowish colored UI (based on the secure payment request UI) instead of our normal payment UI - allows us to receive paymentACK messages for insecure payment requests - allows us to handle expirations for insecure payment request - changed walletmodel, so that all types of payment requests don't touch the addressbook
- as one this pulls main purpose is to change a payment request to be displayed as a single sendcoins entry
- use a QStringList to store valid addresses and format them for GUI and debug.log usage via .join()
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/395d0d5af017bbf6d432471075608efaf4104a03 for binaries and test log. |
|
@Diapolo do you intend to make further changes here or is this ready for merging? |
|
@laanwj I currently have no working local build environment (tried to upgrade to a newer MinGW version, which caused troubles ^^). I intended to add back the delete button for payment requests, but this can be done in another pull. If there are no show-stoppers or bugs in here this can be merged. @gavinandresen Can you perhaps check this out and give an ACK? |
395d0d5 rework an ugly hack in processPaymentRequest() (Philip Kaufmann) 952d2cd make processPaymentRequest() use a single SendCoinsRecipient (Philip Kaufmann) 983cef4 payment-request UI: use SendCoinsRecipient.message for memo (Philip Kaufmann) c6c97e0 [Qt] Rework of payment request UI (mainly for insecure pr) (Philip Kaufmann)
* More/better logging for InstantSend * Implement CRecoveredSigsDb::TruncateRecoveredSig * Truncate recovered sigs for ISLOCKs instead of completely removing them This makes AlreadyHave() return true even when the recovered sig is deleted locally. This avoids re-requesting and re-processing of old recovered sigs. * Also truncate recovered sigs for freshly received ISLOCKs * Fix comment
5 participants
colored UI (based on the secure payment request UI) instead of our
normal payment UI
the addressbook
This is an attempt to make payment requests much more usable via our GUI and already addresses some of the problems mentioned in #2936. I'm unsure, if #3095 also belongs here?
This is considered a preview and open for feedback and bug reports!
New insecure pr UI:
Todo: