OP_CODESEPARATOR bug fix

[sirdeggen]

This took about 16 hours to find 😫

Problem

The problem is that when using ARC to broadcast transactions, go-bt interpreter is used. Therefore the tx below is rejected because the signature will not validate. This is caused by the preimage being wrong within the interpreter around OP_CODESEPARATOR. It should slice off the previous parts of the script including the OP_CODESEPARATOR itself.

Transactions like this will broadcast via WoC and the nodes accept it, but ARC thinks it invalid.

0100000001abdbd5873fbda1b08c19d899993301fd44c0aa735064ebb2248260b7adadf795000000006b483045022100e7813394c7a55941c1acf3c7032046c2aa5bf3a506b4ee09e4cb5761c1850f960220154769af29eef81d56d69eba1d7a5ab37eed15beb9eadcd2cb608ff2e09b3147c321035941a219bcd9688318028afeef55183634f010a933de9d8469ff6e702d96c238ffffffff010271000000000000220687623971234575ab76a914fbcf31b659334eeb086693fc3b4005ce29e1c21788ac00000000

ARC responds:

{
    "detail": "Transaction is malformed and cannot be processed",
    "extraInfo": "arc error 461: script execution failed: false stack entry at end of script execution",
    "instance": null,
    "status": 461,
    "title": "Malformed transaction",
    "txid": "d25ba24ceabf3e6a57010a250225b0e5b80b2140d59dd77a8ac87eb63fb8d317",
    "type": "https://arc.bitcoinsv.com/errors/461"
}

Solution

Off by one bug, the subscript does not include the opcode separator itself, so the slice of the script should be from just after the op code separator - not before.

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% :tada:

Comparison is base (29189f4) 80.61% compared to head (55d53b3) 80.63%. Report is 1 commits behind head on master.

:exclamation: Current head 55d53b3 differs from pull request most recent head 9ef4e75. Consider uploading reports for the commit 9ef4e75 to get more accurate results

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #164 +/- ## ========================================== + Coverage 80.61% 80.63% +0.01% ========================================== Files 38 38 Lines 4112 4115 +3 ========================================== + Hits 3315 3318 +3 Misses 546 546 Partials 251 251 ``` | [Files Changed](https://app.codecov.io/gh/libsv/go-bt/pull/164?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=libsv) | Coverage Δ | | |---|---|---| | [bscript/interpreter/thread.go](https://app.codecov.io/gh/libsv/go-bt/pull/164?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=libsv#diff-YnNjcmlwdC9pbnRlcnByZXRlci90aHJlYWQuZ28=) | `94.35% <100.00%> (+0.04%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[sirdeggen]

Noticing this block here which conditionally removes the OPCODESEPARATOR afterwards but is conditional on NON-forkID sighashes:

// Remove the signature since there is no way for a signature
// to sign itself.
if !t.hasFlag(scriptflag.EnableSighashForkID) || !shf.Has(sighash.ForkID) {
    subScript = subScript.removeOpcodeByData(fullSigBytes)
    subScript = subScript.removeOpcode(bscript.OpCODESEPARATOR)
}

[sirdeggen]

We find in the ABC documentation:

• If the script contains any OP_CODESEPARATOR, the scriptCode is the script but removing everything up to
  and including the last executed OP_CODESEPARATOR
• before the signature checking opcode being executed [...] Notes:
  1. Contrary to the original algorithm, this one does not use FindAndDelete to remove the signature from the script.

Therefore I would recommend that we keep the code as is within this PR.