Skip to content

fix(electrum): verify txid of server-returned transactions#2188

Merged
evanlinjin merged 1 commit intobitcoindevkit:masterfrom
tnull:2026-04-electrum-txid-check
Apr 24, 2026
Merged

fix(electrum): verify txid of server-returned transactions#2188
evanlinjin merged 1 commit intobitcoindevkit:masterfrom
tnull:2026-04-electrum-txid-check

Conversation

@tnull
Copy link
Copy Markdown
Contributor

@tnull tnull commented Apr 23, 2026

Description

An Electrum server could return an arbitrary transaction when fetch_tx() requests a specific txid. The returned transaction was cached and used without verifying that its computed txid matches the requested one.

Add a verification check that tx.compute_txid() == txid after fetching from the server, returning an error on mismatch. Include a unit test with a mock Electrum client that exercises both the mismatch rejection and the matching-txid happy path.

Copy link
Copy Markdown
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

ConceptACK

I think the test is overkill - let's remove it. Otherwise, LGTM.

@evanlinjin evanlinjin moved this to In Progress in BDK Chain Apr 23, 2026
@evanlinjin evanlinjin added this to the Chain 0.24.0 milestone Apr 23, 2026
@evanlinjin
Copy link
Copy Markdown
Member

@tnull Let me know if you would like me to take over (I know you are busy).

An Electrum server could return an arbitrary transaction when
`fetch_tx()` requests a specific txid. The returned transaction was
cached and used without verifying that its computed txid matches the
requested one.

Add a verification check that `tx.compute_txid() == txid` after
fetching from the server, returning an error on mismatch.

Signed-off-by: Elias Rohrer <dev@tnull.de>
@tnull tnull force-pushed the 2026-04-electrum-txid-check branch from 0e68465 to d101a09 Compare April 23, 2026 16:55
@tnull
Copy link
Copy Markdown
Contributor Author

tnull commented Apr 23, 2026

@tnull Let me know if you would like me to take over (I know you are busy).

Dropped the test.

Copy link
Copy Markdown
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

ACK d101a09

@evanlinjin evanlinjin merged commit 8760d87 into bitcoindevkit:master Apr 24, 2026
18 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in BDK Chain Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants