Update encode_qr.py - Adds comments to code (Fixes #582)

[fedebuyito]

Some helpers comments.

Description

Some additional helpers comments on code. (Relative to issue #582, fixed on #610)

This pull request is categorized as a:

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

Checklist

• N/A

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:

• N/A

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.

[kdmukai]

@fedebuyito:

• it's best to reference the Issue number in the description, that way github will auto-link to that discussion. Github doesn't auto-link references in titles.
• And since this is a follow-up PR, also a good idea to reference the earlier PR number in the description so that gets auto-linked as well.

@jdlcdl you mention use_info as the solution in #582 but the discussion there and the new code comment added in this PR doesn't actually explain WHAT that does, only that it provides the correct outcome.

And now that I take a second look at PR #610, is network=1 referring to the "coin type" portion of the derivation path? We should explain that relationship in the code comment as well.

[fedebuyito]

@fedebuyito:

• it's best to reference the Issue number in the description, that way github will auto-link to that discussion. Github doesn't auto-link references in titles.
• And since this is a follow-up PR, also a good idea to reference the earlier PR number in the description so that gets auto-linked as well.

Thank you for your advices, done!

[jdlcdl]

@jdlcdl you mention use_info as the solution in #582 but the discussion there and the new code comment added in this PR doesn't actually explain WHAT that does, only that it provides the correct outcome.

I had not given this as much thought as @fedebuyito and @newtonick did. My thoughts were to include the "use_info" context (which network is this pubkey for?) in all cases, but it would make the qrcode even bigger for mainnet, whereas that's the only reasonable default without an explicit "use_info". That it's only included for testnet seems a much better solution than my original thoughts.

[fedebuyito]

And now that I take a second look at PR #610, is network=1 referring to the "coin type" portion of the derivation path? We should explain that relationship in the code comment as well.

It does not seem to refer to the cointype of the derivation path but to the hrp of the xpub according to the code in urtypes/crypto/hd_key.HDKey class here: https://github.com/selfcustody/urtypes/blob/8ff8e6ebe484d7a0f98ad73f4441708704998b43/src/urtypes/crypto/hd_key.py#L112, where setting use_info=None or use_info.network=0 would apply MAINNET; otherwise, TESTNET.

Perhaps we could add to the comment (df36c1a) that, for TESTNET, network must not be 0:
#Implements "use_info" member on HDKey class (urtypes/crypto packages-libs folder) construct,
#so if working on TESTNET, for Xpub can be exported accordingly, "use_info.network" must to exist and to be != 0.
#Default case, MAINNET: "use_info" = None.

On the other hand, remembering the conversation with @Newtonic, it was recommended to skip "use_info" for mainnet according to https://github.com/BlockchainCommons/Research/blob/master/papers/bcr-2020-007-hdkey.md; different from setting it to "None" as it is now. We could effectively bypass "use_info" with this last conditional argument on code below:

self.ur_hdkey = HDKey({
'key': self.xpub.key.serialize(),
'chain_code': self.xpub.chain_code,
'origin': origin,
'parent_fingerprint': self.xpub.fingerprint,
**({'use_info': CoinInfo(type=None, network=1)} if self.network != SettingsConstants.MAINNET else {}) <------------------
})

I have probe it and works fine and doesn't affect the QR exported.

Maybe it's better implemented this way. I read your opinions, of course, so you can tell me what you think is good.