Skip to content

Commit

Permalink
Merge #15497: rpc: Consistent range arguments in scantxoutset/importm…
Browse files Browse the repository at this point in the history 
…ulti/deriveaddresses

ca253f6 Make deriveaddresses use stop/[start,stop] notation for ranges (Pieter Wuille)
1675b7c Use stop/[start,stop] notation in importmulti desc range (Pieter Wuille)
4566011 Add support for stop/[start,stop] ranges to scantxoutset (Pieter Wuille)
6b9f45e Support ranges arguments in RPC help (Pieter Wuille)
7aa6a8a 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
  • Loading branch information
MarcoFalke committed Mar 1, 2019
2 parents e8612ad + ca253f6 commit a6d7026
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 42 deletions.
15 changes: 9 additions & 6 deletions src/rpc/blockchain.cpp
View file
Expand Up @@ -2159,7 +2159,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])"},
},
},
},
Expand Down Expand Up @@ -2216,7 +2216,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<int64_t, int64_t> range = {0, 1000};
if (scanobject.isStr()) {
desc_str = scanobject.get_str();
} else if (scanobject.isObject()) {
Expand All @@ -2225,8 +2225,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");
Expand All @@ -2237,8 +2237,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<CScript> 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));
Expand Down
32 changes: 16 additions & 16 deletions src/rpc/misc.cpp
View file
Expand Up @@ -185,7 +185,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"
Expand All @@ -199,37 +199,37 @@ 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, RPCArg::Optional::OMITTED_NAMED_ARG, "If a ranged descriptor is used, this specifies the beginning of the range to import."},
{"end", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "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) the derived addresses\n"
},
RPCExamples{
"First three native segwit receive addresses\n" +
HelpExampleCli("deriveaddresses", "\"wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)#trd0mf0l\" 0 2")
HelpExampleCli("deriveaddresses", "\"wpkh([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 provider;
Expand Down
22 changes: 22 additions & 0 deletions src/rpc/util.cpp
View file
Expand Up @@ -200,6 +200,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;
Expand Down Expand Up @@ -405,6 +406,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;
Expand Down Expand Up @@ -464,6 +469,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:
Expand Down Expand Up @@ -494,6 +501,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;
Expand Down Expand Up @@ -523,3 +531,17 @@ std::string RPCArg::ToString(const bool oneline) const
}
assert(false);
}

std::pair<int64_t, int64_t> 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]");
}
4 changes: 4 additions & 0 deletions src/rpc/util.h
View file
Expand Up @@ -38,6 +38,9 @@ unsigned int ParseConfirmTarget(const UniValue& value);
RPCErrorCode RPCErrorFromTransactionError(TransactionError terr);
UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_string = "");

//! Parse a JSON range specified as int64, or [int64, int64]
std::pair<int64_t, int64_t> ParseRange(const UniValue& value);

struct RPCArg {
enum class Type {
OBJ,
Expand All @@ -48,6 +51,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 {
Expand Down
18 changes: 5 additions & 13 deletions src/wallet/rpcdump.cpp
View file
Expand Up @@ -1132,13 +1132,10 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
if (!data.exists("range")) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Descriptor is ranged, please specify the range");
}
const UniValue& range = data["range"];
range_start = range.exists("start") ? range["start"].get_int64() : 0;
if (!range.exists("end")) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "End of range for descriptor must be specified");
}
range_end = range["end"].get_int64();
if (range_end < range_start || range_start < 0) {
auto range = ParseRange(data["range"]);
range_start = range.first;
range_end = range.second;
if (range_start < 0 || (range_end >> 31) != 0 || range_end - range_start >= 1000000) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid descriptor range specified");
}
}
Expand Down Expand Up @@ -1373,12 +1370,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
{"key", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""},
}
},
{"range", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "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 watchonly."},
{"label", RPCArg::Type::STR, /* default */ "''", "Label to assign to the address, only allowed with internal=false"},
Expand Down
13 changes: 8 additions & 5 deletions test/functional/rpc_deriveaddresses.py
View file
Expand Up @@ -27,17 +27,20 @@ def run_test(self):
assert_equal(self.nodes[0].deriveaddresses(descriptor_pubkey), [address])

ranged_descriptor = "wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)#kft60nuy"
assert_equal(self.nodes[0].deriveaddresses(ranged_descriptor, 0, 2), [address, "bcrt1qhku5rq7jz8ulufe2y6fkcpnlvpsta7rq4442dy", "bcrt1qpgptk2gvshyl0s9lqshsmx932l9ccsv265tvaq"])
assert_equal(self.nodes[0].deriveaddresses(ranged_descriptor, [1, 2]), ["bcrt1qhku5rq7jz8ulufe2y6fkcpnlvpsta7rq4442dy", "bcrt1qpgptk2gvshyl0s9lqshsmx932l9ccsv265tvaq"])
assert_equal(self.nodes[0].deriveaddresses(ranged_descriptor, 2), [address, "bcrt1qhku5rq7jz8ulufe2y6fkcpnlvpsta7rq4442dy", "bcrt1qpgptk2gvshyl0s9lqshsmx932l9ccsv265tvaq"])

assert_raises_rpc_error(-8, "Range should not be specified for an un-ranged descriptor", self.nodes[0].deriveaddresses, descsum_create("wpkh(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("wpkh(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("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"))

assert_raises_rpc_error(-8, "Missing range end parameter", self.nodes[0].deriveaddresses, descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), 0)
assert_raises_rpc_error(-8, "End of range is too high", self.nodes[0].deriveaddresses, descsum_create("wpkh(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("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), 2, 0)
assert_raises_rpc_error(-8, "Range is too large", self.nodes[0].deriveaddresses, descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [1000000000, 2000000000])

assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].deriveaddresses, descsum_create("wpkh(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("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [2, 0])

assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].deriveaddresses, descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [-1, 0])

combo_descriptor = descsum_create("combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)")
assert_equal(self.nodes[0].deriveaddresses(combo_descriptor), ["mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", "mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", address, "2NDvEwGfpEqJWfybzpKPHF2XH3jwoQV3D7x"])
Expand Down
1 change: 1 addition & 0 deletions test/functional/rpc_scantxoutset.py
View file
Expand Up @@ -95,6 +95,7 @@ def run_test(self):
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"])
Expand Down
4 changes: 2 additions & 2 deletions test/functional/wallet_importmulti.py
View file
Expand Up @@ -584,7 +584,7 @@ def run_test(self):
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."])
for address in addresses:
Expand Down Expand Up @@ -807,7 +807,7 @@ def run_test(self):
'desc': descsum_create('wpkh([80002067/0h/0h]' + xpub + '/*)'),
'keypool': True,
'timestamp': 'now',
'range' : {'start': 0, 'end': 4}
'range' : [0, 4],
}]
)
for i in range(0, 5):
Expand Down

0 comments on commit a6d7026

Please sign in to comment.