diff options
| author | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-08-14 11:45:28 +0200 |
|---|---|---|
| committer | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-08-14 11:53:47 +0200 |
| commit | 4d4bd5ed7474fac735ae6e0647a87b490e6ff1fa (patch) | |
| tree | e00bdf0e3b8da3f99e50f5ca7770e4b998a32db1 | |
| parent | dabab06a1a1a981231c6376d202b7a66d5cf3179 (diff) | |
| parent | dca28634d779c775678cba402a85fe5bb9b3a5a9 (diff) | |
Merge #17204: wallet: Do not turn OP_1NEGATE in scriptSig into 0x0181 in signing code (sipa)
dca28634d779c775678cba402a85fe5bb9b3a5a9 test: ensure OP_1NEGATE satisfies BIP62 minimal push rule (Jon Atack)
e629d07199b83f4ad313b23a94c9016e3276c52a Do not turn OP_1NEGATE in scriptSig into 0x0181 in signing code (Pieter Wuille)
Pull request description:
A rebase of #13084 which additionally modifies the test code (unaddressed in the original, assuming sipa is too busy to deal with this at the moment).
Relatively simple bugfix so it'd be good to have merged soon.
Turning OP_1NEGATE into 0x0181 results in a larger-than-necessary data push instead of just actually using the OP_1NEGATE opcode (0x4f). This fails the minimal push rule of BIP 62 and makes the result non-standard.
ACKs for top commit:
fjahr:
Code review ACK dca28634d779c775678cba402a85fe5bb9b3a5a9
luke-jr:
ACK dca28634d77
jonatack:
ACK dca28634d779c775678cba402a85fe5bb9b3a5a9
Tree-SHA512: 706d9a2ef20c809dea923e477a873e2fd60db8d0ae64289e510b766a38005c1f31ab0b5883f16b9c7863ff0d3f705e8e413f6121320028ac196b79c3184a4113
| -rw-r--r-- | src/script/sign.cpp | 2 | ||||
| -rw-r--r-- | src/test/transaction_tests.cpp | 2 | ||||
| -rwxr-xr-x | test/functional/rpc_signrawtransaction.py | 20 |
3 files changed, 24 insertions, 0 deletions
diff --git a/src/script/sign.cpp b/src/script/sign.cpp index f425215549..9b3f94f14d 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -186,6 +186,8 @@ static CScript PushAll(const std::vector<valtype>& values) result << OP_0; } else if (v.size() == 1 && v[0] >= 1 && v[0] <= 16) { result << CScript::EncodeOP_N(v[0]); + } else if (v.size() == 1 && v[0] == 0x81) { + result << OP_1NEGATE; } else { result << v; } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 4bf6e734ce..c30f44292c 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -361,6 +361,8 @@ static CScript PushAll(const std::vector<valtype>& values) result << OP_0; } else if (v.size() == 1 && v[0] >= 1 && v[0] <= 16) { result << CScript::EncodeOP_N(v[0]); + } else if (v.size() == 1 && v[0] == 0x81) { + result << OP_1NEGATE; } else { result << v; } diff --git a/test/functional/rpc_signrawtransaction.py b/test/functional/rpc_signrawtransaction.py index 3d08202724..704b65c060 100755 --- a/test/functional/rpc_signrawtransaction.py +++ b/test/functional/rpc_signrawtransaction.py @@ -198,10 +198,30 @@ class SignRawTransactionsTest(BitcoinTestFramework): assert_equal(spending_tx_signed['complete'], True) self.nodes[0].sendrawtransaction(spending_tx_signed['hex']) + def OP_1NEGATE_test(self): + self.log.info("Test OP_1NEGATE (0x4f) satisfies BIP62 minimal push standardness rule") + hex_str = ( + "0200000001FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF" + "FFFFFFFF00000000044F024F9CFDFFFFFF01F0B9F5050000000023210277777777" + "77777777777777777777777777777777777777777777777777777777AC66030000" + ) + prev_txs = [ + { + "txid": "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF", + "vout": 0, + "scriptPubKey": "A914AE44AB6E9AA0B71F1CD2B453B69340E9BFBAEF6087", + "redeemScript": "4F9C", + "amount": 1, + } + ] + txn = self.nodes[0].signrawtransactionwithwallet(hex_str, prev_txs) + assert txn["complete"] + def run_test(self): self.successful_signing_test() self.script_verification_error_test() self.witness_script_test() + self.OP_1NEGATE_test() self.test_with_lock_outputs() |