Skip to content

Conversation

@ctnguyen
Copy link
Contributor

@ctnguyen ctnguyen commented Nov 7, 2025

No description provided.

@oskarszoon oskarszoon changed the title MvP-4333 Add check for P2SH output after genesis activation Add check for P2SH output after genesis activation Nov 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

🤖 Claude Code Review

Status: Complete


Current Review:

This PR implements P2SH output rejection after Genesis activation (issue #4333). The implementation looks correct:

Core logic is sound: The P2SH check is properly gated by policy checks and Genesis activation
Consistency fixed: Uses shared isGenesisActivated variable with > comparison (excluding Genesis block itself)
Test coverage: Comprehensive test cases verify behavior at Genesis height, after Genesis, and with policy skip option
Documentation: Comments explain the > vs >= distinction for Genesis block handling

Minor observations:

  • Test uses hardcoded transaction hex string - this is acceptable for a known P2SH transaction
  • Error message format matches existing patterns in the codebase

History:

  • ✅ Fixed: Previously used >= comparison which would incorrectly reject P2SH in Genesis block 620538. Now correctly uses > comparison consistent with dust limit check.

No blocking issues found.

@ctnguyen ctnguyen requested a review from icellan November 7, 2025 15:39
ctnguyen and others added 3 commits November 7, 2025 22:09
…ld start failing if you ran the checks too long after the commit date
@oskarszoon oskarszoon merged commit 8a0fd2b into bsv-blockchain:main Nov 10, 2025
13 of 14 checks passed
@ctnguyen ctnguyen deleted the Fix/MvP-4333_p2sh_output branch November 10, 2025 14:10
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.

3 participants