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

Trigger send to BTC address flow when camera reads a BTC address #665

Merged
merged 3 commits into from
Oct 10, 2023

Conversation

ademar111190
Copy link
Collaborator

Fixes: #570

How to test

  • Generates a Bitcoin address QR code
    • Try different address formats (1... 3... bc1...)
    • Try adding the amount and leaving it empty
  • Opens c-breez and then open the camera scanner
  • The app should recognize the QR code and open the Send to BTC address page
  • The address and the amount should be correctly filled with the data you added on the QR code

@ademar111190 ademar111190 requested a review from a team October 9, 2023 14:37
@ademar111190 ademar111190 self-assigned this Oct 9, 2023
Copy link
Contributor

@ubbabeck ubbabeck left a comment

Choose a reason for hiding this comment

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

Looks good to me 😃

@roeierez
Copy link
Member

roeierez commented Oct 9, 2023

@ademar111190 This will trigger the onchain page also when the bitcoin address is copied to the clipboard right?
Is that the behavior in breez mobile? Is that what we want?

@ubbabeck
Copy link
Contributor

ubbabeck commented Oct 9, 2023

@ademar111190 This will trigger the onchain page also when the bitcoin address is copied to the clipboard right?
Is that the behavior in breez mobile? Is that what we want?

Good catch,

@ademar111190 This will trigger the onchain page also when the bitcoin address is copied to the clipboard right?
Is that the behavior in breez mobile? Is that what we want?

Good catch I forgot about this when reviewing. We only want to push the reverse_swap page when scanning btc addresses and not when it is in the clipboard.

@ademar111190
Copy link
Collaborator Author

That is a good point, and it is a difference in behavior that may appear in the future as well. When X is inputed, do Y if the source is Z.
I'll think in something to have this in control.

@ademar111190 ademar111190 force-pushed the aa/570-old-btc-address branch from 296cc9a to e7451d7 Compare October 10, 2023 13:16
@ademar111190
Copy link
Collaborator Author

I edit the PR with the following changes:

let me know your thoughts @roeierez @ubbabeck

@ademar111190 ademar111190 force-pushed the aa/570-old-btc-address branch 3 times, most recently from 9d9aa13 to ba275c5 Compare October 10, 2023 14:10
@ademar111190
Copy link
Collaborator Author

I don't know why the CI is not passing, the error:

cd cbreez
  dart format -o none --set-exit-if-changed -l 110 .
  shell: /bin/bash -e {0}
  env:
    FLUTTER_ROOT: /Users/runner/hostedtoolcache/flutter/stable-[3](https://github.com/breez/c-breez/actions/runs/6470526578/job/17566932463?pr=665#step:10:3).7.12-x6[4](https://github.com/breez/c-breez/actions/runs/6470526578/job/17566932463?pr=665#step:10:4)
    PUB_CACHE: /Users/runner/hostedtoolcache/flutter/stable-3.7.12-x64/.pub-cache
    JAVA_HOME: /Users/runner/hostedtoolcache/Java_Zulu_jdk/17.0.8-7/x64
  
Changed lib/bloc/account/account_bloc.dart
Changed lib/routes/create_invoice/create_invoice_page.dart
Formatted 336 files (2 changed) in 2.2[5](https://github.com/breez/c-breez/actions/runs/6470526578/job/17566932463?pr=665#step:10:5) seconds.
Error: Process completed with exit code 1.

but running locally I got no diff:

dart format -l 110 .
Formatted 336 files (0 changed) in 0.65 seconds.

I've checked account_bloc.dart and create_invoice_page.dart both are okay (I even manually edit the comments to make sure it is under 110 lines)

@ademar111190 ademar111190 mentioned this pull request Oct 10, 2023
@ademar111190 ademar111190 force-pushed the aa/570-old-btc-address branch from ba275c5 to eea9199 Compare October 10, 2023 21:38
@ademar111190 ademar111190 merged commit 06b48d4 into main Oct 10, 2023
1 check passed
@ademar111190 ademar111190 deleted the aa/570-old-btc-address branch October 10, 2023 21:43
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.

Old BTC address format support
3 participants