slimcoin-project / pacli

Simple CLI PeerAssets client (extended version).
GNU General Public License v3.0
0 stars 0 forks source link

address set with non existent address #116

Closed buhtignew closed 5 months ago

buhtignew commented 5 months ago

While I was making tests for the issue #112 I've discovered that it was possible to create a label for a non existent address. Specifically I've run address set blabla mqnBdCH2cuoSiQWjKoEZHfov77bzrpmtN2 and everything went smoothly except at the next run of address list I've got an error message:

        General error raised by PeerAssets. Check if your input is correct.

        If you gave a deck as an argument, a possible reason for this error is that you need to initialize the deck.

        To initialize the default decks, use:

        pacli deck init

        To initialize a single deck, use:

        pacli deck init DECKID

The address list -d output was:

  File "~/.local/bin/pacli", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "~/.local/lib/python3.12/site-packages/pacli/__main__.py", line 479, in main
    fire.Fire({
  File "~/.local/lib/python3.12/site-packages/fire/core.py", line 143, in Fire
    component_trace = _Fire(component, args, parsed_flag_args, context, name)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.12/site-packages/fire/core.py", line 477, in _Fire
    component, remaining_args = _CallAndUpdateTrace(
                                ^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.12/site-packages/fire/core.py", line 693, in _CallAndUpdateTrace
    component = fn(*varargs, **kwargs)
                ^^^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.12/site-packages/pacli/extended_classes.py", line 567, in list
    return ei.run_command(self.__list, advanced=advanced, keyring=keyring, coinbalances=coinbalances, labels=labels, full_labels=full_labels, no_labels=without_labels, only_labels=only_labels, named=named, quiet=quiet, p2th=p2th, network=blockchain, include_all=include_all, debug=debug)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.12/site-packages/pacli/extended_interface.py", line 33, in run_command
    result = c(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.12/site-packages/pacli/extended_classes.py", line 641, in __list
    return tc.all_balances(wallet=True,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.12/site-packages/pacli/token_commands.py", line 53, in all_balances
    balance = float(str(provider.getbalance(addr)))
                        ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/.local/lib/python3.12/site-packages/pypeerassets/provider/rpcnode.py", line 97, in getbalance
    values = [ Decimal(v["amount"]) for v in unspent ]
                       ~^^^^^^^^^^
TypeError: string indices must be integers, not 'str'

I was able to solve with address set blabla -d -n command.

I'd suggest us to perform a check against the existing addresses before the address set LABEL ADDRESS command is performed, although it will increase considerable the time needed to run that command.

d5000 commented 5 months ago

I have already added a check to that as part of the fix for the deck set command, so only addresses which could be valid (due to the string they use) are accepted by address set.

Checking the "existence" of addresses is not possible realistically. All blocks would have been checked :) What could be checked is of course if the address is part of the wallet, but that would exclude non-wallet addresses from being labeled, and for example it may make sense to label a gateway address of an AT token, or an exchange partner in the DEX.

I think I'll check if I can fix/catch the error in the address list command instead in this case.

Edit: I've found out that my validity test was too weak, the "...N2" address was actually invalid. Fixed in commit 117b9ed. Anyway address list will not anymore throw an error if an invalid address was stored, e.g. by "misusing" config set.

Can be closed if everything works.

(just a short comment: I'll comment on the issues related to the block locators once I fixed the issue mentioned in issue #86 ).

buhtignew commented 5 months ago

This works I'm closing.