From bec90e253c284f1692429d2e3c05fe586363c35e Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Mon, 20 Apr 2015 00:32:43 -0500 Subject: [PATCH] txscript: Remove unneeded param from NewScript. This commit removes the unnecessary sigScript parameter from the txscript.NewScript function. This has bothered me for a while because it can and really should be obtained from the provided transaction and input index. The way it was, the passed script could technically be different than what is in the transaction. Obviously that would be an improper use of the API, but it's safer and more convenient to simply pull it from the provided transaction and index. Also, since the function signature is changing anyways, make the input index parameter come after the transaction which it references. --- blockchain/scriptval.go | 4 ++-- txscript/example_test.go | 4 ++-- txscript/internal_test.go | 10 ++++------ txscript/opcode_test.go | 7 ++----- txscript/script.go | 17 +++++++++++++---- txscript/script_test.go | 36 ++++++++++++++---------------------- 6 files changed, 37 insertions(+), 41 deletions(-) diff --git a/blockchain/scriptval.go b/blockchain/scriptval.go index fea983bb..3f56c07e 100644 --- a/blockchain/scriptval.go +++ b/blockchain/scriptval.go @@ -83,8 +83,8 @@ out: // Create a new script engine for the script pair. sigScript := txIn.SignatureScript pkScript := originMsgTx.TxOut[originTxIndex].PkScript - engine, err := txscript.NewScript(sigScript, pkScript, - txVI.txInIndex, txVI.tx.MsgTx(), v.flags) + engine, err := txscript.NewScript(pkScript, + txVI.tx.MsgTx(), txVI.txInIndex, v.flags) if err != nil { str := fmt.Sprintf("failed to parse input "+ "%s:%d which references output %s:%d - "+ diff --git a/txscript/example_test.go b/txscript/example_test.go index 64d0a987..3f86a036 100644 --- a/txscript/example_test.go +++ b/txscript/example_test.go @@ -164,8 +164,8 @@ func ExampleSignTxOutput() { flags := txscript.ScriptBip16 | txscript.ScriptVerifyDERSignatures | txscript.ScriptStrictMultiSig | txscript.ScriptDiscourageUpgradableNops - s, err := txscript.NewScript(redeemTx.TxIn[0].SignatureScript, - originTx.TxOut[0].PkScript, 0, redeemTx, flags) + s, err := txscript.NewScript(originTx.TxOut[0].PkScript, redeemTx, 0, + flags) if err != nil { fmt.Println(err) return diff --git a/txscript/internal_test.go b/txscript/internal_test.go index c1c3bd80..b94f0a6c 100644 --- a/txscript/internal_test.go +++ b/txscript/internal_test.go @@ -4202,7 +4202,7 @@ func TestBitcoindInvalidTests(t *testing.T) { continue } tx := createSpendingTx(scriptSig, scriptPubKey) - s, err := NewScript(scriptSig, scriptPubKey, 0, tx, flags) + s, err := NewScript(scriptPubKey, tx, 0, flags) if err == nil { if err := s.Execute(); err == nil { t.Errorf("%s test succeeded when it "+ @@ -4254,7 +4254,7 @@ func TestBitcoindValidTests(t *testing.T) { continue } tx := createSpendingTx(scriptSig, scriptPubKey) - s, err := NewScript(scriptSig, scriptPubKey, 0, tx, flags) + s, err := NewScript(scriptPubKey, tx, 0, flags) if err != nil { t.Errorf("%s failed to create script: %v", name, err) continue @@ -4391,8 +4391,7 @@ testloop: k, i, test) continue testloop } - s, err := NewScript(txin.SignatureScript, pkScript, k, - tx.MsgTx(), flags) + s, err := NewScript(pkScript, tx.MsgTx(), k, flags) if err != nil { t.Errorf("test (%d:%v:%d) failed to create "+ "script: %v", i, test, k, err) @@ -4537,8 +4536,7 @@ testloop: // These are meant to fail, so as soon as the first // input fails the transaction has failed. (some of the // test txns have good inputs, too.. - s, err := NewScript(txin.SignatureScript, pkScript, k, - tx.MsgTx(), flags) + s, err := NewScript(pkScript, tx.MsgTx(), k, flags) if err != nil { continue testloop } diff --git a/txscript/opcode_test.go b/txscript/opcode_test.go index 7c57707c..4b18b6a6 100644 --- a/txscript/opcode_test.go +++ b/txscript/opcode_test.go @@ -507,9 +507,7 @@ func TestScripts(t *testing.T) { flags = txscript.ScriptVerifyDERSignatures } mockTx.TxOut[0].PkScript = test.script - sigScript := mockTx.TxIn[0].SignatureScript - engine, err := txscript.NewScript(sigScript, test.script, 0, - mockTx, flags) + engine, err := txscript.NewScript(test.script, mockTx, 0, flags) if err == nil { err = engine.Execute() } @@ -4285,8 +4283,7 @@ func testOpcode(t *testing.T, test *detailedTest) { tx.TxOut[0].PkScript = test.script - engine, err := txscript.NewScript(tx.TxIn[0].SignatureScript, - tx.TxOut[0].PkScript, 0, tx, 0) + engine, err := txscript.NewScript(tx.TxOut[0].PkScript, tx, 0, 0) if err != nil { if err != test.expectedReturn { t.Errorf("Error return not expected %s: %v %v", diff --git a/txscript/script.go b/txscript/script.go index 1338c3de..5946c067 100644 --- a/txscript/script.go +++ b/txscript/script.go @@ -146,6 +146,10 @@ var ( // ErrInvalidFlags is returned when the passed flags to NewScript contain // an invalid combination. ErrInvalidFlags = errors.New("invalid flags combination") + + // ErrInvalidIndex is returned when the passed input index for the + // provided transaction is out of range. + ErrInvalidIndex = errors.New("invalid input index") ) const ( @@ -699,10 +703,15 @@ const ( ) // NewScript returns a new script engine for the provided tx and input idx with -// a signature script scriptSig and a pubkeyscript scriptPubKey. If bip16 is -// true then it will be treated as if the bip16 threshhold has passed and thus -// pay-to-script hash transactions will be fully validated. -func NewScript(scriptSig []byte, scriptPubKey []byte, txidx int, tx *wire.MsgTx, flags ScriptFlags) (*Script, error) { +// with a pubkeyscript scriptPubKey. If bip16 is true then it will be treated as +// if the bip16 threshhold has passed and thus pay-to-script hash transactions +// will be fully validated. +func NewScript(scriptPubKey []byte, tx *wire.MsgTx, txidx int, flags ScriptFlags) (*Script, error) { + if txidx < 0 || txidx >= len(tx.TxIn) { + return nil, ErrInvalidIndex + } + scriptSig := tx.TxIn[txidx].SignatureScript + var m Script if flags&ScriptVerifySigPushOnly == ScriptVerifySigPushOnly && !IsPushOnlyScript(scriptSig) { return nil, ErrStackNonPushOnly diff --git a/txscript/script_test.go b/txscript/script_test.go index 0777879b..2a332f42 100644 --- a/txscript/script_test.go +++ b/txscript/script_test.go @@ -1636,9 +1636,8 @@ func testTx(t *testing.T, test txTest) { if test.strictSigs { flags |= txscript.ScriptVerifyDERSignatures } - engine, err := txscript.NewScript( - test.tx.TxIn[test.idx].SignatureScript, test.pkScript, - test.idx, test.tx, flags) + engine, err := txscript.NewScript(test.pkScript, test.tx, test.idx, + flags) if err != nil { if err != test.parseErr { t.Errorf("Failed to parse %s: got \"%v\" expected "+ @@ -2470,8 +2469,7 @@ func TestBadPC(t *testing.T) { pkScript := []byte{txscript.OP_NOP} for _, test := range pcTests { - engine, err := txscript.NewScript(tx.TxIn[0].SignatureScript, - pkScript, 0, tx, 0) + engine, err := txscript.NewScript(pkScript, tx, 0, 0) if err != nil { t.Errorf("Failed to create script: %v", err) } @@ -2542,8 +2540,7 @@ func TestCheckErrorCondition(t *testing.T) { txscript.OP_TRUE, } - engine, err := txscript.NewScript(tx.TxIn[0].SignatureScript, pkScript, - 0, tx, 0) + engine, err := txscript.NewScript(pkScript, tx, 0, 0) if err != nil { t.Errorf("failed to create script: %v", err) } @@ -2892,10 +2889,9 @@ nexttest: // Validate tx input scripts scriptFlags := txscript.ScriptBip16 | txscript.ScriptVerifyDERSignatures - for j, txin := range tx.TxIn { - engine, err := txscript.NewScript(txin.SignatureScript, - SigScriptTests[i].inputs[j].txout.PkScript, - j, tx, scriptFlags) + for j := range tx.TxIn { + engine, err := txscript.NewScript(SigScriptTests[i]. + inputs[j].txout.PkScript, tx, j, scriptFlags) if err != nil { t.Errorf("cannot create script vm for test %v: %v", SigScriptTests[i].name, err) @@ -3307,9 +3303,8 @@ func signAndCheck(msg string, tx *wire.MsgTx, idx int, pkScript []byte, hashType txscript.SigHashType, kdb txscript.KeyDB, sdb txscript.ScriptDB, previousScript []byte) error { - sigScript, err := txscript.SignTxOutput( - &chaincfg.TestNet3Params, tx, idx, pkScript, hashType, - kdb, sdb, []byte{}) + sigScript, err := txscript.SignTxOutput(&chaincfg.TestNet3Params, tx, + idx, pkScript, hashType, kdb, sdb, []byte{}) if err != nil { return fmt.Errorf("failed to sign output %s: %v", msg, err) } @@ -3317,11 +3312,10 @@ func signAndCheck(msg string, tx *wire.MsgTx, idx int, pkScript []byte, return checkScripts(msg, tx, idx, sigScript, pkScript) } -func checkScripts(msg string, tx *wire.MsgTx, idx int, - sigScript, pkScript []byte) error { - engine, err := txscript.NewScript(sigScript, pkScript, idx, tx, - txscript.ScriptBip16| - txscript.ScriptVerifyDERSignatures) +func checkScripts(msg string, tx *wire.MsgTx, idx int, sigScript, pkScript []byte) error { + tx.TxIn[idx].SignatureScript = sigScript + engine, err := txscript.NewScript(pkScript, tx, idx, + txscript.ScriptBip16|txscript.ScriptVerifyDERSignatures) if err != nil { return fmt.Errorf("failed to make script engine for %s: %v", msg, err) @@ -4857,9 +4851,7 @@ func TestInvalidFlagCombinations(t *testing.T) { pkScript := []byte{txscript.OP_NOP} for i, test := range tests { - _, err := txscript.NewScript(tx.TxIn[0].SignatureScript, - pkScript, 0, tx, test) - + _, err := txscript.NewScript(pkScript, tx, 0, test) if err != txscript.ErrInvalidFlags { t.Fatalf("TestInvalidFlagCombinations #%d unexpected "+ "error: %v", i, err)