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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Qt] Optimize SendToSelf rendering with a single non-change output #11471

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@jonasschnelli
Member

jonasschnelli commented Oct 9, 2017

Partially fixes #11464

This is a simple improvement to render singe non-change output self-to-self transactions with the corresponding output-address/label.
Multi non-change output self-to-self transaction do keep the (n.a.) label (we could show all the addresses coma separated).

Screen:
bildschirmfoto 2017-10-09 um 14 18 13

@jonasschnelli jonasschnelli added the GUI label Oct 9, 2017

@promag

Concept ACK.

we could show all the addresses coma separated

That sounds sensible.

Show outdated Hide outdated src/qt/transactionrecord.cpp Outdated
Show outdated Hide outdated src/qt/transactionrecord.cpp Outdated
Show outdated Hide outdated src/qt/transactionrecord.cpp Outdated
Show outdated Hide outdated src/qt/transactionrecord.cpp Outdated
Show outdated Hide outdated src/qt/transactiontablemodel.cpp Outdated
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Oct 10, 2017

Member

Optimizing rendering sounds good, do you have FPS numbers? :)

Sorry. Concept ACK.

Member

laanwj commented Oct 10, 2017

Optimizing rendering sounds good, do you have FPS numbers? :)

Sorry. Concept ACK.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Oct 10, 2017

Member

Fixed @promag points.

Optimizing rendering sounds good, do you have FPS numbers? :)

Heh. Yes. Rending is maybe not the best word for decomposing a transactions.

Member

jonasschnelli commented Oct 10, 2017

Fixed @promag points.

Optimizing rendering sounds good, do you have FPS numbers? :)

Heh. Yes. Rending is maybe not the best word for decomposing a transactions.

@Sjors

This comment has been minimized.

Show comment
Hide comment
@Sjors

Sjors Nov 9, 2017

Member

Concept ACK.

I can't get it to work. Before & After:
schermafbeelding 2017-11-09 om 12 56 28

I made a transaction from one non-segwit wallet address to another non-segwit wallet address, without change.

Maybe I'm doing something wrong?

Member

Sjors commented Nov 9, 2017

Concept ACK.

I can't get it to work. Before & After:
schermafbeelding 2017-11-09 om 12 56 28

I made a transaction from one non-segwit wallet address to another non-segwit wallet address, without change.

Maybe I'm doing something wrong?

@luke-jr

I think we should remove "Payment to yourself" (and just show a send+receive pair), but this seems like a strict improvement.

Show outdated Hide outdated src/qt/transactiontablemodel.cpp Outdated

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Nov 11, 2017

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Nov 11, 2017

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Nov 28, 2017

Member

utACK, agree w/ luke-jr to please add /* Falls through. */. This will be useful when we enable -Wimplicit-fallthrough warnings, but also for general readability.

Member

laanwj commented Nov 28, 2017

utACK, agree w/ luke-jr to please add /* Falls through. */. This will be useful when we enable -Wimplicit-fallthrough warnings, but also for general readability.

@jb55

This comment has been minimized.

Show comment
Hide comment
@jb55

jb55 Dec 29, 2017

Contributor

Lightly Tested ACK a010231

dec29-103140

Contributor

jb55 commented Dec 29, 2017

Lightly Tested ACK a010231

dec29-103140

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Mar 11, 2018

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Mar 19, 2018

{
for (const CTxOut& txout : wtx.tx->vout)
{
if (wallet->IsChange(txout)) continue;

This comment has been minimized.

@jonasschnelli

jonasschnelli Apr 10, 2018

Member

Not sure if its worth do add a call to IsChange for every transaction decomposing with the new interface structure...

@jonasschnelli

jonasschnelli Apr 10, 2018

Member

Not sure if its worth do add a call to IsChange for every transaction decomposing with the new interface structure...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment