txscript: fix off-by-one error due to new OP_CODESEPARATOR behavior in segwit

This commit fixes an off-by-one error which is only manifested by the
new behavior of OP_CODESEPARATOR within sig hashes triggered by the
segwit behavior. The current behavior within the Script VM
(txscript.Engine) is known to be fully correct to the extent that it has
been verified. However, once segwit activates a consensus divergence
would emerge due to *when* the program counter was incremented in the
previous code (pre-this-commit).

Currently (pre-segwit) when calculating the pre-image to a transaction
sighash for signature verification, *all* instances of OP_CODESEPARATOR
are removed from the subScript being signed before generating the final
sighash. SegWit has additional nerfed the behavior of OP_CODESEPARATOR
by no longer removing them (and starting after the last instance), but
instead simply starting the subScript to be directly *after* the last
instance of an OP_CODESEPARATOR within the pkScript.

Due to this new behavior, without this commit, an off-by-one error
(which only matters post-segwit), would cause txscript to generate an
incorrect subScript since the instance of OP_CODESEPARATOR would remain
as part of the subScript instead of being sliced off as the new behavior
dictates. The off-by-one error itself is manifested due to a slight
divergence in txscript.Engine’s logic compared to Bitcoin Core.  In
Bitcoin Core script verification is as follows: first the next op-code
is fetched, then program counter is incremented, and finally the op-code
itself is executed. Before this commit, btcd flipped the order
of the last two steps, executing the op-code *before* the program
counter was incremented.

This commit fixes the post-segwit consensus divergence by incrementing
the program-counter *before* the next op-code is executed. It is
important to note that this divergence is only significant post-segwit,
meaning that txscript.Engine is still consensus compliant independent of
this commit.
This commit is contained in:
Olaoluwa Osuntokun 2016-10-18 17:53:52 -07:00 committed by Dave Collins
parent 653459c810
commit b564111aff

View file

@ -288,6 +288,7 @@ func (vm *Engine) Step() (done bool, err error) {
return true, err
}
opcode := &vm.scripts[vm.scriptIdx][vm.scriptOff]
vm.scriptOff++
// Execute the opcode while taking into account several things such as
// disabled opcodes, illegal opcodes, maximum allowed operations per
@ -307,7 +308,6 @@ func (vm *Engine) Step() (done bool, err error) {
}
// Prepare for next instruction.
vm.scriptOff++
if vm.scriptOff >= len(vm.scripts[vm.scriptIdx]) {
// Illegal to have an `if' that straddles two scripts.
if err == nil && len(vm.condStack) != 0 {