gui

gui copied to clipboard

This little change got me back to earth when i realized that the balance hint Your current spendable balance was just a label and not the result of of some random pubkey addresses i imported with importaddress warping into keys for those addresses,

At least for some short moment i felt mainnet btc rich, but this change in hint wording should be done.

edit@saibato Todo: before merge run translation Makefile after we agree in wording.

edit@saibato The prefix word "Your " implies that the user can spend this confirmed and not pending balance even though he has or had never the keys, At multiple spaces in the ui we hint that some utxos are only watched. So this PR changes this hint also at prominent space too, to:

Your current wallet balance`

We could sure even add more here and make a longer text that hints the user to the correct interpretation of this balance shown, discussion welcome.

My understanding is that the language "spendable balance" is deliberate as it does not include unconfirmed transactions which are considered not yet "spendable". If my understanding is correct it is a Concept NACK from me.

My understanding is that the language "spendable balance" is deliberate as it does not include unconfirmed transactions

@michaelfolkson hmm,,. this is not about unconfirmed transactions, they are confirmed and the balance is in sum correct. The wallet has no private keys to this balance, do you think they are "spendable" by this wallet and me?

Spendable as in whoever has the keys can spend them. Not yet spendable would be funds in unconfirmed transactions. The language isn't referring to who has the keys, it is referring to whether the transactions are confirmed or not. In the example image you posted all your funds are spendable.

Your ... is a synonym for whoever hmm...

Prob by this Gavin was tricked?

Unrelated, but the wallet generally considers unconfirmed change as spendable as well

Unrelated, but the wallet generally considers unconfirmed change as spendable as well

Yup, when you begin to see what we do with user eye;s, there is some work ahead.

And this is probably anyway an edge case and this wrong hint is somehow a minor bug, but since we allow watch only wallets, i still would like to have the ui state in any from that those balances are not spendable at all by the wallet that is currently visible to the user.

In general we clearly show that watch-only utxos are unspendable and have split row to diff , but not in this case. An unaware user might think since in this case the ui show's up almost like the normal full spendable wallet. And i must admit when testing back and forth with wallets, for a short moment, even i thought wow they are spendbable, only after inspecting deeper i saw that this was only watched utxos,

@MarcoFalke Btw. since rona has gave me lots of free timeslots, if you know others thing to be addressed, that you have in your backlog and think, would be nice to have, pls ping.

There'd be #1 for example

There'd be #1 for example

thx, for hint, I only have looked in the codebase for two weeks or so i am not so deep in bitcoin and c++ voodoo, aside from net, tor or ip things or a bit gui. But I have a quite remarkable learning by doing pace, if i must or have fun. But i also want not do push others, that can not keep up with such a pace hide away or think, why can she this in minutes? I am quite different from a healthy human. So ping @sehyunc have you begun fix #1?

fixup-Update; Since we now have different wallet types and one ui can show different wallets, it;s still my view that we should indicate this to the user.

I considered @michaelfolkson argument and he is right that those confirmed balances are spendable, if you have the keys, what we know from the wallet, if the keys are there, but even for a watch only wallet, the user might have the keys,

So this fixup changes in addition also the balancetext label to indicated that the wallet displayed has no keys and that in general we could only say that those shown wallet balances are current confirmed.

This is the current way we display watch only wallets, they look the same as a normal spendable full key wallet, although the wallet has no keys and we might not had them also never.

After change;

We already have the watch only icon on the status bar, maybe it should be more visible?

@promag thx, whatever helps, my point is that up till now this design is not optimal and serve;s not perfect our high standards .

An unaware user could even have there keys stripped from there wallet by i.e. some rpc remote or local bytecode insert exploit or simple fileops filesytem wallet.dat exchange. And be visual somewhat unaware of that his keys are gone, until he tries to spend in the other window,

And even long term experts could in my view, be somewhat astound, if they switch to newer node software. Where keyless and multiple wallets are possible.

I can't imagine a case where it would make sense for malware to turn a read/write wallet into watch-only. Typically, they would just spend the coins to a key they control exclusively. Even if they wanted to pull a trick like this, it would be comparable to just corrupt the private keys...

Also, aren't watch-only funds spendable with stuff like hardware wallets?

@luke-jr I know, its not easy to follow or see the links of sense i would like to have changed in core and some of my PR are only hints and I never cared honestly, if any is merged, i done my footprint early on, but be assured, if you at some point will see the 3 stage picture that i see water clear, you will think omg, the heist of the century. imho

Also, aren't watch-only funds spendable with stuff like hardware wallets?

Sure but i would say, that's technically an other wallet?

And btw, I will not push script hint pr's , but honestly why even doubt, that what i outlined here makes obviously some sense, if you compare the overall watch-only wallet layout and the standard HD layout?

And have you hovered about the little disabled hd icon on the bottom, if you tell me great choice of words, turing-test?

@achow101 @meshcollider Mind looking into this PR?

I'm ~0 on this, as promag said, there is already an indicator. And spendable isn't the same as confirmed so I'm not convinced on that change either,

Happy if other people want this though.

Spendable takes into account confirmed/unconfirmed, whether wallet contains all the key(s) needed to spend, whether sufficient time has passed for coinbase output to be spent etc. So as @meshcollider says spendable meets more conditions than just confirmed. As a result it is a Concept NACK on this change. I do wonder though whether it should be made clearer to the GUI user how "spendable" is defined either in the UI or in documentation.

Since the bug pun line confirmed did not resonated to action by review, here a modified version that address solely the poor hint and indication that a wallet is pure watch-only.

Also we can see here that blanked out HD symbol in status line is hardly to distinguish from a blanked eye.

With the description header and the hint, that distinction should be sufficient.

(change: add a headerline Watch-only in the watch only case and applied the same hint from watch-only address from the mixed cases )

Here for reference how the other cases will still look like :

(unchnaged/ by this PR same display as in old version ) Picture of a standard hd wallet with no imported address

(unchanged/ by this PR same display as in old version) Picture of a non hd wallet with imported address

We already have the watch only icon on the status bar, maybe it should be more visible?

Yes, I didn't notice it before this comment actually. Also, with wallets having both spendable and watch-only addresses you have "Watch-only" / "Spendable" above amounts, so one might expect the same behaviour when having loaded wallet with only watch-only addresses.

@kristapsk

Yes, I didn't notice it before this comment actually. Also, with wallets having both spendable and watch-only addresses you have "Watch-only" / "Spendable" above amounts, so one might expect the same behaviour when having loaded wallet with only watch-only addresses.

exactly, if I interpret correctly the case u describe? That's the reason the change is in its last version only for the pure watch only wallets Since as @michaelfolkson pointed out, to change the hint in general is a bit overdone.

Mixed stay untouched since they already display in two columns with a hinting header and this seams to be fine AFAICS.

@Saibato are you going to revisit this?

thx, for testing this.

I merged the suggestion from @hebasto to use the Qt r() function here for auto translate, since we have tr in the object and so we have shorter code. ;-) The effect is the same, and you might want to revisit and confirm that this has still the desired effect

While revisiting this PR I found a minor bug when displaying icons if the wallets are reloaded, https://github.com/bitcoin-core/gui/pull/407 you might want to test that too?

With descriptor wallets, users aren't going to have mixed wallets anymore. So I don't know if this is really necessary. The balance should either be all watch only, or all single key, or all multi-signature, etc.

There probably could be some GUI improvements, but I don't think this is complete/or going in the right direction.

With descriptor wallets, users aren't going to have mixed wallets anymore. So I don't know if this is really necessary. The balance should either be all watch only, or all single key, or all multi-signature, etc.

There probably could be some GUI improvements, but I don't think this is complete/or going in the right direction.

this is for sure an edge case of bare keyless and not mixed wallets. afaics mixed wallets are handled already correct. I would guess, core will for ever have wallets without keys, and some bold obvious distinction in UI to the user that those spendable coins are in a watch-only wallets ( maybe ancient ) without keys, is in my view good UX. Afaics that is what this PR intends and archives.

How would it label a multisignature wallet where less than m privkeys are on the device? It's neither "watch only" or "spendable"?

How would it label a multisignature wallet where less than m privkeys are on the device? It's neither "watch only" or "spendable"?

one could argue not your keys, not your coins, so if you need m and have only m-1 that bare wallet at the moment you watch the balance is still "watch-only" like the other ones of whom we do not call his name?

After further thought, I like the current change (the "Watch-Only" label). I just don't know what the solution is for multisignature wallets.

There's lots of different ways for wallets to be setup and I think that complicates this. Don't know if @Bosch-0 has thought about this. For example:

You could have a cold storage situation with a full node being in watch only mode. Then n wallets each with a privkey. The GUI for the full node vs for the wallets should probably represent the balance in different ways? (The wallets have one "piece of the puzzle" to spend, the node has none).

I think Specter and Nunchuk both go about solving this issue quite eloquently. I think these type of products are what the GUI should strive to be, hopefully a lot easier with the work being done with QML GUI.

You can actually go through this UX flow using n local core wallets in Specter.

@Saibato

Please squash your commits, and rebase the branch to avoid CI failures.