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

Tests that fail for BlueWallet-v7.0.7 upper-cased addresses #666

Closed
wants to merge 2 commits into from

Conversation

jdlcdl
Copy link

@jdlcdl jdlcdl commented Jan 20, 2025

Description

resolves #665

Recently released BlueWallet v7.0.7 changed their bech32 address format for qr-codes.
BlueWallet/BlueWallet#7461

2 commits that add failing test-cases for latest BlueWallet v7.0.7 bech32 addresses that look like "bitcoin:BC1Q..."

TODO: resolve models/decode_qr.py to support uppercased bech32 addresses and avoids future BlueWallet user complaints about not being able to verify addresses.

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.

@@ -297,6 +297,9 @@ def test_bitcoin_address():

main_bech32_address2 = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=12000"
main_bech32_address3 = "BITCOIN:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?junk"
main_bech32_address4 = "bitcoin:BC1QAR0SRRR7XFKVY5L643LYDNW9RE59GTZZWF5MDQ?junk"
main_bech32_address5 = "BITCOIN:BC1QAR0SRRR7XFKVY5L643LYDNW9RE59GTZZWF5MDQ?junk"
main_bech32_address6 = "BC1QAR0SRRR7XFKVY5L643LYDNW9RE59GTZZWF5MDQ"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest refactoring to something like:

bech32_capitalization_variations = [
    "bitcoin:...",
    "BITCOIN:...",
]

And simplifying the for:

for capitalization_variation in bech32_capitalization_variations:
    ...

The main_bech32_address2 test down on lines 358-362 can be removed since it's now redundant. With that removed, there's no need for the var name.

@kdmukai
Copy link
Contributor

kdmukai commented Jan 20, 2025

While we're editing this file, can we rename it to test_decode_qr.py?

As I was altering the bitcoin addr decoding logic, I was looking for a decode_qr test file but only saw the decodepsbtqr which implies it only concerns itself with psbt QRs.

@jdlcdl jdlcdl closed this Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged Not Yet Released
Development

Successfully merging this pull request may close these issues.

BlueWallet addresses may be bech32 in all uppercase.
2 participants