validation: throw when a compressed script can't be decompressed #36280

pull furszy wants to merge 2 commits into bitcoin:master from furszy:2026_compression_throw changing 3 files +96 −9

furszy commented at 8:49 PM on September 16, 2026: member

ScriptCompression::Unser ignores the DecompressScript return value and leaves the script empty on failure, without notifying upper layers about the error. In the worst-case scenario, which is currently only reachable through corruption, an unspendable off-curve P2PK turns into an anyone-can-spend empty script. Given that, properly throwing the error to prevent any future mistakes from happening seems the sanest path to follow.

Aside from that, added coverage for a P2PK script with an uncompressed pubkey that has a valid X but a wrong Y, which makes it unspendable. Checking we store such scripts raw and never encode them as 0x04/0x05, as those encodings serialize X with only Y's parity (not the full Y), which would make decompression recompute Y from X and turn the unspendable script into a spendable one, which would not be good.

test: script compression with off-curve and wrong-Y P2PK pubkeys

Adds coverage for two cases:

The existing compress_p2pk_scripts_not_on_curve test now also covers
the ScriptCompression ser + unser round-trip, and checks the behavior
of a compressed record that can't be decompressed.
The worst-case scenario is a P2PK to an off-curve pubkey, which nobody
can spend, that is turned into an anyone-can-spend script during
decompression without throwing an error to upper layers.
Note: this could only ever happen after corruption today, but better
to add proper coverage for it, and add the missing error signaling
(next commit).

Then added coverage for a P2PK script with an uncompressed pubkey that
has a valid X but a wrong Y, which makes it unspendable. Checking we store
such scripts raw and never encode them as 0x04/0x05, as those encodings
serialize X with only Y's parity (not the full Y), so decompress would
recompute Y from X and turn the unspendable script into a spendable one,
which would be bad.

27c6a2c75b

DrahtBot added the label Validation on Sep 16, 2026
DrahtBot commented at 8:49 PM on September 16, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36280.



Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

Approach ACK sedited

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #35511 (RFC: consensus: Make CAmount a class by hodlinator)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
furszy force-pushed on Sep 16, 2026
DrahtBot added the label CI failed on Sep 16, 2026
sedited commented at 9:49 PM on September 16, 2026: contributor

Approach ACK
DrahtBot removed the label CI failed on Sep 16, 2026

Type	Reviewers
Approach ACK	sedited

compressor: throw when compressed script can't be decompressed

`ScriptCompression::Unser` ignores the `DecompressScript` return value and
leaves the script empty on failure, without notifying upper layers about the error.
In the worst-case scenario, which is currently only reachable through corruption,
this turns an unspendable off-curve P2PK into an anyone-can-spend script.
Given that, properly throwing the error to prevent any future mistakes from
happening seems the sanest path to follow.

The fuzz test has been expanded to cover more than a bare crash.

5500521f8b

furszy force-pushed on Sep 17, 2026

Contributors

Labels

validation: throw when a compressed script can't be decompressed #36280

Code Coverage & Benchmarks

Reviews

Conflicts