From d30f05b25059d79fd5efe94733e9627ed53cbc39 Mon Sep 17 00:00:00 2001 From: Ori Newman Date: Mon, 8 Feb 2021 10:04:19 +0200 Subject: [PATCH] Remove IsPushOnlyScript from mempool validation (#1492) * Remove IsPushOnlyScript from mempool validation * Fix TestCheckTransactionStandard --- domain/consensus/utils/txscript/error.go | 2 +- domain/consensus/utils/txscript/script.go | 11 ------- .../consensus/utils/txscript/script_test.go | 31 +++++++++++++------ domain/miningmanager/mempool/policy.go | 13 -------- domain/miningmanager/mempool/policy_test.go | 12 ------- 5 files changed, 22 insertions(+), 47 deletions(-) diff --git a/domain/consensus/utils/txscript/error.go b/domain/consensus/utils/txscript/error.go index 04c940016..73ddd3105 100644 --- a/domain/consensus/utils/txscript/error.go +++ b/domain/consensus/utils/txscript/error.go @@ -192,7 +192,7 @@ const ( // or to be in compressed format as a 32 byte string prefixed with 0x02 or 0x03 to signal oddness. ErrPubKeyFormat - // ErrCleanStack is returned when after evalution, the stack + // ErrCleanStack is returned when after evaluation, the stack // contains more than one element. ErrCleanStack diff --git a/domain/consensus/utils/txscript/script.go b/domain/consensus/utils/txscript/script.go index a65aa1037..61d677528 100644 --- a/domain/consensus/utils/txscript/script.go +++ b/domain/consensus/utils/txscript/script.go @@ -84,17 +84,6 @@ func isPushOnly(pops []parsedOpcode) bool { return true } -// IsPushOnlyScript returns whether or not the passed script only pushes data. -// -// False will be returned when the script does not parse. -func IsPushOnlyScript(script []byte) (bool, error) { - pops, err := parseScript(script) - if err != nil { - return false, err - } - return isPushOnly(pops), nil -} - // parseScriptTemplate is the same as parseScript but allows the passing of the // template list for testing purposes. When there are parse errors, it returns // the list of parsed opcodes up to the point of failure along with the error. diff --git a/domain/consensus/utils/txscript/script_test.go b/domain/consensus/utils/txscript/script_test.go index 16e2a708f..7e819366c 100644 --- a/domain/consensus/utils/txscript/script_test.go +++ b/domain/consensus/utils/txscript/script_test.go @@ -3723,6 +3723,17 @@ func TestPushedData(t *testing.T) { } } +// isPushOnlyScript returns whether or not the passed script only pushes data. +// +// False will be returned when the script does not parse. +func isPushOnlyScript(script []byte) (bool, error) { + pops, err := parseScript(script) + if err != nil { + return false, err + } + return isPushOnly(pops), nil +} + // TestHasCanonicalPush ensures the canonicalPush function works as expected. func TestHasCanonicalPush(t *testing.T) { t.Parallel() @@ -3734,8 +3745,8 @@ func TestHasCanonicalPush(t *testing.T) { err) continue } - if result, _ := IsPushOnlyScript(script); !result { - t.Errorf("IsPushOnlyScript: test #%d failed: %x\n", i, + if result, _ := isPushOnlyScript(script); !result { + t.Errorf("isPushOnlyScript: test #%d failed: %x\n", i, script) continue } @@ -3760,8 +3771,8 @@ func TestHasCanonicalPush(t *testing.T) { t.Errorf("StandardPushesTests test #%d unexpected error: %v\n", i, err) continue } - if result, _ := IsPushOnlyScript(script); !result { - t.Errorf("StandardPushesTests IsPushOnlyScript test #%d failed: %x\n", i, script) + if result, _ := isPushOnlyScript(script); !result { + t.Errorf("StandardPushesTests isPushOnlyScript test #%d failed: %x\n", i, script) continue } pops, err := parseScript(script) @@ -3889,9 +3900,9 @@ func TestHasCanonicalPushes(t *testing.T) { } } -// TestIsPushOnlyScript ensures the IsPushOnlyScript function returns the +// TestIsPushOnly ensures the isPushOnly function returns the // expected results. -func TestIsPushOnlyScript(t *testing.T) { +func TestIsPushOnly(t *testing.T) { t.Parallel() tests := []struct { @@ -3922,19 +3933,19 @@ func TestIsPushOnlyScript(t *testing.T) { } for _, test := range tests { - isPushOnly, err := IsPushOnlyScript(test.script) + isPushOnly, err := isPushOnlyScript(test.script) if isPushOnly != test.expectedResult { - t.Errorf("IsPushOnlyScript (%s) wrong result\ngot: %v\nwant: "+ + t.Errorf("isPushOnlyScript (%s) wrong result\ngot: %v\nwant: "+ "%v", test.name, isPushOnly, test.expectedResult) } if test.shouldFail && err == nil { - t.Errorf("IsPushOnlyScript (%s) expected an error but got ", test.name) + t.Errorf("isPushOnlyScript (%s) expected an error but got ", test.name) } if !test.shouldFail && err != nil { - t.Errorf("IsPushOnlyScript (%s) expected no error but got: %v", test.name, err) + t.Errorf("isPushOnlyScript (%s) expected no error but got: %v", test.name, err) } } } diff --git a/domain/miningmanager/mempool/policy.go b/domain/miningmanager/mempool/policy.go index ff5538cf7..583797227 100644 --- a/domain/miningmanager/mempool/policy.go +++ b/domain/miningmanager/mempool/policy.go @@ -222,19 +222,6 @@ func checkTransactionStandard(tx *consensusexternalapi.DomainTransaction, policy maxStandardSigScriptSize) return txRuleError(RejectNonstandard, str) } - - // Each transaction input signature script must only contain - // opcodes which push data onto the stack. - isPushOnly, err := txscript.IsPushOnlyScript(txIn.SignatureScript) - if err != nil { - str := fmt.Sprintf("transaction input %d: IsPushOnlyScript: %t. Error %s", i, isPushOnly, err) - return txRuleError(RejectNonstandard, str) - } - if !isPushOnly { - str := fmt.Sprintf("transaction input %d: signature "+ - "script is not push only", i) - return txRuleError(RejectNonstandard, str) - } } // None of the output public key scripts can be a non-standard script or diff --git a/domain/miningmanager/mempool/policy_test.go b/domain/miningmanager/mempool/policy_test.go index 1968db7c2..2919e83a5 100644 --- a/domain/miningmanager/mempool/policy_test.go +++ b/domain/miningmanager/mempool/policy_test.go @@ -235,18 +235,6 @@ func TestCheckTransactionStandard(t *testing.T) { isStandard: false, code: RejectNonstandard, }, - { - name: "Signature script that does more than push data", - tx: consensusexternalapi.DomainTransaction{Version: 0, Inputs: []*consensusexternalapi.DomainTransactionInput{{ - PreviousOutpoint: dummyPrevOut, - SignatureScript: []byte{ - txscript.OpCheckSigVerify}, - Sequence: constants.MaxTxInSequenceNum, - }}, Outputs: []*consensusexternalapi.DomainTransactionOutput{&dummyTxOut}}, - height: 300000, - isStandard: false, - code: RejectNonstandard, - }, { name: "Valid but non standard public key script", tx: consensusexternalapi.DomainTransaction{Version: 0, Inputs: []*consensusexternalapi.DomainTransactionInput{&dummyTxIn}, Outputs: []*consensusexternalapi.DomainTransactionOutput{{