Feature: More robust wallet recreation and support tool "Wallet management" for Spectrum (#2273)

* having a very limites spectrum wallet-backend-admin tool

* remove renumbering of wallets

* enriched alias function

* add wallets_aliases property to wallet manager

* use alias when checking names at wallets endpoint

* get rid of scary "ERROR" in base.jinja

* change title for input field for creating a new wallet

* strip leading and trailing whitespaces in alias()

* pytest added

* fix pytests

* Moving Wallet Link to settings

* a bit of styling

---------

Co-authored-by: moneymanolis <moneymanolis@protonmail.com>
Co-authored-by: Manolis Mandrapilias <70536101+moneymanolis@users.noreply.github.com>
This commit is contained in:
k9ert 2023-03-02 19:59:33 +01:00 committed by GitHub
parent 10d84ddac7
commit 2f21a03c26
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 199 additions and 65 deletions

View file

@ -112,12 +112,17 @@ def to_ascii20(name: str) -> str:
return "".join([c for c in name if ord(c) < 127])[:20]
# TODO: Rename this function to sth. like create_unique_id
def alias(name):
"""
Create a filesystem-friendly alias from a string.
Replaces space with _ and keeps only alphanumeric chars.
Leading and trailing whitespaces are removed.
Replaces space(s) and hyphen(s) with one underscore.
Keeps only alphanumeric chars and returns in lowercase.
"""
name = name.replace(" ", "_")
name = name.strip().replace(" ", "_").replace("-", "_")
while "__" in name:
name = name.replace("__", "_")
return "".join(x for x in name if x.isalnum() or x == "_").lower()

View file

@ -5,7 +5,7 @@ import sys
import pathlib
import sys
from typing import Dict
from typing import Dict, List
from flask_babel import lazy_gettext as _
from flask import copy_current_request_context
from cryptoadvance.specter.rpc import BitcoinRPC
@ -298,9 +298,13 @@ class WalletManager:
return working_folder
@property
def wallets_names(self):
def wallets_names(self) -> List:
return sorted(self.wallets.keys())
@property
def wallets_aliases(self) -> List:
return [wallet.alias for wallet in self.wallets.values()]
@property
def rpc(self):
"""returns a BitcoinRpc depending on the chain"""
@ -354,14 +358,6 @@ class WalletManager:
walletsindir = []
self._check_duplicate_keys(keys)
wallet_alias = alias(name)
i = 2
# Ensure unique wallet alias
while (
os.path.isfile(os.path.join(self.working_folder, "%s.json" % wallet_alias))
or os.path.join(self.rpc_path, wallet_alias) in walletsindir
):
wallet_alias = alias("%s %d" % (name, i))
i += 1
w = self.WalletClass.create(
self.rpc,

View file

@ -11,7 +11,7 @@ from flask_babel import lazy_gettext as _
from flask_login import login_required
from ...commands.psbt_creator import PsbtCreator
from ...helpers import bcur2base64, get_devices_with_keys_by_type, get_txid
from ...helpers import bcur2base64, get_devices_with_keys_by_type, get_txid, alias
from ...key import Key
from ...managers.wallet_manager import purposes
from ...persistence import delete_file
@ -231,13 +231,6 @@ def new_wallet(wallet_type):
).format(device.name)
break
name = wallet_type.title()
wallet_name = name
i = 2
while wallet_name in app.specter.wallet_manager.wallets_names:
wallet_name = "%s %d" % (name, i)
i += 1
return render_template(
"wallet/new_wallet/new_wallet_keys.jinja",
cosigners=devices,
@ -253,8 +246,8 @@ def new_wallet(wallet_type):
address_type = request.form["type"]
sigs_total = int(request.form.get("sigs_total", 1))
sigs_required = int(request.form.get("sigs_required", 1))
if wallet_name in app.specter.wallet_manager.wallets_names:
err = _("Wallet already exists")
if alias(wallet_name) in app.specter.wallet_manager.wallets_aliases:
err = _("Wallet name already exists. Choose a different name!")
if err:
devices = [
app.specter.device_manager.get_by_alias(
@ -805,8 +798,10 @@ def settings(wallet_alias):
flash(_("Wallet name cannot be empty"), "error")
elif wallet_name == wallet.name:
pass
elif wallet_name in app.specter.wallet_manager.wallets_names:
flash(_("Wallet already exists"), "error")
elif alias(wallet_name) in app.specter.wallet_manager.wallets_aliases:
flash(
_("Wallet name already exists. Choose a different name!"), "error"
)
else:
app.specter.wallet_manager.rename_wallet(wallet, wallet_name)
flash("Wallet successfully renamed!")

View file

@ -9,9 +9,11 @@
- isRight: Is the item at the right edge (last item) of the navigation bar?
#}
{% macro menu_item(service_id, tab, title, active_menuitem, isLeft=false, isRight=false) -%}
<a
href="{{ url_for(service_id+'_endpoint.' ~ tab)}}"
class="btn radio {% if isLeft %}left{% endif %} {% if isRight %}right{% endif %} {% if active_menuitem == tab %}checked{% endif %}">
{{ title }}
</a>
<li class="mr-2 py-2 border-b-2 border-transparent {% if active_menuitem == tab %}text-link border-b-2 border-link active{% endif %}">
<a
href="{{ url_for(service_id+'_endpoint.' ~ tab)}}"
class="text-lg inline-block py-1 px-3 hover:text-white rounded-lg hover:bg-dark-700">
{{ title }}
</a>
</li>
{%- endmacro %}

View file

@ -86,7 +86,7 @@
{% endif %}
{% endwith %}
{% if error %}
<message-box type="error">{{ _("ERROR:") }} {{error | safe }}</message-box>
<message-box type="error">{{error | safe }}</message-box>
{% endif %}
</div>

View file

@ -117,6 +117,7 @@
{% for wallet in specter.wallet_manager.failed_load_wallets %}
<div class="bg-dark-800 p-4 rounded-lg ">
<h1>{{ _("Wallet Name") }}: {{wallet.name}}</h1>
<h5>{{ _("Wallet alias") }}: {{wallet.alias}}</h5>
<h2>{{ _("Error Details") }}:</h2>
<p data-style="color: red">{{wallet.loading_error}}</p>
<form class="mt-3" action="{{ url_for('wallets_endpoint.failed_wallets') }}" method="POST" class="row center">

View file

@ -150,6 +150,15 @@
</div>
</text-explainer>
<text-explainer class="p-3">
<span slot="title" class="cursor-pointer">{{ _("Support Tools") }} </span>
<div slot="content" class="mt-3">
<a class="btn" href="{{url_for('spectrum_endpoint.wallets_get') }}">
Wallet Management
</a>
</div>
</text-explainer>
<div class="mt-8">
<button type="submit" class="button bg-accent text-white" name="action" value="save">{{ _("Save") }}</button>
</div>

View file

@ -9,7 +9,7 @@
<h3 class="mt-8">Information</h3>
<div class="floating-wrapper">
<input type="text" pattern="^[^']+" title="Do not use single quotes" id="wallet_name" name="wallet_name" class="peer floating-input" value="{{ wallet_name }}" placeholder=" " required>
<input type="text" pattern="^[^']+" title="Choose a unique name. Capitalization, spaces, and hyphens will not affect uniqueness. Don't use single quotes." id="wallet_name" name="wallet_name" class="peer floating-input" value="{{ wallet_name }}" placeholder=" " required>
<label class="floating-label">Wallet Name</label>
</div>

View file

@ -166,3 +166,27 @@ def index_post():
)
raise Exception(f"Unknown action: {action}")
@spectrum_endpoint.route("/wallets", methods=["GET"])
@login_required
def wallets_get():
wallets = ext().spectrum_node.get_rpc().listwallets()
wallets_dict = {}
for wallet_name in wallets:
rpc = ext().spectrum_node.get_rpc().wallet(wallet_name)
wallets_dict[wallet_name] = rpc.getwalletinfo()
return render_template(
"spectrum/spectrum_wallets.jinja", wallets=wallets, wallets_dict=wallets_dict
)
@spectrum_endpoint.route("/wallets", methods=["POST"])
@login_required
def wallets_post():
wallet_name = request.form["wallet_name"]
# Does not work yet
# ext().spectrum_node.get_rpc().wallet()delete_wallet()
logger.info("Would delete wallet if it would yet be possible!")
flash("Deletion not yet implemented!")
return redirect(url_for(f"{ SpectrumService.get_blueprint_name()}.wallets_get"))

View file

@ -6,11 +6,10 @@
- active_menuitem: Current active tab. Options: 'general', 'settings', ...
#}
{% macro spectrum_menu(active_menuitem) -%}
<nav class="row collapse-on-mobile">
{{ menu_item(service.id, 'index', 'Main', active_menuitem, isLeft=true) }}
{{ menu_item(service.id, 'settings_get', 'Settings', active_menuitem, isRight=true) }}
<a href="javascript:void(0);" class="mobile-nav-icon" onclick="toggleMobileNav(this, `{{ url_for('static', filename='img/expand-more.svg') }}`, `{{ url_for('static', filename='img/expand-less.svg') }}`)">
<img style="width: 36px;" src="{{ url_for('static', filename='img/expand-more.svg') }}"/>
</a>
<nav class="text-center text-dark-200 border-b border-dark-600">
<ul class="flex flex-wrap -mb-px">
{{ menu_item(service.id, 'index', 'Main', active_menuitem) }}
{{ menu_item(service.id, 'wallets_get', 'Wallet Management', active_menuitem, isRight=true) }}
</ul>
</nav>
{%- endmacro %}

View file

@ -1,4 +1,4 @@
{% extends "spectrum/base.jinja" %}
{% extends "spectrum/components/spectrum_tab.jinja" %}
{% block main %}
<h1>{{ _("Connect with Electrum") }}</h1>

View file

@ -0,0 +1,63 @@
{% extends "spectrum/components/spectrum_tab.jinja" %}
{% block title %}{% endblock %}
{% set tab = 'index' %}
{% block content %}
<div>
<h1>{{ _("Wallet Management") }}</h1>
<div class="note">
{{ _("Other than core, Spectrum lets you delete wallets. The limitation of core has been") }}<br/>
{{ _("surprising effects about wallet naming. This could have lead to inconsitencies which are") }}<br/>
{{ _("difficult to solve.") }}<br/>
{{ _("Here, you can delete wallets if you really need to. Use it with care!") }}<br/>
</div>
<div class="table-holder">
<table class="space-y-3 max-w-[700px] m-auto min-w-full divide-y divide-gray-200">
<thead class="bg-gray-50">
<tr>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Name</th>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Version</th>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Balance</th>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Un. Bl.</th>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Imm. Bl.</th>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Tx #</th>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Keyp</th>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Keyp Int</th>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Pay Tx Fee</th>
<th scope="col" class="py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Actions</th>
</tr>
</thead>
<tbody class="divide-gray-200">
{% for wallet_name, wallet in wallets_dict.items() %}
<tr>
<td class="py-4 ">{{ wallet['walletname'] }}</td>
<td class="py-4 ">{{ wallet['walletversion'] }}</td>
<td class="py-4 ">{{ wallet['balance'] }}</td>
<td class="py-4 ">{{ wallet['uconfirmed_balance'] }}</td>
<td class="py-4 ">{{ wallet['immature_balance'] }}</td>
<td class="py-4 ">{{ wallet['txcount'] }}</td>
<td class="py-4 ">{{ wallet['keypoolsize'] }}</td>
<td class="py-4 ">{{ wallet['keypoolsize_hd_internal'] }}</td>
<td class="py-4 ">{{ wallet['paytxfee'] }}</td>
<td class="py-4 ">
<form action="{{ url_for('spectrum_endpoint.wallets_post')}}" method="POST">
<input type="hidden" class="csrf-token" name="csrf_token" value="{{ csrf_token() }}"/>
<input type="hidden" name="wallet_name" value="{{ wallet['walletname'] }}"/>
<button type="submit" name="action" value="delete" class="button">
<img src="/static/img/cross_thick.svg" class="w-5">
<tool-tip id="tooltip-wallet-deletion">
<h4 slot="title">{{ _("Wallet deletion") }}</h4>
<span slot="paragraph">
{{ _("Clicking here would delete the Wallet but only on the Spectrum, not in Specter.") }}
</span>
</tool-tip>
</button>
</form>
</td>
</tr>
{% endfor %}
</tbody>
</table>
</div>
<br/>
</div>
{% endblock %}

View file

@ -1,9 +1,10 @@
import logging
from cryptoadvance.specter.helpers import deep_update, load_jsons, is_ip_private, alias
logger = logging.getLogger(__name__)
def test_deep_update():
import cryptoadvance.specter.helpers as helpers
base_value = {
"pli": {"pla": "blub", "yes": "yes"},
"pli2": ["arrayelm1", "arrayelm2", "arrayelm3"],
@ -18,7 +19,7 @@ def test_deep_update():
"pli": {"newSubKey": "blub2"},
"pli2": ["arrayelm4", "arrayelm5"],
}
helpers.deep_update(base_value, update_value)
deep_update(base_value, update_value)
# There is now a newRootKey
assert len(base_value) == 4
# keys get added
@ -26,19 +27,18 @@ def test_deep_update():
# Arrays get replaced, not appended!
assert len(base_value["pli2"]) == 2
# you cannot delete stuff with empty dicts
helpers.deep_update(base_value, {"newRootKey": {}})
deep_update(base_value, {"newRootKey": {}})
assert base_value["newRootKey"]["pla"] == "blub"
def test_load_jsons(caplog):
caplog.set_level(logging.INFO)
caplog.set_level(logging.DEBUG, logger="cryptoadvance.specter")
import cryptoadvance.specter.helpers as helpers
mydict = helpers.load_jsons("./tests/helpers_testdata")
mydict = load_jsons("./tests/helpers_testdata")
assert mydict["some_jsonfile"]["blub"] == "bla"
assert mydict["some_other_jsonfile"]["bla"] == "blub"
mydict = helpers.load_jsons("./tests/helpers_testdata", "id")
mydict = load_jsons("./tests/helpers_testdata", "id")
assert "some_jsonfile" not in mydict
# instead the value for the key "id" is now used as the top-level key
assert mydict["ID123"]["blub"] == "bla"
@ -56,7 +56,6 @@ def test_load_jsons(caplog):
def test_is_ip_private(caplog):
caplog.set_level(logging.INFO)
caplog.set_level(logging.DEBUG, logger="cryptoadvance.specter")
import cryptoadvance.specter.helpers as helpers
# https://en.wikipedia.org/wiki/Private_network
# For each private network address range, we are testing the lower, arbitrary middle and upper part of the range
@ -68,14 +67,43 @@ def test_is_ip_private(caplog):
# Randomly generated public ip addresses
public_addresses = ["58.28.76.138", "128.5.218.201", "170.45.214.32"]
assert helpers.is_ip_private("localhost")
assert is_ip_private("localhost")
for ip in priv_lo_addresses:
assert helpers.is_ip_private(ip)
assert is_ip_private(ip)
for ip in priv_24_addresses:
assert helpers.is_ip_private(ip)
assert is_ip_private(ip)
for ip in priv_20_addresses:
assert helpers.is_ip_private(ip)
assert is_ip_private(ip)
for ip in priv_16_addresses:
assert helpers.is_ip_private(ip)
assert is_ip_private(ip)
for ip in public_addresses:
assert not helpers.is_ip_private(ip)
assert not is_ip_private(ip)
def test_alias():
the_same_unique_names = [
"ghost wallet",
"Ghost Wallet",
"GHOST WALLET",
"gHoSt wALlEt",
" Ghost-Wallet",
"Ghost-Wallet ",
" Ghost-Wallet ",
"ghost-wallet",
"Ghost_Wallet",
"ghost_wallet",
"ghost wallet",
"ghost----wallet",
"Ghost Wallet?",
"Ghost Wallet***",
]
assert all(alias(name) == "ghost_wallet" for name in the_same_unique_names)
not_the_same_unique_names = [
"ghost wallet",
"ghost wallet 2",
"ghostwallet",
"ghost.wallet",
"ghostwallet123",
"my_ghos_wallet",
]
assert not all(alias(name) == "ghost_wallet" for name in not_the_same_unique_names)

View file

@ -138,7 +138,7 @@ def test_device_wallets(
assert device.wallets(wm)[0].alias == wallet.alias
second_device = device_manager.get_by_alias("specter")
multisig_wallet = wm.create_wallet(
"a_multisig_test_wallet",
"multisig_wallet_for_device_test",
1,
"wsh",
[device.keys[7], second_device.keys[0]],

View file

@ -42,9 +42,11 @@ def test_WalletManager(
device = device_manager.get_by_alias("trezor")
assert device != None
# Lets's create a wallet with the WalletManager
wm.create_wallet("a_test_wallet", 1, "wpkh", [device.keys[5]], [device])
wm.create_wallet(
"wallet_for_wallet_manager_test", 1, "wpkh", [device.keys[5]], [device]
)
# The wallet-name gets its filename and therefore its alias
wallet = wm.wallets["a_test_wallet"]
wallet = wm.wallets["wallet_for_wallet_manager_test"]
assert wallet != None
assert wallet.balance["trusted"] == 0
assert wallet.balance["untrusted_pending"] == 0
@ -76,12 +78,18 @@ def test_WalletManager(
multisig_wallet.update_balance()
assert multisig_wallet.amount_total == 4
# The WalletManager also has a `wallets_names` property, returning a sorted list of the names of all wallets
assert wm.wallets_names == ["a_multisig_test_wallet", "a_test_wallet"]
assert wm.wallets_names == [
"a_multisig_test_wallet",
"wallet_for_wallet_manager_test",
]
# You can rename a wallet using the wallet manager using `rename_wallet`, passing the wallet object and the new name to assign to it
wm.rename_wallet(multisig_wallet, "new_name_test_wallet")
assert multisig_wallet.name == "new_name_test_wallet"
assert wm.wallets_names == ["a_test_wallet", "new_name_test_wallet"]
assert wm.wallets_names == [
"new_name_test_wallet",
"wallet_for_wallet_manager_test",
]
# You can also delete a wallet by passing it to the wallet manager's `delete_wallet` method
# It will delete the json and attempt to remove it from Bitcoin Core
@ -100,7 +108,7 @@ def test_WalletManager(
):
wm.delete_wallet(wallet, node)
# Check that the wallet wasn't deleted in Specter because of the RpcError
assert wm.wallets_names == ["a_test_wallet"]
assert wm.wallets_names == ["wallet_for_wallet_manager_test"]
# The following deletion should not remove the wallet file on the node
assert node_with_empty_datadir.datadir == ""
wm.rpc.loadwallet(wallet_rpc_path) # we need to load the wallet again
@ -109,7 +117,8 @@ def test_WalletManager(
# The wallet in Specter was already deleted, so trying to delete it again should raise a SpecterError
wm.rpc.loadwallet(wallet_rpc_path) # we need to load the wallet again
with pytest.raises(
SpecterError, match="The wallet a_test_wallet has already been deleted."
SpecterError,
match="The wallet wallet_for_wallet_manager_test has already been deleted.",
):
assert wm.delete_wallet(wallet)
@ -137,9 +146,9 @@ def test_WalletManager_2_nodes(
device = device_manager.get_by_alias("trezor")
assert device != None
first_wallet = wm.create_wallet(
"a_test_wallet", 1, "wpkh", [device.keys[5]], [device]
"wallet_for_test_with_two_nodes", 1, "wpkh", [device.keys[5]], [device]
)
assert wm.wallets_names == ["a_test_wallet"]
assert wm.wallets_names == ["wallet_for_test_with_two_nodes"]
assert wm.chain == "regtest"
assert wm.working_folder.endswith("regtest")
assert wm.rpc.port == 18543
@ -154,7 +163,7 @@ def test_WalletManager_2_nodes(
"regtest",
] # wm.rpcs looks like this: {'regtest': <BitcoinRpc http://localhost:18543>, 'regtest2': <BitcoinRpc http://localhost:18544>}
assert wm.rpc.port == 18544
assert wm.wallets_names == ["a_test_wallet"]
assert wm.wallets_names == ["wallet_for_test_with_two_nodes"]
assert wm.chain == "regtest"
assert wm.working_folder.endswith("test")
second_wallet = wm.create_wallet(
@ -162,7 +171,10 @@ def test_WalletManager_2_nodes(
)
# Note: "regtest2" is recognised by the get_network() from embit as Liquid, that is why there is an error in the logs saying the Bitcoin address is not valid since a Liquid address is derived.
assert len(wm.wallets_names) == 2
assert wm.wallets_names == ["a_regtest2_test_wallet", "a_test_wallet"]
assert wm.wallets_names == [
"a_regtest2_test_wallet",
"wallet_for_test_with_two_nodes",
]
def test_WalletManager_check_duplicate_keys(empty_data_folder):
@ -243,7 +255,7 @@ def test_wallet_sortedmulti(
for i in range(2):
if i == 0:
multisig_wallet = wm.create_wallet(
"a_multisig_test_wallet",
"multisig_wallet_for_sortedmulti_test",
1,
"wsh",
[device.keys[7], second_device.keys[0]],
@ -251,7 +263,7 @@ def test_wallet_sortedmulti(
)
else:
multisig_wallet = wm.create_wallet(
"a_multisig_test_wallet",
"another_multisig_wallet_for_sortedmulti_test",
1,
"wsh",
[second_device.keys[0], device.keys[7]],