Skip to content

Remove unnecessary verify_string_with_params wrapper#240

Merged
fergusfinn merged 2 commits into
faster-password-testsfrom
copilot/sub-pr-239
Nov 26, 2025
Merged

Remove unnecessary verify_string_with_params wrapper#240
fergusfinn merged 2 commits into
faster-password-testsfrom
copilot/sub-pr-239

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 26, 2025

Addresses review feedback on #239 to remove a pointless wrapper function.

verify_string_with_params was unnecessary because Argon2 verification always extracts parameters from the hash itself—the params argument was ignored.

Changes

  • Removed verify_string_with_params function
  • Moved implementation directly into verify_string
// Before: wrapper that ignored its params argument
pub fn verify_string_with_params(input: &str, hash: &str, _params: Option<Argon2Params>) -> Result<bool, Error> { ... }
pub fn verify_string(input: &str, hash: &str) -> Result<bool, Error> {
    verify_string_with_params(input, hash, None)
}

// After: single function with the actual implementation
pub fn verify_string(input: &str, hash: &str) -> Result<bool, Error> { ... }

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: fergusfinn <6034059+fergusfinn@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor to remove wrapper based on PR review feedback Remove unnecessary verify_string_with_params wrapper Nov 26, 2025
Copilot AI requested a review from fergusfinn November 26, 2025 10:33
@fergusfinn fergusfinn marked this pull request as ready for review November 26, 2025 10:33
@fergusfinn fergusfinn merged commit 7613935 into faster-password-tests Nov 26, 2025
fergusfinn added a commit that referenced this pull request Nov 26, 2025
…239)

* feat: make Argon2 parameters configurable for faster test execution

This change makes Argon2 password hashing parameters (memory, iterations,
parallelism) configurable through PasswordConfig, with weak defaults in
test environments for dramatically faster test execution.

## Changes

- Added `argon2_memory_kib`, `argon2_iterations`, and `argon2_parallelism`
  to `PasswordConfig` (defaults: 19MB, 2 iterations, 1 thread for production)
- Test config uses weak params (128KB, 1 iteration) for fast execution
- Updated all password hashing calls to use configurable parameters
- Added `hash_string_with_params()` function to accept custom Argon2 params

## Performance Impact

Auth test suite with coverage:
- Before (19MB, 2 iter): 15.38s
- After (128KB, 1 iter): 0.67s
- **23x faster** test execution with coverage

This resolves the issue where `cargo llvm-cov` was significantly slower
than `cargo test` due to instrumenting Argon2's internal hashing loops.
Tests now run faster even with coverage enabled.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Remove unnecessary verify_string_with_params wrapper (#240)

* Initial plan

* refactor: remove unnecessary verify_string_with_params wrapper

Co-authored-by: fergusfinn <6034059+fergusfinn@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fergusfinn <6034059+fergusfinn@users.noreply.github.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fergusfinn <6034059+fergusfinn@users.noreply.github.com>
hachall pushed a commit that referenced this pull request Jan 23, 2026
…239)

* feat: make Argon2 parameters configurable for faster test execution

This change makes Argon2 password hashing parameters (memory, iterations,
parallelism) configurable through PasswordConfig, with weak defaults in
test environments for dramatically faster test execution.

## Changes

- Added `argon2_memory_kib`, `argon2_iterations`, and `argon2_parallelism`
  to `PasswordConfig` (defaults: 19MB, 2 iterations, 1 thread for production)
- Test config uses weak params (128KB, 1 iteration) for fast execution
- Updated all password hashing calls to use configurable parameters
- Added `hash_string_with_params()` function to accept custom Argon2 params

## Performance Impact

Auth test suite with coverage:
- Before (19MB, 2 iter): 15.38s
- After (128KB, 1 iter): 0.67s
- **23x faster** test execution with coverage

This resolves the issue where `cargo llvm-cov` was significantly slower
than `cargo test` due to instrumenting Argon2's internal hashing loops.
Tests now run faster even with coverage enabled.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Remove unnecessary verify_string_with_params wrapper (#240)

* Initial plan

* refactor: remove unnecessary verify_string_with_params wrapper

Co-authored-by: fergusfinn <6034059+fergusfinn@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fergusfinn <6034059+fergusfinn@users.noreply.github.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fergusfinn <6034059+fergusfinn@users.noreply.github.com>
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