search

namecoin / electrum-nmc

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

name_new detection of NameAlreadyExistsError is broken #234

Closed JeremyRand closed 4 years ago

JeremyRand commented 4 years ago

@gits7r reported on Matrix/IRC that registering a name (that doesn't already exist) in the Qt GUI in 3.3.9.1 fails at the name_new step:

Traceback (most recent call last):
File "electrum_nmc\electrum\gui\qt\configure_name_dialog.py", line 155, in register_and_broadcast
File "electrum_nmc\electrum\gui\qt\main_window.py", line 2107, in
File "electrum_nmc\electrum\commands.py", line 149, in _run
File "electrum_nmc\electrum\commands.py", line 123, in func_wrapper
File "electrum_nmc\electrum\commands.py", line 733, in name_autoregister
File "electrum_nmc\electrum\commands.py", line 123, in func_wrapper
File "electrum_nmc\electrum\commands.py", line 604, in name_new
electrum.commands.NameAlreadyExistsError: The name is already registered
JeremyRand commented 4 years ago

This is likely to be breakage caused by the switch from name_show raising an exception when a name doesn't exist to making it return a Fault.

JeremyRand commented 4 years ago

@gits7r This should be fixed in the new master-3.3.10 branch, can you test that?

gits7r commented 4 years ago

master-3.3.10 branch fixes it yes, there is no pop-up. However, it still does not register a new name. The current error is that min. relay fee was not provided, and the server returned an error. But it says like this with all the servers, somehow it does not send the correct fee . I am also using simple DNS editor to add 1 IPv4 address, so maybe it does not include that somehow and the min relay fee is not met?

gits7r commented 4 years ago

Indeed: I had to switch to static fees from Preferences in order for it to work, and it sees the entire 0.01 spent as fee so it warns about high fee.

We should fix the bug that electrum-nmc considers high fee when we try to register domains, because it does not know it's related to registering a name so it sees it as unusual for a 'transaction fee'.

JeremyRand commented 4 years ago

@gits7r Interesting. I suspect I know why this is, and I also suspect that it's already fixed in master by a refactor, but I don't understand why this only started happening now (the bug should have affected previous releases if it affects this one).

In Electrum-NMC prior to 4.0.0, the 0.01 NMC is "hidden" by the serialization/deserialization code, so name_new transactions appear to be paying an extra 0.01 NMC as a "fee" even though the underlying blockchain logic simply has the 0.01 NMC being locked inside the name output. This was removed while merging the PSBT refactor of the transaction code from upstream, because that approach no longer is feasible with the PSBT refactor.

Since this is already fixed in master but can't be backported to 3.3.10, I'll look for a minimal-change hack that can work around this for 3.3.10.

JeremyRand commented 4 years ago

However, it still does not register a new name. The current error is that min. relay fee was not provided, and the server returned an error. But it says like this with all the servers, somehow it does not send the correct fee .

@gits7r does this happen when registering a name from the console (via the name_autoregister command), or is it only reproducible when using the Qt GUI to register a name?

gits7r commented 4 years ago

Only reproducible when using the Qt GUI. It will only allow you to register new names if you have the fee setting to Static (not Mempool or ETA) and it will report on the transaction details warning - HIGH FEE. Somehow it does not know that it's not a fee for a simple transaction, it's a fee for registration (the transaction contains a commitment not just inputs and outputs).

JeremyRand commented 4 years ago

@gits7r Can you paste the exact error message? This happens when broadcasting the name_new step, or the name_firstupdate step?

gits7r commented 4 years ago

It happens with the name_new step (when 0.01 is spent as fee). It only appears when fees are set to Mempool or ETA. The error is excessive fee pop-up (regular message, not bug pop up box).

JeremyRand commented 4 years ago

@gits7r I can't reproduce this with master-3.3.10 branch, fees set to ETA. Are there any other settings necessary to reproduce this? E.g. the Fee slider on the Send tab, or settings on the Transactions tab?

gits7r commented 4 years ago

No, I did not touch the Fee slider on the Send tab. Settings on the transactions tab are:

When you try to register a new name (name_new command broadcast) it will return: "The server returned and error while broadcasting the transaction. Consider trying to connect to a different server, or updating Electrum-NMC.

min relay fee not met"

gits7r commented 4 years ago

It's true that with ETA it does not return this error. only with Mempool.

In all cases we will see Warning - high fee! on the transaction details tab. We might want to remove the high fee warning for transactions that contain a salt commitment (pre-registration, name_new process).

JeremyRand commented 4 years ago

@gits7r I've created https://github.com/namecoin/electrum-nmc/issues/241 for the issue with Mempool fee estimation; it's not related to name transactions (and also affects master branch).

JeremyRand commented 4 years ago

Closing this, as all of the relevant bugs are fixed in master AFAICT, and the only bug remaining in master-3.3.10 is the fee warning for name_new transactions in the Transaction dialog, which isn't particularly severe, is annoying to fix in that branch, and is fixed in master. @gits7r If you think any of these bugs needs additional patches please open a new issue.