GUI: Replace send-to-self with dual send+receive entries #15115
Conversation
|
Apparently tests need updating. Concept ACK/NACK? |
|
Before: After: In #12578 the "Sent to" row would only have 0.1 and the fee would be in a separate row. |
I cannot reproduce that. |
|
Concept ACK - seems like a nice simplification, and IMO the table is more clear |
|
Does this approach work for special transactions (e.g., CoinJoin, Pay-to-Endpoint etc)? |
It can't. If you have a tx from A & B to C & D, there's no logical way to know the correct logical transaction(s) that comprise it, without some kind of metadata. |
|
(It might display it nicer, though, as a single "net change" entry) |
|
Haven't tested, but in the screenshot above "Received with" is before "Sent to". Shouldn't it be other way around? |
|
@kristapsk yap, it makes more sense if you consider the order important. |
|
Tests and sorting fixed. Ready for review. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
Concept ACK |
luke-jr
force-pushed
the
rm_send2self
branch
2 times, most recently
from
da5656e to
14bb8db
Compare
|
Rebased |
|
Maybe also a QT test that produces a variate of transaction types including CoinJoins with an optional to manually verify the transaction table (or a produced screenshot) would be great. |
hebasto
left a comment
hebasto
left a comment
There was a problem hiding this comment.
Tested 14bb8db on Linux Mint 19.3 (x86_64):
- one "recipient"
- master
- this PR
- master
- two "recipients"
- master
- this PR
- master
-
I'd prefer to keep the "Payment to yourself" type as descriptive enough.
-
Could a send-to-self tx has only two records: combined debit and combined credit?
How would that even work? Why?
That breaks labels and defeats the point of having multiple recipients. If you want that as a user, you can just use one recipient? |
On master, one could easily spot send-to-self txs in the tx list, and use sorting in "Type" column. I believe it is convenient. Without "Payment to yourself" type such UX is lost.
Agree.
It is up to a user how many txouts one wants to create in a send-to-self tx, no? |
What is the use case?
But you want to make anything other than 1 (in effect) require multiple transactions... |
For example, a user want to review send-to-self txs made some time ago... |
|
What is the use case for "send-to-self txs"? The only time I've ever seen actual sends-to-self occur, are also cases in which deviations of behaviour are bad... |
... tend to agree with you.
The scope of this PR is to how to present the existing "send-to-self" txs to a user in the GUI, as I understand it. The discussions about "should send-to-self txs exist" or "what is the use case for send-to-self txs" deserve their own issues/pulls :) |
|
This replaces send-to-self. As in, they cease to exist conceptually. (And they never had any existence beyond conceptually.) |
…g whether a transaction is a send or receive This changes behaviour (IMO for the better) in the case where some but not all inputs are from us, and the net amount is positive.
If we didn't send it, it can't possibly be change, even if that's the key's purpose
|
Rebased. This has quite a few Concept ACKs - how about some code review? :/ |
Lets try and make that happen over in the GUI repo. |
8 participants
Makes the GUI transaction list more like the RPC, and IMO clearer in general.
As a side effect, this also fixes the GUI entries when a transaction is a net profit to us, but some inputs were also from us.