primitives: Remove alloc requirement from HexPrimitive by mpbagot · Pull Request #5480 · rust-bitcoin/rust-bitcoin

mpbagot · 2026-01-05T08:48:05Z

The HexPrimitive type in primitives provides encoding and decoding functionality to/from hex for arbitrary Encodable/Decodable types in primitives. Currently, this type relies on alloc to implement this functionality, which is less than ideal for types that don't otherwise require alloc (e.g. Header).

Patch 1 converts the decoding logic in from_str to be alloc-less, and adjusts the feature gates on the relevant impls on Header.
Patch 2 converts the encoding logic in hex_write_with_case to be alloc-less and adjusts the relevant feature gates.
Patch 3 moves the hex_codec module from lib.rs into a new file. No logic changes.
Patch 4 rearranges the hex_codec module to match the style of other modules.
Patch 5 adjusts function docs and replaces all pub visibility with pub(crate) in hex_codec.

Closes #5469

apoelstra · 2026-01-05T21:44:31Z

Merged #5381. This needs rebase now. (Sorry, I think maybe the formatting PR conflicted with a ton of stuff.)

tcharding

ACK 97b26bf

tcharding · 2026-01-15T01:53:42Z

primitives/src/hex_codec.rs

+    let extra_len = if f.alternate() { 2 } else { 0 };
+    let total_len = len + extra_len;


I already wrote it today so I won't hark on the point but these two lines are new and not a code move but are in commit titled primitives: Move hex_codec to independent module.

I must have messed up a rebase/amend at some point. I thought that I had definitely made the last patch move-only.

The new patches move this and one other difference into the second patch. The third patch should be only a straight code move now.

tcharding · 2026-01-15T01:54:56Z

FTR the buffer stuff looked like it should be supported by the language but from a brief look at Iterator there is only array_chunks that is a nightly feature.

tcharding · 2026-01-15T01:55:05Z

Good effort @mpbagot

tcharding

ACK 852adb5

tcharding · 2026-01-15T23:13:26Z

primitives/src/lib.rs

 pub mod witness;

+#[cfg(feature = "hex")]
+pub(crate) mod hex_codec;


Suggested change

pub(crate) mod hex_codec;

mod hex_codec;

pub(crate) mod is equivalent to a private module (unless I'm missing something).

You're right. It is. I tend to try and be more explicit with things than I really need to be.

hmm, that is fair and we often do such things (e.g., our usage of ref in matching: match *self { Self::Foo(ref e)).

However uniformity is king IMO, if you want to introduce this style I rekon either raise an issue to discuss it or raise a PR that does it everywhere in the repo. If you feel super strongly about it I probably wouldn't NACK it but FWIW it doesn't seem useful to me because it is such a basic language feature. Don't let me hold you back though.

tcharding · 2026-01-15T23:14:19Z

primitives/src/hex_codec.rs

@@ -0,0 +1,193 @@
+use core::fmt;


This file needs an SPDX licence string.

Also some module level docs would be great please. FYI the linter does not enforce this because its a private module.

And since I'm doing a nit-picky as all hell review:

Can you re-order the file so that HexPrimitive is at the top, the error code is at the bottom, and the hex_write_with_case is below where its called (the fmt::LowerHex and fmt::UpperHex impls).

This is based on the principle that the most important thing goes at the top.

This is one instance where an additional patch on top of the current ones is the best because it preserves the move-only nature of the existing one. Could also be a follow up PR but I don't rekon Andrew is going to get to merging this before you get back to it.

I've done this as two new patches, since adding module/license docs is a different operation than rearranging code. If you'd rather it be squashed or reordered, I can do so. Is it acceptable for code to exist for a patch without the license string?

tcharding

ACK ddec4b9

tcharding · 2026-01-18T21:43:57Z

Thanks for humouring me and re-arranging the code. I'm aware that not all devs are as retarded as me and have to have everything exactly in the right place.

mpbagot · 2026-01-23T00:56:53Z

Rebased to fix a conflict in primitives/src/block.rs.

mpbagot · 2026-01-24T03:08:12Z

Rebased to fix a conflict in primitives/src/block.rs.

tcharding

ACK 7033611

mpbagot · 2026-02-02T05:51:46Z

Rebased to fix a conflict in primitives/src/lib.rs, squashed license + doc patch into move patch, and fixed a bug with the no-alloc from_str that Jamil's tests caught.

tcharding

ACK 5da334d

apoelstra · 2026-02-03T17:28:49Z

Needs rebase again (in the API files, ugh)

tcharding

ACK 6b38d69

apoelstra · 2026-02-14T04:01:01Z

Needs rebase.

tcharding

ACK b24baea

apoelstra · 2026-02-21T02:52:29Z

In 5c22144:

primitives does not build with cargo test --no-default-features --features alloc. (This affects all the commits except the tip I think)

mpbagot · 2026-02-21T03:31:47Z

Rebased on latest master and should have fixed rbmt test runs for all commits in the chain.

tcharding · 2026-02-23T22:33:33Z

primitives/src/block.rs

-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        fmt::UpperHex::fmt(&crate::hex_codec::HexPrimitive(self), f)
-    }
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { crate::hex_codec::HexPrimitive(self).fmt(f) }


How does the compiler resolve this call to fmt? Said another way, looking at this change I can't tell if the UpperHex::fmt method is the one being called and not for example Display::fmt?

But it does because we have a test

// And these should yield uppercase hex let upper_hex = lower_hex.to_ascii_uppercase(); assert_eq!(upper_hex, format!("{:X}", header));

Is the compiler using trait method function name because the implementation is a simple call through? This seems like a lot of magic. Or is it something to do with the fact that we call the method chained onto the constructor call?

tcharding · 2026-02-23T23:16:52Z

I've reviewed this much more in depth now. For the coming suggestions feel free to ignore, use, or do follow up. While reviewing I hacked a bunch locally.

tcharding · 2026-02-23T23:21:46Z

I moved the logic from hex_write_with_case to an inherent method on HexPrimitive and then called through like this

impl<T: Encodable + Decodable> fmt::Display for HexPrimitive<'_, T> {
    #[inline]
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.fmt(f, Case::Lower) }
}

In case its useful:

    /// Writes an Encodable object to the given formatter in the requested case.
    fn fmt(&self, f: &mut fmt::Formatter, case: Case) -> fmt::Result {

I messed with the HexPrimitive type like this:

/// Hex encoding wrapper type for `Encodable` + `Decodable` types.
///
/// Implements `Display`, `Debug`, `LowerHex`, and `UpperHex` as well as an inherent `from_str`
/// method that returns `T`.
pub(crate) struct HexPrimitive<'a, T: Encodable + Decodable>(pub &'a T);

I messed around with from_str function like this:

    /// Parses a given string into an instance of the type `T`.
    ///
    /// Since `FromStr` would return an instance of `Self` and thus a &T, this function
    /// is implemented directly on the struct to return the owned instance of T.
    /// Other `FromStr` implementations can directly return the result of
    /// [`HexPrimitive::from_str`].
    ///
    /// # Errors
    ///
    /// * `OddLength` or `InvalidChar` if decode the hex string to bytes fails.
    /// * `Decode` if consensus decoding the hex-decoded bytes fails.
    pub(crate) fn from_str(s: &str) -> Result<T, ParsePrimitiveError<T>> {
        // The iterators are ustable (exist but private in 1.0).
        let iter = hex_unstable::HexToBytesIter::new(s).map_err(ParsePrimitiveError::from)?;

I removed any pub from hex_codec and used pub(crate) just to be explicit.

And finally one last nit, I imported HexPrimitive anywhere its used instead of using crate::hex_codec::HexPrimitive.

tcharding · 2026-02-23T23:23:40Z

I investigated the error stuff from hex and you've got it spot on, its the hex-conservative v1.0 crate that is making this code more ugly than it needs to be. \me hangs his head in shame.

tcharding · 2026-02-24T00:27:32Z

primitives/src/hex_codec.rs

+    /// [`ParsePrimitiveError::OddLengthString`] if the input string is an odd length.
+    /// [`ParsePrimitiveError::Decode`] if an error occurs during decoding of the object.
+    pub(crate) fn from_str(s: &str) -> Result<T, ParsePrimitiveError<T>> {
+        let iter = hex_unstable::HexToBytesIter::new(s).map_err(ParsePrimitiveError::from)?;


I did rust-bitcoin/hex-conservative#203 to help with this change. No need to wait though because all the unstable stuff is hidden.

The from_str implementation for parsing types from hex strings currently relies on alloc due to the use of hex::decode_to_vec. This locks Header FromStr decoding behind alloc. Implement from_str without alloc, and adjust feature gates on HeaderDecoder to match new relaxed requirements.

The Display/Debug/UpperHex/LowerHex implementations on HexPrimitive all relied on alloc due to the implementation of hex_write_with_case. This locks LowerHex and UpperHex on Header behind alloc, and led to an unnecessary custom implementation of Display. Change hex_write_with_case to remove alloc requirement, and remove corresponding feature gates. Move hex_write_with_case to inherent function on HexPrimitive, and rename to fmt_hex. Replace Display impl on Header with call through to HexPrimitive. Remove feature gates on Header Upper/LowerHex.

The hex_codec module in lib.rs of primitives now makes up 2/3rds of the file. At this scale, it is better suited to be moved into its own file. Move the hex_codec module in lib.rs to a new module file, and import it as a pub(crate) mod.

The hex_codec module was created over various patches and thus doesn't follow best practice for ordering. Errors should be near the bottom, with important types near the top. Rearrange hex_codec to put HexPrimitive at the top, and ParsePrimitiveError at the bottom.

The hex_codec module should be entirely hidden to outside the crate. While all types are pub(crate), the inner field of HexPrimitive was pub.

mpbagot · 2026-02-24T04:26:39Z

I've incorporated most of your suggestions, @tcharding. I changed the fmt inherent function to fmt_hex. I know the function signature already differs from the fmt traits' fmt functions, but I feel like there's getting to be too much pollution with the plain name fmt.

tcharding

ACK 118791e

apoelstra

ACK 118791e; successfully ran local tests

github-actions bot added C-bitcoin PRs modifying the bitcoin crate C-primitives labels Jan 5, 2026

mpbagot force-pushed the feature/hex-no-alloc branch from 3b8b8ad to 97b26bf Compare January 6, 2026 06:24

github-actions bot removed the C-bitcoin PRs modifying the bitcoin crate label Jan 6, 2026

mpbagot marked this pull request as ready for review January 6, 2026 07:43

tcharding previously approved these changes Jan 15, 2026

View reviewed changes

tcharding reviewed Jan 15, 2026

View reviewed changes

mpbagot dismissed tcharding’s stale review via 852adb5 January 15, 2026 07:08

mpbagot force-pushed the feature/hex-no-alloc branch from 97b26bf to 852adb5 Compare January 15, 2026 07:08

tcharding previously approved these changes Jan 15, 2026

View reviewed changes

tcharding reviewed Jan 15, 2026

View reviewed changes

mpbagot dismissed tcharding’s stale review via ddec4b9 January 16, 2026 05:59

mpbagot force-pushed the feature/hex-no-alloc branch from 852adb5 to ddec4b9 Compare January 16, 2026 05:59

tcharding previously approved these changes Jan 18, 2026

View reviewed changes

mpbagot mentioned this pull request Jan 23, 2026

primitives: Improve test coverage of ParsePrimitiveError #5558

Merged

mpbagot dismissed tcharding’s stale review via 42deb5a January 23, 2026 00:56

mpbagot force-pushed the feature/hex-no-alloc branch from ddec4b9 to 42deb5a Compare January 23, 2026 00:56

mpbagot force-pushed the feature/hex-no-alloc branch from 42deb5a to 7033611 Compare January 24, 2026 03:07

tcharding previously approved these changes Jan 27, 2026

View reviewed changes

mpbagot dismissed tcharding’s stale review via 5da334d February 2, 2026 05:51

mpbagot force-pushed the feature/hex-no-alloc branch from 7033611 to 5da334d Compare February 2, 2026 05:51

tcharding previously approved these changes Feb 2, 2026

View reviewed changes

mpbagot dismissed tcharding’s stale review via 6b38d69 February 4, 2026 00:00

mpbagot force-pushed the feature/hex-no-alloc branch from 5da334d to 6b38d69 Compare February 4, 2026 00:00

tcharding previously approved these changes Feb 4, 2026

View reviewed changes

mpbagot dismissed tcharding’s stale review via b24baea February 14, 2026 05:08

mpbagot force-pushed the feature/hex-no-alloc branch from 6b38d69 to b24baea Compare February 14, 2026 05:08

tcharding approved these changes Feb 17, 2026

View reviewed changes

mpbagot mentioned this pull request Feb 19, 2026

Implement stringly traits for Block using hex #5703

Merged

mpbagot force-pushed the feature/hex-no-alloc branch from b24baea to 0dfb450 Compare February 21, 2026 03:30

mpbagot force-pushed the feature/hex-no-alloc branch from 0dfb450 to 1b3958b Compare February 23, 2026 02:14

tcharding reviewed Feb 23, 2026

View reviewed changes

tcharding reviewed Feb 24, 2026

View reviewed changes

mpbagot added 5 commits February 24, 2026 14:52

primitives: Move hex_codec to independent module

58cf501

The hex_codec module in lib.rs of primitives now makes up 2/3rds of the file. At this scale, it is better suited to be moved into its own file. Move the hex_codec module in lib.rs to a new module file, and import it as a pub(crate) mod.

Clean up function comments and visibility in hex_codec

118791e

The hex_codec module should be entirely hidden to outside the crate. While all types are pub(crate), the inner field of HexPrimitive was pub.

mpbagot force-pushed the feature/hex-no-alloc branch from 1b3958b to 118791e Compare February 24, 2026 04:22

tcharding approved these changes Feb 24, 2026

View reviewed changes

apoelstra approved these changes Mar 2, 2026

View reviewed changes

apoelstra merged commit b7ab8bb into rust-bitcoin:master Mar 3, 2026
28 checks passed

mpbagot mentioned this pull request Mar 3, 2026

primitives: Update API files #5764

Merged

		let extra_len = if f.alternate() { 2 } else { 0 };
		let total_len = len + extra_len;

Conversation

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

Uh oh!

Uh oh!

apoelstra commented Jan 5, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mpbagot Jan 15, 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!

tcharding commented Jan 15, 2026

Uh oh!

tcharding commented Jan 15, 2026

Uh oh!

tcharding left a comment

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!

Choose a reason for hiding this comment

Uh oh!

tcharding Jan 15, 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!

mpbagot Jan 16, 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!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding commented Jan 18, 2026

Uh oh!

mpbagot commented Jan 23, 2026

Uh oh!

mpbagot commented Jan 24, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

mpbagot commented Feb 2, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra commented Feb 3, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra commented Feb 14, 2026

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

apoelstra commented Feb 21, 2026

Uh oh!

mpbagot commented Feb 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tcharding Feb 23, 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!

tcharding commented Feb 23, 2026

mpbagot commented Jan 5, 2026 •

edited

Loading

mpbagot Jan 15, 2026 •

edited

Loading

tcharding Jan 15, 2026 •

edited

Loading

mpbagot Jan 16, 2026 •

edited

Loading

tcharding Feb 23, 2026 •

edited

Loading

tcharding commented Feb 23, 2026 •

edited

Loading