PaymentRequest GUI bugs #2936

I consider these low priority, for two reasons:

1. I think the vast majority of payment requests will be signed and will involve payment to just one merchant.

2. I think the whole "send" GUI needs a redesign; I tried to do the absolute minimum amount of GUI work to demonstrate payment protocol functionality (I'm a terrible GUI designer).

From michagogo:

Binary used: http://jenkins.bluematt.me/pull-tester/a41d5fe01947f2f878c055670986a165af800f9a/bitcoin/bitcoin-qt.exe

On the unsigned payment request test, even though the address and amount are editable, editing them doesn't have any effect -- the address and value entered into the web form are what ends up getting sent regardless of what's entered into those boxes.

In the Signed payment request test: I don't know if this is intentional, but if clicking on more CLICK TO PAY links before confirming the first send causes the other payments to be added as outputs to the transaction, with no way to remove individual transactions without using the Clear All button.

When sending multiple PaymentACKs, the dialog boxes are modal on top of each other, and out of order.

A signed request, even one with multiple outputs, will show up in the Send tab as one entry, while an unsigned request with multiple outputs shows up as one entry per output.

Either unsigned requests cannot expire and the request generator just doesn't make that clear, or expiration is ignored in unsigned requests.

I'm not sure if I like the behavior of "unsigned payment requests add the address to your address book with the contents of the memo field if it's not already there"... I'd maybe make it a checkbox or something, if not remove it completely -- oh, just realized it's because it uses the "Label" field. Seems like a bad idea to me, since payment-protocol addresses are probably not going to be reused...

Receiving an unsigned payment request with a1, b1, and c1 amounts for three addresses and modifying them to a2, b2 and c2 would result in sending a1+b1+c1+b2+c2 (a2 is not sent) while the confirmation box before sending the payment shows a2+b2+c2 (as would be expected to be sent).

I believe unsigned payment requests should not be enabled until this is fixed since there's too much risk of sending more bitcoins than expected.

@jherrerob Is that behaviour with current master? I introduced some small changes and fixes in that area over the past days and more pull-requests are open that are related to payment-requests in general.

The version I'm using I would say it's less than 2 weeks old.

Bitcoin-QT shows this version: v0.8.2-469-gacb3ebc-beta

And git log -1 returns this:

commit acb3ebc45506701fdb7e060d8b941a5ba6a1b5ac Merge: d922c85 bf3a20a Author: Gavin Andresen gavinandresen@gmail.com Date: Thu Oct 3 22:52:38 2013 -0700

Merge pull request [#2947](/bitcoin-bitcoin/2947/) from gmaxwell/theyre_maturing_faster_these_days

[wallet] Consider generated coins mature at COINBASE_MATURITY+1

I can update and retest if you you believe it's already fixed.

I'm not sure it's fixed, just wanted to be sure...

@gavinandresen I need your help to investigate the reported problems.

Consider an insecure payment-request, this is a request, which doesn't contain an auth-merchant, right? It is otherwiese a normal payment-request, which can contain multiple send-to-addresses, right?

Currently GUI code differentiates between secure and insecure payment-requests, the insecure ones generate one sendcoinsentry for each send-to-address (displayed in the GUI), while secure ones don't do this (one entry and no addresses displayed in the sendcoins dialog).

Now consider an insecure payment-request with 3 send-to-addresses (A, B and C). Current code attaches the payment-request to the first displayed entry (only showing address A), while still adding 2 entries for the other addresses (B and C).

What does this result in? Payments to address A, B and C via the payment-request embedded in GUI entry 1 and payments to B and C via the separate GUI entries!

Seems like a heavy bug IMHO?

@Diapolo:

As I said, I think payment requests need a GUI redesign-- I don't think wedging them into the "one sendcoinsentry per destination address" works.

The design should drive the coding, not the other way around. "We" need to figure out the correct user experience; perhaps payment requests shouldn't even appear in the Send tab, but should be their own dialog boxes. As I said, I'm a terrible UI designer, so I hope @laanwj or somebody else will chime in with an opinion.

To be honest I think it was an excellent idea to make payment requests appear on the send tab. This keeps sending in one place, which is safe and recognizable.

But IMO it should always be one sendcoins entry per payment request, even for non-signed/secure one, so that they can be removed in one go. That entry should show the total amount sent so that there is no confusion in how much is sent. The background color could be used to distinguish signed and non-signed ones. It should not be editable in either case.

It usually doesn't really matter for the user to which address(es) which amount is sent, although it'd be useful to be able to expand it and/or show details for those curious.

@laanwj What you propose shouldn't be that hard (except some detailed display), I can take a look in the next days, as I'm quite im that part of the code now. But if you are already working on that area too, I'm waiting for your input. Can you take a look at the other related pulls and merge the small ones, so I have an up-to-date branch I can start working with :)?

@diapolo, yes I'll take another look

As for the interaction with the address book, I agree there should be as little as possible. And we should really get rid of the address book as it is and change it to "historical addresses". But that's a different topic (#2429).

Also we need to introduce per-transaction messages somehow (I think this is already supported in the core?). This would allow showing what the payment request was about in the "Transactions" tab instead of abusing the label for that.

I'm working on a fix currently, which fixes wrong amounts for insecure payment requests and also allows insecure requests to be able to expire and much more...

Closing payment protocol specific issues; as that functionality is going to be deprecated in the near future it's not worth working on.