Fixes #8125
Qt: Show transaction size in transaction details window #8672
pull hsjoberg wants to merge 2 commits into bitcoin:master from hsjoberg:qttxsizeindetails changing 3 files +13 −0-
hsjoberg commented at 8:38 PM on September 6, 2016: contributor
-
fdf82fba31
Adding method GetTotalSize() to CTransaction
GetTotalSize() returns the total transaction size (including witness) in bytes.
-
qt: Adding transaction size to transaction details window c015634400
-
sipa commented at 8:42 PM on September 6, 2016: member
Wouldn't it be more useful to show the transaction weight? That's what matters for fee estimates etc.
-
luke-jr commented at 8:52 PM on September 6, 2016: member
I suggest size + feerate-based-on-weight.
-
- fanquake added the label GUI on Sep 7, 2016
-
jonasschnelli commented at 6:19 AM on September 7, 2016: contributor
I agree with @luke-jr:
size + ferrate-based-on-weight -
sipa commented at 6:25 AM on September 7, 2016: member
Which size? We have:
- Base size
- Total size (base + witness)
- Virtual size (base + witness/4)
- Weight (4*base + witness)
My assumption was that after segwit, only vsize would matter. For old transactions it is identical to total and base size, and for others it remains proportional to fees with that.
-
jonasschnelli commented at 6:36 AM on September 7, 2016: contributor
Depends what users will do with the size information. But IMO size should be the "total size" (base + witness). I guess the size has usefulness besides fee calculation.
-
sipa commented at 6:37 AM on September 7, 2016: member
I agree it may be useful, but total size isn't the first a user should see
- it's not relevant for fee calculation, propagation time, priority, or block space usage.
-
jonasschnelli commented at 6:42 AM on September 7, 2016: contributor
I agree that the virtual size does make more sense to display. We just need to clearly distinct between size (concrete hard size of a data structure) and virtual size (including other variables and attached to a bigger context).
I'm also not opposed to display multiple size types (at least base, total and virtual).
-
laanwj commented at 7:09 AM on September 8, 2016: member
Which size? We have:
- Base size
- Total size (base + witness)
- Virtual size (base + witness/4)
- Weight (4*base + witness)
I think the "easy to implement" tag was a bit deceptive in this case :) #8125 had no discussion about what size at all, I think everyone assumed it to be the size of gettransaction hex.
anyhow, utACK: Let's call it "Total size" and let people add other sizes later if they want.
-
hsjoberg commented at 8:19 AM on September 8, 2016: contributor
@laanwj Indeed, what seemed like an easy task blew up completely. I can continue adding other types of sizes if we decide which ones to add.
anyhow, utACK: Let's call it "Total size" and let people add other sizes later if they want.
Will fix.
-
MarcoFalke commented at 10:59 PM on September 9, 2016: member
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
-
laanwj commented at 9:12 AM on September 13, 2016: member
@marcofalke These are two logically separate commits, one adds the function, the other adds the information to the transaction description HTML, I don't think squashing is necessary.
ACK after the 'size'→ 'total size' disambiguation.
-
fanquake commented at 9:01 AM on September 16, 2016: member
It's pretty self explanatory, but here are some OS X screenshots.
-
MarcoFalke commented at 9:22 AM on September 16, 2016: member
utACK c015634
-
paveljanik commented at 10:12 AM on September 16, 2016: contributor
- jonasschnelli merged this on Sep 20, 2016
- jonasschnelli closed this on Sep 20, 2016
- jonasschnelli referenced this in commit 6052d50910 on Sep 20, 2016
-
jonasschnelli commented at 1:35 PM on September 20, 2016: contributor
Tested ACK https://github.com/bitcoin/bitcoin/pull/8672/commits/c015634400f112f8b7f037c82690f1332d7971a3, merge-fixed
size->total size. - luke-jr referenced this in commit 73cd55204a on Oct 20, 2016
- luke-jr referenced this in commit 98381f1592 on Oct 20, 2016
- hsjoberg deleted the branch on Mar 28, 2017
- codablock referenced this in commit a7623c884d on Sep 7, 2017
- UdjinM6 referenced this in commit 9707ca5cea on Sep 9, 2017
- MarcoFalke locked this on Sep 8, 2021
