gui: Add transaction record type Fee #12578

promag · 2018-03-01T18:31:13Z

Currently, in the transactions table, the fee is added as debit to the first output. It's a different behaviour from gettransaction RPC where the values are seen separately:

gettransaction 56d4c03f65f32acecd80ec6ec1b5cd12eddcd35d705f2d4c82a90c85b1cf872a

{
  ...
  "details": [
    {
      "account": "",
      "address": "2N3AFooPuYvN6zWPpThPB8w8V48Ph31PuKH",
      "category": "send",
      "amount": -1,
      "label": "",
      "vout": 0,
      "fee": -0.0000022,
      "abandoned": false
    },
    {
      "account": "",
      "address": "2N42JYZHhwRXgrpuztod6CcEXfUDLTqbhY7",
      "category": "send",
      "amount": -1,
      "label": "",
      "vout": 1,
      "fee": -0.0000022,
      "abandoned": false
    }
  ],
  ...
}

Before:

After:

The row background now alternates based on txid, so in the capture above, the first 3 rows refer to the same transaction.

laanwj · 2018-03-05T18:42:51Z

Concept ACK, I think it is clearer to list the fee separately. On the other hand, this compounds the issue that it's not possible to see in the list what transaction output records group to one transaction.

promag · 2018-03-05T18:55:09Z

@laanwj that's an existing problem isn't?

laanwj · 2018-03-05T19:29:20Z

@laanwj that's an existing problem isn't?

Yes, that's what I said. However this compounds it, because it means every transaction will get multiple records, whereas it used to be that this only happens for (rarer) multi-sends. Not an argument against doing this, but something to be aware of, as people will ask "what does this fee belong to". They can find out by looking at the transaction details.

promag · 2018-03-05T20:22:22Z

Yeah, in the typical listing it will show an alternating "Fee" row.

I was thinking in adding a checkbox to show/hide individual fee rows.

Another thing that could help the grouping is to change from alternate rows into "alternate txids", but that is something more complicated to do (as it depends on the proxy model parameters).

laanwj · 2018-03-05T20:45:19Z

Another thing that could help the grouping is to change from alternate rows into "alternate txids", but that is something more complicated to do (as it depends on the proxy model parameters).

Yeah that can be done in a future PR. Sounds like a good idea.
I wonder how that will/should interact with sorting on alternative columns, though.

I was thinking in adding a checkbox to show/hide individual fee rows.

Might be overkill, just adds an extra configuration that needs to be supported. But no strong opinion.

Do add it to the release notes, this is something that users are bound to notice.

luke-jr · 2018-03-05T22:45:31Z

Concept ACK. It might (or might not) make sense to only do it for multi-debits...

promag · 2018-03-06T14:02:07Z

@laanwj I've implemented alternating rows based on transaction id (see after image above). The implementation is not efficient but it's enough to evaluate the concept.

@luke-jr even for single-debit it's weird to have the fee associated to that address, sounds like you sent that value there.

promag · 2018-03-06T20:07:23Z

Added release notes.

jonasschnelli · 2018-04-10T18:32:58Z

Concept ACK especially with the grouping (odd/even background alternative). IMO the grouping could be more visible (group entries from a transaction with an outline border line or similar)

promag · 2018-04-12T19:00:03Z

group entries from a transaction with an outline border line or similar

I'll try something along that, but it's not something usual right?

jonasschnelli · 2018-04-12T19:40:19Z

I'll try something along that, but it's not something usual right?

I guess that is something that could be improved in a follow up PR.

promag · 2018-04-13T21:02:57Z

Rebased.

laanwj · 2018-05-10T20:20:43Z

Concept ACK

jonasschnelli · 2018-05-10T20:25:48Z

Build: https://bitcoin.jonasschnelli.ch/build/603

jonasschnelli · 2018-05-14T06:28:59Z

Tested ACK ae7a25fafc08482ba8af2174c3e8002bbe5a3bac

Before / After:

promag · 2018-05-24T15:47:09Z

Rebased due to conflict in src/qt/test/wallettests.cpp.

promag · 2018-06-24T15:05:59Z

Rebased due to release notes conflict.

jb55 · 2018-08-24T20:29:55Z

Concept ACK

practicalswift · 2019-01-09T20:41:47Z

src/qt/transactionview.cpp

+    }
+
+public:
+    TransactionRecordDelegate(QSortFilterProxyModel* proxy) : m_proxy(proxy) {}


This should be explicit :-)

DrahtBot · 2019-05-07T17:07:19Z

The last travis run for this pull request was 239 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.

DrahtBot · 2019-05-09T17:49:12Z

Needs rebase

promag · 2019-06-30T09:02:20Z

Closing for now. The implementation is not ideal anyway.

Github-Pull: bitcoin/bitcoin bitcoin#12578 Rebased-From: 82dd3e0

Github-Pull: bitcoin/bitcoin bitcoin#12578 Rebased-From: 550f1f3

Github-Pull: bitcoin/bitcoin bitcoin#12578 Rebased-From: 264f9c4

This reverts commit 264f9c4d4f7c1aeb3e89eedddcd88e47e8bba938. This was included in ElementsProject#569 which received no review, and is a backport of upstream PR bitcoin/bitcoin#12578 which was later closed. It leaks memory which is causing CI to fail.

fanquake added the GUI label Mar 1, 2018

promag force-pushed the 2018-03-fee-transaction-record branch from 8d115aa to 9d6ae2e Compare March 6, 2018 13:56

promag force-pushed the 2018-03-fee-transaction-record branch from 9d6ae2e to 2bbf744 Compare March 6, 2018 20:06

promag force-pushed the 2018-03-fee-transaction-record branch from 2bbf744 to ed5e1c3 Compare April 13, 2018 20:59

promag force-pushed the 2018-03-fee-transaction-record branch from ed5e1c3 to ae7a25f Compare April 13, 2018 21:06

maflcko changed the title ~~Add transaction record type Fee~~ gui: Add transaction record type Fee May 10, 2018

promag force-pushed the 2018-03-fee-transaction-record branch from ae7a25f to ca5d3d6 Compare May 24, 2018 15:46

DrahtBot added the Needs rebase label Jun 11, 2018

promag force-pushed the 2018-03-fee-transaction-record branch from ca5d3d6 to 1c778ac Compare June 24, 2018 15:05

DrahtBot removed the Needs rebase label Jun 24, 2018

DrahtBot added the Needs rebase label Aug 13, 2018

gui: Avoid TransactionRecord instance for IsMine outputs

82dd3e0

practicalswift reviewed Jan 9, 2019

View reviewed changes

DrahtBot closed this May 7, 2019

DrahtBot reopened this May 7, 2019

DrahtBot added the Needs rebase label May 9, 2019

promag closed this Jun 30, 2019

kallewoof pushed a commit to kallewoof/bitcoin that referenced this pull request Oct 4, 2019

gui: Avoid TransactionRecord instance for IsMine outputs

41c8d4c

Github-Pull: bitcoin/bitcoin bitcoin#12578 Rebased-From: 82dd3e0

kallewoof pushed a commit to kallewoof/bitcoin that referenced this pull request Oct 4, 2019

gui: Add transaction record type Fee

3edcdf8

Github-Pull: bitcoin/bitcoin bitcoin#12578 Rebased-From: 550f1f3

kallewoof pushed a commit to kallewoof/bitcoin that referenced this pull request Oct 4, 2019

gui: Alternate row background based on txid

1655947

Github-Pull: bitcoin/bitcoin bitcoin#12578 Rebased-From: 264f9c4

laanwj removed the Needs rebase label Oct 24, 2019

promag mentioned this pull request Oct 26, 2020

Call interfaces::Wallet::getWalletTxs asynchronous #20241

Closed

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

gui: Add transaction record type Fee #12578

gui: Add transaction record type Fee #12578

Uh oh!

Conversation

promag commented Mar 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Mar 5, 2018

Uh oh!

promag commented Mar 5, 2018

Uh oh!

laanwj commented Mar 5, 2018

Uh oh!

promag commented Mar 5, 2018

Uh oh!

laanwj commented Mar 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luke-jr commented Mar 5, 2018

Uh oh!

promag commented Mar 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Mar 6, 2018

Uh oh!

jonasschnelli commented Apr 10, 2018

Uh oh!

promag commented Apr 12, 2018

Uh oh!

jonasschnelli commented Apr 12, 2018

Uh oh!

promag commented Apr 13, 2018

Uh oh!

laanwj commented May 10, 2018

Uh oh!

jonasschnelli commented May 10, 2018

Uh oh!

jonasschnelli commented May 14, 2018

Uh oh!

promag commented May 24, 2018

Uh oh!

promag commented Jun 24, 2018

Uh oh!

jb55 commented Aug 24, 2018

Uh oh!

practicalswift Jan 9, 2019

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented May 7, 2019

Uh oh!

DrahtBot commented May 9, 2019

Uh oh!

promag commented Jun 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

promag commented Mar 1, 2018 •

edited

Loading

laanwj commented Mar 5, 2018 •

edited

Loading

promag commented Mar 6, 2018 •

edited

Loading