Skip to content

Improve slicing and error handling around slices#279

Merged
apoelstra merged 6 commits into
ElementsProject:masterfrom
apoelstra:2026-06/slicing-improvements
Jun 15, 2026
Merged

Improve slicing and error handling around slices#279
apoelstra merged 6 commits into
ElementsProject:masterfrom
apoelstra:2026-06/slicing-improvements

Conversation

@apoelstra

Copy link
Copy Markdown
Member

Uses the bitcoin-internals library which provides us a bunch of array/slice methods that aren't available in std in our MSRV. Eventually we will be able to drop the dep and get these features directly from std.

This replaces a bunch of slice accesses with arrays, cleaning up error paths and eliminating panics (though most are inaccessible assuming you use known-valid data from the blockchain).

Builds on #278 (which I will merge in the next hour).

Will do a followup that cleans up the error types; I deliberately avoided breaking the API to make this one easy to backport.

@apoelstra apoelstra force-pushed the 2026-06/slicing-improvements branch 6 times, most recently from 529130d to e97b79a Compare June 15, 2026 18:57
I would like the ArrayExt trait, which has a bunch of unsafe code, and
which implements stuff that is missing in stdlib. This is already in
our dep tree through rust-bitcoin so adding a direct dependency adds
no new code exposure.
These unwraps and indexing (which hide more panic paths) irritated me, and also
would need to be tweaked once we get rid of the Hash::from_slice methods. I
figured I'd preemptively get rid of them.
I don't *think* it's possible to create a rangeproof with a sidechannel smaller
than 64 bytes (if you create a 0-sized "proof of exact value" then unwinding
will fail entirely, and anything larger I think has at least one ring, so 128
bytes or more). Unsure.

But better not to assume this by indexing recklessly into the sidechannel
message.
We should redo the error types here as well at some point.
Currently we allow decoding segwit v0 programs which have uncompressed/hybrid
keys (not allowed) and I suspect that if you provide a too-short address then
you'll get a panic here.
Lol, this code was so close and yet so far.
@apoelstra apoelstra force-pushed the 2026-06/slicing-improvements branch from e97b79a to 5e0d577 Compare June 15, 2026 19:00
@philipr-za

philipr-za commented Jun 15, 2026

Copy link
Copy Markdown

ACK 5e0d577

The type inference that auto-magically figures out the sizes for split_array is neat!

@apoelstra

Copy link
Copy Markdown
Member Author

Yeah, these are great APIs and frustratingly they have been unstable for a long time and still not in stable https://doc.rust-lang.org/std/primitive.array.html#method.split_array_ref

@apoelstra

Copy link
Copy Markdown
Member Author

ACK 5e0d577; successfully ran local tests

@apoelstra apoelstra merged commit 9304937 into ElementsProject:master Jun 15, 2026
10 checks passed
apoelstra added a commit that referenced this pull request Jun 16, 2026
982a50f Disable fuzztest in CI (Philip Robinson)
ad00563 Bump version to 0.25.3 (Philip Robinson)
6275989 Fix docstring in blech32 (Philip Robinson)
675ca26 Pin deps for Fuzztest job (Philip Robinson)
ea42e11 add `--no-deps` to doc test (Philip Robinson)
70276cc pin bitcoin crates in test.sh (Philip Robinson)
2941a39 Remove `-- -D warnings` from test.sh (Philip Robinson)
250c4f6 pset: fix slicing in KeySource::deserialize (Philip Robinson)
b06dac9 address: do a better job slicing bech32 data (Philip Robinson)
2a8b46f transaction: use better error typing for pegin destructuring (Philip Robinson)
8b15649 blind: use array splitting in TxOut::unblind (fix potential DoS?) (Philip Robinson)
179c6da rewrite Address::from_base58 to eliminate all the unwraps (Philip Robinson)
8cd2a4d Add arrays.rs and slices.rs from bitcoin-internals 0.5 (Philip Robinson)

Pull request description:

  There are a number of useful improvements from #279 we would like to include in 0.25.x.

ACKs for top commit:
  apoelstra:
     utACK 982a50f

Tree-SHA512: 51f786c01982d107193f9e75be13a1087a24372664eb70fbbb0eb3ee3d2bfbf64b94ac573a22fe20d412c717f54d5c3a37a4010099fe6d56892fdd486e026c62
apoelstra added a commit that referenced this pull request Jun 16, 2026
6fee387 Bump version to 0.26.2 (Philip Robinson)
4428361 pset: fix slicing in KeySource::deserialize (Philip Robinson)
6e082a9 address: do a better job slicing bech32 data (Philip Robinson)
acf5bc8 transaction: use better error typing for pegin destructuring (Philip Robinson)
6659691 blind: use array splitting in TxOut::unblind (fix potential DoS?) (Philip Robinson)
6005720 rewrite Address::from_base58 to eliminate all the unwraps (Andrew Poelstra)
d216a7c Add arrays.rs and slices.rs from bitcoin-internals 0.5 (Philip Robinson)

Pull request description:

  There are a number of useful improvements from #279 we would like to include in 0.26.x.

ACKs for top commit:
  apoelstra:
    ACK 6fee387; successfully ran local tests

Tree-SHA512: dd5d142dd76cbe472c67823989f79ed1bb1024282b788cd840bd4cf71db95ef277aa4eeb416ebe829a01f4dd22ed672dbc293e181bddaa9ccb0814e47dd1cc19
@apoelstra apoelstra deleted the 2026-06/slicing-improvements branch June 16, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants