From f5e52489c72c23680e329536af3b91e104f67775 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 1 Mar 2019 09:13:05 -0500 Subject: [PATCH 1/2] Merge #15497: rpc: Consistent range arguments in scantxoutset/importmulti/deriveaddresses ca253f6ebf Make deriveaddresses use stop/[start,stop] notation for ranges (Pieter Wuille) 1675b7ce55 Use stop/[start,stop] notation in importmulti desc range (Pieter Wuille) 4566011631 Add support for stop/[start,stop] ranges to scantxoutset (Pieter Wuille) 6b9f45e81b Support ranges arguments in RPC help (Pieter Wuille) 7aa6a8aefb Add ParseRange function to parse args of the form int/[int,int] (Pieter Wuille) Pull request description: This introduces a consistent notation for RPC arguments in `scantxoutset`, `importmulti`, and `deriveaddresses`, either: * `"range" : int` to just specify the end of the range * `"range" : [int,int]` to specify both the begin and the end of the range. For `scantxoutset`, this is a backward compatible new feature. For the two other RPCs, it's an incompatible change, but neither of them has been in a release so far. Because of that non-released reason, this only makes sense in 0.18, in my opinion. I suggest this as an alternative to #15496, which only makes `deriveaddresses` compatible with `importmulti`, but not with the existing `scantxoutset` RPC. I also think `[int,int]` is more convenient than `{"start":int,"stop":int}`. I realize this is technically a feature added to `scantxoutset` after the feature freeze. If desired, I'll drop the `scantxoutset` changes. Tree-SHA512: 1cbebb90cf34f106786dbcec7afbf3f43fb8b7e46cc7e6763faf1bc1babf12375a1b3c3cf86ee83c21ed2171d99b5a2f60331850bc613db25538c38b6a056676 --- src/rpc/blockchain.cpp | 15 +++++++----- src/rpc/misc.cpp | 32 +++++++++++++------------- src/rpc/util.cpp | 22 ++++++++++++++++++ src/rpc/util.h | 4 ++++ src/wallet/rpcdump.cpp | 18 ++++----------- test/functional/rpc_deriveaddresses.py | 13 +++++++---- test/functional/rpc_scantxoutset.py | 1 + test/functional/wallet_importmulti.py | 2 +- 8 files changed, 66 insertions(+), 41 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 2dce9e1c96..ce817e7eb3 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2492,7 +2492,7 @@ UniValue scantxoutset(const JSONRPCRequest& request) {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "An object with output descriptor and metadata", { {"desc", RPCArg::Type::STR, RPCArg::Optional::NO, "An output descriptor"}, - {"range", RPCArg::Type::NUM, /* default */ "1000", "Up to what child index HD chains should be explored"}, + {"range", RPCArg::Type::RANGE, /* default */ "1000", "The range of HD chain indexes to explore (either end or [begin,end])"}, }, }, }, @@ -2549,7 +2549,7 @@ UniValue scantxoutset(const JSONRPCRequest& request) // loop through the scan objects for (const UniValue& scanobject : request.params[1].get_array().getValues()) { std::string desc_str; - int range = 1000; + std::pair range = {0, 1000}; if (scanobject.isStr()) { desc_str = scanobject.get_str(); } else if (scanobject.isObject()) { @@ -2558,8 +2558,8 @@ UniValue scantxoutset(const JSONRPCRequest& request) desc_str = desc_uni.get_str(); UniValue range_uni = find_value(scanobject, "range"); if (!range_uni.isNull()) { - range = range_uni.get_int(); - if (range < 0 || range > 1000000) throw JSONRPCError(RPC_INVALID_PARAMETER, "range out of range"); + range = ParseRange(range_uni); + if (range.first < 0 || (range.second >> 31) != 0 || range.second >= range.first + 1000000) throw JSONRPCError(RPC_INVALID_PARAMETER, "range out of range"); } } else { throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object"); @@ -2570,8 +2570,11 @@ UniValue scantxoutset(const JSONRPCRequest& request) if (!desc) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor '%s'", desc_str)); } - if (!desc->IsRange()) range = 0; - for (int i = 0; i <= range; ++i) { + if (!desc->IsRange()) { + range.first = 0; + range.second = 0; + } + for (int i = range.first; i <= range.second; ++i) { std::vector scripts; if (!desc->Expand(i, provider, scripts, provider)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys: '%s'", desc_str)); diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 5e1b20ed06..7d95d8553f 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -341,7 +341,7 @@ UniValue getdescriptorinfo(const JSONRPCRequest& request) UniValue deriveaddresses(const JSONRPCRequest& request) { - if (request.fHelp || request.params.empty() || request.params.size() > 3) { + if (request.fHelp || request.params.empty() || request.params.size() > 2) { throw std::runtime_error( RPCHelpMan{"deriveaddresses", "\nDerives one or more addresses corresponding to an output descriptor.\n" @@ -354,8 +354,7 @@ UniValue deriveaddresses(const JSONRPCRequest& request) "For more information on output descriptors, see the documentation in the doc/descriptors.md file.\n", { {"descriptor", RPCArg::Type::STR, RPCArg::Optional::NO, "The descriptor"}, - {"begin", RPCArg::Type::NUM, /* default */ "", "If a ranged descriptor is used, this specifies the beginning of the range to import"}, - {"end", RPCArg::Type::NUM, /* default */ "", "If a ranged descriptor is used, this specifies the end of the range to import"} + {"range", RPCArg::Type::RANGE, RPCArg::Optional::OMITTED_NAMED_ARG, "If a ranged descriptor is used, this specifies the end or the range (in [begin,end] notation) to derive."}, }, RPCResult{ "\"address\" (array) A json array of the derived addresses\n" @@ -365,29 +364,30 @@ UniValue deriveaddresses(const JSONRPCRequest& request) }, RPCExamples{ "\nFirst three receive addresses\n" - + HelpExampleCli("deriveaddresses", "\"pkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#trd0mf0l\" 0 2") + + HelpExampleCli("deriveaddresses", "\"pkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#trd0mf0l\" \"[0,2]\"") } }.ToString()); } - RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VNUM, UniValue::VNUM}); + RPCTypeCheck(request.params, {UniValue::VSTR, UniValueType()}); // Range argument is checked later const std::string desc_str = request.params[0].get_str(); - int range_begin = 0; - int range_end = 0; + int64_t range_begin = 0; + int64_t range_end = 0; - if (request.params.size() >= 2) { - if (request.params.size() == 2) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing range end parameter"); - } - range_begin = request.params[1].get_int(); - range_end = request.params[2].get_int(); - if (range_begin < 0) { + if (request.params.size() >= 2 && !request.params[1].isNull()) { + auto range = ParseRange(request.params[1]); + if (range.first < 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Range should be greater or equal than 0"); } - if (range_begin > range_end) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Range end should be equal to or greater than begin"); + if ((range.second >> 31) != 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "End of range is too high"); } + if (range.second >= range.first + 1000000) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Range is too large"); + } + range_begin = range.first; + range_end = range.second; } FlatSigningProvider key_provider; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 8dabed3f39..44dab395cc 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -293,6 +293,7 @@ struct Sections { case RPCArg::Type::STR: case RPCArg::Type::NUM: case RPCArg::Type::AMOUNT: + case RPCArg::Type::RANGE: case RPCArg::Type::BOOL: { if (outer_type == OuterType::NAMED_ARG) return; // Nothing more to do for non-recursive types on first recursion auto left = indent; @@ -483,6 +484,10 @@ std::string RPCArg::ToDescriptionString() const ret += "numeric or string"; break; } + case Type::RANGE: { + ret += "numeric or array"; + break; + } case Type::BOOL: { ret += "boolean"; break; @@ -542,6 +547,8 @@ std::string RPCArg::ToStringObj(const bool oneline) const return res + "\"hex\""; case Type::NUM: return res + "n"; + case Type::RANGE: + return res + "n or [n,n]"; case Type::AMOUNT: return res + "amount"; case Type::BOOL: @@ -572,6 +579,7 @@ std::string RPCArg::ToString(const bool oneline) const return "\"" + m_name + "\""; } case Type::NUM: + case Type::RANGE: case Type::AMOUNT: case Type::BOOL: { return m_name; @@ -602,6 +610,20 @@ std::string RPCArg::ToString(const bool oneline) const assert(false); } +std::pair ParseRange(const UniValue& value) +{ + if (value.isNum()) { + return {0, value.get_int64()}; + } + if (value.isArray() && value.size() == 2 && value[0].isNum() && value[1].isNum()) { + int64_t low = value[0].get_int64(); + int64_t high = value[1].get_int64(); + if (low > high) throw JSONRPCError(RPC_INVALID_PARAMETER, "Range specified as [begin,end] must not have begin after end"); + return {low, high}; + } + throw JSONRPCError(RPC_INVALID_PARAMETER, "Range must be specified as end or as [begin,end]"); +} + RPCErrorCode RPCErrorFromTransactionError(TransactionError terr) { switch (terr) { diff --git a/src/rpc/util.h b/src/rpc/util.h index 112b160974..02908359eb 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -87,6 +87,9 @@ unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target); /** Returns, given services flags, a list of humanly readable (known) network services */ UniValue GetServicesNames(ServiceFlags services); +//! Parse a JSON range specified as int64, or [int64, int64] +std::pair ParseRange(const UniValue& value); + struct RPCArg { enum class Type { OBJ, @@ -97,6 +100,7 @@ struct RPCArg { OBJ_USER_KEYS, //!< Special type where the user must set the keys e.g. to define multiple addresses; as opposed to e.g. an options object where the keys are predefined AMOUNT, //!< Special type representing a floating point amount (can be either NUM or STR) STR_HEX, //!< Special type that is a STR with only hex chars + RANGE, //!< Special type that is a NUM or [NUM,NUM] }; enum class Optional { diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index f9c33f12e3..594ad5e6b9 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1297,13 +1297,10 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map> 31) != 0 || range_end - range_start >= 1000000) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid descriptor range specified"); } } @@ -1513,12 +1510,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) {"key", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""}, } }, - {"range", RPCArg::Type::OBJ, /* default */ "", "If a ranged descriptor is used, this specifies the start and end of the range to import", - { - {"start", RPCArg::Type::NUM, /* default */ "0", "Start of the range to import"}, - {"end", RPCArg::Type::NUM, RPCArg::Optional::NO, "End of the range to import (inclusive)"}, - } - }, + {"range", RPCArg::Type::RANGE, RPCArg::Optional::OMITTED, "If a ranged descriptor is used, this specifies the end or the range (in the form [begin,end]) to import"}, {"internal", RPCArg::Type::BOOL, /* default */ "false", "Stating whether matching outputs should be treated as not incoming payments (also known as change)"}, {"watchonly", RPCArg::Type::BOOL, /* default */ "false", "Stating whether matching outputs should be considered watched even when not all private keys are provided."}, {"label", RPCArg::Type::STR, /* default */ "''", "Label to assign to the address, only allowed with internal=false"}, diff --git a/test/functional/rpc_deriveaddresses.py b/test/functional/rpc_deriveaddresses.py index 5bfeffbdba..82436a4dbc 100755 --- a/test/functional/rpc_deriveaddresses.py +++ b/test/functional/rpc_deriveaddresses.py @@ -27,17 +27,20 @@ class DeriveaddressesTest(BitcoinTestFramework): assert_equal(self.nodes[0].deriveaddresses(descriptor_pubkey), [address]) ranged_descriptor = "pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)#77vpsvm5" - assert_equal(self.nodes[0].deriveaddresses(ranged_descriptor, 0, 2), [address, "ydccVGNV2EcEouAxbbgdu8pi8gkdaqkiav", "yMENst4XYP3ZSNvsCEm587GbSSXZUfhpWG"]) + assert_equal(self.nodes[0].deriveaddresses(ranged_descriptor, [1, 2]), ["ydccVGNV2EcEouAxbbgdu8pi8gkdaqkiav", "yMENst4XYP3ZSNvsCEm587GbSSXZUfhpWG"]) + assert_equal(self.nodes[0].deriveaddresses(ranged_descriptor, 2), [address, "ydccVGNV2EcEouAxbbgdu8pi8gkdaqkiav", "yMENst4XYP3ZSNvsCEm587GbSSXZUfhpWG"]) - assert_raises_rpc_error(-8, "Range should not be specified for an un-ranged descriptor", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)"), 0, 2) + assert_raises_rpc_error(-8, "Range should not be specified for an un-ranged descriptor", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)"), [0, 2]) assert_raises_rpc_error(-8, "Range must be specified for a ranged descriptor", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)")) - assert_raises_rpc_error(-8, "Missing range end parameter", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), 0) + assert_raises_rpc_error(-8, "End of range is too high", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), 10000000000) - assert_raises_rpc_error(-8, "Range end should be equal to or greater than begin", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), 2, 0) + assert_raises_rpc_error(-8, "Range is too large", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [1000000000, 2000000000]) - assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), -1, 0) + assert_raises_rpc_error(-8, "Range specified as [begin,end] must not have begin after end", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [2, 0]) + + assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].deriveaddresses, descsum_create("pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [-1, 0]) combo_descriptor = descsum_create("combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)") assert_equal(self.nodes[0].deriveaddresses(combo_descriptor), ["yZTyMdEJjZWJi6CwY6g3WurLESH3UsWrrM", "yZTyMdEJjZWJi6CwY6g3WurLESH3UsWrrM", "93EpXofs6W7eNiuj4gu2LJh8L8opowW1jz"]) diff --git a/test/functional/rpc_scantxoutset.py b/test/functional/rpc_scantxoutset.py index 1316d578f6..9fe6d9c6d8 100755 --- a/test/functional/rpc_scantxoutset.py +++ b/test/functional/rpc_scantxoutset.py @@ -93,6 +93,7 @@ class ScantxoutsetTest(BitcoinTestFramework): assert_equal(self.nodes[0].scantxoutset("start", [ {"desc": "combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)", "range": 1500}])['total_amount'], Decimal("28.672")) assert_equal(self.nodes[0].scantxoutset("start", [ {"desc": "combo(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/*)", "range": 1499}])['total_amount'], Decimal("12.288")) assert_equal(self.nodes[0].scantxoutset("start", [ {"desc": "combo(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/*)", "range": 1500}])['total_amount'], Decimal("28.672")) + assert_equal(self.nodes[0].scantxoutset("start", [ {"desc": "combo(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/*)", "range": [1500,1500]}])['total_amount'], Decimal("16.384")) # Test the reported descriptors for a few matches assert_equal(descriptors(self.nodes[0].scantxoutset("start", [ {"desc": "combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/0h/0'/*)", "range": 1499}])), ["pkh([0c5f9a1e/0'/0'/0]026dbd8b2315f296d36e6b6920b1579ca75569464875c7ebe869b536a7d9503c8c)#dzxw429x", "pkh([0c5f9a1e/0'/0'/1]033e6f25d76c00bedb3a8993c7d5739ee806397f0529b1b31dda31ef890f19a60c)#43rvceed"]) diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index 3c1370481c..ed931fc0ee 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -498,7 +498,7 @@ class ImportMultiTest(BitcoinTestFramework): self.log.info("Should import the ranged descriptor with specified range as solvable") self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", - "range": {"end": 1}}, + "range": 1}, success=True, warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) From 60f98530d791b0045c71604bc1e771c9a4ba2c85 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 10 May 2019 08:09:34 -0400 Subject: [PATCH 2/2] Merge #15744: refactor: Extract ParseDescriptorRange 510c6532ba Extract ParseDescriptorRange (Ben Woosley) Pull request description: So as to be consistently informative when the checks fail, and to protect against unintentional divergence among the checks. ACKs for commit 510c65: meshcollider: Oh apologies, yes. Thanks :) utACK https://github.com/bitcoin/bitcoin/pull/15744/commits/510c6532bae9abc5beda1c126c945923a64680cb MarcoFalke: utACK 510c6532bae9abc5beda1c126c945923a64680cb sipa: utACK 510c6532bae9abc5beda1c126c945923a64680cb Tree-SHA512: b1f0792bfaa163890a20654a0fc2c4c4a996659916bf5f4a495662436b39326692a1a0c825caafd859e48c05f5dd1865c4f7c28092be5074edda3c94f94f9f8b --- src/rpc/blockchain.cpp | 3 +-- src/rpc/misc.cpp | 14 ++------------ src/rpc/util.cpp | 20 +++++++++++++++++++- src/rpc/util.h | 2 +- src/wallet/rpcdump.cpp | 8 ++------ test/functional/rpc_scantxoutset.py | 9 ++++++++- test/functional/wallet_importmulti.py | 15 +++++++++++++++ 7 files changed, 48 insertions(+), 23 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index ce817e7eb3..3185917107 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2558,8 +2558,7 @@ UniValue scantxoutset(const JSONRPCRequest& request) desc_str = desc_uni.get_str(); UniValue range_uni = find_value(scanobject, "range"); if (!range_uni.isNull()) { - range = ParseRange(range_uni); - if (range.first < 0 || (range.second >> 31) != 0 || range.second >= range.first + 1000000) throw JSONRPCError(RPC_INVALID_PARAMETER, "range out of range"); + range = ParseDescriptorRange(range_uni); } } else { throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object"); diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 7d95d8553f..df43c88b6d 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -30,6 +30,7 @@ #include #include +#include #ifdef HAVE_MALLOC_INFO #include #endif @@ -376,18 +377,7 @@ UniValue deriveaddresses(const JSONRPCRequest& request) int64_t range_end = 0; if (request.params.size() >= 2 && !request.params[1].isNull()) { - auto range = ParseRange(request.params[1]); - if (range.first < 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Range should be greater or equal than 0"); - } - if ((range.second >> 31) != 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "End of range is too high"); - } - if (range.second >= range.first + 1000000) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Range is too large"); - } - range_begin = range.first; - range_end = range.second; + std::tie(range_begin, range_end) = ParseDescriptorRange(request.params[1]); } FlatSigningProvider key_provider; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 44dab395cc..7e726d6df3 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -11,6 +11,8 @@ #include #include +#include + InitInterfaces* g_rpc_interfaces = nullptr; void RPCTypeCheck(const UniValue& params, @@ -610,7 +612,7 @@ std::string RPCArg::ToString(const bool oneline) const assert(false); } -std::pair ParseRange(const UniValue& value) +static std::pair ParseRange(const UniValue& value) { if (value.isNum()) { return {0, value.get_int64()}; @@ -624,6 +626,22 @@ std::pair ParseRange(const UniValue& value) throw JSONRPCError(RPC_INVALID_PARAMETER, "Range must be specified as end or as [begin,end]"); } +std::pair ParseDescriptorRange(const UniValue& value) +{ + int64_t low, high; + std::tie(low, high) = ParseRange(value); + if (low < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Range should be greater or equal than 0"); + } + if ((high >> 31) != 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "End of range is too high"); + } + if (high >= low + 1000000) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Range is too large"); + } + return {low, high}; +} + RPCErrorCode RPCErrorFromTransactionError(TransactionError terr) { switch (terr) { diff --git a/src/rpc/util.h b/src/rpc/util.h index 02908359eb..dee31ebb16 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -88,7 +88,7 @@ unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target); UniValue GetServicesNames(ServiceFlags services); //! Parse a JSON range specified as int64, or [int64, int64] -std::pair ParseRange(const UniValue& value); +std::pair ParseDescriptorRange(const UniValue& value); struct RPCArg { enum class Type { diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 594ad5e6b9..8288136282 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -1297,12 +1298,7 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map> 31) != 0 || range_end - range_start >= 1000000) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid descriptor range specified"); - } + std::tie(range_start, range_end) = ParseDescriptorRange(data["range"]); } const UniValue& priv_keys = data.exists("keys") ? data["keys"].get_array() : UniValue(); diff --git a/test/functional/rpc_scantxoutset.py b/test/functional/rpc_scantxoutset.py index 9fe6d9c6d8..3651237fcc 100755 --- a/test/functional/rpc_scantxoutset.py +++ b/test/functional/rpc_scantxoutset.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the scantxoutset rpc call.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, Decimal +from test_framework.util import assert_equal, assert_raises_rpc_error, Decimal import shutil import os @@ -65,6 +65,13 @@ class ScantxoutsetTest(BitcoinTestFramework): assert_equal(self.nodes[0].scantxoutset("start", [ "addr(" + addr1 + ")", "addr(" + addr2 + ")", "addr(" + addr3 + ")"])['total_amount'], Decimal("0.007")) assert_equal(self.nodes[0].scantxoutset("start", [ "addr(" + addr1 + ")", "addr(" + addr2 + ")", "pkh(" + pubk3 + ")"])['total_amount'], Decimal("0.007")) + self.log.info("Test range validation.") + assert_raises_rpc_error(-8, "End of range is too high", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": -1}]) + assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [-1, 10]}]) + assert_raises_rpc_error(-8, "End of range is too high", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [(2 << 31 + 1) - 1000000, (2 << 31 + 1)]}]) + assert_raises_rpc_error(-8, "Range specified as [begin,end] must not have begin after end", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [2, 1]}]) + assert_raises_rpc_error(-8, "Range is too large", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [0, 1000001]}]) + self.log.info("Test extended key derivation.") # Run various scans, and verify that the sum of the amounts of the matches corresponds to the expected subset. # Note that all amounts in the UTXO set are powers of 2 multiplied by 0.001 BTC, so each amounts uniquely identifies a subset. diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index ed931fc0ee..94342d2f0d 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -502,6 +502,21 @@ class ImportMultiTest(BitcoinTestFramework): success=True, warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": -1}, + success=False, error_code=-8, error_message='End of range is too high') + + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [-1, 10]}, + success=False, error_code=-8, error_message='Range should be greater or equal than 0') + + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [(2 << 31 + 1) - 1000000, (2 << 31 + 1)]}, + success=False, error_code=-8, error_message='End of range is too high') + + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [2, 1]}, + success=False, error_code=-8, error_message='Range specified as [begin,end] must not have begin after end') + + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [0, 1000001]}, + success=False, error_code=-8, error_message='Range is too large') + # Test importing of a P2PKH address via descriptor key = self.get_key() self.log.info("Should import a p2pkh address from descriptor")