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

Update encode_qr.py - Fixes #582 #610

Merged
merged 4 commits into from
Dec 24, 2024
Merged

Update encode_qr.py - Fixes #582 #610

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
17d1524
Update encode_qr.py
fedebuyito Sep 18, 2024
405d0c8
Update encode_qr.py
fedebuyito Sep 23, 2024
8d769c1
Update test_encodepsbtqr.py
fedebuyito Sep 23, 2024
0496eea
Update encode_qr.py
fedebuyito Dec 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/seedsigner/models/encode_qr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from seedsigner.models.settings import SettingsConstants

from urtypes.crypto import PSBT as UR_PSBT
from urtypes.crypto import Account, HDKey, Output, Keypath, PathComponent, SCRIPT_EXPRESSION_TAG_MAP
from urtypes.crypto import Account, HDKey, Output, Keypath, PathComponent, SCRIPT_EXPRESSION_TAG_MAP, CoinInfo



Expand Down Expand Up @@ -344,11 +344,13 @@ def derivation_to_keypath(path: str) -> list:
return Keypath(arr, self.root.my_fingerprint, len(arr))

origin = derivation_to_keypath(self.derivation)
self.use_info = None if self.network == "M" else CoinInfo(type=None, network=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should always use the constants instead of hard-coded strings.

self.use_info = None if self.network == SettingsConstants.MAINNET else...

Copy link
Contributor

Choose a reason for hiding this comment

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

An explanatory comment would help here and/or a reference to Issue #582.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. Done on commit 0496eea bellow.

Copy link
Contributor

Choose a reason for hiding this comment

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

@fedebuyito I meant in the code itself:

        origin = derivation_to_keypath(self.derivation)

        # For testnet we need to... blah blah... because... blah blah...
        self.use_info = None if self.network == "M" else CoinInfo(type=None, network=1)

It's not urgent, but since this PR is already merged, can you add that code comment and submit that change as its own PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @kdmukai .Yes, sure (sorry for the misunderstanding). Here it is: df36c1a .

PR #643


self.ur_hdkey = HDKey({ 'key': self.xpub.key.serialize(),
'chain_code': self.xpub.chain_code,
'origin': origin,
'parent_fingerprint': self.xpub.fingerprint})
'parent_fingerprint': self.xpub.fingerprint,
'use_info': self.use_info })

ur_outputs = []

Expand Down
20 changes: 17 additions & 3 deletions tests/test_encodepsbtqr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,26 @@ def test_ur_xpub_qr():

e = UrXpubQrEncoder(
seed=Seed(mnemonic.split(), passphrase="pass"),
network=SettingsConstants.TESTNET,
network=SettingsConstants.MAINNET,
derivation="m/48h/1h/0h/2h",
qr_density=SettingsConstants.DENSITY__MEDIUM
)

assert e.next_part() == "UR:CRYPTO-ACCOUNT/1-4/LPADAACSKPCYMOMNLGRYHDCKOEADCYSSMECPONAOLYTAADMETAADDLOXAXHDCLAOKSRLNLKPUEGYATHPMNSNIYMUECBY"
assert e.next_part() == "UR:CRYPTO-ACCOUNT/2-4/LPAOAACSKPCYMOMNLGRYHDCKKKGHZMLUZORPVDGUOTECSTTKTOLPCWPTNTLKZTTIZTBEAAHDCXVDTPMYRSTDMOPSCXFZ"
assert e.next_part() == "UR:CRYPTO-ACCOUNT/3-4/LPAXAACSKPCYMOMNLGRYHDCKSPZSBZSPGERLGDATUYNLPYBTGYIYYKBTWTAOSWKSVTSGCHBYDKYAVDAMTAADMONDGDFD"
assert e.next_part() == "UR:CRYPTO-ACCOUNT/4-4/LPAAAACSKPCYMOMNLGRYHDCKDYOTADLOCSDYYKADYKAEYKAOYKAOCYSSMECPONAXAAAYCYIOREKKJKAEAEAEWZWDMYON"
assert e.next_part() == "UR:CRYPTO-ACCOUNT/4-4/LPAAAACSKPCYMOMNLGRYHDCKDYOTADLOCSDYYKADYKAEYKAOYKAOCYSSMECPONAXAAAYCYIOREKKJKAEAEAEWZWDMYON"


e = UrXpubQrEncoder(
seed=Seed(mnemonic.split(), passphrase="pass"),
network=SettingsConstants.TESTNET,
derivation="m/48h/1h/0h/2h",
qr_density=SettingsConstants.DENSITY__MEDIUM
)

assert e.next_part() == "UR:CRYPTO-ACCOUNT/1-5/LPADAHCSKECYRTPEDKMOHDCFOEADCYSSMECPONAOLYTAADMETAADDLONAXHDCLAOKSRLNLKPUENSAHBTHS"
assert e.next_part() == "UR:CRYPTO-ACCOUNT/2-5/LPAOAHCSKECYRTPEDKMOHDCFGYATHPMNSNKKGHZMLUZORPVDGUOTECSTTKTOLPCWPTNTLKZTTIZTNDJSCF"
assert e.next_part() == "UR:CRYPTO-ACCOUNT/3-5/LPAXAHCSKECYRTPEDKMOHDCFZTBEAAHDCXVDTPMYRSTDSPZSBZSPGERLGDATUYNLPYBTGYIYYKBDFGWPKE"
assert e.next_part() == "UR:CRYPTO-ACCOUNT/4-5/LPAAAHCSKECYRTPEDKMOHDCFBTWTAOSWKSVTSGCHBYDKYAVDAHTAADEHOYAOADAMTAADDYOTADGYBKBWFE"
assert e.next_part() == "UR:CRYPTO-ACCOUNT/5-5/LPAHAHCSKECYRTPEDKMOHDCFLOCSDYYKADYKAEYKAOYKAOCYSSMECPONAXAAAYCYIOREKKJKAETODLFYWP"
Loading