Skip to content

fix(serder): majority() uses banned unwrap_or(u64::MAX) sentinel conversion #153

Description

@joeldsouzax

Problem

majority() in cesr/src/serder/builder/icp.rs:69-73 converts usize -> u64 with the banned sentinel pattern:

pub(crate) fn majority(n: usize) -> u64 {
    let m = 1.max(n.div_ceil(2));
    // SAFETY: usize to u64 is lossless on all supported platforms (64-bit)
    u64::try_from(m).unwrap_or(u64::MAX)
}

Same class of defect fixed for ample() in #147 / #152: unwrap_or(sentinel) on a failed conversion is banned per the repo arithmetic-safety rules — on a hypothetical usize > 64-bit target it would silently cap the signing threshold to u64::MAX instead of surfacing an error. The // SAFETY comment is also misleading — that marker is conventionally reserved for unsafe blocks, and the claim it makes is exactly the kind of thing the type system should enforce rather than a comment.

Unlike #147 there is no numeric divergence: the formula max(1, ceil(n/2)) matches keripy's default signing threshold (eventing.py:459 @ de59bc7d: max(1, ceil(len(keys) / 2)); same shape at :471 for nsith and :488 for toad).

Fix

Mirror the #152 treatment of ample():

  • checked conversion with a typed error path (Result<u64, SerderError> with SerderError::Validation), or restructure so the conversion is infallible by construction
  • callers: InceptionBuilder/DelegatedInceptionBuilder/rotation builders' unwrap_or_else(|| Tholder::Simple(majority(...))) closures propagate through the builders' existing Result
  • drop the misleading // SAFETY comment

Acceptance

  • No unwrap_or/sentinel conversion in majority(); error path is typed and tested
  • Table test pinning majority(n) for n = 0..=13 against keripy's max(1, ceil(n/2)) expectations
  • Existing builder default-threshold tests stay green (no behavior change on realistic inputs)

Found while fixing #147 (see PR #152, "Out of scope" note).

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions