Skip to content

fix: codebase correctness audit fixes (#145)#151

Merged
sgbett merged 7 commits intomasterfrom
feature/145-codebase-audit
Mar 7, 2026
Merged

fix: codebase correctness audit fixes (#145)#151
sgbett merged 7 commits intomasterfrom
feature/145-codebase-audit

Conversation

@sgbett
Copy link
Copy Markdown
Owner

@sgbett sgbett commented Mar 7, 2026

Summary

Addresses all findings from the v0.2.0 codebase correctness audit (#145):

  • B1 [HIGH]: Fix TypeError crash in script parser on truncated OP_PUSHDATA1/2/4 opcodes — now raises ArgumentError with descriptive message
  • B2 [MEDIUM]: Fix Transaction#to_beef silently dropping second merkle path when two ancestors share a block height — now uses Beef#merge_bump
  • S1 [LOW]: Replace BN→Integer→mod roundtrip with mod_add in PrivateKey#derive_child
  • S2 [LOW]: Fix misleading "internal byte order" YARD docs on txid and BEEF methods (actually display byte order)
  • T1-T5: Add missing test coverage for FORKID enforcement, ExtendedKey fingerprint chain, mnemonic entropy round-trip, truncated scripts, and BEEF same-height ancestors

Test plan

  • Full suite: 1508 examples, 0 failures, 5 pending (up from 1497)
  • All 9 BRC-42 conformance vectors pass after mod_add refactor
  • Truncated script parsing confirmed fixed (3 crash variants)
  • BEEF same-height test verifies both ancestors get distinct bump indices

🤖 Generated with Claude Code

sgbett and others added 6 commits March 7, 2026 04:20
Cross-SDK comparison at v0.2.0 showing ~65% coverage (up from ~50%).
All Tier 1 items complete. Replaces interim v0.1.0 report.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add bounds checking in parse_chunks for OP_PUSHDATA1/2/4 opcodes
with missing or partial length bytes. Raises ArgumentError with
descriptive message instead of crashing with TypeError.

Implements sub-task #146 of HLR #145.

Co-Authored-By: Claude <noreply@anthropic.com>
…147)

Replace manual bump_map (keyed by height) with Beef#merge_bump which
correctly handles multiple merkle paths at the same block height.
The old code silently dropped the second path when two ancestors were
proven in the same block.

Implements sub-task #147 of HLR #145.

Co-Authored-By: Claude <noreply@anthropic.com>
Replace BN→Integer→mod→String→BN roundtrip with OpenSSL's mod_add,
matching the pattern used in ExtendedKey#child.

Implements sub-task #148 of HLR #145.

Co-Authored-By: Claude <noreply@anthropic.com>
Transaction#txid returns display byte order (reversed from natural
hash), not internal byte order as previously documented. Fix YARD
comments on txid, BeefTx#txid, and all BEEF lookup method parameters.

Implements sub-task #149 of HLR #145.

Co-Authored-By: Claude <noreply@anthropic.com>
…150)

- FORKID enforcement: verify interpreter rejects sighash without FORKID
- ExtendedKey fingerprint chain: verify parent→child→grandchild integrity
- Mnemonic entropy round-trip: verify all 5 valid entropy lengths

Implements sub-task #150 of HLR #145.

Co-Authored-By: Claude <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Co-Authored-By: Claude <noreply@anthropic.com>
@sgbett sgbett merged commit 9474a2d into master Mar 7, 2026
1 of 7 checks passed
@sgbett sgbett deleted the feature/145-codebase-audit branch March 24, 2026 00:15
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.

1 participant