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

Compare generated addresses with the expected ones in the client library #166

Merged
merged 6 commits into from
Jun 19, 2023

Conversation

bigspider
Copy link
Collaborator

@bigspider bigspider commented May 29, 2023

Supersedes #141 with a more generic solution: derive addresses from the wallet policy on the client side, and fail if they differ.

The libraries used for client side are generation are externally developed.

  • based on python-bip380 for python; considered not production-ready, but good enough for our purpose, since it's only used to compare miniscript addresses.
  • uses rust-miniscript for Rust; as it's a relatively large import, the paranoid_client feature is kept as an optional feature, but enabled by default.

For JavaScript, there doesn't seem to be a viable library for descriptors. bitcoinerlab/miniscript is currently the only one, which wraps the WASM-compiled version of https://github.com/sipa/miniscript which unfortunately doesn't directly export the miniscript => Script compilation. Therefore, the JS library is still explicitly blacklisting app versions with known bugs, rather than using the generic approach above.

@codecov-commenter
Copy link

codecov-commenter commented May 29, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (d1c950b) 83.96% compared to head (dbd5723) 83.96%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #166   +/-   ##
========================================
  Coverage    83.96%   83.96%           
========================================
  Files           17       17           
  Lines         2189     2189           
========================================
  Hits          1838     1838           
  Misses         351      351           
Flag Coverage Δ
unittests 83.96% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@bigspider bigspider force-pushed the client-check-address branch 4 times, most recently from ff349f1 to f944e7a Compare May 30, 2023 11:06
@bigspider bigspider changed the base branch from develop to master June 13, 2023 13:54
@bigspider bigspider changed the base branch from master to develop June 13, 2023 13:54
sgliner-ledger
sgliner-ledger previously approved these changes Jun 19, 2023
bitcoin_client_js/src/lib/appClient.ts Outdated Show resolved Hide resolved
@bigspider bigspider merged commit f1d379a into develop Jun 19, 2023
50 checks passed
@bigspider bigspider deleted the client-check-address branch June 19, 2023 08:25
@sonarcloud
Copy link

sonarcloud bot commented Jun 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

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.

None yet

3 participants