From 60f98530d791b0045c71604bc1e771c9a4ba2c85 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 10 May 2019 08:09:34 -0400 Subject: [PATCH] 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")