[Qt] show conflicts of unconfirmed transactions in the UI #7826
Conversation
|
Another option would be to hide these transactions completely. |
|
Before this PR: Sending 49.9BTC and RBF with a 40.0BTC transaction. After this PR: |
| return true; | ||
| } | ||
|
|
||
| if (status.cur_num_blocks != chainActive.Height()) |
There was a problem hiding this comment.
Nit: I think you can leave those three lines the way it was before: return status.cur_num_blocks != chainActive.Height();
I don't think you want to hide wtx without telling the user/ without having the user told you to do so. |
91b63c5 to
614073f
Compare
Yes. I agree. IMO the "mark as conflicted" solution is better. |
|
Hmm, so I have a non-rbf transaction back from august: and sendrawt returns |
|
@MarcoFalke: does you non-rbf transaction back from august is in conflict with another transaction in your wallet? Only if it's so, the conflicted state icon will be shown. |
|
No, it didn't conflict within the wallet. Tested ACK 614073f |
|
One could argue to use another status/icon for this e.g. https://raw.githubusercontent.com/jonasschnelli/bitcoin/47196712ec7f3ff2f5f814650d40366cb269fe56/src/qt/res/icons/transaction_replaceable.png with the conflicted icon (cross) in the lower left. |
614073f to
8844ef1
Compare
On RPC level, we show possible mempool conflicts already via detecting them over the CWallet::GetConflicts() function. Currently, the GUI shows only conflicts with transaction in a block. This results in displaying replaced transaction (RBF) wrong. This PR will mark unconfirmed transactions that are _not_ in our mempool and have conflicts as conflicted.
|
Rebased and changed the icon for transactions with conflict but not being in the mempool (RBF): |
|
These seem to be two list items referring to the same transaction. |
|
@luke-jr: this PR does not focus on hiding out transactions from the wallet (a lot more complexity). This PR is a tiny fix to improve the GUI RBF attribution. |
|
Another solution would be (idea once shortly discusses with @gmaxwell on IRC):
|
|
@jonasschnelli Any progress here? |
|
@sipa: I'm working on a option to auto-hide mempool conflicts (transactions that are not in the blockchain and mempool) (RBF/abandoned txes). |
|
I think this solution is fine as it is. The auto-hide option can be written in a separate PR. Would be great for 0.13. |
|
Removing this from the 0.13 milestone as it missed the feature freeze.
Agree, would have been nice, just like #7817. I think we have a lack of GUI testers/reviewers, as useful GUI changes like this linger way too long. |
|
This (simple) PR looks for reviewers... |
|
I think the problem is that it may not be obvious for everyone to set up the appropriate transactions to test this. I think it may help to add "proposed steps to reproduce" for harder to test pulls in general. |
|
82962bd Works for standard transactions:
Does not work for non-standard transactions:
|
|
Closing for now due to inactivity... |
Currently, the GUI shows only conflicts with transaction in a block.
This results in displaying replaced transaction (RBF) wrong (the replaced transaction is no longer in the mempool, but still in the wallet-transactionlist, conflicts are not detected).
This PR will mark unconfirmed transactions that are not in our mempool and have conflicts as conflicted.