Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: #109 change md5 to sha256 #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: #109 change md5 to sha256 #112

wants to merge 1 commit into from

Conversation

zvdy
Copy link

@zvdy zvdy commented Feb 25, 2025

Change hasher function from MD5 to SHA256 #109

Summary

Changes the hashing algorithm in utils.hasher() from MD5 to SHA256 to improve security and prevent potential hash collisions.

Technical Details

  • Changed hasher function to use hashlib.sha256 instead of hashlib.md5
  • Added docstring clarifying hash function choice
  • No interface changes as function signature and return type remain identical

Impact Analysis

This change affects batching logic in:

  • interest_rate_swap/proto_utils.py
  • forward_rate_agreement/proto_utils.py
  • american_option/proto_utils.py

The change is backwards compatible since:

  1. Hash values are only used internally for grouping similar instruments
  2. SHA256 maintains the same deterministic properties required for batching
  3. Output length difference between MD5 (128 bits) and SHA256 (256 bits) doesn't affect the grouping functionality

Testing

All existing tests should pass without modification since:

  • The hasher function's interface remains unchanged
  • Batching behavior remains deterministic
  • No functional changes to how instruments are grouped

Motivation

While MD5 was sufficient for our non-cryptographic use case, switching to SHA256:

  1. Follows modern security best practices
  2. Provides better collision resistance as noted in the Python hashlib docs and Wikipedia
  3. Has minimal performance impact on our batching use case
  4. Fixes Replace MD5 with SHA-256 in hasher function for improved security #109

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace MD5 with SHA-256 in hasher function for improved security
1 participant