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

README note: don't mount .img before signature verification #539

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

akarve
Copy link
Contributor

@akarve akarve commented Apr 1, 2024

Description

Clarify instructions for new users. If you happen to double click the .img before running the verification code on a Mac
then verification fails and it looks scary for no reason.

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

So I did run pytest and two tests are failing, which I'm glad to fix but have nothing to do with my PR.
See comments for more.

If you modified or added functionality/workflow, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms/os:

Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.

@akarve
Copy link
Contributor Author

akarve commented Apr 1, 2024

Here's what happens when I run pytest on an M1 chip. Two QR code unit tests fail:

Note: I needed to conda install zbar" outside of any requirements.txt provided
as apparently that didn't get installed with pyzbar

============================= test session starts ==============================
platform darwin -- Python 3.10.14, pytest-6.2.4, py-1.11.0, pluggy-0.13.1
rootdir: /Users/akarve/code/seedsigner, configfile: pytest.ini, testpaths: tests
plugins: cov-4.1.0
collected 87 items

tests/test_bip85.py .                                                    [  1%]
tests/test_controller.py .....                                           [  6%]
tests/test_decodepsbtqr.py ............                                  [ 20%]
tests/test_embit_utils.py .....                                          [ 26%]
tests/test_encodepsbtqr.py ......                                        [ 33%]
tests/test_flows.py ........                                             [ 42%]
tests/test_flows_psbt.py ..                                              [ 44%]
tests/test_flows_seed.py ..............                                  [ 60%]
tests/test_flows_settings.py .....                                       [ 66%]
tests/test_flows_tools.py ....                                           [ 71%]
tests/test_flows_view.py ....                                            [ 75%]
tests/test_mnemonic_generation.py ........                               [ 85%]
tests/test_psbt_parser.py .                                              [ 86%]
tests/test_seed.py .                                                     [ 87%]
tests/test_seedqr.py .FF                                                 [ 90%]
tests/test_settings.py ......                                            [ 97%]
tests/test_settingsqr_decoder.py ..                                      [100%]

=================================== FAILURES ===================================
______________________ test_compact_seedqr_encode_decode _______________________

    def test_compact_seedqr_encode_decode():
        """ Should encode 24- and 12- word mnemonics to CompactSeedQR format and decode
            them back again to their original mnemonic seed phrase.
        """
        # 24-word seed
>       run_encode_decode_test(os.urandom(32), mnemonic_length=24, qr_type=QRType.SEED__COMPACTSEEDQR)

tests/test_seedqr.py:59: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

entropy = b'\xebb\xa5\xb1Qm\x80\xd9d\xf0\xbe\xdd\xd9\xa21\x1b\xcb\xb9\x9a,\x11)\xfa\x196\xc42o\xcd\x11\xc0\xad'
mnemonic_length = 24, qr_type = 'seed__compactseedqr'

    def run_encode_decode_test(entropy: bytes, mnemonic_length, qr_type):
        """ Helper method to re-run multiple variations of the same encode/decode test """
        print(entropy)
        seed_phrase = bip39.mnemonic_from_bytes(entropy).split()
        print(seed_phrase)
        assert len(seed_phrase) == mnemonic_length
    
        e = EncodeQR(seed_phrase=seed_phrase, qr_type=qr_type)
        data = e.next_part()
        print(data)
    
        qr = QR()
        image = qr.qrimage(
            data=data,
            width=240,
            height=240,
            border=3
        )
    
        decoder = DecodeQR()
        status = decoder.add_image(image)
>       assert status == DecodeQRStatus.COMPLETE
E       assert <DecodeQRStatus.INVALID: 5> == <DecodeQRStatus.COMPLETE: 3>
E        +  where <DecodeQRStatus.COMPLETE: 3> = DecodeQRStatus.COMPLETE

tests/test_seedqr.py:34: AssertionError
----------------------------- Captured stdout call -----------------------------
b'\xebb\xa5\xb1Qm\x80\xd9d\xf0\xbe\xdd\xd9\xa21\x1b\xcb\xb9\x9a,\x11)\xfa\x196\xc42o\xcd\x11\xc0\xad'
['twin', 'benefit', 'hobby', 'people', 'submit', 'hole', 'need', 'blind', 'tape', 'snack', 'middle', 'daring', 'romance', 'snack', 'raccoon', 'cement', 'wheel', 'situate', 'rain', 'gospel', 'wolf', 'material', 'actor', 'release']
b'\xebb\xa5\xb1Qm\x80\xd9d\xf0\xbe\xdd\xd9\xa21\x1b\xcb\xb9\x9a,\x11)\xfa\x196\xc42o\xcd\x11\xc0\xad'
____________________ test_compact_seedqr_handles_null_bytes ____________________

    def test_compact_seedqr_handles_null_bytes():
        """ Should properly encode a CompactSeedQR with null bytes (b'\x00') in the input
            entropy and decode it back to the original mnemonic seed.
        """
        # 24-word seed, null bytes at the front
        entropy = b'\x00' + os.urandom(31)
>       run_encode_decode_test(entropy, mnemonic_length=24, qr_type=QRType.SEED__COMPACTSEEDQR)

tests/test_seedqr.py:72: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

entropy = b'\x004B\x82\xeeb\x95\xb2S\xa6V_\xda\xb5{\xc5\x05\x92h%\x7f\xbd0\x0cs\xde\xe6\xb9\x84GJ\x97'
mnemonic_length = 24, qr_type = 'seed__compactseedqr'

    def run_encode_decode_test(entropy: bytes, mnemonic_length, qr_type):
        """ Helper method to re-run multiple variations of the same encode/decode test """
        print(entropy)
        seed_phrase = bip39.mnemonic_from_bytes(entropy).split()
        print(seed_phrase)
        assert len(seed_phrase) == mnemonic_length
    
        e = EncodeQR(seed_phrase=seed_phrase, qr_type=qr_type)
        data = e.next_part()
        print(data)
    
        qr = QR()
        image = qr.qrimage(
            data=data,
            width=240,
            height=240,
            border=3
        )
    
        decoder = DecodeQR()
        status = decoder.add_image(image)
>       assert status == DecodeQRStatus.COMPLETE
E       assert <DecodeQRStatus.INVALID: 5> == <DecodeQRStatus.COMPLETE: 3>
E        +  where <DecodeQRStatus.COMPLETE: 3> = DecodeQRStatus.COMPLETE

tests/test_seedqr.py:34: AssertionError
----------------------------- Captured stdout call -----------------------------
b'\x004B\x82\xeeb\x95\xb2S\xa6V_\xda\xb5{\xc5\x05\x92h%\x7f\xbd0\x0cs\xde\xe6\xb9\x84GJ\x97'
['ability', 'pear', 'pass', 'system', 'citizen', 'summer', 'excess', 'skull', 'garlic', 'stick', 'galaxy', 'meat', 'float', 'cross', 'nominee', 'waste', 'copy', 'glory', 'waste', 'sniff', 'obscure', 'casual', 'clean', 'usage']
b'\x004B\x82\xeeb\x95\xb2S\xa6V_\xda\xb5{\xc5\x05\x92h%\x7f\xbd0\x0cs\xde\xe6\xb9\x84GJ\x97'
=========================== short test summary info ============================
FAILED tests/test_seedqr.py::test_compact_seedqr_encode_decode - assert <Deco...
FAILED tests/test_seedqr.py::test_compact_seedqr_handles_null_bytes - assert ...
========================= 2 failed, 85 passed in 0.93s =========================

@kdmukai
Copy link
Contributor

kdmukai commented Apr 1, 2024

Note the manual installation instructions here: https://github.com/SeedSigner/seedsigner/blob/dev/docs/manual_installation.md#install-zbar

Errors reading Compact SeedQR format most likely point to an incompatibility with reading raw byte data format out of a QR code.

Also verify that you're running with our fork of pyzbar:
https://github.com/SeedSigner/seedsigner/blob/dev/docs/manual_installation.md#install-zbar

@akarve
Copy link
Contributor Author

akarve commented Apr 1, 2024

Thanks. I'm running the tests locally, as opposed to in Docker. I was able to brew install zbar and can verify I'm on your fork of pyzbar as that's what's in requirements.txt. But getting the same errors. Should I break this out into a separate docs PR for getting Mac unit tests running and unencumber this PR?

@newtonick newtonick merged commit c7738c1 into SeedSigner:dev Apr 29, 2024
1 check passed
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.

3 participants