From ec0803a0f5269487cab3d17b7eeb41ea98f400a2 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 23 Jun 2024 12:47:57 +0000 Subject: [PATCH 01/12] refactor: align `TxToUniv` argument list with upstream In bitcoin#20286, a new `TxToUniv` will be defined and in order to minimize upstream deviation caused by having to change function calls to account for `const CSpentIndexTxInfo*` being placed _before_ `const CTxUndo*` (requiring us to therefore specify `nullptr` as those calls do not expect spend information) instead of letting the default value remain. To allow for that, their ordering will be swapped. To prevent a confusion with the last two arguments between the `core_io` definition and the `rpc/blockchain` definition, let's do the inversion here too before the backport. --- src/core_io.h | 2 +- src/core_write.cpp | 2 +- src/rpc/blockchain.cpp | 2 +- src/rpc/rawtransaction.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core_io.h b/src/core_io.h index 1d19461e53..170f927428 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -47,6 +47,6 @@ std::string EncodeHexTx(const CTransaction& tx); std::string SighashToStr(unsigned char sighash_type); void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex); void ScriptToUniv(const CScript& script, UniValue& out, bool include_address); -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, const CSpentIndexTxInfo* ptxSpentInfo = nullptr, const CTxUndo* txundo = nullptr); +void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, const CTxUndo* txundo = nullptr, const CSpentIndexTxInfo* ptxSpentInfo = nullptr); #endif // BITCOIN_CORE_IO_H diff --git a/src/core_write.cpp b/src/core_write.cpp index c27b0c0f58..e921643f72 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -192,7 +192,7 @@ void ScriptPubKeyToUniv(const CScript& scriptPubKey, out.pushKV("addresses", a); } -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, const CSpentIndexTxInfo* ptxSpentInfo, const CTxUndo* txundo) +void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, const CTxUndo* txundo, const CSpentIndexTxInfo* ptxSpentInfo) { uint256 txid = tx.GetHash(); entry.pushKV("txid", txid.GetHex()); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 49b39817f1..6749f8ac4b 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -226,7 +226,7 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIn // coinbase transaction (i == 0) doesn't have undo data const CTxUndo* txundo = (have_undo && i) ? &blockUndo.vtxundo.at(i - 1) : nullptr; UniValue objTx(UniValue::VOBJ); - TxToUniv(*tx, uint256(), objTx, true, nullptr, txundo); + TxToUniv(*tx, uint256(), objTx, true, txundo); bool fLocked = isman.IsLocked(tx->GetHash()); objTx.pushKV("instantlock", fLocked || result["chainlock"].get_bool()); objTx.pushKV("instantlock_internal", fLocked); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 3222098339..c70fca08e9 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -84,7 +84,7 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, CTxMemPool& mempo } } - TxToUniv(tx, uint256(), entry, true, &txSpentInfo); + TxToUniv(tx, uint256(), entry, true, /* txundo = */ nullptr, &txSpentInfo); bool chainLock = false; if (!hashBlock.IsNull()) { From 7c59923845d026ee89fd1a644395bb86d85bdf66 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 13 Mar 2020 14:12:50 +0000 Subject: [PATCH 02/12] merge bitcoin#18344: Fix nit in getblockchaininfo --- src/rpc/blockchain.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6749f8ac4b..792e2d1151 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1726,7 +1726,7 @@ RPCHelpMan getblockchaininfo() {RPCResult::Type::NUM, "since", "height of the first block to which the status applies"}, {RPCResult::Type::NUM, "activation_height", "expected activation height for this softfork (only for \"locked_in\" status)"}, {RPCResult::Type::NUM, "min_activation_height", "minimum height of blocks for which the rules may be enforced"}, - {RPCResult::Type::OBJ, "statistics", "numeric statistics about BIP9 signalling for a softfork", + {RPCResult::Type::OBJ, "statistics", "numeric statistics about BIP9 signalling for a softfork (only for \"started\" status)", { {RPCResult::Type::NUM, "period", "the length in blocks of the BIP9 signalling period"}, {RPCResult::Type::NUM, "threshold", "the number of blocks with the version bit set required to activate the feature"}, From 7cddf70c5836c2b72eb42d1a1c6e09c6e997b5ea Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 23 Jun 2024 21:14:02 +0000 Subject: [PATCH 03/12] merge bitcoin#20867: Support up to 20 keys for multisig under Segwit context --- src/rpc/util.cpp | 4 +- src/script/descriptor.cpp | 5 +- src/script/interpreter.cpp | 2 +- src/script/interpreter.h | 2 + src/script/standard.cpp | 55 ++++++++++++++++----- src/test/descriptor_tests.cpp | 33 ++++++++++++- test/functional/wallet_importdescriptors.py | 36 ++++++++++++++ 7 files changed, 119 insertions(+), 18 deletions(-) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 80abf1418a..c9292461c1 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -277,8 +277,8 @@ CTxDestination AddAndGetMultisigDestination(const int required, const std::vecto if ((int)pubkeys.size() < required) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("not enough keys supplied (got %u keys, but need at least %d to redeem)", pubkeys.size(), required)); } - if (pubkeys.size() > 16) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Number of keys involved in the multisignature address creation > 16\nReduce the number"); + if (pubkeys.size() > MAX_PUBKEYS_PER_MULTISIG) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Number of keys involved in the multisignature address creation > %d\nReduce the number", MAX_PUBKEYS_PER_MULTISIG)); } script_out = GetScriptForMultisig(required, pubkeys); diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index cc17ea947a..054d8de795 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -978,8 +978,8 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, Span 16) { - error = strprintf("Cannot have %u keys in multisig; must have between 1 and 16 keys, inclusive", providers.size()); + if (providers.empty() || providers.size() > MAX_PUBKEYS_PER_MULTISIG) { + error = strprintf("Cannot have %u keys in multisig; must have between 1 and %d keys, inclusive", providers.size(), MAX_PUBKEYS_PER_MULTISIG); return nullptr; } else if (thres < 1) { error = strprintf("Multisig threshold cannot be %d, must be at least 1", thres); @@ -995,6 +995,7 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, Span MAX_SCRIPT_ELEMENT_SIZE) { error = strprintf("P2SH script is too large, %d bytes is larger than %d bytes", script_size + 3, MAX_SCRIPT_ELEMENT_SIZE); return nullptr; diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index d3a994c5ca..c84d78c25f 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -221,7 +221,7 @@ bool static CheckPubKeyEncoding(const valtype &vchPubKey, unsigned int flags, co return true; } -bool static CheckMinimalPush(const valtype& data, opcodetype opcode) { +bool CheckMinimalPush(const valtype& data, opcodetype opcode) { // Excludes OP_1NEGATE, OP_1-16 since they are by definition minimal assert(0 <= opcode && opcode <= OP_PUSHDATA4); if (data.size() == 0) { diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 8e4e9bc8f6..74f2d4e897 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -179,6 +179,8 @@ using MutableTransactionSignatureChecker = GenericTransactionSignatureChecker >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* error = nullptr); bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = nullptr); +bool CheckMinimalPush(const std::vector& data, opcodetype opcode); + int FindAndDelete(CScript& script, const CScript& b); #endif // BITCOIN_SCRIPT_INTERPRETER_H diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 93eb6a5d9d..0d4a921d7f 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -70,21 +70,53 @@ static constexpr bool IsSmallInteger(opcodetype opcode) return opcode >= OP_1 && opcode <= OP_16; } -static bool MatchMultisig(const CScript& script, unsigned int& required, std::vector& pubkeys) +static constexpr bool IsPushdataOp(opcodetype opcode) +{ + return opcode > OP_FALSE && opcode <= OP_PUSHDATA4; +} + +static constexpr bool IsValidMultisigKeyCount(int n_keys) +{ + return n_keys > 0 && n_keys <= MAX_PUBKEYS_PER_MULTISIG; +} + +static bool GetMultisigKeyCount(opcodetype opcode, valtype data, int& count) +{ + if (IsSmallInteger(opcode)) { + count = CScript::DecodeOP_N(opcode); + return IsValidMultisigKeyCount(count); + } + + if (IsPushdataOp(opcode)) { + if (!CheckMinimalPush(data, opcode)) return false; + try { + count = CScriptNum(data, /* fRequireMinimal = */ true).getint(); + return IsValidMultisigKeyCount(count); + } catch (const scriptnum_error&) { + return false; + } + } + + return false; +} + +static bool MatchMultisig(const CScript& script, int& required_sigs, std::vector& pubkeys) { opcodetype opcode; valtype data; + int num_keys; + CScript::const_iterator it = script.begin(); if (script.size() < 1 || script.back() != OP_CHECKMULTISIG) return false; - if (!script.GetOp(it, opcode, data) || !IsSmallInteger(opcode)) return false; - required = CScript::DecodeOP_N(opcode); + if (!script.GetOp(it, opcode, data) || !GetMultisigKeyCount(opcode, data, required_sigs)) return false; while (script.GetOp(it, opcode, data) && CPubKey::ValidSize(data)) { pubkeys.emplace_back(std::move(data)); } - if (!IsSmallInteger(opcode)) return false; - unsigned int keys = CScript::DecodeOP_N(opcode); - if (pubkeys.size() != keys || keys < required) return false; + if (!GetMultisigKeyCount(opcode, data, num_keys)) return false; + + if (pubkeys.size() != static_cast(num_keys) || num_keys < required_sigs) return false; + return (it + 1 == script.end()); } @@ -121,12 +153,12 @@ TxoutType Solver(const CScript& scriptPubKey, std::vector> keys; if (MatchMultisig(scriptPubKey, required, keys)) { - vSolutionsRet.push_back({static_cast(required)}); // safe as required is in range 1..16 + vSolutionsRet.push_back({static_cast(required)}); // safe as required is in range 1..20 vSolutionsRet.insert(vSolutionsRet.end(), keys.begin(), keys.end()); - vSolutionsRet.push_back({static_cast(keys.size())}); // safe as size is in range 1..16 + vSolutionsRet.push_back({static_cast(keys.size())}); // safe as size is in range 1..20 return TxoutType::MULTISIG; } @@ -240,10 +272,11 @@ CScript GetScriptForMultisig(int nRequired, const std::vector& keys) { CScript script; - script << CScript::EncodeOP_N(nRequired); + script << nRequired; for (const CPubKey& key : keys) script << ToByteVector(key); - script << CScript::EncodeOP_N(keys.size()) << OP_CHECKMULTISIG; + script << keys.size() << OP_CHECKMULTISIG; + return script; } diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 8d0c812b6f..ff28bb2b67 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include