New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
ui: show tx & script version on tx details #1863
Conversation
Every output script has an independent version. From the screenshot, it appears this is assuming there is only one script version for the entire transaction, which isn't the case. |
Hm understood @davecgh was mistakenly assuming all output scripts from a tx had to be the same version. Sweet, will add it to the outputs table 馃憤 |
To be clear, the tx version field looks good and is correct. It's just the script version that is unique per output. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks proper to me.
</td> | ||
<td class="text-right medium-sans text-nowrap pr-2 py-2" | ||
>Time: | ||
<td class="text-right medium-sans text-nowrap pr-2 py-2">Tx Raw: </td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Back to "Raw Tx" please. This is shorthand for something like "raw transaction data:"
</td> | ||
<td class="text-left py-1 text-secondary">{{.FormattedSize}}</td> | ||
<td class="text-right medium-sans text-nowrap pr-2 py-2">Rate:</td> | ||
<td class="text-right medium-sans text-nowrap pr-2 py-2">Tx Version: </td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just "Version" here is good, no "Tx" since this is the "Transaction Details" section.
Also, the new spaces after all these ":" seems to serve no purpose too since there's a newline after the </td>
or maybe just because they are different table cells. Can you get rid of them while making these changes? Unless they actually have a visual effect, and I do not think they do (I don't see any in testing).
*TxBasic | ||
TxVersion int32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works. I'm cooking up a PR to do some mempool and DB work, and in that I'm moving TxVersion
to TxBasic
and renaming it to just Version
. Just a heads up. This is good like this for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works, and I have the minor requested changes in a subsequent diff that adjust the mempool structures.
This diff adds the tx and script versions to the tx details areabox.
Edit:
Before:
After:
Let me know if any UI changes are needed for a better UX 馃憤
closes #1851