search

BitBoxSwiss / bitbox-wallet-app

The BitBoxApp for desktop and mobile.
https://bitbox.swiss/app
Apache License 2.0
251 stars 82 forks source link

frontend: label change #2788

Open NicolaLS opened 3 months ago

NicolaLS commented 3 months ago

Label utxos that are to change addresses from our account (subaccounts respectively) as change to give the user more information about the utxo when selecting it in coin control. requestly-isChange-for-all-tx-small png requestly-isChange-for-all-tx

NicolaLS commented 3 months ago

LGTM! Only one doubt about the first commit comment:

Possible false negative in case the user used another wallet and derived some change address behind the current change gap limit.

I think that in case the user received on a change address uncovered by our gap limit he will miss the whole utxo, not only the change badge. wdyt?

Right, I don't know why I thought he would see the utxo. I'll amend the commit message, thanks.

Beerosagos commented 2 months ago

@NicolaLS is this ready for BE review or does it need other changes?

NicolaLS commented 2 months ago

@NicolaLS is this ready for BE review or does it need other changes?

@Beerosagos It's ready, I added the IsChange method to the account and added a test (also fixed test package name). I still think something like this would be better rather than adding a method to account, but this could be a future PR/refactor too.

NicolaLS commented 1 week ago

@Beerosagos adressed all change requests and @benma is okay with changing the tests package if I understood correctly: https://github.com/BitBoxSwiss/bitbox-wallet-app/pull/2788#discussion_r1770985912

So should be ready for final review/merge.

thisconnect commented 1 week ago

please add a changelog entry 🙏