Skip to content

fix(txscript): allocate fresh slice in opcodeInvert to avoid aliasing#777

Open
ordishs wants to merge 2 commits intobsv-blockchain:mainfrom
ordishs:security/4576-opinvert-immutable
Open

fix(txscript): allocate fresh slice in opcodeInvert to avoid aliasing#777
ordishs wants to merge 2 commits intobsv-blockchain:mainfrom
ordishs:security/4576-opinvert-immutable

Conversation

@ordishs
Copy link
Copy Markdown
Collaborator

@ordishs ordishs commented Apr 29, 2026

Closes #4576.

Summary

opcodeInvert used PeekByteArray(0) and XOR'd bytes in place. After OP_DUP (which copies only the slice header), both stack items share the same backing array — so OP_DUP … OP_INVERT mutates the duplicate as well. This diverges from Bitcoin Core / SV Node script semantics.

Fix

Pop, allocate a fresh slice, invert into the new slice, push. Stack transformation a -> ~a is unchanged; the result is now an independent slice with no aliasing.

Test plan

  • Regression test: push a byte slice, push again (aliased), invert top, assert the second copy on the stack is unchanged.
  • Basic correctness: invert known bytes, assert top of stack matches bitwise-NOT.
  • go test -race ./services/legacy/txscript/... passes.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026 •
edited
Loading

🤖 Claude Code Review

Status: Complete

Current Review:

No issues found. The fix correctly addresses the aliasing bug in opcodeInvert:

  • Root cause correctly identified: The old implementation used PeekByteArray(0) and mutated bytes in-place, causing aliasing problems after OP_DUP (which only copies the slice header, not the backing array).

  • Fix is correct: Pop the value, allocate a fresh slice, invert into new slice, push result. This ensures stack isolation while preserving the a -> ~a transformation semantics.

  • Test coverage is thorough:

    • Regression test explicitly validates no aliasing after OP_DUP + OP_INVERT
    • Basic correctness tests cover empty, single-byte, and multi-byte cases
    • Test confirms bitwise-NOT semantics match expected behavior

  • Verified no similar issues: Other opcodes using PeekByteArray(0) are read-only operations (opcodeIfDup, opcodeSize, etc.). Binary opcodes like opcodeXor already pop operands and allocate fresh output slices.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026 •
edited
Loading

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-777 (fc4a92a)

Summary

  • Regressions: 0
  • Improvements: 0
  • Unchanged: 142
  • Significance level: p < 0.05
All benchmark results (sec/op)
Benchmark Baseline Current Change p-value
_NewBlockFromBytes-4 1.664µ 1.695µ ~ 0.700
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 61.50n 61.51n ~ 1.000
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 61.56n 62.60n ~ 0.100
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 61.78n 64.76n ~ 0.500
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 33.08n 33.12n ~ 0.700
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 55.79n 53.76n ~ 0.200
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 126.8n 120.1n ~ 0.400
MiningCandidate_Stringify_Short-4 264.9n 269.0n ~ 0.100
MiningCandidate_Stringify_Long-4 1.918µ 1.935µ ~ 0.400
MiningSolution_Stringify-4 1005.0n 995.2n ~ 0.200
BlockInfo_MarshalJSON-4 1.799µ 1.797µ ~ 0.400
NewFromBytes-4 126.8n 126.1n ~ 0.300
Mine_EasyDifficulty-4 51.79µ 52.17µ ~ 0.400
Mine_WithAddress-4 5.404µ 5.359µ ~ 1.000
DirectSubtreeAdd/4_per_subtree-4 59.16n 65.09n ~ 0.100
DirectSubtreeAdd/64_per_subtree-4 31.46n 31.58n ~ 0.400
DirectSubtreeAdd/256_per_subtree-4 30.93n 30.55n ~ 0.700
DirectSubtreeAdd/1024_per_subtree-4 29.19n 29.42n ~ 0.600
DirectSubtreeAdd/2048_per_subtree-4 28.82n 28.85n ~ 0.500
SubtreeProcessorAdd/4_per_subtree-4 280.5n 279.4n ~ 0.700
SubtreeProcessorAdd/64_per_subtree-4 274.0n 276.3n ~ 0.400
SubtreeProcessorAdd/256_per_subtree-4 275.1n 280.5n ~ 0.400
SubtreeProcessorAdd/1024_per_subtree-4 267.2n 269.6n ~ 0.100
SubtreeProcessorAdd/2048_per_subtree-4 269.7n 269.7n ~ 1.000
SubtreeProcessorRotate/4_per_subtree-4 272.9n 276.6n ~ 0.100
SubtreeProcessorRotate/64_per_subtree-4 271.1n 276.4n ~ 0.100
SubtreeProcessorRotate/256_per_subtree-4 271.4n 274.6n ~ 0.100
SubtreeProcessorRotate/1024_per_subtree-4 269.9n 273.4n ~ 0.100
SubtreeNodeAddOnly/4_per_subtree-4 54.37n 54.71n ~ 0.700
SubtreeNodeAddOnly/64_per_subtree-4 34.62n 34.68n ~ 1.000
SubtreeNodeAddOnly/256_per_subtree-4 33.69n 33.68n ~ 0.700
SubtreeNodeAddOnly/1024_per_subtree-4 32.98n 32.78n ~ 0.400
SubtreeCreationOnly/4_per_subtree-4 113.8n 115.1n ~ 0.100
SubtreeCreationOnly/64_per_subtree-4 412.9n 407.8n ~ 0.700
SubtreeCreationOnly/256_per_subtree-4 1.417µ 1.364µ ~ 0.100
SubtreeCreationOnly/1024_per_subtree-4 4.454µ 4.463µ ~ 1.000
SubtreeCreationOnly/2048_per_subtree-4 8.453µ 8.522µ ~ 0.700
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 271.5n 272.4n ~ 1.000
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 269.9n 270.4n ~ 1.000
ParallelGetAndSetIfNotExists/1k_nodes-4 590.6µ 601.0µ ~ 0.400
ParallelGetAndSetIfNotExists/10k_nodes-4 1.330m 1.389m ~ 0.100
ParallelGetAndSetIfNotExists/50k_nodes-4 6.750m 6.772m ~ 0.400
ParallelGetAndSetIfNotExists/100k_nodes-4 13.45m 13.37m ~ 0.100
SequentialGetAndSetIfNotExists/1k_nodes-4 666.3µ 661.9µ ~ 1.000
SequentialGetAndSetIfNotExists/10k_nodes-4 2.861m 2.801m ~ 1.000
SequentialGetAndSetIfNotExists/50k_nodes-4 10.48m 10.55m ~ 0.700
SequentialGetAndSetIfNotExists/100k_nodes-4 20.14m 19.90m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 636.6µ 634.4µ ~ 0.700
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 4.163m 4.204m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 16.88m 16.68m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 694.6µ 705.6µ ~ 0.200
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 5.633m 5.893m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 37.69m 37.74m ~ 1.000
DiskTxMap_SetIfNotExists-4 3.715µ 3.681µ ~ 0.500
DiskTxMap_SetIfNotExists_Parallel-4 3.534µ 3.500µ ~ 1.000
DiskTxMap_ExistenceOnly-4 315.8n 315.8n ~ 0.800
Queue-4 188.7n 192.5n ~ 0.100
AtomicPointer-4 3.642n 3.649n ~ 1.000
ReorgOptimizations/DedupFilterPipeline/Old/10K-4 849.7µ 820.6µ ~ 0.400
ReorgOptimizations/DedupFilterPipeline/New/10K-4 824.4µ 794.2µ ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/10K-4 126.2µ 118.8µ ~ 0.200
ReorgOptimizations/AllMarkFalse/New/10K-4 64.26µ 64.07µ ~ 0.700
ReorgOptimizations/HashSlicePool/Old/10K-4 58.19µ 70.74µ ~ 0.100
ReorgOptimizations/HashSlicePool/New/10K-4 11.29µ 11.21µ ~ 0.100
ReorgOptimizations/NodeFlags/Old/10K-4 5.388µ 6.133µ ~ 0.100
ReorgOptimizations/NodeFlags/New/10K-4 1.866µ 2.634µ ~ 0.100
ReorgOptimizations/DedupFilterPipeline/Old/100K-4 9.255m 10.140m ~ 0.100
ReorgOptimizations/DedupFilterPipeline/New/100K-4 9.462m 9.943m ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/100K-4 1.188m 1.179m ~ 0.400
ReorgOptimizations/AllMarkFalse/New/100K-4 705.3µ 705.7µ ~ 0.400
ReorgOptimizations/HashSlicePool/Old/100K-4 562.9µ 599.9µ ~ 0.100
ReorgOptimizations/HashSlicePool/New/100K-4 204.4µ 196.0µ ~ 0.400
ReorgOptimizations/NodeFlags/Old/100K-4 52.65µ 55.31µ ~ 0.200
ReorgOptimizations/NodeFlags/New/100K-4 18.06µ 18.98µ ~ 0.100
TxMapSetIfNotExists-4 46.49n 46.53n ~ 0.700
TxMapSetIfNotExistsDuplicate-4 38.62n 38.63n ~ 1.000
ChannelSendReceive-4 610.5n 608.9n ~ 1.000
BlockAssembler_AddTx-4 0.02886n 0.03161n ~ 0.400
AddNode-4 11.68 11.75 ~ 0.100
AddNodeWithMap-4 12.30 12.48 ~ 1.000
CalcBlockWork-4 465.9n 468.1n ~ 0.700
CalculateWork-4 640.4n 627.9n ~ 1.000
BuildBlockLocatorString_Helpers/Size_10-4 1.316µ 1.307µ ~ 0.200
BuildBlockLocatorString_Helpers/Size_100-4 15.05µ 14.91µ ~ 1.000
BuildBlockLocatorString_Helpers/Size_1000-4 124.4µ 124.1µ ~ 1.000
CatchupWithHeaderCache-4 104.4m 104.4m ~ 0.700
_BufferPoolAllocation/16KB-4 4.359µ 3.486µ ~ 0.700
_BufferPoolAllocation/32KB-4 8.318µ 7.217µ ~ 0.100
_BufferPoolAllocation/64KB-4 15.42µ 15.03µ ~ 0.700
_BufferPoolAllocation/128KB-4 29.08µ 30.51µ ~ 0.100
_BufferPoolAllocation/512KB-4 124.4µ 117.2µ ~ 0.100
_BufferPoolConcurrent/32KB-4 18.13µ 19.19µ ~ 0.100
_BufferPoolConcurrent/64KB-4 27.58µ 31.40µ ~ 0.100
_BufferPoolConcurrent/512KB-4 146.4µ 149.2µ ~ 0.400
_SubtreeDeserializationWithBufferSizes/16KB-4 638.9µ 629.7µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/32KB-4 662.7µ 617.1µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/64KB-4 665.2µ 624.5µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/128KB-4 657.3µ 618.7µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/512KB-4 716.9µ 631.2µ ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/16KB-4 35.79m 35.68m ~ 0.400
_SubtreeDataDeserializationWithBufferSizes/32KB-4 35.31m 35.70m ~ 0.200
_SubtreeDataDeserializationWithBufferSizes/64KB-4 35.24m 35.69m ~ 0.200
_SubtreeDataDeserializationWithBufferSizes/128KB-4 35.41m 35.64m ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/512KB-4 35.14m 35.52m ~ 0.400
_PooledVsNonPooled/Pooled-4 735.7n 671.3n ~ 0.100
_PooledVsNonPooled/NonPooled-4 6.688µ 7.159µ ~ 0.100
_MemoryFootprint/Current_512KB_32concurrent-4 6.533µ 7.098µ ~ 0.100
_MemoryFootprint/Proposed_32KB_32concurrent-4 9.019µ 11.021µ ~ 0.100
_MemoryFootprint/Alternative_64KB_32concurrent-4 8.629µ 10.298µ ~ 0.100
_prepareTxsPerLevel-4 416.5m 403.7m ~ 0.400
_prepareTxsPerLevelOrdered-4 3.738m 4.041m ~ 0.700
_prepareTxsPerLevel_Comparison/Original-4 425.2m 422.0m ~ 0.700
_prepareTxsPerLevel_Comparison/Optimized-4 4.267m 3.718m ~ 0.100
SubtreeSizes/10k_tx_4_per_subtree-4 1.401m 1.372m ~ 0.100
SubtreeSizes/10k_tx_16_per_subtree-4 337.8µ 325.6µ ~ 0.200
SubtreeSizes/10k_tx_64_per_subtree-4 80.80µ 78.92µ ~ 0.700
SubtreeSizes/10k_tx_256_per_subtree-4 20.05µ 19.67µ ~ 0.100
SubtreeSizes/10k_tx_512_per_subtree-4 9.881µ 9.721µ ~ 0.100
SubtreeSizes/10k_tx_1024_per_subtree-4 4.935µ 4.836µ ~ 0.200
SubtreeSizes/10k_tx_2k_per_subtree-4 2.450µ 2.403µ ~ 0.200
BlockSizeScaling/10k_tx_64_per_subtree-4 77.64µ 75.66µ ~ 0.100
BlockSizeScaling/10k_tx_256_per_subtree-4 19.62µ 19.32µ ~ 0.200
BlockSizeScaling/10k_tx_1024_per_subtree-4 4.821µ 4.767µ ~ 0.100
BlockSizeScaling/50k_tx_64_per_subtree-4 402.2µ 397.2µ ~ 0.400
BlockSizeScaling/50k_tx_256_per_subtree-4 96.13µ 96.64µ ~ 0.100
BlockSizeScaling/50k_tx_1024_per_subtree-4 24.55µ 24.02µ ~ 1.000
SubtreeAllocations/small_subtrees_exists_check-4 160.9µ 161.4µ ~ 1.000
SubtreeAllocations/small_subtrees_data_fetch-4 172.3µ 172.4µ ~ 1.000
SubtreeAllocations/small_subtrees_full_validation-4 334.4µ 333.3µ ~ 0.700
SubtreeAllocations/medium_subtrees_exists_check-4 9.480µ 9.481µ ~ 1.000
SubtreeAllocations/medium_subtrees_data_fetch-4 10.06µ 10.07µ ~ 0.700
SubtreeAllocations/medium_subtrees_full_validation-4 19.53µ 19.45µ ~ 0.800
SubtreeAllocations/large_subtrees_exists_check-4 2.271µ 2.272µ ~ 0.700
SubtreeAllocations/large_subtrees_data_fetch-4 2.440µ 2.465µ ~ 0.400
SubtreeAllocations/large_subtrees_full_validation-4 4.891µ 4.904µ ~ 0.700
StoreBlock_Sequential/BelowCSVHeight-4 323.3µ 323.4µ ~ 1.000
StoreBlock_Sequential/AboveCSVHeight-4 324.6µ 318.9µ ~ 1.000
GetUtxoHashes-4 275.8n 279.1n ~ 0.100
GetUtxoHashes_ManyOutputs-4 46.38µ 47.42µ ~ 0.100
_NewMetaDataFromBytes-4 233.7n 235.0n ~ 0.500
_Bytes-4 623.6n 621.6n ~ 0.400
_MetaBytes-4 576.0n 570.3n ~ 0.100

Threshold: >10% with p < 0.05 | Generated: 2026-05-04 13:46 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ordishs