search

namecoin / namecoin-core

Namecoin full node + wallet based on the current Bitcoin Core codebase.
https://www.namecoin.org/
MIT License
463 stars 146 forks source link

Add "address_type" arg to DestinationAddressHelper #427

Open JeremyRand opened 3 years ago

JeremyRand commented 3 years ago

Is your feature request related to a problem? Please describe.

There is no way to specify the address type of an address generated by name_update, other than changing the default for the entire wallet via the -addresstype command-line flag.

Describe the solution you'd like

We should add an address_type option to DestinationAddressHelper (which currently handles the destAddress option). If set, this sets the address type of the generated address, similarly to the address_type argument that currently exists for getnewaddress.

Describe alternatives you've considered

We could name the option addressType, but that spelling is inconsistent with upstream's existing getnewaddress method.

We could require users to manually call getnewaddress with the desired address_type and then pass that to name_update, but that is not good UX.

Additional context

Once this is implemented, I intend to make the name_update Qt GUI offer a tickbox for this, similar to the Generate native segwit (Bech32) address tickbox that exists on the Receive tab.

domob1812 commented 3 years ago

Sounds useful, especially as backend for the Qt UI checkbox. (Apart from that, i.e. for pure RPC/CLI users, I'd say they should just change the address type globally to segwit.)

As an alternative, what do you think about just flipping the default address type to segwit and not bothering with adding a choice in the UI?

JeremyRand commented 3 years ago

As an alternative, what do you think about just flipping the default address type to segwit and not bothering with adding a choice in the UI?

As long as upstream still offers a choice for currency transactions (AFAIK they still do), I think we should do the same for name transactions. Also I'm hesitant to make Bech32 the default until we've finished phasing out the old 0.13.99 client that can't send to Bech32.

JeremyRand commented 3 years ago

Hmm, that said, AFAIK the main reason upstream still supports giving the user a choice is because Alice may want to receive coins from Bob, but Alice supports Bech32 while Bob doesn't. This is not really a valid scenario with the auto-generated addresses in name_update since Alice and Bob are the same wallet. Maybe it would make sense to have the DestinationAddressHelper default to Bech32 even if the -addresstype setting in namecoin.conf isn't Bech32, and not bother with giving the user a choice since there's no good reason for them to change it here?

Gapcoin1 commented 3 years ago

Would it be more Fluid to make the Tick box backwards. As in just flipping the code to make the Tick Box represent Old addresses while Bech32 be the New Default?

JeremyRand commented 3 years ago

Would it be more Fluid to make the Tick box backwards. As in just flipping the code to make the Tick Box represent Old addresses while Bech32 be the New Default?

@Gapcoin1 For receiving coins/names from other people, I don't think we're ready to make Bech32 the default. For sending names to oneself, I don't see any reason to allow anything other than Bech32. Am I missing a use case?

yanmaani commented 3 years ago

Maybe not a very good one, but I sometimes run non-release builds on mainnet to use some new feature, while using a stable build for most usages. I imagine some other people might be doing that, but with 13.99. It seems undesirable if they should send their coins to a Bech32 address and then lose access to them in the old version.

My $0.02: if they want to upgrade to Bech32, let them, if not, don't force them.