From: valued mammal <valuedmammal@protonmail.com>
Date: Thu, 20 Feb 2025 16:27:48 +0000 (-0500)
Subject: Merge bitcoindevkit/bdk#1837: `bdk_electrum`: Handle negative heights properly
X-Git-Tag: wallet-1.2.0~26
X-Git-Url: http://internal-gitweb-vhost/script/%22https:/struct.DecoderReader.html?a=commitdiff_plain;h=ad216854b89b91fe8a39853bcbcfe4963848b7a7;p=bdk

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

161d715bedd6af5996c4cf24b6bc1f733c1228ea test(electrum): Chained mempool sync (志宇)
75020521db4e5268960d1b84fe13482f9c6c9457 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: 18bc5c6ccebd810e253cc88a69554ef6a27168df1961a5d77e2044bf30f4e27d44cb3a2499f5d29f0a3bd0eb349cacd7ea870f9c7225dd09798c7e19091a0f04
---

ad216854b89b91fe8a39853bcbcfe4963848b7a7