Skip to content

fix(random): random_bit_list should set exact length#318

Merged
iamquang95 merged 1 commit intomainfrom
iamquang95/bug/random-bit-list
Apr 10, 2026
Merged

fix(random): random_bit_list should set exact length#318
iamquang95 merged 1 commit intomainfrom
iamquang95/bug/random-bit-list

Conversation

@iamquang95
Copy link
Copy Markdown
Collaborator

Should fix this: #317

@iamquang95 iamquang95 marked this pull request as ready for review April 10, 2026 12:07
@iamquang95
Copy link
Copy Markdown
Collaborator Author

@claude

@claude
Copy link
Copy Markdown

claude bot commented Apr 10, 2026

Claude finished @iamquang95's task in 1m 17s —— View job


Code Review

  • Read changed file
  • Check Go reference implementation
  • Analyze correctness of the fix
  • Post review feedback

Summary

The fix correctly addresses a real bug where random_bit_list could set fewer than length bits due to duplicate indices. The original code looped length times, but could land on the same bit multiple times via rng.gen::<usize>() % 256, giving you a bitlist with fewer set bits than requested.


Analysis

The root cause (original code):

// Old: duplicate bit_idx values possible → fewer than `length` bits actually set
for _ in 0..length {
    let bit_idx = rng.r#gen::<usize>() % 256;
    bytes[byte_idx] |= 1 << bit_offset;
}

The fix:

// New: index::sample guarantees `length` distinct indices
for bit_idx in index::sample(&mut rng, 256, length) { ... }

rand::seq::index::sample draws length unique indices from [0, 256) without replacement — exactly what's needed here. This is the correct, idiomatic fix.


Notes

1. Intentional divergence from Go — The Go reference (charon/testutil/random.go) has the same duplicate-index issue:

func RandomBitList(length int) bitfield.Bitlist {
    size := 256
    resp := bitfield.NewBitlist(uint64(size))
    for range length {
        resp.SetBitAt(uint64(rand.Intn(size)), true)  // duplicates possible
    }
    return resp
}

This PR makes Pluto's version stricter than the Go counterpart. Per AGENTS.md, the default is functional equivalence with Go, but since this is test utility code and the Go version's behavior appears to be a latent bug rather than an intentional design choice, the fix is justified. The overall test semantics (generating a bitlist with some bits set) are preserved.

2. The assert! guard is appropriate — Calling index::sample(rng, 256, n) with n > 256 would panic with a cryptic message from inside rand. The explicit assert! with a clear message is good practice.

3. Edge caseslength = 0 produces index::sample(&mut rng, 256, 0) which returns an empty iterator, leaving bytes all-zero. That's correct behavior.


Verdict

The fix is correct, clean, and idiomatic. No issues found. ✅

@iamquang95 iamquang95 merged commit 90236b3 into main Apr 10, 2026
13 of 15 checks passed
@iamquang95 iamquang95 deleted the iamquang95/bug/random-bit-list branch April 10, 2026 13:25
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.

3 participants