Skip to content

Conversation

strmfos
Copy link
Contributor

@strmfos strmfos commented Oct 2, 2025

Describe your changes

  • prevent panic in InputFile::parse_word when inputs omit the 0x prefix
  • return descriptive error for malformed word strings and keep behavior otherwise unchanged
  • verified with cargo test internal::tests::test_merkle_data_parsing and cargo test miden_vm::internal::tests::test_merkle_data_parsing

Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

The fix to use strip_prefix() is correct, but this change needs proper test coverage.

It would be great if you could add unit tests to the test_merkle_data_parsing module (around line 370) to verify:

  1. Error handling for missing 0x prefix
  2. Edge cases like empty strings or just 0x
  3. Valid hex parsing still works as expected

@strmfos
Copy link
Contributor Author

strmfos commented Oct 3, 2025

Thanks for the review! Added the requested unit tests in test_merkle_data_parsing module:

  • test_parse_word_missing_0x_prefix() - verifies error handling for missing "0x" prefix
  • test_parse_word_edge_cases() - covers empty strings, just "0x", and short hex
  • test_parse_word_valid_hex() - ensures valid hex parsing still works

Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

Thanks!

@huitseeker huitseeker requested a review from bitwalker October 4, 2025 03:05
@huitseeker huitseeker merged commit 03b9f77 into 0xMiden:next Oct 4, 2025
}

/// Parse a `Word` from a hex string.
pub fn parse_word(word_hex: &str) -> Result<Word, String> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Theoretically, we should be able to replace this function with Word::try_from() - though, this would not support "incomplete strings" (i.e., the string would need to be 66 characters). Maybe it is worth updating how Word::try_from() works in miden-crypto? cc @huitseeker.

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.

4 participants