test: adding test for InstructionIndices::nth byte position bug
What the developer wrote
Authored by Alexey Gradoboev
test: adding test for InstructionIndices::nth byte position bug
Remove the custom `nth` implementation for `InstructionIndices` because
it captures position via `next_with` and returns as byte offset instead of advancing it in later next_fn calls.
Adding test to show that nth position is not in sync with returned opcode value, no real fix in the commit
Fixes #6377
What changed, and why it matters
This commit only adds a new test to the rust-bitcoin library. The test demonstrates that a method called `InstructionIndices::nth` can report incorrect byte positions when used in certain ways. The commit message explicitly says there is 'no real fix in the commit' and that the custom `nth` implementation should be removed because it returns a byte offset without properly advancing the iterator. So this is a test that exposes a bug, not a patch that fixes it.
Treat this commit as a bug-reporting test, not a security fix. A follow-up commit should remove the custom `nth` implementation as suggested in the commit message, run the new test to confirm the fix, and review any downstream consumers of `InstructionIndices` that depend on accurate byte positions.
Security signals we found
Iterator position desynchronization in script instruction parsing
Incorrect byte offset reporting could affect code relying on accurate script indices
No fix included; only regression test added
Evidence from the diff
The diff adds a unit test instruction_indices_nth_extended in bitcoin/src/blockdata/script/tests.rs. It constructs a 4-byte script (OP_FALSE, OP_PUSHBYTES_1 0x69, OP_NOP3) and checks that InstructionIndices::nth returns the correct instruction and byte position for indices 0, 1, 2, returns None for index 3, and that mixed nth/skip/next usage keeps positions consistent. The commit message states the custom nth implementation captures position via next_with and returns it as a byte offset instead of advancing it in later next_fn calls, causing desynchronization. No production code is changed.
Changed components
bitcoin/src/blockdata/script/tests.rsInstructionIndices iterator (script instruction iteration)Inspect captured patch +37 / −0
diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs
index e1ad86b4..42e95aca 100644
--- a/bitcoin/src/blockdata/script/tests.rs
+++ b/bitcoin/src/blockdata/script/tests.rs
@@ -1115,3 +1115,40 @@ fn p2sh_p2wsh_script_sig() {
assert_eq!(redeem_script.as_bytes()[1], 0x20); // push 32 bytes
assert_eq!(redeem_script.len(), 34);
}
+
+#[test]
+#[allow(clippy::iter_nth_zero, clippy::iter_skip_next)]
+fn instruction_indices_nth_extended() {
+ // Script bytes: OP_FALSE (00) | OP_PUSHBYTES_1 (01) 0x69 | OP_NOP3 (b2)
+ // Instructions begin at byte positions: 0, 1, 3.
+ let script = ScriptBuf::from_hex_no_length_prefix("000169b2").unwrap();
+
+ // basic nth() calls from a fresh iterator
+ let pos_0 = script.instruction_indices().nth(0).unwrap().unwrap();
+ assert_eq!(pos_0.1, Instruction::PushBytes(PushBytes::empty()), "nth(0) must be OP_FALSE");
+ assert_eq!(pos_0.0, 0, "nth(0) returned wrong instruction position");
+
+ let pos_1 = script.instruction_indices().nth(1).unwrap().unwrap();
+ assert_eq!(pos_1.1, Instruction::PushBytes([105].as_ref()), "nth(1) must be OP_PUSHBYTES_1");
+ assert_eq!(pos_1.0, 1, "nth(1) returned wrong instruction position");
+
+ let pos_2 = script.instruction_indices().nth(2).unwrap().unwrap();
+ assert_eq!(pos_2.1, Instruction::Op(OP_NOP3), "nth(2) must be OP_NOP3");
+ assert_eq!(pos_2.0, 3, "nth(2) returned wrong instruction position");
+
+ // out-of-bounds index
+ assert!(
+ script.instruction_indices().nth(3).is_none(),
+ "nth(3) must return None since there are only 3 instructions"
+ );
+
+ // random access via .nth()
+ let mut iter = script.instruction_indices();
+ assert_eq!(iter.nth(1).unwrap().unwrap().0, 1);
+ assert_eq!(iter.nth(0).unwrap().unwrap().0, 3);
+ assert!(iter.nth(0).is_none());
+
+ // skips 2nd, returns 3rd
+ let pos_skip = script.instruction_indices().skip(2).next().unwrap().unwrap().0;
+ assert_eq!(pos_skip, 3, "skip(2).next() returned wrong position");
+}
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.