address balance additional tests

[buhtignew]

I've tested address balance once again and got the following points to share with you:

1) In case I use the address that is not in my wallet I'm getting {'balance': 0.0} message. Is possible for pacli to understand that the address doesn't make part of the wallet and to output the relative message?

2) In case I use address address balance -a LABEL I'm getting the following message:

  File "~/.local/bin/pacli", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "~/.local/lib/python3.11/site-packages/pacli/__main__.py", line 441, in main
    fire.Fire({
  File "~/.local/lib/python3.11/site-packages/fire/core.py", line 141, in Fire
    component_trace = _Fire(component, args, parsed_flag_args, context, name)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.11/site-packages/fire/core.py", line 475, in _Fire
    component, remaining_args = _CallAndUpdateTrace(
                                ^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.11/site-packages/fire/core.py", line 691, in _CallAndUpdateTrace
    component = fn(*varargs, **kwargs)
                ^^^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.11/site-packages/pacli/extended_classes.py", line 612, in balance
    {'balance': float(provider.getbalance(address))}
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.11/site-packages/pypeerassets/provider/rpcnode.py", line 97, in getbalance
    values = [ Decimal(v["amount"]) for v in unspent ]
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.11/site-packages/pypeerassets/provider/rpcnode.py", line 97, in <listcomp>
    values = [ Decimal(v["amount"]) for v in unspent ]
                       ~^^^^^^^^^^
TypeError: string indices must be integers, not 'str'

Maybe it would make sense to output something like "Sorry, no LABEL address in the wallet".

3) Seems like if I use the address balance SOMETEXTHERE I'm getting the balance of the whole wallet. This usage is not described in the help.

4) It looks like the output of address balance and address balance -k for any address is the same. However if I run address balance using the keyring label I'm getting the whole wallet balance and the same if I use address balance -k with a not keyring label. Would it make sense to suppress the -k flag and to output for the labels the message whether it's a keyring label or not, together with the balance?

[d5000]

This was an old vanilla command I modified only in a rudimentary form (adding support for labels and addresses which aren't the main address).

The command was now modified in the following form:

If you input any string behind -a which is not an address (your point 3), it will throw an error. If the balance is 0 it checks if the address is the wallet (your point 1) and if not throws an error. Error described in 2 is catched and also will output a proper error message.

The functionality you discovered in 3 is one I wasn't aware of, but yes, according to how the command works it should show the sum of all unspent outputs - i.e. the "balance" - of the whole wallet. I "legalized" this feature adding a -w flag, but if you don't add this it won't work anymore (see above) to avoid confusion.

Your point 4 is probably WONTFIX. You probably have all your keyring labels also as labels in the external config file, thus it outputs the same balance with -k and not. Keyring labels are completely independent from external config labels, so there can be duplicates.

Try to use a label you haven't in one of both lists (i.e. isn't at the same time a keyring label and a extended config file label), you can find the differences with address list and address list -k. If the output is still the same, then let me know.

I'd also mention that everything which is keyring related is low priority for me now. Since the external config file was added this is not a functionality which is needed if PeerAssets is used with a local SLM/PPC node.

Fixes were applied in commit 7ec2029.

[buhtignew]

I've tested the address balance command with an address that is not mine (using the address balance -a mpmZCVusQ6WoWHcFTAizmEcqSXdu5YX5PV command) and the output was as expected. However when I've run address balance -a mgUkRF2eosnFQpMKqAYxuzVLxccTPyNxYw (using the address that is in my wallet) I've got the same message: Error: This address is not in your wallet. Command works only for wallet addresses.. I think this can create some confusion or even stress for the user, outputting zero balance would be more appropriate in this case.

I've tested then address balance -a donor_address1 and got Error: Not a valid address or label.. Maybe it would be better to throw Error: Not a valid address to avoid confusion for the user.

Then I've tested address balance safafafsas and got the balance of the main address. Maybe we should made it explicit that there is no such a label, since the code is probably makes this check in any case. Something like No such label stored, the balance of the main address ADDRESS is: Something similar is already present if I use the -k flag with whatever address, (for instance address balance mqKRFEhXsY5n2QNeKzq9QTDC2huG3k1fNR -k) where I receive the following error Error: Label mqKRFEhXsY5n2QNeKzq9QTDC2huG3k1fNR was not stored in the keyring., but there is no clarification what is the balance outputted afterwards, so the user may think that still it's the balance of the address he has was looking for, mqKRFEhXsY5n2QNeKzq9QTDC2huG3k1fNR in this case, while it's the balance of the main address.

The address balance -w works great, I think it's the right solution.

Try to use a label you haven't in one of both lists (i.e. isn't at the same time a keyring label and a extended config file label), you can find the differences with address list and address list -k. If the output is still the same, then let me know.

I've made this test and the issue is not were I was looking for it. The fact, already mentioned above, that the main address balance is being outputted in any case without it being explained has created confusion to me. In case I use address balance with the keyring label and omit the -k flag (the address balance origin_address command in my case) there is no warning message at all, I'm just getting the balance of the main address which can make me wrongly assume it's my origin_address balance. In case I use the extended configuration label with the -k flag the situation is slightly better, because I'm receiving an error Error: Label donor_address1 was not stored in the keyring., but the balance I'm still getting in this case may also create confusion.

[d5000]

outputting zero balance would be more appropriate in this case.

Confirmed. This was a bug, because if there's zero balance the system checks if the address is in the wallet or not. But it didn't take into account empty addresses as part of the wallet :)

the main address balance is being outputted in any case without it being explained

The reason for the confusion was actually that I used an older function to process the label string which did less checks; I have now changed it to the one I use habitually. This should trigger all necessary error messages.

I have reworked the command now: it will now accept addresses and labels without the -a flag. So its usage is now: pacli address balance for the main address, pacli address balance ADDRESS, pacli address balance LABEL or pacli address balance -w. The error messages and docstrings have been changed partly, according to your suggestions.

Commit: ebd8b4e

[buhtignew]

I have reworked the command now: it will now accept addresses and labels without the -a flag.

Great improvement, I was in doubt whether it was relevant to ask you for.

I have now changed it to the one I use habitually. This should trigger all necessary error messages.

Now it seems like everything works as expected except that in case I use an extended file label with the -k flag (address balance old_default -k for instance in my case) I'm getting no balance and the following error message: Error: No valid address string or non-existing label., but if I use the keyring label without the -k flag (address balance origin_address in my case) the output is the corresponding address balance. For consistency we should or output just the error message in both cases or the balance without the error message. If we opt for the error message I think it should be changed for the extended file label used with -k into how it was previously Error: Label origin_address was not stored in the keyring., while for the keyring label used without the -k the message Error: No valid address string or non-existing label. should be appropriate. If we opt for the balance output the -k flag becomes obsolete.

[d5000]

I thought this had been solved when fixing another issue, but there seems still to be a strange behaviour which is probably what you meant in your last post:

If you have a label stored in the keyring only, and call address balance without -k, then it will be shown "as if" it was a label stored in the extended config file.

I have changed that behaviour now, so in the case described an error is shown (where it's precised if the label was searched in the keyring or in the extended config file).

Commit: c54c1df

PS: Was pushed today to the repo.

[buhtignew]

This works.

I think we can close this issue.