spesmilo / spesmilo/electrum

payto command with unsigned flag not working, and similar

Open
#3,394 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug 🐞 CLI/RPC ▶
Dominant language
Python
Stars
8.6k
Forks
3.5k
Avg merge
2d 8h
Merged PRs (30d)
61

Description

On git master 9425319dcd4c16a63ee3559bde433472b44ec165.

The payto CLI command with the --unsigned flag does not work with a passworded wallet.

user@debian:~/wspace/electrum$ ./electrum --testnet daemon start
command-decorator() id(known_commands) 140364120445320
starting daemon (PID 10009)

user@debian:~/wspace/electrum$ ./electrum --testnet daemon load_wallet -w /home/user/.electrum/testnet/wallets/test_segwit_asdasd
command-decorator() id(known_commands) 139636863779784
Password:
true

user@debian:~/wspace/electrum$ ./electrum --testnet -w /home/user/.electrum/testnet/wallets/test_segwit_asdasd payto -W "1234" -v -u -f 0.0001 --rbf tb1q4gv3cqhvptestl05sh7qy0galpww9wt0xrr55r '!'
command-decorator() id(known_commands) 139653340300168
[SimpleConfig] electrum directory /home/user/.electrum/testnet
[SimpleConfig] electrum directory /home/user/.electrum/testnet
init_cmdline() id(known_commands) 139653340300168
init_cmdline(). cmd.requires_password is being set to False
[WalletStorage] wallet path /home/user/.electrum/testnet/wallets/test_segwit_asdasd
init_cmdline(). password is being set to None
command-decorator-func_wrapper() id(known_commands) 140364120445320
Password required

So, even when providing a password to the payto command with the -W flag, if the --unsigned flag is present, the command fails.

The changes I've made for the extra log lines above:

diff --git a/electrum b/electrum
index 17acb309..8e71c9de 100755
--- a/electrum
+++ b/electrum
@@ -208,12 +208,14 @@ def init_cmdline(config_options, server):
     config = SimpleConfig(config_options)
     cmdname = config.get('cmd')
     cmd = known_commands[cmdname]
+    print('init_cmdline() id(known_commands)', id(known_commands))
 
     if cmdname == 'signtransaction' and config.get('privkey'):
         cmd.requires_wallet = False
         cmd.requires_password = False
 
     if cmdname in ['payto', 'paytomany'] and config.get('unsigned'):
+        print('init_cmdline(). cmd.requires_password is being set to False')
         cmd.requires_password = False
 
     if cmdname in ['payto', 'paytomany'] and config.get('broadcast'):
@@ -244,6 +246,7 @@ def init_cmdline(config_options, server):
                 print_msg("Error: Password required")
                 sys.exit(1)
     else:
+        print('init_cmdline(). password is being set to None')
         password = None
 
     config_options['password'] = password
diff --git a/lib/commands.py b/lib/commands.py
index 0d8907d7..35519e3f 100644
--- a/lib/commands.py
+++ b/lib/commands.py
@@ -75,8 +75,11 @@ def command(s):
         global known_commands
         name = func.__name__
         known_commands[name] = Command(func, s)
+        if name == 'payto':
+            print('command-decorator() id(known_commands)', id(known_commands))
         @wraps(func)
         def func_wrapper(*args, **kwargs):
+            print('command-decorator-func_wrapper() id(known_commands)', id(known_commands))
             c = known_commands[func.__name__]
             wallet = args[0].wallet
             password = kwargs.get('password')

At least these lines should be subject to scrutiny:
https://github.com/spesmilo/electrum/blob/9425319dcd4c16a63ee3559bde433472b44ec165/electrum#L210-L220
They are modifying cmd from known_commands but as the log shows above, a different instance of known_commands than what the daemon sees. As the daemon runs in a separate process. Is known_commands intended to be a singleton? Because there is at least one instance of it in every electrum process.
Also, if known_commands is supposed to be a singleton, I'm not sure it makes sense to change the properties of commands in it, such as cmd.requires_password = False, based on modifiers (such as --unsigned). Because the next time the same command is invoked the modifier might not be present. So I'm not sure what the programmer's intentions were here.

To be clear, the payto command with the --unsigned modifier might not be the only thing broken due to this.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with init_cmdline in electrum around lines 210-220 and the command decorator and wrapper in lib/commands.py. Reproduce the passworded-wallet payto --unsigned failure, then trace how command metadata and password handling differ between processes. Done means the reported command works and related modifier handling is verified without unintended state persisting between invocations.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
cli
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.