consensus_encoding: add u64 support to compact size by nyonson · Pull Request #5784 · rust-bitcoin/rust-bitcoin

nyonson · 2026-03-05T21:04:25Z

Add u64 support to the Compact Size codec since there are non prefix length use cases (#5762). Kind of related to #5752.

I believe this is the simplest way to add support, but the asymmetry kind of annoys me. On the encoding side, only a new constructor is necessary. But on the decoding side a new type is required since there is a 1:1 with the associated output. I don't think symmetry is a strong enough reason to put a type on the encoding side though.

I didn't put much though into the names, focusing on minimal change. But maybe it would be worth it to name each encoder/decoder after its use case instead of impl? So the existing one would be called something like LengthPrefixDecoder and so on.

The first commit moves the existing compact size stuff into its own ::compact_size module, to kind of corner this weirdness. This creates kind of a funky dependency graph with ::decode and ::decode::decoders (and the equivalent encoders side), but none of that is exposed from the crate since it is a private module.

tcharding · 2026-03-06T22:05:38Z

Are we writing the 'decoding a compact size should fail if value over 32MB' bug back in by providing the CompactSizeU64Decoder or does the p2p usecase not have this check in Core? If we are not, and there is legit use case for this type, I rekon we need some good docs on the type explaining how we got here and when to use the type. Otherwise users are just going to go 'yeah I want a u64 - YOLO'. Remember our mandate is 'write API's that are easy to use and hard to misuse'. Even when Core bites us in the arse ...

tcharding · 2026-03-06T22:08:30Z

Part of me wants to move all the compact size stuff into its own ::compact_size module

I rekon we should do this. The module layout in this crate is simply a code organisation thing so we are free to do it how we like. Top level private file compact_size.rs would serve use well I rekon. Do remember that private modules don't get their rustdocs included so if we add module level docs explaining all this decoding limit kerfuffle they won't be user facing.

nyonson · 2026-03-06T22:21:39Z

Part of me wants to move all the compact size stuff into its own ::compact_size module

I rekon we should do this. The module layout in this crate is simply a code organisation thing so we are free to do it how we like. Top level private file compact_size.rs would serve use well I rekon. Do remember that private modules don't get their rustdocs included so if we add module level docs explaining all this decoding limit kerfuffle they won't be user facing.

Cool, think I can tack on the move to ::compact_size here with another commit? Or will that be a pain to review?

tcharding · 2026-03-06T22:25:33Z

Do it, its easy to review if its a separate patch because of git zebra config. At the front of the PR or the back, its the same for me to review.

apoelstra · 2026-03-09T02:05:52Z

Needs rebase.

rustaceanrob · 2026-03-09T07:22:05Z

Per ServiceFlag, check out my comment pls

Bortlesboat

ACK ccbfdaf. Encoding/decoding is symmetric across all prefix boundaries, non-minimal rejection is correct at each tier, and the round-trip tests are thorough.

nyonson · 2026-03-10T03:46:50Z

 pub type bitcoin_primitives::Wtxid::Err = hex_conservative::error::DecodeFixedLengthBytesError
 pub type bitcoin_primitives::block::Block::Decoder = bitcoin_primitives::block::BlockDecoder
-pub type bitcoin_primitives::block::Block::Encoder<'e> where Self: 'e = bitcoin_consensus_encoding::encode::encoders::Encoder2<bitcoin_primitives::block::HeaderEncoder<'e>, bitcoin_consensus_encoding::encode::encoders::Encoder2<bitcoin_consensus_encoding::encode::encoders::CompactSizeEncoder, bitcoin_consensus_encoding::encode::encoders::SliceEncoder<'e, bitcoin_primitives::transaction::Transaction>>>
+pub type bitcoin_primitives::block::Block::Encoder<'e> where Self: 'e = bitcoin_consensus_encoding::encode::encoders::Encoder2<bitcoin_primitives::block::HeaderEncoder<'e>, bitcoin_consensus_encoding::encode::encoders::Encoder2<bitcoin_consensus_encoding::compact_size::CompactSizeEncoder, bitcoin_consensus_encoding::encode::encoders::SliceEncoder<'e, bitcoin_primitives::transaction::Transaction>>>


Hm, need to dig in to why these APIs changed.

EDIT: maybe just a limitation of the docs-as-type-checker system?

nyonson · 2026-03-10T04:29:41Z

cc51369: moved compact size stuff into its own module and beef'd up the docs.

For some reason this move triggered a mutant, not sure how yet, but tossed in a test to cover it. There is also a confusing diff in the primitives API files due to the move, but I think it is ok?

nyonson · 2026-03-10T13:54:34Z

Ah, the mutant timeouts are on master already, going to deal with those separately in #5806 and then rebase.

apoelstra · 2026-03-11T19:27:24Z

Needs rebase.

nyonson · 2026-03-12T16:27:43Z

fe83d4f: rebased and toned down some of the yelling in the docs

apoelstra

ACK fe83d4f; successfully ran local tests

apoelstra · 2026-03-14T12:41:11Z

@tcharding this is a nontrivial API change to consensus-encoding so we need your ACK

tcharding · 2026-03-16T03:53:26Z

-
-/// The maximum length of a compact size encoding.
-const SIZE: usize = 9;
+pub use crate::compact_size::CompactSizeEncoder;


I don't think we should do this. And if we do want to do it we should mirror it for decoders.

Was the concern here about the encoder constructors? Or did I just make the commits confusing?

Oh it was adding 'pub use' in encoders but the decoder is not re-exported in decoders. But I don't think we should do either, just the crate root is enough IMO.

ooo, and I missed this while reviewing this morning. Thanks for flagging it.

Ah I see now, yea, I did this really round-about. Will fix.

tcharding · 2026-03-16T06:35:59Z

Didn't finish review sorry mate, will come back tomorrow.

tcharding

ACK fe83d4f

tcharding · 2026-03-18T00:37:23Z

Sorry for taking so long.

nyonson · 2026-03-18T16:54:01Z

2c9b904: simplified the compact_size re-export from the root.

nyonson · 2026-03-18T17:00:11Z

 pub const bitcoin_hashes::HashEngine::LEN: usize
 pub const bitcoin_hashes::IsByteArray::LEN: usize
 pub const bitcoin_hashes::hash160::Hash::DISPLAY_BACKWARD: bool
+pub const bitcoin_hashes::hash160::Hash::LEN: usize


Huh, dunno why all these hashes changes showed up now.

Ah, the version of cargo-public-api is only set int he CI rust.yml workflow, it isn't shared for local dev. Need to run on cargo +nightly install --locked cargo-public-api --version 0.50.1 for now. The new cargo-rbmt tools command will handle this in the future.

Yep that bit me before, sorry I didn't notice during review.

Public API is unchanged; lib.rs re-exports all the same types.

nyonson · 2026-03-18T20:19:09Z

85a6da3: regenerated api files with v0.50.1

tcharding

ACK 85a6da3

apoelstra

ACK 85a6da3; successfully ran local tests

Manually revert 91a6e64 (`git revert` didn't work.) This was a backport of rust-bitcoin#5697 which added range checking to the `VarInt` type. On master we did this then realized there are legitimate use cases for the full range of a `u64`. We then added `u64` support to compact size (rust-bitcoin#5784). Note that we cannot break the API so the new stuff introduced in `0.32.9` has to stay in there. Comment the code as such. NB: 'compact size' is the new name for varint. This backport then broke downstream users.

Manually revert 91a6e64 Note that we cannot break the API so the new stuff introduced in `0.32.9` has to stay in there. Comment the code as such. This was a backport of rust-bitcoin#5697 which added range checking to the `VarInt` type. On master we did this then realized there are legitimate use cases for the full range of a `u64`. We then added `u64` support to compact size (rust-bitcoin#5784). NB: 'compact size' is the new name for varint. This backport then broke downstream users.

Manually revert 91a6e64 Note that we cannot break the API so the new stuff introduced in `0.32.9` has to stay in there. Comment the code as such. This was a backport of #5697 which added range checking to the `VarInt` type. On master we did this then realized there are legitimate use cases for the full range of a `u64`. We then added `u64` support to compact size (#5784). NB: 'compact size' is the new name for varint. This backport then broke downstream users.

3bcd7c9 Manually revert VarInt range check (Tobin C. Harding) Pull request description: Manually revert 91a6e64 Note that we cannot break the API so the new stuff introduced in `0.32.9` has to stay in there. Comment the code as such. This was a backport of #5697 which added range checking to the `VarInt` type. On master we did this then realized there are legitimate use cases for the full range of a `u64`. We then added `u64` support to compact size (#5784). NB: 'compact size' is the new name for varint. This backport then broke downstream users. Close: #6138 ACKs for top commit: apoelstra: ACK 3bcd7c9; successfully ran local tests Tree-SHA512: d01078f4f8fae59c439fd404a643abfb353bcec1757ebfbcabde487ff8620d927387f91d0e749a7bbd36163f2386290d301f17c0cff9e628e37585329daf116b

github-actions Bot added C-consensus_encoding PRs modifying the consensus-encoding crate test labels Mar 5, 2026

nyonson force-pushed the compact-size-u64 branch 2 times, most recently from 82364b5 to ccbfdaf Compare March 5, 2026 21:50

tcharding mentioned this pull request Mar 9, 2026

Release tracking PR: The whole stack up to bitcoin 0.34.0-beta #5796

Closed

nyonson mentioned this pull request Mar 9, 2026

Tracking issue: Release consensus_encoding 1.0.0 #5686

Closed

3 tasks

Bortlesboat approved these changes Mar 9, 2026

View reviewed changes

nyonson force-pushed the compact-size-u64 branch 3 times, most recently from 66d4143 to 7c28f91 Compare March 10, 2026 03:38

nyonson commented Mar 10, 2026

View reviewed changes

nyonson force-pushed the compact-size-u64 branch 3 times, most recently from e1322cf to cc51369 Compare March 10, 2026 04:25

nyonson force-pushed the compact-size-u64 branch from cc51369 to 093025b Compare March 10, 2026 13:53

github-actions Bot added the C-primitives label Mar 10, 2026

nyonson force-pushed the compact-size-u64 branch from 093025b to 1596b2c Compare March 10, 2026 17:00

github-actions Bot removed the C-primitives label Mar 10, 2026

nyonson force-pushed the compact-size-u64 branch from 1596b2c to 3be0feb Compare March 10, 2026 17:12

yancyribbens mentioned this pull request Mar 11, 2026

p2p: Implement encoding traits for AddrV2Payload #5762

Merged

nyonson force-pushed the compact-size-u64 branch from 3be0feb to 5535272 Compare March 12, 2026 02:44

nyonson force-pushed the compact-size-u64 branch from 5535272 to fe83d4f Compare March 12, 2026 16:26

apoelstra approved these changes Mar 13, 2026

View reviewed changes

tcharding reviewed Mar 16, 2026

View reviewed changes

tcharding approved these changes Mar 18, 2026

View reviewed changes

nyonson force-pushed the compact-size-u64 branch from fe83d4f to 2c9b904 Compare March 18, 2026 16:52

nyonson commented Mar 18, 2026

View reviewed changes

nyonson added 6 commits March 18, 2026 13:04

consensus_encoding: move compact size into its own module

a06bf03

Public API is unchanged; lib.rs re-exports all the same types.

consensus_encoding: add a u64 constructor to compact size encoder

773b4a3

consensus_encoding: add a u64 decoder for compact size

77a60ae

consensus_encoding: add round trip test for compact size codecs

4614515

consensus_encoding: const the compact size prefix values

bb7bf48

api: update for CompactSizeU64Decoder

85a6da3

nyonson force-pushed the compact-size-u64 branch from 2c9b904 to 85a6da3 Compare March 18, 2026 20:16

tcharding approved these changes Mar 19, 2026

View reviewed changes

apoelstra approved these changes Mar 21, 2026

View reviewed changes

apoelstra merged commit d283a89 into rust-bitcoin:master Mar 21, 2026
28 checks passed

apoelstra mentioned this pull request May 6, 2026

VarInt breaking change in 0.32.9 #6138

Open

tcharding mentioned this pull request May 14, 2026

Manually revert VarInt range check #6182

Merged

Uh oh!

Conversation

nyonson commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tcharding commented Mar 6, 2026

Uh oh!

tcharding commented Mar 6, 2026

Uh oh!

nyonson commented Mar 6, 2026

Uh oh!

tcharding commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apoelstra commented Mar 9, 2026

Uh oh!

rustaceanrob commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bortlesboat left a comment

Choose a reason for hiding this comment

Uh oh!

nyonson Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nyonson commented Mar 10, 2026

Uh oh!

nyonson commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apoelstra commented Mar 11, 2026

Uh oh!

nyonson commented Mar 12, 2026

Uh oh!

apoelstra left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra commented Mar 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tcharding commented Mar 16, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding commented Mar 18, 2026

Uh oh!

nyonson commented Mar 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nyonson commented Mar 18, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

nyonson commented Mar 5, 2026 •

edited

Loading

tcharding commented Mar 6, 2026 •

edited

Loading

rustaceanrob commented Mar 9, 2026 •

edited

Loading

nyonson Mar 10, 2026 •

edited

Loading

nyonson commented Mar 10, 2026 •

edited

Loading