Skip to content

Implement map for Bitfield#81

Open
dknopik wants to merge 2 commits intosigp:mainfrom
dknopik:map
Open

Implement map for Bitfield#81
dknopik wants to merge 2 commits intosigp:mainfrom
dknopik:map

Conversation

@dknopik
Copy link
Copy Markdown
Member

@dknopik dknopik commented Apr 17, 2026

This one might be a bit overkill...

Bitfields have a maximum or fixed size. When handling existing bitfields and creating other bitfields of the same size, we have annoying code such as this:

let existing_bitfield: Bitfield<Variable<U10>> = ...;
let new_bitfield = Bitfield::<Variable<U10>>::with_capacity(existing_bitfield.len()).expect("as the types are the same, len is guaranteed to be small enough");

The expect is not really nice to see, and this is brittle of one of the types changes. This new map function would allow to clone a bitfield while infallibly setting the data as we desire, and the type system would let us know if one of the types change.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.47%. Comparing base (d3ecadb) to head (5beff5d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
+ Coverage   82.36%   82.47%   +0.10%     
==========================================
  Files          13       13              
  Lines        1140     1147       +7     
==========================================
+ Hits          939      946       +7     
  Misses        201      201              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the main motivation for this to just be able to create new bitlists with the same length as existing ones without needing expect? Using map for this seems like a pretty abstract solution.

A simpler solution could be to just add a dedicated clone_zeroed method or something which would create a fresh bitlist with the existing length.

I do like this map impl though, it's very cute and I'd be happy to include this if it actually has use for more than just creating fresh bitlists.

@dknopik
Copy link
Copy Markdown
Member Author

dknopik commented Apr 22, 2026

Haha, yeah, maybe it is too abstract. I think what happened here is that this was one of my initial ideas for the not problem, but changed my mind and did #80 instead. I also think something similar would be more useful for VariableList, and an argument could be made that also having map on what is essentially a packed VariableList<bool, N> would be good for consistency.

clone_zeroed sounds good to me regardless, and would be sufficient for my use case in connection with not, as I actually need an all ones bitmap (map(|_| true)).

Other use cases for this might be something like a bitwise operation while avoiding to create another bit vector beforehand:

fn add_attestation_idxs_to_bits(bits: Bitfield<Variable<U256>>, attestation_indices: BTreeMap<usize>) -> Bitfield<Variable<U256>> {
    let mut n = 0;
    bits.map(|b| {
        let ret = attestation_indices.contains(n) || b;
        n += 1;
        ret
    })
}

A map_inplace variant might be useful as well if we decide to go ahead with this...

@dknopik dknopik mentioned this pull request Apr 24, 2026
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