slimcoin-project / pacli

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

Address list command with the -k flag #47

Closed buhtignew closed 6 months ago

buhtignew commented 6 months ago

I've run address list -k and got the following error message before I've got the expected output:

/usr/lib/python3.11/site-packages/jeepney/low_level.py:314: ResourceWarning: unclosed <socket.socket fd=8, family=1, type=1, proto=0, raddr=/run/user/1000/bus>
  valtype = parse_signature(list(sig))
ResourceWarning: Enable tracemalloc to get the object allocation traceback

I have an impression that we've already spoken about it and that the issue is not solvable for the moment. I've mentioned it in case I'm wrong and the issue has never been discussed (since I haven't found this discussion here). _ I've discovered that the addresses that are labeled only in keyring without having coins nor tokens on them are considered void by the address list command so they are visible only with address list -i or address list -k. I'm not sure we should do anything with it though, considering that the keyring labels are basically obsolete from the point of view of our code.

In case we decide to do anything with it, we can add "keyring label" column into address list output and to consider an address as not void if there is a keyring label attached.

d5000 commented 6 months ago

For the first point, you're right, this was already discussed and for now it's WONTFIX.

For the second, I wrote already in issue #42 keyring and extended configuration file labels are totally independent. You may have both the same ones for most of your addresses due to the --import_all_keyring_addresses feature but I see nothing which has to be done.

I'm closing.

buhtignew commented 6 months ago

For the first point, you're right, this was already discussed and for now it's WONTFIX.

What if (also in order for us not to come back to the question each time I forgot we've already discussed about :-) ) we output a short warning before the address list with the -k flag is being executed saying something like: "Keyring usage has become obsolete, please not consider the following error message, the required info will be provided soon after the error message."

d5000 commented 6 months ago

I think it's better to mention that in the manual. The idea is to mention the keyring options there but not recommending them, and then the potential resource warning can also be mentioned.

About a message I'm more sceptical, before I add such a message I'd need to understand the error better, I don't want to give users the impression the program wants to "downplay" the error. I also don't think this has any priority now.

buhtignew commented 6 months ago

I understand your point.

Then another question: what if we suppress the -k option at all?

d5000 commented 6 months ago

I have thought about this too but for now I intend to preserve it if it doesn't lead to mayor complications (and I don't think this ResourceWarning is such one).

The point is that the keyring options allow you to use pacli also without any wallet at all (pacli is then your wallet), with a block explorer in charge of most queries, or, if we get this at any point, with a light wallet or an alternative implementation. Using it with Peercoin this way is possible for example, you only need one of the supported block explorers.

Cryptoid for Peercoin is among the supported options. It's possible it could be adapted to the Slimcoin cryptoid - and we would de facto have a light wallet for SLM (a centralized one though, as the block explorer in theory could misbehave, but that's true for all centralized wallets). This could be something for a post-launch update, if cryptoid is still available for SLM.

buhtignew commented 6 months ago

Interesting insights.

I'm closing this for now since it was already closed previously.