From 8beb0dec54550f434561e09b1bd9087c2633d2df Mon Sep 17 00:00:00 2001 From: "Owain G. Ainsworth" Date: Fri, 14 Mar 2014 18:27:01 +0000 Subject: [PATCH] Make opcode.Exec run a bunch of unconditional checks Add push length and number of operations to this to match bitcoind behaviour. found that we differed here by their negative tests. --- opcode.go | 31 ++++++++++++++++++++++--------- script.go | 28 ++++++---------------------- 2 files changed, 28 insertions(+), 31 deletions(-) diff --git a/opcode.go b/opcode.go index 52b930b9..c9c1747f 100644 --- a/opcode.go +++ b/opcode.go @@ -932,17 +932,35 @@ func (pop *parsedOpcode) conditional() bool { } } +// exec peforms execution on the opcode. It takes into account whether or not +// it is hidden by conditionals, but some rules still must be tested in this +// case. func (pop *parsedOpcode) exec(s *Script) error { - // *sigh* bitcoind pretty much mandates that we violate layering here. - // Any opcode that isn't just adding data to the stack counts here - // as an operation. Note that OP_RESERVED is less than OP_16 and thus - // is counted as a push opcode here. + // Disabled opcodes are ``fail on program counter''. + if pop.disabled() { + return StackErrOpDisabled + } + + // Always-illegal opcodes are ``fail on program counter''. + if pop.alwaysIllegal() { + return StackErrAlwaysIllegal + } + + // Note that this includes OP_RESERVED which counts as a push operation. if pop.opcode.value > OP_16 { s.numOps++ if s.numOps > MaxOpsPerScript { return StackErrTooManyOperations } + } else if len(pop.data) > MaxScriptElementSize { + return StackErrElementTooBig + } + + // If we are not a conditional opcode and we aren't executing, then + // we are done now. + if s.condStack[0] != OpCondTrue && !pop.conditional() { + return nil } return pop.opcode.opfunc(pop, s) } @@ -1050,11 +1068,6 @@ func opcodeFalse(op *parsedOpcode, s *Script) error { } func opcodePushData(op *parsedOpcode, s *Script) error { - // This max script element test must occur at execution time instead - // of parse time to match bitcoind behaviour. - if len(op.data) > MaxScriptElementSize { - return StackErrElementTooBig - } s.dstack.PushByteArray(op.data) return nil } diff --git a/script.go b/script.go index f750858a..8f72339a 100644 --- a/script.go +++ b/script.go @@ -595,29 +595,13 @@ func (m *Script) Step() (done bool, err error) { // verify that it is pointing to a valid script address err = m.validPC() if err != nil { - return + return true, err } opcode := m.scripts[m.scriptidx][m.scriptoff] - executeInstr := true - if m.condStack[0] != OpCondTrue { - // some opcodes still 'activate' if on the non-executing side - // of conditional execution - if opcode.disabled() { - return true, StackErrOpDisabled - } else if opcode.alwaysIllegal() { - return true, StackErrAlwaysIllegal - } else if opcode.conditional() { - executeInstr = true - } else { - executeInstr = false - } - } - if executeInstr { - err = opcode.exec(m) - if err != nil { - return - } + err = opcode.exec(m) + if err != nil { + return true, err } // prepare for next instruction @@ -664,10 +648,10 @@ func (m *Script) Step() (done bool, err error) { } m.lastcodesep = 0 if m.scriptidx >= len(m.scripts) { - done = true + return true, nil } } - return + return false, nil } // curPC returns either the current script and offset, or an error if the