Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Import electrum multisig wallet (with available seed) #1543

Merged

Conversation

relativisticelectron
Copy link
Collaborator

@relativisticelectron relativisticelectron commented Jan 29, 2022

  1. A space was in recv_descriptor
  2. hw_type key does not exist in the electrum wallet dict if the seed exists in the electrum wallet

How to reproduce the bug: Create a 2/2 testnet electrum multisig wallet (with electrum seeds). The file will look like:

{
    "addr_history": {

    },
    "addresses": {
        "change": [
        ],
        "receiving": [
        ]
    },
    "fiat_value": {},
    "frozen_coins": {},
    "invoices": {},
    "labels": {},
    "payment_requests": {},
    "prevouts_by_scripthash": {....},
    "seed_version": 41,
    "spent_outpoints": {......
    },
    "transactions": {.....    },
    "tx_fees": {.....    },
    "txi": {},
    "txo": {.....    },
    "use_encryption": false,
    "verified_tx3": {},
    "wallet_type": "2of2",
    "x1/": {
        "derivation": "m/1'",
        "pw_hash_version": 1,
        "root_fingerprint": "....",
        "seed": ".....",
        "seed_type": "segwit",
        "type": "bip32",
        "xprv": "...",
        "xpub": "..."
    },
    "x2/": {
        "derivation": "m/1'",
        "pw_hash_version": 1,
        "root_fingerprint": "...",
        "type": "bip32",
        "xprv": null,
        "xpub": "..."
    }
}

- 1. bug was a space in recv_descriptor
- 2. bug was that hw_type key does not exist in the electrum wallet dict if the seed in th electrum wallet
@netlify
Copy link

netlify bot commented Jan 29, 2022

✔️ Deploy Preview for specter-desktop-docs ready!

🔨 Explore the source changes: c3663a9

🔍 Inspect the deploy log: https://app.netlify.com/sites/specter-desktop-docs/deploys/61f94c9300e04a0007df1fc8

😎 Browse the preview: https://deploy-preview-1543--specter-desktop-docs.netlify.app

@relativisticelectron relativisticelectron marked this pull request as ready for review January 29, 2022 20:15
@relativisticelectron relativisticelectron marked this pull request as draft January 29, 2022 20:21
Copy link
Collaborator Author

@relativisticelectron relativisticelectron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong comment....

Copy link
Collaborator Author

@relativisticelectron relativisticelectron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I show this warning to the user?

src/cryptoadvance/specter/util/wallet_importer.py Outdated Show resolved Hide resolved
@k9ert k9ert requested a review from moneymanolis January 31, 2022 14:42
@k9ert
Copy link
Contributor

k9ert commented Jan 31, 2022

Can you check the code-style? That's red right now. See:
https://docs.specter.solutions/desktop/development/#code-style

Removed logger.warning

Added missing import
@relativisticelectron
Copy link
Collaborator Author

Can you check the code-style? That's red right now. See: https://docs.specter.solutions/desktop/development/#code-style

Hi @k9ert : The black coding style problem comes from the file: src/cryptoadvance/specter/managers/config_manager.py However I never touched this file.

@relativisticelectron relativisticelectron marked this pull request as ready for review February 1, 2022 08:05
@k9ert k9ert merged commit eaeac1f into cryptoadvance:master Feb 1, 2022
@relativisticelectron relativisticelectron deleted the import_electrum_multisig branch February 1, 2022 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants