From b564111aff3ce8669bbc5547e7791ab99885ee37 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 18 Oct 2016 17:53:52 -0700 Subject: [PATCH] txscript: fix off-by-one error due to new OP_CODESEPARATOR behavior in segwit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- txscript/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/txscript/engine.go b/txscript/engine.go index 9bea68614..3c2e4142c 100644 --- a/txscript/engine.go +++ b/txscript/engine.go @@ -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 {