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.

Sign up for GitHub

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

Jump to bottom

Address validation in Spectrum #11

Open
moneymanolis opened this issue Nov 19, 2022 · 5 comments · May be fixed by #42
Open

Address validation in Spectrum #11

moneymanolis opened this issue Nov 19, 2022 · 5 comments · May be fixed by #42
Labels
good first issue Good for newcomers question Further information is requested

Comments

@moneymanolis
Copy link
Collaborator

How will the address validation in Spectrum work without Bitcoin Core code in the background?
@moneymanolis moneymanolis added the question Further information is requested label Nov 19, 2022
@stepansnigirev
Copy link
Contributor

For every address you have a derivation path and wallet descriptor, so what's the problem?

@moneymanolis
Copy link
Collaborator Author

moneymanolis commented Nov 19, 2022 •
edited
Loading

If someone enters an invalid address in Specter Desktop when sending Bitcoin Core currently throws an error. What prevents sth. like this in Spectrum?

@stepansnigirev
Copy link
Contributor

It must be converted to script during PSBT creation, so if embit can't decode the address rpc call will fail.
I am not sure if spectrum checks wether the address is in the correct network thouhg.

@moneymanolis
Copy link
Collaborator Author

Thanks, good point. Will check with the network.

@k9ert
Copy link
Contributor

k9ert commented Jan 18, 2023 •
edited
Loading

Status Quo

spectrum

In wallet_send.jinja, there is also a very shallow validity check in validAddress (not empty, address.length < 14, no whitespace). Passing that stupidiy test, currently we get an exception like this:

Traceback (most recent call last):
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/embit/script.py", line 183, in address_to_scriptpubkey
    data = base58.decode_check(addr)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/embit/base58.py", line 76, in decode_check
    raise ValueError(
ValueError: Checksum mismatch: expected b'\xc7\xce\xdb\xb0', calculated b'\x92\x14W\xba'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/app.py", line 1523, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask/app.py", line 1509, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/flask_login/utils.py", line 269, in decorated_view
    return func(*args, **kwargs)
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/server_endpoints/wallets/wallets.py", line 531, in send_new
    psbt = psbt_creator.create_psbt(wallet)
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/commands/psbt_creator.py", line 120, in create_psbt
    self.psbt_as_object: SpecterPSBT = wallet.createpsbt(
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/wallet.py", line 1684, in createpsbt
    r = self.rpc.walletcreatefundedpsbt(
  File "/home/kim/src/specter-desktop/src/cryptoadvance/specter/rpc.py", line 495, in fn
    r = self.multi([(method, *args)], **kwargs)[0]
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/cryptoadvance/specterext/spectrum/bridge_rpc.py", line 86, in multi
    result = [self.spectrum.jsonrpc(item,wallet_name=self.wallet_name, catch_exceptions=False) for item in payload]
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/cryptoadvance/specterext/spectrum/bridge_rpc.py", line 86, in <listcomp>
    result = [self.spectrum.jsonrpc(item,wallet_name=self.wallet_name, catch_exceptions=False) for item in payload]
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/cryptoadvance/spectrum/spectrum.py", line 353, in jsonrpc
    raise e
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/cryptoadvance/spectrum/spectrum.py", line 343, in jsonrpc
    res = m(wallet, *args, **kwargs)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/cryptoadvance/spectrum/spectrum.py", line 57, in wrapper
    return f(*args, **kwargs)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/cryptoadvance/spectrum/spectrum.py", line 986, in walletcreatefundedpsbt
    btc_to_sat(amount), EmbitScript.from_address(addr)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/embit/script.py", line 86, in from_address
    return address_to_scriptpubkey(addr)
  File "/home/kim/src/specter-desktop/.env/lib/python3.10/site-packages/embit/script.py", line 195, in address_to_scriptpubkey
    raise EmbitError("Invalid bech32 address")
embit.base.EmbitError: Invalid bech32 address

Bitcoin Core

After passing the stupidity test, we get a SpecterError which comes from a RPC-Error:
image

ToDo

I guess we need to catch the embit.base.EmbitError in https://github.com/cryptoadvance/spectrum/blob/master/src/cryptoadvance/spectrum/spectrum.py#L986 and raise an Spectrum.RPCError with the text: "Invalid Bitcoin address: {invalid_address}".

@k9ert k9ert added the good first issue Good for newcomers label Feb 9, 2023
@PaarthAgarwal PaarthAgarwal linked a pull request Feb 10, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixed address validation in spectrum PaarthAgarwal/spectrum
3 participants
@k9ert @stepansnigirev @moneymanolis