bitcoindevkit / bdk / 13439987517 / 1

79%

master: 79%

push

github

Merge bitcoindevkit/bdk#1837: `bdk_electrum`: Handle negative heights properly

<a class=hub.com/bitcoindevkit/bdk/commit/161d715bedd6af5996c4cf24b6bc1f733c1228ea">161d715be<a href="https://github.com/bitcoindevkit/bdk/commit/ad216854b89b91fe8a39853bcbcfe4963848b7a7"> test(electrum): Chained mempool sync (志宇)
<a class="double-link" href="https://github.com/bitcoindevkit/bdk/commit/75020521db4e5268960d1b84fe13482f9c6c9457">75020521d</a> fix(electrum): Handle negative heights properly (志宇)

Pull request description:

  ### Description

  This fixes a problem with `bdk_electrum` making bogus `transaction_get_merkle` calls to the Electrum server.

  In electrum, heights returned alongside txs may be 0 or -1. 0 means the tx is unconfirmed. -1 means the tx is unconfirmed and spends an unconfirmed tx.

  Previously, the codebase assumed that heights cannot be negative and used a `i32 as usize` cast (which would lead to the casted height being 18446744073709551615). Then the codebase will try to call `transaction_get_merkle` with the overflowed height.

  ### Notes to the reviewers

  The test introduced in this PR does not fail with the old codebase. What ends up happening is that `transaction_get_merke` will be called with the overflow height and the Electrum server will return with "merkle not found".

  To see the failure, you can change the `height as usize` casts to use `.try_into().expect()` then you will see a panic.

  These changes should be applied as `1.0.1` and `1.1.1` fixes.

  ### Changelog notice

  * Fix `bdk_electrum` handling of negative spk-history height.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [x] I've added tests to reproduce the issue which are now passing

  ~* [ ] This pull request breaks the existing API~
  ~* [ ] I'm linking the issue being fixed by this PR~

ACKs for top commit:
  LagginTimes:
    ACK 161d715bedd6af5996c4cf24b6bc1f733c1228ea

Tree-SHA512: 18bc5c6cc1961a5d77e2044bf30f4e27d44cb3a24... (continued)

12146 of 14113 relevant lines covered (86.06%)

22350.62 hits per line