Skip to content

Absorb elements from the validated input slice - #41

Open
HDauven wants to merge 1 commit into
mainfrom
fix/absorb-validated-input
Open

Absorb elements from the validated input slice#41
HDauven wants to merge 1 commit into
mainfrom
fix/absorb-validated-input

Conversation

@HDauven

@HDauven HDauven commented Sep 12, 2026

Copy link
Copy Markdown
Member

Summary

Borrow the input once in Sponge::absorb, so the slice checked for sufficient length is also the slice that is absorbed.

Previously, the two as_ref() calls could return different slices. A safe Rust adapter that returns two elements on its first call and an empty slice on the second makes absorb(2, ...) report success without absorbing either element.

This behavior dates to SAFE's initial implementation; it was not introduced by #40. This PR is based directly on current main, after #40.

Changes

  • Reuse the validated slice throughout absorption.
  • Add one regression using the existing Rotate test backend.
  • Add an Unreleased changelog entry.

No new API, dependencies, allocations or changes to field/permutation logic. Ordinary slice/array inputs retain their existing behavior. The diff is three files, +23/-2 lines.

Validation

  • The exact regression fails on unmodified main in debug and release (expected 9, actual 0), and passes with the fix.
  • make test, make cq, no-default-features tests, no_std builds with and without encryption, docs and frozen-vector checks pass.
  • Selected local Poseidon, Phoenix-core, Merkle and Dusk-core test suites pass with this SAFE candidate patched in.
  • Additional local checks passed: one million seeded model inputs in each of debug, release and 32-bit execution; 1.54 million coverage-guided executions; allocation/cleanup tests and focused Miri checks. Selected deliberately faulty implementations were detected.

Investigation harnesses, fuzz corpora and mutation tests are intentionally outside this PR. These are bounded correctness checks, not a proof of cryptographic security; no deployed impact or authentication bypass was demonstrated.

Read AsRef input once so validation and absorption use the same view. A changing-view adapter could previously satisfy the length check and then supply no elements while absorb still reported success.

Add a regression using the existing Rotate backend and record the fix in the changelog.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved review issues remain.

Pull request overview

Fixes Sponge::absorb by reusing the slice validated for length, preventing stateful AsRef inputs from being inconsistently handled.

Changes:

  • Reuse the validated input slice.
  • Add a regression test for stateful AsRef inputs.
  • Document the fix in the changelog.
File summaries
File Description
tests/sponge.rs Adds regression coverage.
src/sponge.rs Reuses the validated input slice.
CHANGELOG.md Records the fix.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@HDauven
HDauven added this pull request to stack #43 September 12, 2026 22:16
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