Skip to content

removes FN-DSA (Falcon)#19

Merged
cryptoquick merged 3 commits intocryptoquick:mainfrom
notmike-5:remove-fn-dsa
Jun 27, 2025
Merged

removes FN-DSA (Falcon)#19
cryptoquick merged 3 commits intocryptoquick:mainfrom
notmike-5:remove-fn-dsa

Conversation

@notmike-5
Copy link
Copy Markdown
Collaborator

Changes remove FN-DSA implementation, tests, and documentation.

@notmike-5 notmike-5 mentioned this pull request Jun 20, 2025
Copy link
Copy Markdown
Owner

@cryptoquick cryptoquick left a comment

Choose a reason for hiding this comment

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

Just one review item

Also pls run cargo fmt and check CI

Comment thread src/lib.rs Outdated
Comment on lines 192 to 199
/// Creates an Algorithm variant infallibly from an index 0..2.
/// Maps 0 -> SECP256K1_SCHNORR (1), 1 -> ML_DSA_44 (2), 2 -> SLH_DSA_128S (4)
/// TODO: ensure this is correct mapping with FN_DSA out!
pub fn algorithm_from_index(index: u8) -> Algorithm {
let valid_index = index % 4; // Ensure value is within 0..3
let bits = 1u8 << valid_index; // Map 0->1, 1->2, 2->4, 3->8
let valid_index = index % 4; // Ensure value is within 0..2
let bits = 1u8 << valid_index; // Map 0->1, 1->2, 2->4, 3->8 ...
Algorithm::from(bits)
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Let's remove this function

Copy link
Copy Markdown
Collaborator Author

@notmike-5 notmike-5 Jun 26, 2025

Choose a reason for hiding this comment

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

✅ done in 9e17578

Comment thread benches/REPORT.md
Comment on lines +12 to +13
| ML-DSA-44 | *Needs Update* | *Needs Update* | *Needs Update* |
| SLH-DSA-128S | *Needs Update* | *Needs Update* | *Needs Update* |
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Also, this is weird... I think we may have lost some benchmark code somewhere

Copy link
Copy Markdown
Collaborator Author

@notmike-5 notmike-5 Jun 26, 2025

Choose a reason for hiding this comment

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

Yeah, I noticed this as well. Had a look into it and this is what the benchmark report returns with. Looks like some previous change in the cargo bench workflow, but unrelated to the changes in this PR.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Makes sense

Comment thread benches/REPORT.md
@cryptoquick cryptoquick merged commit b9e481b into cryptoquick:main Jun 27, 2025
3 of 4 checks passed
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