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

Update encode_qr.py #610

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

fedebuyito
Copy link

Implementing "use_info.network" member provided on HDKey/CoinInfo classes (urtypes/crypto packages-libs folder) for testnet public key exporting option.

Description

Describe the change simply. Provide a reason for the change.

Include screenshots of any new or modified screens (or at least explain why they were omitted)

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

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.

Implementing "use_info.network" member provided on HDKey/CoinInfo classes (urtypes/crypto packages-libs folder) for testnet public key exporting option.
@fedebuyito
Copy link
Author

#582 Exporting for testnet when option is selected.

@jgmontoya
Copy link

jgmontoya commented Sep 18, 2024

I couldn't replicate the failing test locally (using python 3.12), @newtonick @kdmukai could you re-run the ci? I suspect that test is maybe just flaky

image

@newtonick
Copy link
Collaborator

newtonick commented Sep 19, 2024

I can't reproduce the failed CI locally either

image

--I have no idea where this 5 UR frame is coming from or where/why there would be an assertion to compare the 4th and 5th frame to be equal. Odd.-- I looked closer and I have a better understanding of what is going on, but need to dig deeper.

@newtonick
Copy link
Collaborator

newtonick commented Sep 19, 2024

I'm able to reproduce the failed CI locally now

@fedebuyito
Copy link
Author

Thank you, @newtonick. What could you conclude? Any particular on the enviroment to take present for this PR?

@jgmontoya
Copy link

@newtonick could you comment on how were you able to reproduce the failing test?

@newtonick newtonick added this to the 0.9.0 milestone Sep 20, 2024
Change in "self.use_info" assignation for to be "None" when mainnet.
Updates first test (in test_ur_xpub_qr) to "MAINNET" and creates following for "TESTNET"
@fedebuyito
Copy link
Author

According to suggested by @newtonick, the commits (pytested on local) for implementing "use_info" only on testnet in order to avoid bloat the QR. Updating mainnet case and creating new one for testnet in test_encodepsbtqr.py which was been affected. #582

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