BIP352: fix byte unpacking and avoid mutable default argument #1916

1. 
   fifalodm commented at 3:42 pm on August 8, 2025: none

   • Fixed deser_compact_size to correctly unpack the first byte instead of reading twice.

   • Updated scanning to use Optional[Dict] with safe default (None → {}) to prevent mutable default argument issues.

2. Update bitcoin_utils.py c9f989b05e
3. Update reference.py 948f137345
4. jonatack added the label Proposed BIP modification on Aug 8, 2025
5. 
   murchandamus commented at 9:31 pm on August 18, 2025: contributor

   I’m not familiar enough with Python to know at first glance—is this fixing an actual bug in the reference implementation or is this just a stylistic improvement?
6. 
   fifalodm commented at 8:46 am on August 19, 2025: none

   Thanks for the review!

   These are actual bug fixes, not just stylistic changes:

   1. deser_compact_size bug
      The old code did b = f.read(1) and then immediately called struct.unpack("<B", f.read(1))[0].
      That skipped the first byte and unpacked the next one.

      • For values like 0xfd/0xfe/0xff (multi-byte varints), the prefix byte was lost and parsing went out of sync.
      • Even for small values (≤252), the stream advanced one byte too far.

      The fix correctly unpacks the already-read b, so the prefix is handled properly.

   2. Mutable default argument in scanning
      Using {} as a default creates a single shared dict across calls. This can leak state between function calls.
      Switching to labels: Optional[Dict[str, str]] = None and initializing inside the function avoids that Python pitfall.

   So both changes address correctness issues, not just style.

7. 
   jonatack commented at 4:17 pm on August 19, 2025: member

   cc BIP authors @RubenSomsen @josibake for feedback
8. jonatack assigned RubenSomsen on Aug 19, 2025
9. jonatack added the label Pending acceptance on Aug 19, 2025

Contributors
fifalodm murchandamus jonatack

Labels
Proposed BIP modification Pending acceptance