BP-WG / bp-wallet

Modern, lightweight & standard-compliant bitcoin wallet runtime & cli without rust-bitcoin dependencies
Apache License 2.0
17 stars 12 forks source link

fix electrum floating-point arithmetic issue #61

Closed zoedberg closed 2 months ago

zoedberg commented 2 months ago

Closes https://github.com/BP-WG/bp-wallet/issues/62

Issue was due to the conversion from float to integer in let value = (vout.value * 100_000_000.0) as u64;

With this PR instead of parsing the TX outputs of the JSON TX returned by electrum we use the TX deserialized from its hex, this way we don't have to deal with floating-point arithmetic issues.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.

Project coverage is 0.0%. Comparing base (dbfb8ba) to head (81346f6). Report is 3 commits behind head on master.

Files Patch % Lines
src/indexers/electrum.rs 0.0% 9 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #61 +/- ## ===================================== Coverage 0.0% 0.0% ===================================== Files 23 23 Lines 2128 2123 -5 ===================================== + Misses 2128 2123 -5 ``` | [Flag](https://app.codecov.io/gh/BP-WG/bp-wallet/pull/61/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BP-WG) | Coverage Δ | | |---|---|---| | [rust](https://app.codecov.io/gh/BP-WG/bp-wallet/pull/61/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BP-WG) | `0.0% <0.0%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BP-WG#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dr-orlovsky commented 2 months ago

Closing in favor of merged #63