[HLR] Codebase correctness audit — v0.2.0

[sgbett]

Codebase Correctness Audit — v0.2.0

Date: 2026-03-07
Spec suite: 1497 examples, 0 failures, 5 pending
Ruby 2.7 compatibility: Verified — no post-2.7 features found

Summary

9 findings across the codebase: 2 bugs (1 HIGH, 1 MEDIUM), 2 semantic issues (LOW), and 5 test gaps.

Task Breakdown

Bugs

• [Task 1] B1: Fix truncated OP_PUSHDATA crash in script parser #146 - [Task 1] B1: Fix truncated OP_PUSHDATA crash in script parser (HIGH)
• [Task 2] B2: Fix Transaction#to_beef BUMP deduplication at same block height #147 - [Task 2] B2: Fix Transaction#to_beef BUMP deduplication at same block height (MEDIUM)

Semantic Fixes

• [Task 3] S1: Use BN.mod_add in PrivateKey#derive_child #148 - [Task 3] S1: Use BN.mod_add in PrivateKey#derive_child (LOW)
• [Task 4] S2: Fix Transaction#txid byte-order documentation #149 - [Task 4] S2: Fix Transaction#txid byte-order documentation (LOW)

Test Coverage

• [Task 5] T1-T5: Add missing test coverage from audit #150 - [Task 5] T1-T5: Add missing test coverage from audit (MEDIUM)

Total Tasks: 5
Critical Path: Task 1 → Task 2 → Task 5 (T1/T2 tests); Tasks 3-4 independent

Findings Detail

B1. [HIGH] Script parse_chunks crashes on truncated OP_PUSHDATA — script.rb:397-414

When a script binary contains OP_PUSHDATA1/2/4 but is truncated before the length bytes, getbyte/byteslice returns nil, causing TypeError. Confirmed with all three variants:

BSV::Script::Script.new("\x4c".b).chunks        # TypeError
BSV::Script::Script.new("\x4d\x01".b).chunks    # TypeError
BSV::Script::Script.new("\x4e".b).chunks         # TypeError

B2. [MEDIUM] Transaction#to_beef deduplicates BUMPs by height only — transaction.rb:304-309

The to_beef convenience method uses bump_map[height] to deduplicate merkle paths. When two ancestors are proven in the same block, the second merkle path is silently dropped. Beef#merge_transaction handles this correctly; only the convenience method is affected.

S1. [LOW] PrivateKey#derive_child uses Integer roundtrip for mod — private_key.rb:161

Converts BN→Integer→mod→String→BN instead of using mod_add. Correct but inconsistent with ExtendedKey#child.

S2. [LOW] Transaction#txid comment says "internal byte order" but returns display order — transaction.rb:361

Code is self-consistent (BEEF comparisons compensate correctly) but docs could mislead external callers.

T1-T5. Test Gaps

• T1. [MEDIUM] Truncated script parsing
• T2. [MEDIUM] BEEF with multiple ancestors at same block height
• T3. [MEDIUM] FORKID enforcement in sighash validation
• T4. [LOW] ExtendedKey fingerprint chain integrity
• T5. [LOW] Mnemonic#to_entropy round-trip

Auditor Notes

• False positive rejected: BEEF .reverse calls on prev_tx_id are correct — txid returns display order while prev_tx_id is wire order, so .reverse is needed for comparison.
• Ruby 2.7 compat: Full scan confirmed no post-2.7 features across all .rb files.
• Cryptographic correctness: All algorithms verified against cross-SDK conformance vectors. No issues found.