Skip to content

fix(types): prevent panic in confirmation_time on empty attestations#176

Open
giwaov wants to merge 1 commit intopodnetwork:mainfrom
giwaov:fix/confirmation-time-panic
Open

fix(types): prevent panic in confirmation_time on empty attestations#176
giwaov wants to merge 1 commit intopodnetwork:mainfrom
giwaov:fix/confirmation-time-panic

Conversation

@giwaov
Copy link
Contributor

@giwaov giwaov commented Mar 2, 2026

Summary

VerifiableLog::confirmation_time() indexed into the attestations slice using attestations[len / 2] without checking whether the slice was empty. When attestations.len() is 0, this evaluates to attestations[0] and panics with an index-out-of-bounds.

Both unit tests in types/src/ledger/log.rs construct a VerifiableLog with an empty attestations vec, so this is a realistic code path — not just a theoretical edge case.

Change

Return Option<Timestamp> instead of Timestamp, using slice::get so callers handle the absent case explicitly rather than the process crashing.

// Before
pub fn confirmation_time(&self) -> Timestamp {
    let num_attestations = self.pod_metadata.attestations.len();
    self.pod_metadata.attestations[num_attestations / 2].timestamp
}

// After
pub fn confirmation_time(&self) -> Option<Timestamp> {
    let attestations = &self.pod_metadata.attestations;
    attestations.get(attestations.len() / 2).map(|a| a.timestamp)
}

Test plan

  • Confirm confirmation_time() returns None when called on a VerifiableLog with no attestations (as constructed in existing unit tests)
  • Confirm all callers of confirmation_time() are updated to handle Option<Timestamp>

`confirmation_time()` indexed into `attestations` using
`attestations[len / 2]` without checking whether the slice was empty.
When `attestations` is empty this panics with an index-out-of-bounds.

Both unit tests in the same file construct `VerifiableLog` with an
empty attestations vec, so this is a realistic code path.

Change the return type to `Option<Timestamp>` and use `slice::get` so
callers can handle the absent case without the process crashing.
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.

1 participant