CommitWatch RSS
AI-generated analysisPublished automatically and not human-verified. Validated context appears in community notes below.
← Watch feed
Informational 18 Bitcoin

Merge rust-bitcoin/rust-bitcoin#6690: consensus_encoding: expose lower level encoder/decoder interfaces

RB76aac6ea View source ↗
Public commit record

What the developer wrote

Authored by Andrew Poelstra

100/100 · Strong
Merge rust-bitcoin/rust-bitcoin#6690: consensus_encoding: expose lower level encoder/decoder interfaces

9d91a6a0bb1e44877e00a3db667ae8d8f1da2009 api: update files (Nick Johnson)
c5bb5fade04716258e6521d4815e781a4ef8a4ab consensus_encoding: expose vec decoder drivers (Nick Johnson)
e5713bdc687e2c52b5097a4a3b1e4b8abe30315c consensus_encoding: add an encoder iterator driver (Nick Johnson)

Pull request description:

From the doc findings in https://github.com/rust-bitcoin/rust-bitcoin/pull/6651, there are only a handful (like, 2.5) of collection-based types in `consensus_encoding` which are tied to the higher level consensus codec `Encode`/`Decode` traits. These two patches expose the internals of the lower-level `Encoder`/`Decoder` traits which is a nice to have in rust-psbt where simple wrappers could then be defined for `PsbtEncode`/`PsbtDecode`.

## Encoding

I first tried to make some sort of `SliceEncoderWith` type following the pattern on the decoding-side, but this gets hairy due to the lifetimes present on the encoding-side. So I turned to `push_decode` and saw how Kixunil used an [iterator pattern there](https://github.com/Kixunil/push_decode/blob/master/src/encoders/iter.rs). I copied that over with a few tweaks and updated `SliceEncoder` to delegate to it.

One question I have here is that the new `IterEncoder` type defensively fuses the input iterator `let mut iter = iter.into_iter().fuse();`. I figure this is best practice, but it is useless for the existing `SliceEncoder` and there might be a performance hit?

The biggest change from `push_decode` is dropping the `I::Item: Into<E>, E: Encoder` bound and instead have an explicit type mapping, `Encoders`, for the type glue. This isn't a huge deal since not exposed to the caller, but it is what connects the `IterEncoder` logic to the existing `SliceEncoder`. But here are the details.

The little wrapper class in action:

```rust
IterEncoder::new(Encoders::new(txs))
```

vs. potentially the `Into` approach with a turbofish:

```rust
IterEncoder::<_, TransactionEncoder>::new(txs.iter())
```

I think to avoid having to add some `From` impl manually to every existing encodable type, we would want some sort of blanket impl. As far as I can tell, the type system can't handle this because of the orphan rule. `From` is external *and* `<T as Encode>::Encoder<'e>` is uncovered, external. We could add a new trait, but that would have the same issues outside of `consensus_encoding`. Maybe I am missing a better pattern for this though.

```rust
impl<'e, T: Encode> From<&'e T> for <T as Encode>::Encoder<'e> {
fn from(t: &'e T) -> Self { t.encoder() }
}
```

## Decoding

The decoding side is simpler on paper since there are no lifetimes to worry about, but I ended up exposing two new types for necessary flexibility: `VecDecoderWith` and `ExactVecDecoderWith`. While the API expands, these all delegate to each other so I don't think a large maintenance burden.

I maintained the use of a `Default` bound like we have on our existing `Decoder` drivers. Recently convinced myself that this is fine: https://github.com/rust-bitcoin/rust-bitcoin/issues/6674.


ACKs for top commit:
apoelstra:
ACK 9d91a6a0bb1e44877e00a3db667ae8d8f1da2009; successfully ran local tests


Tree-SHA512: e8e532139ebbde24c0ee5995d1e13eecce22716d8fe5a7dbe44c6a95e6243a8b47899cbc4c34dd70ebc3fba4d4daa17f7b41ddba8313a5ed970cbfefd9249f8b
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
The short version

What changed, and why it matters

This commit is a routine library refactor: it exposes lower-level building blocks for encoding and decoding lists of Bitcoin data, and rewrites existing list encoders/decoders to use those new building blocks. There is no direct security fix or vulnerability being patched. It is an API expansion and internal cleanup in the consensus_encoding crate.

Recommended action

No immediate action required. Treat as a normal dependency/API update. If using the new public types, review their documented behavior and ensure downstream code does not bypass existing length limits.

Security signals we found

01

Refactor only: no new parsing rules or relaxed bounds

02

Existing MAX_VEC_SIZE limit retained in VecDecoderWith

03

No mention of security, CVE, advisory, or vulnerability in commit message or PR description

04

No changes to unsafe code, cryptography, or consensus-critical validation

Risk score

Why this scored 18/100

Our methodology →
Potential impact 2/30
Exploitability 1/25
Stealth signal 1/15
Affected reach 3/15
Confidence 8/10
Evidence quality 3/5
Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

More from rust-bitcoin

Related watches

Informational 15 AI analysisMessage 96 · Strong
RB Rust Bitcoinrust-bitcoin BitcoinCryptographic libraries

Merge rust-bitcoin/rust-bitcoin#6713: build(deps): bump cargo-bins/cargo-binstall from 1.19.1 to 1.20.0

This is a routine update to a GitHub Actions workflow that bumps the version of a helper tool called cargo-binstall from 1.19.1 to 1.20.0. The tool is only used during automated version-compatibility checks in CI and is not part of the act…

617302d3by Andrew Poelstra+1−11 file
No security note in commit
Open full analysis →Source commit ↗
Informational 15 AI analysisMessage 96 · Strong
RB Rust Bitcoinrust-bitcoin BitcoinCryptographic libraries

Merge rust-bitcoin/rust-bitcoin#6714: build(deps): bump taiki-e/install-action from 2.81.4 to 2.81.10

This is a routine Dependabot update that changes the version of a third-party GitHub Action used only in automated testing workflows. It does not touch the actual Bitcoin library code, user-facing APIs, or any shipped software. There is no…

e8250092by Andrew Poelstra+2−22 files
No security note in commit
Open full analysis →Source commit ↗
Informational 15 AI analysisMessage 100 · Strong
RB Rust Bitcoinrust-bitcoin BitcoinCryptographic libraries

Merge rust-bitcoin/rust-bitcoin#6592: primitives: Update API test

This commit only updates an internal test file (primitives/tests/api.rs) that checks whether public types implement expected Rust traits and are exported correctly. It adds coverage for new types introduced in earlier development and reorg…

0969e567by Andrew Poelstra+322−461 file
No security note in commit
Open full analysis →Source commit ↗