Skip to content

Conversation

@famouswizard
Copy link

fixed several parsing bugs in key handling logic:

  • find_key_end_position: removed '/' from the set of terminating characters. This prevents derivation paths like .../44'/0'/0'/... from being truncated at the first /.
  • parse_key_expressions: implemented proper parenthesis depth tracking for musig(...) expressions instead of stopping at the first ), allowing correct parsing of nested expressions.
  • parse_key_expressions: added boundary checks (key_pos_end < len(descriptor)) before indexing descriptor[key_pos_end] to avoid IndexError at string ends.

these changes ensure the parser correctly handles complex key expressions and avoids crashes on edge cases.

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

This looks like AI slop to me, so I’m gonna close it. Please feel free to explain what actual issue this resolves should there be any merit to this, so it can be considered for reopening. cc: @bigspider.

@bigspider
Copy link
Contributor

The code is meant mostly for illustrative purposes. However:

  • find_key_end_position definitely intends to truncate at the first /
  • nested musig is explicitly forbidden by the specifications in BIP-388 (and, today, by descriptors)
  • pretty sure key_pos_end can never be larger than or equal to len(descriptor), by construction.

So I don't think those are improvements.

@murchandamus
Copy link
Contributor

murchandamus commented Jan 13, 2026

Thanks for your time, @bigspider.

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.

3 participants