Skip to content

fix: return merkle branch roots by requested type#7

Open
pinehill99 wants to merge 2 commits into
BitgesellOfficial:masterfrom
pinehill99:pybgl-merkle-branches-return-type
Open

fix: return merkle branch roots by requested type#7
pinehill99 wants to merge 2 commits into
BitgesellOfficial:masterfrom
pinehill99:pybgl-merkle-branches-return-type

Conversation

@pinehill99
Copy link
Copy Markdown

Summary

  • Fix merkle_root_from_branches() so hex=True returns a hex string and hex=False returns raw bytes.
  • Add a focused regression covering both return types and the hex/raw round-trip.

Validation

  • git diff --check
  • PYTHONPYCACHEPREFIX=/private/tmp/pybgl-pycache python3 -m py_compile pybgl/functions/block.py pybgl/test/block.py
  • Stubbed focused execution of merkle_root_from_branches() confirmed hex=True returns str, hex=False returns bytes, and root_hex == root_raw.hex().

Note: python3 -m unittest pybgl.test.block -q cannot load in this local checkout because native _sha3_hash is not built; it fails before tests run. The change is isolated to the pure-Python merkle branch helper.

Bounty context: BitgesellOfficial/bitgesell#39 and BitgesellOfficial/bitgesell#81.

@MyTH-zyxeon
Copy link
Copy Markdown

Review-assist pass for the #39/#81 bounty context:

This is a clean, narrow fix candidate: the current implementation returns bytes for hex=True and then attempts bytes_from_hex() on an already-bytes value for hex=False, so the new root_hex == root_raw.hex() regression is the right acceptance spine.

Before merging, I would ask for three quick maintainer checks:

  1. Confirm the intended API contract is exactly hex=True -> str and hex=False -> bytes for merkle_root_from_branches(), matching nearby helpers.
  2. Run the focused test in an environment where the native _sha3_hash extension is built, since the author notes local unittest collection currently fails before this regression can execute.
  3. Consider adding one branch input as raw bytes, not only hex strings, so the existing if type(h) == str normalization path stays covered.

No claim/payment action from me here; this is just a review-assist note to make the bounty acceptance criteria explicit.

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