Skip to content

Commit 510c653

Browse files
committed
Extract ParseDescriptorRange
So as to be consistently informative when the checks fail, and to protect against unintentional divergence among the checks.
1 parent ba54342 commit 510c653

File tree

7 files changed

+48
-23
lines changed

7 files changed

+48
-23
lines changed

src/rpc/blockchain.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2224,8 +2224,7 @@ UniValue scantxoutset(const JSONRPCRequest& request)
22242224
desc_str = desc_uni.get_str();
22252225
UniValue range_uni = find_value(scanobject, "range");
22262226
if (!range_uni.isNull()) {
2227-
range = ParseRange(range_uni);
2228-
if (range.first < 0 || (range.second >> 31) != 0 || range.second >= range.first + 1000000) throw JSONRPCError(RPC_INVALID_PARAMETER, "range out of range");
2227+
range = ParseDescriptorRange(range_uni);
22292228
}
22302229
} else {
22312230
throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object");

src/rpc/misc.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <warnings.h>
2424

2525
#include <stdint.h>
26+
#include <tuple>
2627
#ifdef HAVE_MALLOC_INFO
2728
#include <malloc.h>
2829
#endif
@@ -214,18 +215,7 @@ UniValue deriveaddresses(const JSONRPCRequest& request)
214215
int64_t range_end = 0;
215216

216217
if (request.params.size() >= 2 && !request.params[1].isNull()) {
217-
auto range = ParseRange(request.params[1]);
218-
if (range.first < 0) {
219-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Range should be greater or equal than 0");
220-
}
221-
if ((range.second >> 31) != 0) {
222-
throw JSONRPCError(RPC_INVALID_PARAMETER, "End of range is too high");
223-
}
224-
if (range.second >= range.first + 1000000) {
225-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Range is too large");
226-
}
227-
range_begin = range.first;
228-
range_end = range.second;
218+
std::tie(range_begin, range_end) = ParseDescriptorRange(request.params[1]);
229219
}
230220

231221
FlatSigningProvider provider;

src/rpc/util.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include <tinyformat.h>
99
#include <util/strencodings.h>
1010

11+
#include <tuple>
12+
1113
InitInterfaces* g_rpc_interfaces = nullptr;
1214

1315
// Converts a hex string to a public key if possible
@@ -529,7 +531,7 @@ std::string RPCArg::ToString(const bool oneline) const
529531
assert(false);
530532
}
531533

532-
std::pair<int64_t, int64_t> ParseRange(const UniValue& value)
534+
static std::pair<int64_t, int64_t> ParseRange(const UniValue& value)
533535
{
534536
if (value.isNum()) {
535537
return {0, value.get_int64()};
@@ -542,3 +544,19 @@ std::pair<int64_t, int64_t> ParseRange(const UniValue& value)
542544
}
543545
throw JSONRPCError(RPC_INVALID_PARAMETER, "Range must be specified as end or as [begin,end]");
544546
}
547+
548+
std::pair<int64_t, int64_t> ParseDescriptorRange(const UniValue& value)
549+
{
550+
int64_t low, high;
551+
std::tie(low, high) = ParseRange(value);
552+
if (low < 0) {
553+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Range should be greater or equal than 0");
554+
}
555+
if ((high >> 31) != 0) {
556+
throw JSONRPCError(RPC_INVALID_PARAMETER, "End of range is too high");
557+
}
558+
if (high >= low + 1000000) {
559+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Range is too large");
560+
}
561+
return {low, high};
562+
}

src/rpc/util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ RPCErrorCode RPCErrorFromTransactionError(TransactionError terr);
3939
UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_string = "");
4040

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

4444
struct RPCArg {
4545
enum class Type {

src/wallet/rpcdump.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <wallet/rpcwallet.h>
2323

2424
#include <stdint.h>
25+
#include <tuple>
2526

2627
#include <boost/algorithm/string.hpp>
2728
#include <boost/date_time/posix_time/posix_time.hpp>
@@ -1144,12 +1145,7 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
11441145
if (!data.exists("range")) {
11451146
throw JSONRPCError(RPC_INVALID_PARAMETER, "Descriptor is ranged, please specify the range");
11461147
}
1147-
auto range = ParseRange(data["range"]);
1148-
range_start = range.first;
1149-
range_end = range.second;
1150-
if (range_start < 0 || (range_end >> 31) != 0 || range_end - range_start >= 1000000) {
1151-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid descriptor range specified");
1152-
}
1148+
std::tie(range_start, range_end) = ParseDescriptorRange(data["range"]);
11531149
}
11541150

11551151
const UniValue& priv_keys = data.exists("keys") ? data["keys"].get_array() : UniValue();

test/functional/rpc_scantxoutset.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55
"""Test the scantxoutset rpc call."""
66
from test_framework.test_framework import BitcoinTestFramework
7-
from test_framework.util import assert_equal
7+
from test_framework.util import assert_equal, assert_raises_rpc_error
88

99
from decimal import Decimal
1010
import shutil
@@ -67,6 +67,13 @@ def run_test(self):
6767
assert_equal(self.nodes[0].scantxoutset("start", [ "addr(" + addr_P2SH_SEGWIT + ")", "addr(" + addr_LEGACY + ")", "addr(" + addr_BECH32 + ")"])['total_amount'], Decimal("0.007"))
6868
assert_equal(self.nodes[0].scantxoutset("start", [ "addr(" + addr_P2SH_SEGWIT + ")", "addr(" + addr_LEGACY + ")", "combo(" + pubk3 + ")"])['total_amount'], Decimal("0.007"))
6969

70+
self.log.info("Test range validation.")
71+
assert_raises_rpc_error(-8, "End of range is too high", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": -1}])
72+
assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [-1, 10]}])
73+
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)]}])
74+
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]}])
75+
assert_raises_rpc_error(-8, "Range is too large", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [0, 1000001]}])
76+
7077
self.log.info("Test extended key derivation.")
7178
# Run various scans, and verify that the sum of the amounts of the matches corresponds to the expected subset.
7279
# Note that all amounts in the UTXO set are powers of 2 multiplied by 0.001 BTC, so each amounts uniquely identifies a subset.

test/functional/wallet_importmulti.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,21 @@ def run_test(self):
591591
key.p2sh_p2wpkh_addr,
592592
solvable=True)
593593

594+
self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": -1},
595+
success=False, error_code=-8, error_message='End of range is too high')
596+
597+
self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [-1, 10]},
598+
success=False, error_code=-8, error_message='Range should be greater or equal than 0')
599+
600+
self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [(2 << 31 + 1) - 1000000, (2 << 31 + 1)]},
601+
success=False, error_code=-8, error_message='End of range is too high')
602+
603+
self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [2, 1]},
604+
success=False, error_code=-8, error_message='Range specified as [begin,end] must not have begin after end')
605+
606+
self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [0, 1000001]},
607+
success=False, error_code=-8, error_message='Range is too large')
608+
594609
# Test importing of a P2PKH address via descriptor
595610
key = get_key(self.nodes[0])
596611
self.log.info("Should import a p2pkh address from descriptor")

0 commit comments

Comments
 (0)