BIP352: fix loop range issue in get_pubkey_from_input function

[famouswizard]

I fixed an issue in the get_pubkey_from_input function where the loop for extracting the public key from the scriptSig wasn't correctly handling the range of indices. The loop now starts from the correct index, ensuring that the last 33 bytes of the vin.scriptSig are properly checked against the scriptPubKey hash. This change ensures the function works as intended without going out of bounds or missing the correct data.

Changes:

• Adjusted the loop range to correctly shift the window over the last 33 bytes of vin.scriptSig.
• Added a check to ensure that we don't go out of bounds when slicing the data.

This should resolve the issue with incorrectly processed scriptSig data.

[quapka]

Is it possible to add, previously failing and now passing, test vectors? I suppose they could go to this file https://github.com/bitcoin/bips/blob/master/bip-0352/send_and_receive_test_vectors.json

[jonatack]

Is it possible to add, previously failing and now passing, test vectors?

Agree, and please also make separate commits for (1) test coverage, if any is missing, (2) the proposed fix with updated test coverage, (3) code refactoring, and (4) code style and grammar changes.

[jonatack]

@famouswizard do you plan to update here, based on the feedback?

[jonatack]

Let's close this PR if no author update by May 1.

[murchandamus]

Please add an explanation to your commit message describing what problem you discovered and how your code change mitigates.

[murchandamus]

I’m not sure what issue is being fixed here. Before this loop, we have already determined that we are looking at a P2PKH input. The P2PKH input script consists of a signature and the pubkey. The pubkey bytes are the last 33 bytes of the input script. However, P2PKH is malleable and could have additional junk bytes after the pubkey. This loop starts by reading the last 33 bytes and in each iteration goes forward one byte until it finds the pubkey.

If I understand this code and the change right, the changed code skips reading the last 33 bytes which means that it breaks support for P2PKH. I may misunderstand what’s going on, if that is so, please elaborate what problem was discovered that this fixes.