BIP-0374: fix incorrect bit index and modernize CSV reader usage in test vector scripts #1817

pull Pronoss wants to merge 3 commits into bitcoin:master from Pronoss:fix/update-bip-0374 changing 3 files +4 −4

Pronoss commented at 7:00 PM on April 10, 2025: contributor
- Corrected a logic bug in gen_test_vectors.py
  Previously, the message tampering logic mistakenly used the proof damage index (proof_damage_pos) instead of the correct message index (msg_damage_pos). This could result in ineffective or misleading bit-flip tests.
  Now fixed to use the correct index for tampering the message.
- Modernized usage of csv.reader in run_test_vectors.py
  Replaced the outdated reader.__next__() with the idiomatic and Python 3–safe next(reader) call for skipping headers.
Update run_test_vectors.py bf67fcbe0f
Update gen_test_vectors.py 3ef94ac86c
jonatack commented at 3:57 PM on April 11, 2025: member

Pinging BIP authors @andrewtoth, @RubenSomsen and @theStack for feedback.
jonatack added the label Proposed BIP modification on Apr 11, 2025
jonatack added the label Pending acceptance on Apr 11, 2025

in bip-0374/gen_test_vectors.py:114 in 3ef94ac86c outdated

 110 | @@ -111,7 +111,7 @@ def gen_all_verify_proof_vectors(f):
 111 |      # modifying message should fail (flip one bit)
 112 |      msg_damage_pos = random_scalar_int(idx, "damage_pos") % 256
 113 |      msg_damaged = list(msg)
 114 | -    msg_damaged[proof_damage_pos // 8] ^= (1 << (msg_damage_pos % 8))
 115 | +    msg_damaged[msg_damage_pos // 8] ^= (1 << (msg_damage_pos % 8))

jonatack commented at 4:00 PM on April 11, 2025:

LGTM, at first glance

theStack commented at 11:30 PM on April 14, 2025: contributor

Previously, the message tampering logic mistakenly used the proof damage index (proof_damage_pos) instead of the correct message index (msg_damage_pos). This could result in ineffective or misleading bit-flip tests.

Good catch, thanks for fixing (seems I back then copy-and-pasted from the proof damaging above and forgot to adapt the array index :face_with_peeking_eye: )! Note though that we also track the generated test vectors data, so you should run the fixed gen_test_vectors.py script and include the re-generated .csv files in the commit.
Regenerate test vectors after fixing message tampering logic in gen_test_vectors.py 125cbdabeb
Pronoss commented at 8:25 AM on April 15, 2025: contributor

@theStack Regenerated test_vectors_*.csv using the fixed gen_test_vectors.py and included them in the commit as requested. Ready for review.
andrewtoth commented at 1:43 PM on April 16, 2025: contributor

ACK 125cbdabeb900e255bc23b382fa6b4faf9d99b05
jonatack merged this on Apr 16, 2025
jonatack closed this on Apr 16, 2025
jbride referenced this in commit 2c33f1bcb9 on Jun 11, 2025

Contributors

Labels

Proposed BIP modification Pending acceptance