From 18dfea0dd082af18dfb02981b7ee1cd44d514388 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Sat, 14 Jul 2018 16:03:28 -0700 Subject: [PATCH 1/3] Always create 70 byte signatures with low R values When extra entropy is not specified by the caller, CKey::Sign will now always create a signature that has a low R value and is at most 70 bytes. The resulting signature on the stack will be 71 bytes when the sighash byte is included. Using low R signatures means that the resulting DER encoded signature will never need to have additional padding to account for high R values. --- src/bench/verify_script.cpp | 2 +- src/key.cpp | 24 ++++++++++++++++++-- src/key.h | 2 +- src/test/key_tests.cpp | 36 ++++++++++++++++++++++++++++++ src/test/script_tests.cpp | 2 +- test/functional/data/rpc_psbt.json | 8 ------- test/util/data/txcreatesignv1.hex | 2 +- test/util/data/txcreatesignv1.json | 16 ++++++------- 8 files changed, 70 insertions(+), 22 deletions(-) diff --git a/src/bench/verify_script.cpp b/src/bench/verify_script.cpp index ae60588c2d8..1d599b2685f 100644 --- a/src/bench/verify_script.cpp +++ b/src/bench/verify_script.cpp @@ -76,7 +76,7 @@ static void VerifyScriptBench(benchmark::State& state) CMutableTransaction txSpend = BuildSpendingTransaction(scriptSig, txCredit); CScriptWitness& witness = txSpend.vin[0].scriptWitness; witness.stack.emplace_back(); - key.Sign(SignatureHash(witScriptPubkey, txSpend, 0, SIGHASH_ALL, txCredit.vout[0].nValue, SigVersion::WITNESS_V0), witness.stack.back(), 0); + key.Sign(SignatureHash(witScriptPubkey, txSpend, 0, SIGHASH_ALL, txCredit.vout[0].nValue, SigVersion::WITNESS_V0), witness.stack.back()); witness.stack.back().push_back(static_cast(SIGHASH_ALL)); witness.stack.push_back(ToByteVector(pubkey)); diff --git a/src/key.cpp b/src/key.cpp index 94be179bfb4..69af255be6b 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -189,7 +189,20 @@ CPubKey CKey::GetPubKey() const { return result; } -bool CKey::Sign(const uint256 &hash, std::vector& vchSig, uint32_t test_case) const { +// Check that the sig has a low R value and will be less than 71 bytes +bool SigHasLowR(const secp256k1_ecdsa_signature* sig) +{ + unsigned char compact_sig[64]; + secp256k1_ecdsa_signature_serialize_compact(secp256k1_context_sign, compact_sig, sig); + + // In DER serialization, all values are interpreted as big-endian, signed integers. The highest bit in the integer indicates + // its signed-ness; 0 is positive, 1 is negative. When the value is interpreted as a negative integer, it must be converted + // to a positive value by prepending a 0x00 byte so that the highest bit is 0. We can avoid this prepending by ensuring that + // our highest bit is always 0, and thus we must check that the first byte is less than 0x80. + return compact_sig[0] < 0x80; +} + +bool CKey::Sign(const uint256 &hash, std::vector& vchSig, bool grind, uint32_t test_case) const { if (!fValid) return false; vchSig.resize(CPubKey::SIGNATURE_SIZE); @@ -197,7 +210,14 @@ bool CKey::Sign(const uint256 &hash, std::vector& vchSig, uint32_ unsigned char extra_entropy[32] = {0}; WriteLE32(extra_entropy, test_case); secp256k1_ecdsa_signature sig; - int ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, test_case ? extra_entropy : nullptr); + uint32_t counter = 0; + int ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, (!grind && test_case) ? extra_entropy : nullptr); + + // Grind for low R + while (ret && !SigHasLowR(&sig) && grind) { + WriteLE32(extra_entropy, ++counter); + ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, extra_entropy); + } assert(ret); secp256k1_ecdsa_signature_serialize_der(secp256k1_context_sign, vchSig.data(), &nSigLen, &sig); vchSig.resize(nSigLen); diff --git a/src/key.h b/src/key.h index f573a18a4ef..ce49b3df02d 100644 --- a/src/key.h +++ b/src/key.h @@ -114,7 +114,7 @@ public: * Create a DER-serialized signature. * The test_case parameter tweaks the deterministic nonce. */ - bool Sign(const uint256& hash, std::vector& vchSig, uint32_t test_case = 0) const; + bool Sign(const uint256& hash, std::vector& vchSig, bool grind = true, uint32_t test_case = 0) const; /** * Create a compact signature (65 bytes), which allows reconstructing the used public key. diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index 64c57f0705f..acd213dac21 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -152,4 +152,40 @@ BOOST_AUTO_TEST_CASE(key_test1) BOOST_CHECK(detsigc == ParseHex("2052d8a32079c11e79db95af63bb9600c5b04f21a9ca33dc129c2bfa8ac9dc1cd561d8ae5e0f6c1a16bde3719c64c2fd70e404b6428ab9a69566962e8771b5944d")); } +BOOST_AUTO_TEST_CASE(key_signature_tests) +{ + // When entropy is specified, we should see at least one high R signature within 20 signatures + CKey key = DecodeSecret(strSecret1); + std::string msg = "A message to be signed"; + uint256 msg_hash = Hash(msg.begin(), msg.end()); + std::vector sig; + bool found = false; + + for (int i = 1; i <=20; ++i) { + sig.clear(); + key.Sign(msg_hash, sig, false, i); + found = sig[3] == 0x21 && sig[4] == 0x00; + if (found) { + break; + } + } + BOOST_CHECK(found); + + // When entropy is not specified, we should always see low R signatures that are less than 70 bytes in 256 tries + // We should see at least one signature that is less than 70 bytes. + found = true; + bool found_small = false; + for (int i = 0; i < 256; ++i) { + sig.clear(); + std::string msg = "A message to be signed" + std::to_string(i); + msg_hash = Hash(msg.begin(), msg.end()); + key.Sign(msg_hash, sig); + found = sig[3] == 0x20; + BOOST_CHECK(sig.size() <= 70); + found_small |= sig.size() < 70; + } + BOOST_CHECK(found); + BOOST_CHECK(found_small); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 510910e149c..63a98928216 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -369,7 +369,7 @@ public: std::vector vchSig, r, s; uint32_t iter = 0; do { - key.Sign(hash, vchSig, iter++); + key.Sign(hash, vchSig, false, iter++); if ((lenS == 33) != (vchSig[5 + vchSig[3]] == 33)) { NegateSignatureS(vchSig); } diff --git a/test/functional/data/rpc_psbt.json b/test/functional/data/rpc_psbt.json index 4e2f08f2749..d800fa97a5c 100644 --- a/test/functional/data/rpc_psbt.json +++ b/test/functional/data/rpc_psbt.json @@ -57,14 +57,6 @@ ], "psbt" : "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAABBEdSIQKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgfyEC2rYf9JoU22p9ArDNH7t4/EsYMStbTlTa5Nui+/71NtdSriIGApWDvzmuCmCXR60Zmt3WNPphCFWdbFzTm0whg/GrluB/ENkMak8AAACAAAAAgAAAAIAiBgLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU21xDZDGpPAAAAgAAAAIABAACAAQMEAQAAAAABASAAwusLAAAAABepFLf1+vQOPUClpFmx2zU18rcvqSHohwEEIgAgjCNTFzdDtZXftKB7crqOQuN5fadOh/59nXSX47ICiQMBBUdSIQMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3CECOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnNSriIGAjrdkE89bc9Z3bkGsN7iNSm3/7ntUOXoYVGSaGAiHw5zENkMak8AAACAAAAAgAMAAIAiBgMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3BDZDGpPAAAAgAAAAIACAACAAQMEAQAAAAAiAgOppMN/WZbTqiXbrGtXCvBlA5RJKUJGCzVHU+2e7KWHcRDZDGpPAAAAgAAAAIAEAACAACICAn9jmXV9Lv9VoTatAsaEsYOLZVbl8bazQoKpS2tQBRCWENkMak8AAACAAAAAgAUAAIAA", "result" : "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAAiAgKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgf0cwRAIgdAGK1BgAl7hzMjwAFXILNoTMgSOJEEjn282bVa1nnJkCIHPTabdA4+tT3O+jOCPIBwUUylWn3ZVE8VfBZ5EyYRGMAQEDBAEAAAABBEdSIQKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgfyEC2rYf9JoU22p9ArDNH7t4/EsYMStbTlTa5Nui+/71NtdSriIGApWDvzmuCmCXR60Zmt3WNPphCFWdbFzTm0whg/GrluB/ENkMak8AAACAAAAAgAAAAIAiBgLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU21xDZDGpPAAAAgAAAAIABAACAAAEBIADC6wsAAAAAF6kUt/X69A49QKWkWbHbNTXyty+pIeiHIgIDCJ3BDHrG21T5EymvYXMz2ziM6tDCMfcjN50bmQMLAtxHMEQCIGLrelVhB6fHP0WsSrWh3d9vcHX7EnWWmn84Pv/3hLyyAiAMBdu3Rw2/LwhVfdNWxzJcHtMJE+mWzThAlF2xIijaXwEBAwQBAAAAAQQiACCMI1MXN0O1ld+0oHtyuo5C43l9p06H/n2ddJfjsgKJAwEFR1IhAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcIQI63ZBPPW3PWd25BrDe4jUpt/+57VDl6GFRkmhgIh8Oc1KuIgYCOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnMQ2QxqTwAAAIAAAACAAwAAgCIGAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcENkMak8AAACAAAAAgAIAAIAAIgIDqaTDf1mW06ol26xrVwrwZQOUSSlCRgs1R1Ptnuylh3EQ2QxqTwAAAIAAAACABAAAgAAiAgJ/Y5l1fS7/VaE2rQLGhLGDi2VW5fG2s0KCqUtrUAUQlhDZDGpPAAAAgAAAAIAFAACAAA==" - }, - { - "privkeys" : [ - "cT7J9YpCwY3AVRFSjN6ukeEeWY6mhpbJPxRaDaP5QTdygQRxP9Au", - "cNBc3SWUip9PPm1GjRoLEJT6T41iNzCYtD7qro84FMnM5zEqeJsE" - ], - "psbt" : "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAABBEdSIQKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgfyEC2rYf9JoU22p9ArDNH7t4/EsYMStbTlTa5Nui+/71NtdSriIGApWDvzmuCmCXR60Zmt3WNPphCFWdbFzTm0whg/GrluB/ENkMak8AAACAAAAAgAAAAIAiBgLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU21xDZDGpPAAAAgAAAAIABAACAAQMEAQAAAAABASAAwusLAAAAABepFLf1+vQOPUClpFmx2zU18rcvqSHohwEEIgAgjCNTFzdDtZXftKB7crqOQuN5fadOh/59nXSX47ICiQMBBUdSIQMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3CECOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnNSriIGAjrdkE89bc9Z3bkGsN7iNSm3/7ntUOXoYVGSaGAiHw5zENkMak8AAACAAAAAgAMAAIAiBgMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3BDZDGpPAAAAgAAAAIACAACAAQMEAQAAAAAiAgOppMN/WZbTqiXbrGtXCvBlA5RJKUJGCzVHU+2e7KWHcRDZDGpPAAAAgAAAAIAEAACAACICAn9jmXV9Lv9VoTatAsaEsYOLZVbl8bazQoKpS2tQBRCWENkMak8AAACAAAAAgAUAAIAA", - "result" : "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAAiAgLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU210gwRQIhAPYQOLMI3B2oZaNIUnRvAVdyk0IIxtJEVDk82ZvfIhd3AiAFbmdaZ1ptCgK4WxTl4pB02KJam1dgvqKBb2YZEKAG6gEBAwQBAAAAAQRHUiEClYO/Oa4KYJdHrRma3dY0+mEIVZ1sXNObTCGD8auW4H8hAtq2H/SaFNtqfQKwzR+7ePxLGDErW05U2uTbovv+9TbXUq4iBgKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgfxDZDGpPAAAAgAAAAIAAAACAIgYC2rYf9JoU22p9ArDNH7t4/EsYMStbTlTa5Nui+/71NtcQ2QxqTwAAAIAAAACAAQAAgAABASAAwusLAAAAABepFLf1+vQOPUClpFmx2zU18rcvqSHohyICAjrdkE89bc9Z3bkGsN7iNSm3/7ntUOXoYVGSaGAiHw5zRzBEAiBl9FulmYtZon/+GnvtAWrx8fkNVLOqj3RQql9WolEDvQIgf3JHA60e25ZoCyhLVtT/y4j3+3Weq74IqjDym4UTg9IBAQMEAQAAAAEEIgAgjCNTFzdDtZXftKB7crqOQuN5fadOh/59nXSX47ICiQMBBUdSIQMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3CECOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnNSriIGAjrdkE89bc9Z3bkGsN7iNSm3/7ntUOXoYVGSaGAiHw5zENkMak8AAACAAAAAgAMAAIAiBgMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3BDZDGpPAAAAgAAAAIACAACAACICA6mkw39ZltOqJdusa1cK8GUDlEkpQkYLNUdT7Z7spYdxENkMak8AAACAAAAAgAQAAIAAIgICf2OZdX0u/1WhNq0CxoSxg4tlVuXxtrNCgqlLa1AFEJYQ2QxqTwAAAIAAAACABQAAgAA=" } ], "combiner" : [ diff --git a/test/util/data/txcreatesignv1.hex b/test/util/data/txcreatesignv1.hex index a46fcc88cbc..40039319bd2 100644 --- a/test/util/data/txcreatesignv1.hex +++ b/test/util/data/txcreatesignv1.hex @@ -1 +1 @@ -01000000018594c5bdcaec8f06b78b596f31cd292a294fd031e24eec716f43dac91ea7494d000000008b48304502210096a75056c9e2cc62b7214777b3d2a592cfda7092520126d4ebfcd6d590c99bd8022051bb746359cf98c0603f3004477eac68701132380db8facba19c89dc5ab5c5e201410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ffffffff01a0860100000000001976a9145834479edbbe0539b31ffd3a8f8ebadc2165ed0188ac00000000 +01000000018594c5bdcaec8f06b78b596f31cd292a294fd031e24eec716f43dac91ea7494d000000008a4730440220131432090a6af42da3e8335ff110831b41a44f4e9d18d88f5d50278380696c7202200fc2e48938f323ad13625890c0ea926c8a189c08b8efc38376b20c8a2188e96e01410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ffffffff01a0860100000000001976a9145834479edbbe0539b31ffd3a8f8ebadc2165ed0188ac00000000 diff --git a/test/util/data/txcreatesignv1.json b/test/util/data/txcreatesignv1.json index 64e5137f4b5..7a06aa9ffea 100644 --- a/test/util/data/txcreatesignv1.json +++ b/test/util/data/txcreatesignv1.json @@ -1,18 +1,18 @@ { - "txid": "977e7cd286cb72cd470d539ba6cb48400f8f387d97451d45cdb8819437a303af", - "hash": "977e7cd286cb72cd470d539ba6cb48400f8f387d97451d45cdb8819437a303af", + "txid": "ffc7e509ec3fd60a182eb712621d41a47dc7d4ff310a70826c2fb0e9afb3fa02", + "hash": "ffc7e509ec3fd60a182eb712621d41a47dc7d4ff310a70826c2fb0e9afb3fa02", "version": 1, - "size": 224, - "vsize": 224, - "weight": 896, + "size": 223, + "vsize": 223, + "weight": 892, "locktime": 0, "vin": [ { "txid": "4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485", "vout": 0, "scriptSig": { - "asm": "304502210096a75056c9e2cc62b7214777b3d2a592cfda7092520126d4ebfcd6d590c99bd8022051bb746359cf98c0603f3004477eac68701132380db8facba19c89dc5ab5c5e2[ALL] 0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8", - "hex": "48304502210096a75056c9e2cc62b7214777b3d2a592cfda7092520126d4ebfcd6d590c99bd8022051bb746359cf98c0603f3004477eac68701132380db8facba19c89dc5ab5c5e201410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8" + "asm": "30440220131432090a6af42da3e8335ff110831b41a44f4e9d18d88f5d50278380696c7202200fc2e48938f323ad13625890c0ea926c8a189c08b8efc38376b20c8a2188e96e[ALL] 0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8", + "hex": "4730440220131432090a6af42da3e8335ff110831b41a44f4e9d18d88f5d50278380696c7202200fc2e48938f323ad13625890c0ea926c8a189c08b8efc38376b20c8a2188e96e01410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8" }, "sequence": 4294967295 } @@ -32,5 +32,5 @@ } } ], - "hex": "01000000018594c5bdcaec8f06b78b596f31cd292a294fd031e24eec716f43dac91ea7494d000000008b48304502210096a75056c9e2cc62b7214777b3d2a592cfda7092520126d4ebfcd6d590c99bd8022051bb746359cf98c0603f3004477eac68701132380db8facba19c89dc5ab5c5e201410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ffffffff01a0860100000000001976a9145834479edbbe0539b31ffd3a8f8ebadc2165ed0188ac00000000" + "hex": "01000000018594c5bdcaec8f06b78b596f31cd292a294fd031e24eec716f43dac91ea7494d000000008a4730440220131432090a6af42da3e8335ff110831b41a44f4e9d18d88f5d50278380696c7202200fc2e48938f323ad13625890c0ea926c8a189c08b8efc38376b20c8a2188e96e01410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ffffffff01a0860100000000001976a9145834479edbbe0539b31ffd3a8f8ebadc2165ed0188ac00000000" } From 48b1473c898129a99212e2db36c61cf93625ea17 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Sat, 14 Jul 2018 19:19:44 -0700 Subject: [PATCH 2/3] Use 71 byte signature for DUMMY_SIGNATURE_CREATOR Changes DUMMY_SIGNATURE_CREATOR to create 71 byte dummy signatures. Update comments to reflect this change --- src/script/sign.cpp | 14 +++++++------- src/wallet/wallet.cpp | 4 ++-- src/wallet/wallet.h | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index fa09adbaf88..c103e3c2f02 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -423,16 +423,16 @@ public: bool CreateSig(const SigningProvider& provider, std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override { // Create a dummy signature that is a valid DER-encoding - vchSig.assign(72, '\000'); + vchSig.assign(71, '\000'); vchSig[0] = 0x30; - vchSig[1] = 69; + vchSig[1] = 68; vchSig[2] = 0x02; - vchSig[3] = 33; + vchSig[3] = 32; vchSig[4] = 0x01; - vchSig[4 + 33] = 0x02; - vchSig[5 + 33] = 32; - vchSig[6 + 33] = 0x01; - vchSig[6 + 33 + 32] = SIGHASH_ALL; + vchSig[4 + 32] = 0x02; + vchSig[5 + 32] = 32; + vchSig[6 + 32] = 0x01; + vchSig[6 + 32 + 32] = SIGHASH_ALL; return true; } }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 16568d0e050..173f1b786c9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1540,7 +1540,7 @@ int64_t CWalletTx::GetTxTime() const return n ? n : nTimeReceived; } -// Helper for producing a max-sized low-S signature (eg 72 bytes) +// Helper for producing a max-sized low-S low-R signature (eg 71 bytes) bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout) const { // Fill in dummy signatures for fee calculation. @@ -1556,7 +1556,7 @@ bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout) const return true; } -// Helper for producing a bunch of max-sized low-S signatures (eg 72 bytes) +// Helper for producing a bunch of max-sized low-S low-R signatures (eg 71 bytes) bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector &txouts) const { // Fill in dummy signatures for fee calculation. diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 06a7c0a7526..8054cfadf03 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1308,7 +1308,7 @@ public: }; // Calculate the size of the transaction assuming all signatures are max size -// Use DummySignatureCreator, which inserts 72 byte signatures everywhere. +// Use DummySignatureCreator, which inserts 71 byte signatures everywhere. // NOTE: this requires that all inputs must be in mapWallet (eg the tx should // be IsAllFromMe). int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet); From e306be742932d4ea5aca0ea4768e54b2fc3dc6a0 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 7 Aug 2018 16:59:53 -0700 Subject: [PATCH 3/3] Use 72 byte dummy signatures when watching only inputs may be used With watching only inputs, we do not know how large the signatures for those inputs will be as their signers may not have implemented 71 byte signatures. Thus we estimate their fees using the 72 byte dummy signature to ensure that we pay enough fees. This only effects fundrawtransaction when includeWatching is true. --- src/script/sign.cpp | 22 +++++++++++++--------- src/script/sign.h | 4 +++- src/wallet/wallet.cpp | 29 ++++++++++++++--------------- src/wallet/wallet.h | 27 +++++++++++++++------------ 4 files changed, 45 insertions(+), 37 deletions(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index c103e3c2f02..66d942d7c5d 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -417,22 +417,25 @@ public: const DummySignatureChecker DUMMY_CHECKER; class DummySignatureCreator final : public BaseSignatureCreator { +private: + char m_r_len = 32; + char m_s_len = 32; public: - DummySignatureCreator() {} + DummySignatureCreator(char r_len, char s_len) : m_r_len(r_len), m_s_len(s_len) {} const BaseSignatureChecker& Checker() const override { return DUMMY_CHECKER; } bool CreateSig(const SigningProvider& provider, std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override { // Create a dummy signature that is a valid DER-encoding - vchSig.assign(71, '\000'); + vchSig.assign(m_r_len + m_s_len + 7, '\000'); vchSig[0] = 0x30; - vchSig[1] = 68; + vchSig[1] = m_r_len + m_s_len + 4; vchSig[2] = 0x02; - vchSig[3] = 32; + vchSig[3] = m_r_len; vchSig[4] = 0x01; - vchSig[4 + 32] = 0x02; - vchSig[5 + 32] = 32; - vchSig[6 + 32] = 0x01; - vchSig[6 + 32 + 32] = SIGHASH_ALL; + vchSig[4 + m_r_len] = 0x02; + vchSig[5 + m_r_len] = m_s_len; + vchSig[6 + m_r_len] = 0x01; + vchSig[6 + m_r_len + m_s_len] = SIGHASH_ALL; return true; } }; @@ -450,7 +453,8 @@ bool LookupHelper(const M& map, const K& key, V& value) } -const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR = DummySignatureCreator(); +const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR = DummySignatureCreator(32, 32); +const BaseSignatureCreator& DUMMY_MAXIMUM_SIGNATURE_CREATOR = DummySignatureCreator(33, 32); const SigningProvider& DUMMY_SIGNING_PROVIDER = SigningProvider(); bool IsSolvable(const SigningProvider& provider, const CScript& script) diff --git a/src/script/sign.h b/src/script/sign.h index 96ef59fbe86..a614d17a9f2 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -80,8 +80,10 @@ public: bool CreateSig(const SigningProvider& provider, std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override; }; -/** A signature creator that just produces 72-byte empty signatures. */ +/** A signature creator that just produces 71-byte empty signatures. */ extern const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR; +/** A signature creator that just produces 72-byte empty signatures. */ +extern const BaseSignatureCreator& DUMMY_MAXIMUM_SIGNATURE_CREATOR; typedef std::pair> SigPair; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 173f1b786c9..3a473e4de37 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1541,29 +1541,28 @@ int64_t CWalletTx::GetTxTime() const } // Helper for producing a max-sized low-S low-R signature (eg 71 bytes) -bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout) const +// or a max-sized low-S signature (e.g. 72 bytes) if use_max_sig is true +bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig) const { // Fill in dummy signatures for fee calculation. const CScript& scriptPubKey = txout.scriptPubKey; SignatureData sigdata; - if (!ProduceSignature(*this, DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) - { + if (!ProduceSignature(*this, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) { return false; - } else { - UpdateInput(tx_in, sigdata); } + UpdateInput(tx_in, sigdata); return true; } // Helper for producing a bunch of max-sized low-S low-R signatures (eg 71 bytes) -bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector &txouts) const +bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector &txouts, bool use_max_sig) const { // Fill in dummy signatures for fee calculation. int nIn = 0; for (const auto& txout : txouts) { - if (!DummySignInput(txNew.vin[nIn], txout)) { + if (!DummySignInput(txNew.vin[nIn], txout, use_max_sig)) { return false; } @@ -1572,7 +1571,7 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector return true; } -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet) +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig) { std::vector txouts; // Look up the inputs. We should have already checked that this transaction @@ -1586,14 +1585,14 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall assert(input.prevout.n < mi->second.tx->vout.size()); txouts.emplace_back(mi->second.tx->vout[input.prevout.n]); } - return CalculateMaximumSignedTxSize(tx, wallet, txouts); + return CalculateMaximumSignedTxSize(tx, wallet, txouts, use_max_sig); } // txouts needs to be in the order of tx.vin -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts) +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts, bool use_max_sig) { CMutableTransaction txNew(tx); - if (!wallet->DummySignTx(txNew, txouts)) { + if (!wallet->DummySignTx(txNew, txouts, use_max_sig)) { // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) // implies that we can sign for every input. return -1; @@ -1601,11 +1600,11 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall return GetVirtualTransactionSize(txNew); } -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet) +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, bool use_max_sig) { CMutableTransaction txn; txn.vin.push_back(CTxIn(COutPoint())); - if (!wallet->DummySignInput(txn.vin[0], txout)) { + if (!wallet->DummySignInput(txn.vin[0], txout, use_max_sig)) { // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) // implies that we can sign for every input. return -1; @@ -2332,7 +2331,7 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const bool solvable = IsSolvable(*this, pcoin->tx->vout[i].scriptPubKey); bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable)); - vCoins.push_back(COutput(pcoin, i, nDepth, spendable, solvable, safeTx)); + vCoins.push_back(COutput(pcoin, i, nDepth, spendable, solvable, safeTx, (coinControl && coinControl->fAllowWatchOnly))); // Checks the sum amount of all UTXO's. if (nMinimumSumAmount != MAX_MONEY) { @@ -2881,7 +2880,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac txNew.vin.push_back(CTxIn(coin.outpoint,CScript())); } - nBytes = CalculateMaximumSignedTxSize(txNew, this); + nBytes = CalculateMaximumSignedTxSize(txNew, this, coin_control.fAllowWatchOnly); if (nBytes < 0) { strFailReason = _("Signing transaction failed"); return false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 8054cfadf03..d1c0218d9e5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -276,7 +276,7 @@ public: }; //Get the marginal bytes of spending the specified output -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet); +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, bool use_max_sig = false); /** * A transaction with a bunch of additional info that only the owner cares about. @@ -461,9 +461,9 @@ public: CAmount GetChange() const; // Get the marginal bytes if spending the specified output from this transaction - int GetSpendSize(unsigned int out) const + int GetSpendSize(unsigned int out, bool use_max_sig = false) const { - return CalculateMaximumSignedInputSize(tx->vout[out], pwallet); + return CalculateMaximumSignedInputSize(tx->vout[out], pwallet, use_max_sig); } void GetAmounts(std::list& listReceived, @@ -507,6 +507,9 @@ public: /** Whether we know how to spend this output, ignoring the lack of keys */ bool fSolvable; + /** Whether to use the maximum sized, 72 byte signature when calculating the size of the input spend. This should only be set when watch-only outputs are allowed */ + bool use_max_sig; + /** * Whether this output is considered safe to spend. Unconfirmed transactions * from outside keys and unconfirmed replacement transactions are considered @@ -514,13 +517,13 @@ public: */ bool fSafe; - COutput(const CWalletTx *txIn, int iIn, int nDepthIn, bool fSpendableIn, bool fSolvableIn, bool fSafeIn) + COutput(const CWalletTx *txIn, int iIn, int nDepthIn, bool fSpendableIn, bool fSolvableIn, bool fSafeIn, bool use_max_sig_in = false) { - tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; nInputBytes = -1; + tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; nInputBytes = -1; use_max_sig = use_max_sig_in; // If known and signable by the given wallet, compute nInputBytes // Failure will keep this value -1 if (fSpendable && tx) { - nInputBytes = tx->GetSpendSize(i); + nInputBytes = tx->GetSpendSize(i, use_max_sig); } } @@ -976,14 +979,14 @@ public: void ListAccountCreditDebit(const std::string& strAccount, std::list& entries); bool AddAccountingEntry(const CAccountingEntry&); bool AddAccountingEntry(const CAccountingEntry&, WalletBatch *batch); - bool DummySignTx(CMutableTransaction &txNew, const std::set &txouts) const + bool DummySignTx(CMutableTransaction &txNew, const std::set &txouts, bool use_max_sig = false) const { std::vector v_txouts(txouts.size()); std::copy(txouts.begin(), txouts.end(), v_txouts.begin()); - return DummySignTx(txNew, v_txouts); + return DummySignTx(txNew, v_txouts, use_max_sig); } - bool DummySignTx(CMutableTransaction &txNew, const std::vector &txouts) const; - bool DummySignInput(CTxIn &tx_in, const CTxOut &txout) const; + bool DummySignTx(CMutableTransaction &txNew, const std::vector &txouts, bool use_max_sig = false) const; + bool DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig = false) const; CFeeRate m_pay_tx_fee{DEFAULT_PAY_TX_FEE}; unsigned int m_confirm_target{DEFAULT_TX_CONFIRM_TARGET}; @@ -1311,6 +1314,6 @@ public: // Use DummySignatureCreator, which inserts 71 byte signatures everywhere. // NOTE: this requires that all inputs must be in mapWallet (eg the tx should // be IsAllFromMe). -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet); -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts); +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig = false); +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts, bool use_max_sig = false); #endif // BITCOIN_WALLET_WALLET_H