namecoin / electrum-nmc

Namecoin port of Electrum Bitcoin client.
https://www.namecoin.org/
MIT License
29 stars 24 forks source link

Requesting names UI & Functionality #371

Closed robertmin1 closed 1 month ago

robertmin1 commented 2 months ago

Continuation of #363

robertmin1 commented 2 months ago

One observation I can make is that we must transform the invoice into a paid status after sending it

robertmin1 commented 1 month ago

The cli currently lacks a "pay invoice" command. My plan is to focus on implementing for GUI exclusively, unless circumstances change.

robertmin1 commented 1 month ago

The other thing we should probably do is, make the "Check name availability..." button check whether a name_new is already in the wallet for that name (by doing the same check that the name_firstupdate command does), and if so, the UI should change to a new UI that only does name_firstupdate (as opposed to name_autoregister) -Jeremy

JeremyRand commented 1 month ago

The cli currently lacks a "pay invoice" command. My plan is to focus on implementing for GUI exclusively, unless circumstances change.

I think the name_new RPC command will work fine for this, if you pass the destination, commitment, and amount params (can you confirm this?). It won't be able to accept these params in URI format, but that's an upstream limitation and isn't our problem.

robertmin1 commented 1 month ago

The other thing we should probably do is, make the "Check name availability..." button check whether a name_new is already in the wallet for that name (by doing the same check that the name_firstupdate command does), and if so, the UI should change to a new UI that only does name_firstupdate (as opposed to name_autoregister) -Jeremy

Is this the check I should be looking at

For the UI, I plan to open show_configure_name with a custom is_new parameter, such as firstupdate. Then using this parameter to modify the UI and its functionality here.

@JeremyRand

robertmin1 commented 1 month ago

The cli currently lacks a "pay invoice" command. My plan is to focus on implementing for GUI exclusively, unless circumstances change.

I think the name_new RPC command will work fine for this, if you pass the destination, commitment, and amount params (can you confirm this?). It won't be able to accept these params in URI format, but that's an upstream limitation and isn't our problem.

Yes, that should work, do we plug it into payto or create an independent command?

JeremyRand commented 1 month ago

The cli currently lacks a "pay invoice" command. My plan is to focus on implementing for GUI exclusively, unless circumstances change.

I think the name_new RPC command will work fine for this, if you pass the destination, commitment, and amount params (can you confirm this?). It won't be able to accept these params in URI format, but that's an upstream limitation and isn't our problem.

Yes, that should work, do we plug it into payto or create an independent command?

Neither, AFAICT this should already work out of the box if you call name_new on the console with those params present. So there's nothing left to do CLI-wise unless I've missed something.

JeremyRand commented 1 month ago

The other thing we should probably do is, make the "Check name availability..." button check whether a name_new is already in the wallet for that name (by doing the same check that the name_firstupdate command does), and if so, the UI should change to a new UI that only does name_firstupdate (as opposed to name_autoregister) -Jeremy

Is this the check I should be looking at

Yes. Feel free to factor that out into a separate function.

For the UI, I plan to open show_configure_name with a custom is_new parameter, such as firstupdate. Then using this parameter to modify the UI and its functionality here.

Yes that sounds fine; we already have an autoregister and update code path there, adding a 3rd firstupdate codepath should be straightforward.

robertmin1 commented 1 month ago

Alright!

robertmin1 commented 1 month ago

The other thing we should probably do is, make the "Check name availability..." button check whether a name_new is already in the wallet for that name (by doing the same check that the name_firstupdate command does), and if so, the UI should change to a new UI that only does name_firstupdate (as opposed to name_autoregister) -Jeremy

Is this the check I should be looking at

Yes. Feel free to factor that out into a separate function.

For the UI, I plan to open show_configure_name with a custom is_new parameter, such as firstupdate. Then using this parameter to modify the UI and its functionality here.

Yes that sounds fine; we already have an autoregister and update code path there, adding a 3rd firstupdate codepath should be straightforward.

If we make a separate the function, the password needs to be passed here This would mean prompting the user to enter the password for every check name availability?

JeremyRand commented 1 month ago

If we make a separate the function, the password needs to be passed here This would mean prompting the user to enter the password for every check name availability?

Good point, I hadn't thought of that. OK, in that case, how about you instead edit the name_autoregister function, so that it attempts to call name_firstupdate by itself, and only does the name_new if name_firstupdate returns an error due to a missing input? This also improves UX for the CLI/RPC interface.

robertmin1 commented 1 month ago

This works well, definitely easier to implement

JeremyRand commented 1 month ago

Code review passes for f784cae4b4f8f3c4f4aee2c593603b5f9bb5da61 other than the things I noted above. Will test shortly.

JeremyRand commented 1 month ago

Testing successful for f784cae4b4f8f3c4f4aee2c593603b5f9bb5da61 other than the things I noted above.

JeremyRand commented 1 month ago

ACK a24ce487d331873cba24bfbee77de1ad4aafd8f9