diff --git a/contrib/testgen/README.md b/contrib/testgen/README.md index 4d9054095571f..1efe07d483a0c 100644 --- a/contrib/testgen/README.md +++ b/contrib/testgen/README.md @@ -4,5 +4,5 @@ Utilities to generate test vectors for the data-driven Dash tests. Usage: - PYTHONPATH=../../test/functional/test_framework ./gen_key_io_test_vectors.py valid 50 > ../../src/test/data/key_io_valid.json - PYTHONPATH=../../test/functional/test_framework ./gen_key_io_test_vectors.py invalid 50 > ../../src/test/data/key_io_invalid.json + ./gen_key_io_test_vectors.py valid 50 > ../../src/test/data/key_io_valid.json + ./gen_key_io_test_vectors.py invalid 50 > ../../src/test/data/key_io_invalid.json diff --git a/contrib/testgen/base58.py b/contrib/testgen/base58.py deleted file mode 100644 index 87341ccf96dcd..0000000000000 --- a/contrib/testgen/base58.py +++ /dev/null @@ -1,115 +0,0 @@ -# Copyright (c) 2012-2020 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -''' -Bitcoin base58 encoding and decoding. - -Based on https://bitcointalk.org/index.php?topic=1026.0 (public domain) -''' -import hashlib - -# for compatibility with following code... -class SHA256: - new = hashlib.sha256 - -if str != bytes: - # Python 3.x - def ord(c): - return c - def chr(n): - return bytes( (n,) ) - -__b58chars = '123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz' -__b58base = len(__b58chars) -b58chars = __b58chars - -def b58encode(v): - """ encode v, which is a string of bytes, to base58. - """ - long_value = 0 - for (i, c) in enumerate(v[::-1]): - if isinstance(c, str): - c = ord(c) - long_value += (256**i) * c - - result = '' - while long_value >= __b58base: - div, mod = divmod(long_value, __b58base) - result = __b58chars[mod] + result - long_value = div - result = __b58chars[long_value] + result - - # Bitcoin does a little leading-zero-compression: - # leading 0-bytes in the input become leading-1s - nPad = 0 - for c in v: - if c == 0: - nPad += 1 - else: - break - - return (__b58chars[0]*nPad) + result - -def b58decode(v, length = None): - """ decode v into a string of len bytes - """ - long_value = 0 - for i, c in enumerate(v[::-1]): - pos = __b58chars.find(c) - assert pos != -1 - long_value += pos * (__b58base**i) - - result = bytes() - while long_value >= 256: - div, mod = divmod(long_value, 256) - result = chr(mod) + result - long_value = div - result = chr(long_value) + result - - nPad = 0 - for c in v: - if c == __b58chars[0]: - nPad += 1 - continue - break - - result = bytes(nPad) + result - if length is not None and len(result) != length: - return None - - return result - -def checksum(v): - """Return 32-bit checksum based on SHA256""" - return SHA256.new(SHA256.new(v).digest()).digest()[0:4] - -def b58encode_chk(v): - """b58encode a string, with 32-bit checksum""" - return b58encode(v + checksum(v)) - -def b58decode_chk(v): - """decode a base58 string, check and remove checksum""" - result = b58decode(v) - if result is None: - return None - if result[-4:] == checksum(result[:-4]): - return result[:-4] - else: - return None - -def get_bcaddress_version(strAddress): - """ Returns None if strAddress is invalid. Otherwise returns integer version of address. """ - addr = b58decode_chk(strAddress) - if addr is None or len(addr)!=21: - return None - version = addr[0] - return ord(version) - -if __name__ == '__main__': - # Test case (from http://gitorious.org/bitcoin/python-base58.git) - assert get_bcaddress_version('15VjRaDX9zpbA8LVnbrCAFzrVzN7ixHNsC') == 0 - _ohai = 'o hai'.encode('ascii') - _tmp = b58encode(_ohai) - assert _tmp == 'DYB3oMS' - assert b58decode(_tmp, 5) == _ohai - print("Tests passed") diff --git a/contrib/testgen/gen_key_io_test_vectors.py b/contrib/testgen/gen_key_io_test_vectors.py index 94f798081975c..ba045ec1c5052 100755 --- a/contrib/testgen/gen_key_io_test_vectors.py +++ b/contrib/testgen/gen_key_io_test_vectors.py @@ -1,20 +1,24 @@ #!/usr/bin/env python3 -# Copyright (c) 2012-2021 The Bitcoin Core developers +# Copyright (c) 2012-2022 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. ''' Generate valid and invalid base58 address and private key test vectors. Usage: - PYTHONPATH=../../test/functional/test_framework ./gen_key_io_test_vectors.py valid 50 > ../../src/test/data/key_io_valid.json - PYTHONPATH=../../test/functional/test_framework ./gen_key_io_test_vectors.py invalid 50 > ../../src/test/data/key_io_invalid.json + ./gen_key_io_test_vectors.py valid 50 > ../../src/test/data/key_io_valid.json + ./gen_key_io_test_vectors.py invalid 50 > ../../src/test/data/key_io_invalid.json ''' -# 2012 Wladimir J. van der Laan -# Released under MIT License -import os + from itertools import islice -from base58 import b58encode_chk, b58decode_chk, b58chars +import os import random +import sys + +sys.path.append(os.path.join(os.path.dirname(__file__), '../../test/functional')) + +from test_framework.address import base58_to_byte, byte_to_base58, b58chars # noqa: E402 +from test_framework.script import OP_DUP, OP_EQUAL, OP_EQUALVERIFY, OP_HASH160, OP_CHECKSIG # noqa: E402 # key types PUBKEY_ADDRESS = 76 @@ -28,15 +32,6 @@ PRIVKEY_REGTEST = 239 # script -OP_0 = 0x00 -OP_1 = 0x51 -OP_2 = 0x52 -OP_16 = 0x60 -OP_DUP = 0x76 -OP_EQUAL = 0x87 -OP_EQUALVERIFY = 0x88 -OP_HASH160 = 0xa9 -OP_CHECKSIG = 0xac pubkey_prefix = (OP_DUP, OP_HASH160, 20) pubkey_suffix = (OP_EQUALVERIFY, OP_CHECKSIG) script_prefix = (OP_HASH160, 20) @@ -65,8 +60,10 @@ def is_valid(v): '''Check vector v for validity''' if len(set(v) - set(b58chars)) > 0: return False - result = b58decode_chk(v) - if result is None: + try: + payload, version = base58_to_byte(v) + result = bytes([version]) + payload + except ValueError: # thrown if checksum doesn't match return False for template in templates: prefix = bytearray(template[0]) @@ -83,7 +80,8 @@ def gen_valid_base58_vector(template): suffix = bytearray(template[2]) dst_prefix = bytearray(template[4]) dst_suffix = bytearray(template[5]) - rv = b58encode_chk(prefix + payload + suffix) + assert len(prefix) == 1 + rv = byte_to_base58(payload + suffix, prefix[0]) return rv, dst_prefix + payload + dst_suffix def gen_valid_vectors(): @@ -124,7 +122,8 @@ def gen_invalid_base58_vector(template): else: suffix = bytearray(template[2]) - val = b58encode_chk(prefix + payload + suffix) + assert len(prefix) == 1 + val = byte_to_base58(payload + suffix, prefix[0]) if random.randint(0,10)<1: # line corruption if randbool(): # add random character to end val += random.choice(b58chars) @@ -152,7 +151,6 @@ def gen_invalid_vectors(): yield val, if __name__ == '__main__': - import sys import json iters = {'valid':gen_valid_vectors, 'invalid':gen_invalid_vectors} try: diff --git a/doc/REST-interface.md b/doc/REST-interface.md index 2aa6f5ee8d45f..5b25d76fc30f2 100644 --- a/doc/REST-interface.md +++ b/doc/REST-interface.md @@ -47,18 +47,24 @@ The HTTP request and response are both handled entirely in-memory. With the /notxdetails/ option JSON response will only contain the transaction hash instead of the complete transaction details. The option only affects the JSON response. #### Blockheaders -`GET /rest/headers//.` +`GET /rest/headers/.?count=` Given a block hash: returns amount of blockheaders in upward direction. Returns empty if the block doesn't exist or it isn't in the active chain. +*Deprecated (but not removed) since 23.0:* +`GET /rest/headers//.` + #### Blockfilter Headers -`GET /rest/blockfilterheaders///.` +`GET /rest/blockfilterheaders//.?count=` Given a block hash: returns amount of blockfilter headers in upward direction for the filter type . Returns empty if the block doesn't exist or it isn't in the active chain. +*Deprecated (but not removed) since 23.0:* +`GET /rest/blockfilterheaders///.` + #### Blockfilters `GET /rest/blockfilter//.` diff --git a/doc/release-notes-24098.md b/doc/release-notes-24098.md new file mode 100644 index 0000000000000..bc74ef6558789 --- /dev/null +++ b/doc/release-notes-24098.md @@ -0,0 +1,20 @@ +Notable changes +=============== + +Updated REST APIs +----------------- + +- The `/headers/` and `/blockfilterheaders/` endpoints have been updated to use + a query parameter instead of path parameter to specify the result count. The + count parameter is now optional, and defaults to 5 for both endpoints. The old + endpoints are still functional, and have no documented behaviour change. + + For `/headers`, use + `GET /rest/headers/.?count=` + instead of + `GET /rest/headers//.` (deprecated) + + For `/blockfilterheaders/`, use + `GET /rest/blockfilterheaders//.?count=` + instead of + `GET /rest/blockfilterheaders///.` (deprecated) diff --git a/doc/release-notes-24629.md b/doc/release-notes-24629.md new file mode 100644 index 0000000000000..36ccda440586c --- /dev/null +++ b/doc/release-notes-24629.md @@ -0,0 +1,7 @@ +Updated RPCs +------------ + +- The return value of the `pruneblockchain` method had an off-by-one bug, + returning the height of the block *after* the most recent pruned. This has + been corrected, and it now returns the height of the last pruned block as + documented. diff --git a/src/Makefile.am b/src/Makefile.am index 1b26de7005fcb..c1fe354201e67 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -302,6 +302,7 @@ BITCOIN_CORE_H = \ psbt.h \ random.h \ randomenv.h \ + rest.h \ rpc/blockchain.h \ rpc/client.h \ rpc/evo_util.h \ diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index bc788ac810f8b..a6ffeb1c21077 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -88,6 +88,7 @@ if ENABLE_WALLET bench_bench_dash_SOURCES += bench/coin_selection.cpp bench_bench_dash_SOURCES += bench/wallet_balance.cpp bench_bench_dash_SOURCES += bench/wallet_create.cpp +bench_bench_dash_SOURCES += bench/wallet_loading.cpp bench_bench_dash_LDADD += $(BDB_LIBS) $(SQLITE_LIBS) endif diff --git a/src/Makefile.test.include b/src/Makefile.test.include index bed7081dd6d4c..703a36d5157b3 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -121,6 +121,7 @@ BITCOIN_TESTS =\ test/getarg_tests.cpp \ test/governance_validators_tests.cpp \ test/hash_tests.cpp \ + test/httpserver_tests.cpp \ test/i2p_tests.cpp \ test/interfaces_tests.cpp \ test/key_io_tests.cpp \ @@ -156,6 +157,7 @@ BITCOIN_TESTS =\ test/raii_event_tests.cpp \ test/random_tests.cpp \ test/ratecheck_tests.cpp \ + test/rest_tests.cpp \ test/reverselock_tests.cpp \ test/rpc_tests.cpp \ test/sanity_tests.cpp \ diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp new file mode 100644 index 0000000000000..7fdacc24529a6 --- /dev/null +++ b/src/bench/wallet_loading.cpp @@ -0,0 +1,133 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +using wallet::CWallet; +using wallet::DatabaseFormat; +using wallet::DatabaseOptions; +using wallet::ISMINE_SPENDABLE; +using wallet::MakeWalletDatabase; +using wallet::TxStateInactive; +using wallet::WALLET_FLAG_DESCRIPTORS; +using wallet::WalletContext; +using wallet::WalletDatabase; + +static const std::shared_ptr BenchLoadWallet(std::unique_ptr database, WalletContext& context, DatabaseOptions& options) +{ + bilingual_str error; + std::vector warnings; + auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings); + NotifyWalletLoaded(context, wallet); + if (context.chain) { + wallet->postInitProcess(); + } + return wallet; +} + +static void BenchUnloadWallet(std::shared_ptr&& wallet) +{ + SyncWithValidationInterfaceQueue(); + wallet->m_chain_notifications_handler.reset(); + UnloadWallet(std::move(wallet)); +} + +static void AddTx(CWallet& wallet) +{ + bilingual_str error; + CTxDestination dest; + wallet.GetNewDestination("", dest, error); + + CMutableTransaction mtx; + mtx.vout.push_back({COIN, GetScriptForDestination(dest)}); + mtx.vin.push_back(CTxIn()); + + wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}); +} + +static std::unique_ptr DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options) +{ + auto new_database = CreateMockWalletDatabase(options); + + // Get a cursor to the original database + auto batch = database.MakeBatch(); + batch->StartCursor(); + + // Get a batch for the new database + auto new_batch = new_database->MakeBatch(); + + // Read all records from the original database and write them to the new one + while (true) { + CDataStream key(SER_DISK, CLIENT_VERSION); + CDataStream value(SER_DISK, CLIENT_VERSION); + bool complete; + batch->ReadAtCursor(key, value, complete); + if (complete) break; + new_batch->Write(key, value); + } + + return new_database; +} + +static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet) +{ + const auto test_setup = MakeNoLogFileContext(); + test_setup->m_args.ForceSetArg("-unsafesqlitesync", "1"); + + WalletContext context; + context.args = &test_setup->m_args; + context.chain = test_setup->m_node.chain.get(); + + // Setup the wallet + // Loading the wallet will also create it + DatabaseOptions options; + if (legacy_wallet) { + options.require_format = DatabaseFormat::BERKELEY; + } else { + options.create_flags = WALLET_FLAG_DESCRIPTORS; + options.require_format = DatabaseFormat::SQLITE; + } + auto database = CreateMockWalletDatabase(options); + auto wallet = BenchLoadWallet(std::move(database), context, options); + + // Generate a bunch of transactions and addresses to put into the wallet + for (int i = 0; i < 1000; ++i) { + AddTx(*wallet); + } + + database = DuplicateMockDatabase(wallet->GetDatabase(), options); + + // reload the wallet for the actual benchmark + BenchUnloadWallet(std::move(wallet)); + + bench.epochs(5).run([&] { + wallet = BenchLoadWallet(std::move(database), context, options); + + // Cleanup + database = DuplicateMockDatabase(wallet->GetDatabase(), options); + BenchUnloadWallet(std::move(wallet)); + }); +} + +#ifdef USE_BDB +static void WalletLoadingLegacy(benchmark::Bench& bench) { WalletLoading(bench, /*legacy_wallet=*/true); } +BENCHMARK(WalletLoadingLegacy); +#endif + +#ifdef USE_SQLITE +static void WalletLoadingDescriptors(benchmark::Bench& bench) { WalletLoading(bench, /*legacy_wallet=*/false); } +BENCHMARK(WalletLoadingDescriptors); +#endif diff --git a/src/chain.cpp b/src/chain.cpp index 2480047d024fe..765c4d55a6ba5 100644 --- a/src/chain.cpp +++ b/src/chain.cpp @@ -19,11 +19,9 @@ std::string CBlockIndex::ToString() const pprev, nHeight, hashMerkleRoot.ToString(), GetBlockHash().ToString()); } -void CChain::SetTip(CBlockIndex *pindex) { - if (pindex == nullptr) { - vChain.clear(); - return; - } +void CChain::SetTip(CBlockIndex& block) +{ + CBlockIndex* pindex = █ vChain.resize(pindex->nHeight + 1); while (pindex && vChain[pindex->nHeight] != pindex) { vChain[pindex->nHeight] = pindex; diff --git a/src/chain.h b/src/chain.h index f7fe7584936bf..2fdac98e85c05 100644 --- a/src/chain.h +++ b/src/chain.h @@ -470,7 +470,7 @@ class CChain } /** Set/initialize a chain with a given tip. */ - void SetTip(CBlockIndex* pindex); + void SetTip(CBlockIndex& block); /** Return a CBlockLocator that refers to a block in this chain (by default the tip). */ CBlockLocator GetLocator(const CBlockIndex* pindex = nullptr) const; diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 51d8c3e0d8c29..dc4d6c5dc0d43 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -21,15 +21,17 @@ #include #include +#include #include #include -#include #include #include -#include +#include #include +#include +#include #include @@ -670,6 +672,37 @@ HTTPRequest::RequestMethod HTTPRequest::GetRequestMethod() const } } +std::optional HTTPRequest::GetQueryParameter(const std::string& key) const +{ + const char* uri{evhttp_request_get_uri(req)}; + + return GetQueryParameterFromUri(uri, key); +} + +std::optional GetQueryParameterFromUri(const char* uri, const std::string& key) +{ + evhttp_uri* uri_parsed{evhttp_uri_parse(uri)}; + const char* query{evhttp_uri_get_query(uri_parsed)}; + std::optional result; + + if (query) { + // Parse the query string into a key-value queue and iterate over it + struct evkeyvalq params_q; + evhttp_parse_query_str(query, ¶ms_q); + + for (struct evkeyval* param{params_q.tqh_first}; param != nullptr; param = param->next.tqe_next) { + if (param->key == key) { + result = param->value; + break; + } + } + evhttp_clear_headers(¶ms_q); + } + evhttp_uri_free(uri_parsed); + + return result; +} + void RegisterHTTPHandler(const std::string &prefix, bool exactMatch, const HTTPRequestHandler &handler) { LogPrint(BCLog::HTTP, "Registering HTTP handler for %s (exactmatch %d)\n", prefix, exactMatch); diff --git a/src/httpserver.h b/src/httpserver.h index 92c8cf1843a81..5ab3f189273af 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -5,8 +5,9 @@ #ifndef BITCOIN_HTTPSERVER_H #define BITCOIN_HTTPSERVER_H -#include #include +#include +#include static const int DEFAULT_HTTP_THREADS=4; static const int DEFAULT_HTTP_WORKQUEUE=16; @@ -82,6 +83,17 @@ class HTTPRequest */ RequestMethod GetRequestMethod() const; + /** Get the query parameter value from request uri for a specified key, or std::nullopt if the + * key is not found. + * + * If the query string contains duplicate keys, the first value is returned. Many web frameworks + * would instead parse this as an array of values, but this is not (yet) implemented as it is + * currently not needed in any of the endpoints. + * + * @param[in] key represents the query parameter of which the value is returned + */ + std::optional GetQueryParameter(const std::string& key) const; + /** * Get the request header specified by hdr, or an empty string. * Return a pair (isPresent,string). @@ -114,6 +126,20 @@ class HTTPRequest void WriteReply(int nStatus, const std::string& strReply = ""); }; +/** Get the query parameter value from request uri for a specified key, or std::nullopt if the key + * is not found. + * + * If the query string contains duplicate keys, the first value is returned. Many web frameworks + * would instead parse this as an array of values, but this is not (yet) implemented as it is + * currently not needed in any of the endpoints. + * + * Helper function for HTTPRequest::GetQueryParameter. + * + * @param[in] uri is the entire request uri + * @param[in] key represents the query parameter of which the value is returned + */ +std::optional GetQueryParameterFromUri(const char* uri, const std::string& key); + /** Event handler closure. */ class HTTPClosure diff --git a/src/init.cpp b/src/init.cpp index d2e165619bf11..15929f4898310 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1041,7 +1041,7 @@ namespace { // Variables internal to initialization process only int nMaxConnections; int nUserMaxConnections; int nFD; -ServiceFlags nLocalServices = ServiceFlags(NODE_NETWORK | NODE_NETWORK_LIMITED | NODE_HEADERS_COMPRESSED); +ServiceFlags nLocalServices = ServiceFlags(NODE_NETWORK_LIMITED | NODE_HEADERS_COMPRESSED); int64_t peer_connect_timeout; std::set g_enabled_filter_types; @@ -2221,12 +2221,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } // ********************************************************* Step 10: data directory maintenance - - // if pruning, unset the service bit and perform the initial blockstore prune + // if pruning, perform the initial blockstore prune // after any wallet rescanning has taken place. if (fPruneMode) { - LogPrintf("Unsetting NODE_NETWORK on prune mode\n"); - nLocalServices = ServiceFlags(nLocalServices & ~NODE_NETWORK); if (!fReindex) { LOCK(cs_main); for (CChainState* chainstate : chainman.GetAll()) { @@ -2234,6 +2231,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) chainstate->PruneAndFlush(); } } + } else { + LogPrintf("Setting NODE_NETWORK on non-prune mode\n"); + nLocalServices = ServiceFlags(nLocalServices | NODE_NETWORK); } // As PruneAndFlush can take several minutes, it's possible the user diff --git a/src/rest.cpp b/src/rest.cpp index 790b55af47acc..56b4c58614071 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -3,6 +3,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include + #include #include #include @@ -30,6 +32,8 @@ #include #include +#include + #include using node::GetTransaction; @@ -39,21 +43,14 @@ using node::ReadBlockFromDisk; static const size_t MAX_GETUTXOS_OUTPOINTS = 15; //allow a max of 15 outpoints to be queried at once static constexpr unsigned int MAX_REST_HEADERS_RESULTS = 2000; -enum class RetFormat { - UNDEF, - BINARY, - HEX, - JSON, -}; - static const struct { - RetFormat rf; + RESTResponseFormat rf; const char* name; } rf_names[] = { - {RetFormat::UNDEF, ""}, - {RetFormat::BINARY, "bin"}, - {RetFormat::HEX, "hex"}, - {RetFormat::JSON, "json"}, + {RESTResponseFormat::UNDEF, ""}, + {RESTResponseFormat::BINARY, "bin"}, + {RESTResponseFormat::HEX, "hex"}, + {RESTResponseFormat::JSON, "json"}, }; struct CCoin { @@ -157,25 +154,28 @@ static LLMQContext* GetLLMQContext(const CoreContext& context, HTTPRequest* req) return node_context->llmq_ctx.get(); } -static RetFormat ParseDataFormat(std::string& param, const std::string& strReq) +RESTResponseFormat ParseDataFormat(std::string& param, const std::string& strReq) { - const std::string::size_type pos = strReq.rfind('.'); - if (pos == std::string::npos) - { - param = strReq; + // Remove query string (if any, separated with '?') as it should not interfere with + // parsing param and data format + param = strReq.substr(0, strReq.rfind('?')); + const std::string::size_type pos_format{param.rfind('.')}; + + // No format string is found + if (pos_format == std::string::npos) { return rf_names[0].rf; } - param = strReq.substr(0, pos); - const std::string suff(strReq, pos + 1); - + // Match format string to available formats + const std::string suffix(param, pos_format + 1); for (const auto& rf_name : rf_names) { - if (suff == rf_name.name) + if (suffix == rf_name.name) { + param.erase(pos_format); return rf_name.rf; + } } - /* If no suffix is found, return original string. */ - param = strReq; + // If no suffix is found, return RESTResponseFormat::UNDEF and original string without query string return rf_names[0].rf; } @@ -211,18 +211,28 @@ static bool rest_headers(const CoreContext& context, if (!CheckWarmup(req)) return false; std::string param; - const RetFormat rf = ParseDataFormat(param, strURIPart); + const RESTResponseFormat rf = ParseDataFormat(param, strURIPart); std::vector path = SplitString(param, '/'); - if (path.size() != 2) - return RESTERR(req, HTTP_BAD_REQUEST, "No header count specified. Use /rest/headers//.."); - - const auto parsed_count{ToIntegral(path[0])}; + std::string raw_count; + std::string hashStr; + if (path.size() == 2) { + // deprecated path: /rest/headers// + hashStr = path[1]; + raw_count = path[0]; + } else if (path.size() == 1) { + // new path with query parameter: /rest/headers/?count= + hashStr = path[0]; + raw_count = req->GetQueryParameter("count").value_or("5"); + } else { + return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/headers/.?count="); + } + + const auto parsed_count{ToIntegral(raw_count)}; if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) { - return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, path[0])); + return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, raw_count)); } - std::string hashStr = path[1]; uint256 hash; if (!ParseHashStr(hashStr, hash)) return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr); @@ -248,7 +258,7 @@ static bool rest_headers(const CoreContext& context, } switch (rf) { - case RetFormat::BINARY: { + case RESTResponseFormat::BINARY: { CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION); for (const CBlockIndex *pindex : headers) { ssHeader << pindex->GetBlockHeader(); @@ -260,7 +270,7 @@ static bool rest_headers(const CoreContext& context, return true; } - case RetFormat::HEX: { + case RESTResponseFormat::HEX: { CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION); for (const CBlockIndex *pindex : headers) { ssHeader << pindex->GetBlockHeader(); @@ -271,7 +281,7 @@ static bool rest_headers(const CoreContext& context, req->WriteReply(HTTP_OK, strHex); return true; } - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { const LLMQContext* llmq_ctx = GetLLMQContext(context, req); if (!llmq_ctx) return false; @@ -298,7 +308,7 @@ static bool rest_block(const CoreContext& context, if (!CheckWarmup(req)) return false; std::string hashStr; - const RetFormat rf = ParseDataFormat(hashStr, strURIPart); + const RESTResponseFormat rf = ParseDataFormat(hashStr, strURIPart); uint256 hash; if (!ParseHashStr(hashStr, hash)) @@ -327,7 +337,7 @@ static bool rest_block(const CoreContext& context, } switch (rf) { - case RetFormat::BINARY: { + case RESTResponseFormat::BINARY: { CDataStream ssBlock(SER_NETWORK, PROTOCOL_VERSION); ssBlock << block; std::string binaryBlock = ssBlock.str(); @@ -336,7 +346,7 @@ static bool rest_block(const CoreContext& context, return true; } - case RetFormat::HEX: { + case RESTResponseFormat::HEX: { CDataStream ssBlock(SER_NETWORK, PROTOCOL_VERSION); ssBlock << block; std::string strHex = HexStr(ssBlock) + "\n"; @@ -345,7 +355,7 @@ static bool rest_block(const CoreContext& context, return true; } - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { const LLMQContext* llmq_ctx = GetLLMQContext(context, req); if (!llmq_ctx) return false; @@ -377,16 +387,31 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con if (!CheckWarmup(req)) return false; std::string param; - const RetFormat rf = ParseDataFormat(param, strURIPart); + const RESTResponseFormat rf = ParseDataFormat(param, strURIPart); std::vector uri_parts = SplitString(param, '/'); - if (uri_parts.size() != 3) { - return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/blockfilterheaders///"); + std::string raw_count; + std::string raw_blockhash; + if (uri_parts.size() == 3) { + // deprecated path: /rest/blockfilterheaders/// + raw_blockhash = uri_parts[2]; + raw_count = uri_parts[1]; + } else if (uri_parts.size() == 2) { + // new path with query parameter: /rest/blockfilterheaders//?count= + raw_blockhash = uri_parts[1]; + raw_count = req->GetQueryParameter("count").value_or("5"); + } else { + return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/blockfilterheaders//.?count="); + } + + const auto parsed_count{ToIntegral(raw_count)}; + if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) { + return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, raw_count)); } uint256 block_hash; - if (!ParseHashStr(uri_parts[2], block_hash)) { - return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + uri_parts[2]); + if (!ParseHashStr(raw_blockhash, block_hash)) { + return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + raw_blockhash); } BlockFilterType filtertype; @@ -399,11 +424,6 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con return RESTERR(req, HTTP_BAD_REQUEST, "Index is not enabled for filtertype " + uri_parts[0]); } - const auto parsed_count{ToIntegral(uri_parts[1])}; - if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) { - return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, uri_parts[1])); - } - std::vector headers; headers.reserve(*parsed_count); { @@ -442,7 +462,7 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con } switch (rf) { - case RetFormat::BINARY: { + case RESTResponseFormat::BINARY: { CDataStream ssHeader{SER_NETWORK, PROTOCOL_VERSION}; for (const uint256& header : filter_headers) { ssHeader << header; @@ -453,7 +473,7 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con req->WriteReply(HTTP_OK, binaryHeader); return true; } - case RetFormat::HEX: { + case RESTResponseFormat::HEX: { CDataStream ssHeader{SER_NETWORK, PROTOCOL_VERSION}; for (const uint256& header : filter_headers) { ssHeader << header; @@ -464,7 +484,7 @@ static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, con req->WriteReply(HTTP_OK, strHex); return true; } - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { UniValue jsonHeaders(UniValue::VARR); for (const uint256& header : filter_headers) { jsonHeaders.push_back(header.GetHex()); @@ -486,7 +506,7 @@ static bool rest_block_filter(const CoreContext& context, HTTPRequest* req, cons if (!CheckWarmup(req)) return false; std::string param; - const RetFormat rf = ParseDataFormat(param, strURIPart); + const RESTResponseFormat rf = ParseDataFormat(param, strURIPart); // request is sent over URI scheme /rest/blockfilter/filtertype/blockhash std::vector uri_parts = SplitString(param, '/'); @@ -541,7 +561,7 @@ static bool rest_block_filter(const CoreContext& context, HTTPRequest* req, cons } switch (rf) { - case RetFormat::BINARY: { + case RESTResponseFormat::BINARY: { CDataStream ssResp{SER_NETWORK, PROTOCOL_VERSION}; ssResp << filter; @@ -550,7 +570,7 @@ static bool rest_block_filter(const CoreContext& context, HTTPRequest* req, cons req->WriteReply(HTTP_OK, binaryResp); return true; } - case RetFormat::HEX: { + case RESTResponseFormat::HEX: { CDataStream ssResp{SER_NETWORK, PROTOCOL_VERSION}; ssResp << filter; @@ -559,7 +579,7 @@ static bool rest_block_filter(const CoreContext& context, HTTPRequest* req, cons req->WriteReply(HTTP_OK, strHex); return true; } - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { UniValue ret(UniValue::VOBJ); ret.pushKV("filter", HexStr(filter.GetEncodedFilter())); std::string strJSON = ret.write() + "\n"; @@ -581,10 +601,10 @@ static bool rest_chaininfo(const CoreContext& context, HTTPRequest* req, const s if (!CheckWarmup(req)) return false; std::string param; - const RetFormat rf = ParseDataFormat(param, strURIPart); + const RESTResponseFormat rf = ParseDataFormat(param, strURIPart); switch (rf) { - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { JSONRPCRequest jsonRequest; jsonRequest.context = context; jsonRequest.params = UniValue(UniValue::VARR); @@ -607,10 +627,10 @@ static bool rest_mempool_info(const CoreContext& context, HTTPRequest* req, cons const CTxMemPool* mempool = GetMemPool(context, req); if (!mempool) return false; std::string param; - const RetFormat rf = ParseDataFormat(param, strURIPart); + const RESTResponseFormat rf = ParseDataFormat(param, strURIPart); switch (rf) { - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { const LLMQContext* llmq_ctx = GetLLMQContext(context, req); if (!llmq_ctx) return false; @@ -633,10 +653,10 @@ static bool rest_mempool_contents(const CoreContext& context, HTTPRequest* req, const CTxMemPool* mempool = GetMemPool(context, req); if (!mempool) return false; std::string param; - const RetFormat rf = ParseDataFormat(param, strURIPart); + const RESTResponseFormat rf = ParseDataFormat(param, strURIPart); switch (rf) { - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { const LLMQContext* llmq_ctx = GetLLMQContext(context, req); if (!llmq_ctx) return false; @@ -658,7 +678,7 @@ static bool rest_tx(const CoreContext& context, HTTPRequest* req, const std::str if (!CheckWarmup(req)) return false; std::string hashStr; - const RetFormat rf = ParseDataFormat(hashStr, strURIPart); + const RESTResponseFormat rf = ParseDataFormat(hashStr, strURIPart); uint256 hash; if (!ParseHashStr(hashStr, hash)) @@ -677,7 +697,7 @@ static bool rest_tx(const CoreContext& context, HTTPRequest* req, const std::str } switch (rf) { - case RetFormat::BINARY: { + case RESTResponseFormat::BINARY: { CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); ssTx << tx; @@ -687,7 +707,7 @@ static bool rest_tx(const CoreContext& context, HTTPRequest* req, const std::str return true; } - case RetFormat::HEX: { + case RESTResponseFormat::HEX: { CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); ssTx << tx; @@ -697,7 +717,7 @@ static bool rest_tx(const CoreContext& context, HTTPRequest* req, const std::str return true; } - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { UniValue objTx(UniValue::VOBJ); TxToUniv(*tx, /*block_hash=*/hashBlock, /*entry=*/objTx); std::string strJSON = objTx.write() + "\n"; @@ -717,7 +737,7 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st if (!CheckWarmup(req)) return false; std::string param; - const RetFormat rf = ParseDataFormat(param, strURIPart); + const RESTResponseFormat rf = ParseDataFormat(param, strURIPart); std::vector uriParts; if (param.length() > 1) @@ -764,14 +784,14 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st } switch (rf) { - case RetFormat::HEX: { + case RESTResponseFormat::HEX: { // convert hex to bin, continue then with bin part std::vector strRequestV = ParseHex(strRequestMutable); strRequestMutable.assign(strRequestV.begin(), strRequestV.end()); [[fallthrough]]; } - case RetFormat::BINARY: { + case RESTResponseFormat::BINARY: { try { //deserialize only if user sent a request if (strRequestMutable.size() > 0) @@ -791,7 +811,7 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st break; } - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { if (!fInputParsed) return RESTERR(req, HTTP_BAD_REQUEST, "Error: empty request"); break; @@ -845,7 +865,7 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st } switch (rf) { - case RetFormat::BINARY: { + case RESTResponseFormat::BINARY: { // serialize data // use exact same output as mentioned in Bip64 CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION); @@ -857,7 +877,7 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st return true; } - case RetFormat::HEX: { + case RESTResponseFormat::HEX: { CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION); ssGetUTXOResponse << chainman.ActiveChain().Height() << chainman.ActiveChain().Tip()->GetBlockHash() << bitmap << outs; std::string strHex = HexStr(ssGetUTXOResponse) + "\n"; @@ -867,7 +887,7 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st return true; } - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { UniValue objGetUTXOResponse(UniValue::VOBJ); // pack in some essentials @@ -907,7 +927,7 @@ static bool rest_blockhash_by_height(const CoreContext& context, HTTPRequest* re { if (!CheckWarmup(req)) return false; std::string height_str; - const RetFormat rf = ParseDataFormat(height_str, str_uri_part); + const RESTResponseFormat rf = ParseDataFormat(height_str, str_uri_part); int32_t blockheight = -1; // Initialization done only to prevent valgrind false positive, see https://github.com/bitcoin/bitcoin/pull/18785 if (!ParseInt32(height_str, &blockheight) || blockheight < 0) { @@ -927,19 +947,19 @@ static bool rest_blockhash_by_height(const CoreContext& context, HTTPRequest* re pblockindex = active_chain[blockheight]; } switch (rf) { - case RetFormat::BINARY: { + case RESTResponseFormat::BINARY: { CDataStream ss_blockhash(SER_NETWORK, PROTOCOL_VERSION); ss_blockhash << pblockindex->GetBlockHash(); req->WriteHeader("Content-Type", "application/octet-stream"); req->WriteReply(HTTP_OK, ss_blockhash.str()); return true; } - case RetFormat::HEX: { + case RESTResponseFormat::HEX: { req->WriteHeader("Content-Type", "text/plain"); req->WriteReply(HTTP_OK, pblockindex->GetBlockHash().GetHex() + "\n"); return true; } - case RetFormat::JSON: { + case RESTResponseFormat::JSON: { req->WriteHeader("Content-Type", "application/json"); UniValue resp = UniValue(UniValue::VOBJ); resp.pushKV("blockhash", pblockindex->GetBlockHash().GetHex()); diff --git a/src/rest.h b/src/rest.h new file mode 100644 index 0000000000000..49b1c333d052d --- /dev/null +++ b/src/rest.h @@ -0,0 +1,28 @@ +// Copyright (c) 2015-2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_REST_H +#define BITCOIN_REST_H + +#include + +enum class RESTResponseFormat { + UNDEF, + BINARY, + HEX, + JSON, +}; + +/** + * Parse a URI to get the data format and URI without data format + * and query string. + * + * @param[out] param The strReq without the data format string and + * without the query string (if any). + * @param[in] strReq The URI to be parsed. + * @return RESTResponseFormat that was parsed from the URI. + */ +RESTResponseFormat ParseDataFormat(std::string& param, const std::string& strReq); + +#endif // BITCOIN_REST_H diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index ae6ca2b23ea84..9ae25ef11ba3f 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1086,7 +1086,7 @@ static RPCHelpMan pruneblockchain() const CBlockIndex& block{*CHECK_NONFATAL(active_chain.Tip())}; const CBlockIndex* last_block{active_chainstate.m_blockman.GetFirstStoredBlock(block)}; - return static_cast(last_block->nHeight); + return static_cast(last_block->nHeight - 1); }, }; } @@ -1457,7 +1457,7 @@ RPCHelpMan getblockchaininfo() {RPCResult::Type::STR_HEX, "chainwork", "total amount of work in active chain, in hexadecimal"}, {RPCResult::Type::NUM, "size_on_disk", "the estimated size of the block and undo files on disk"}, {RPCResult::Type::BOOL, "pruned", "if the blocks are subject to pruning"}, - {RPCResult::Type::NUM, "pruneheight", /* optional */ true, "lowest-height complete block stored (only present if pruning is enabled)"}, + {RPCResult::Type::NUM, "pruneheight", /* optional */ true, "height of the last block pruned, plus one (only present if pruning is enabled)"}, {RPCResult::Type::BOOL, "automatic_pruning", /* optional */ true, "whether automatic pruning is enabled (only present if pruning is enabled)"}, {RPCResult::Type::NUM, "prune_target_size", /* optional */ true, "the target size used by pruning (only present if automatic pruning is enabled)"}, {RPCResult::Type::OBJ, "softforks", "status of softforks in progress", @@ -2671,6 +2671,12 @@ static RPCHelpMan dumptxoutset() FILE* file{fsbridge::fopen(temppath, "wb")}; CAutoFile afile{file, SER_DISK, CLIENT_VERSION}; + if (afile.IsNull()) { + throw JSONRPCError( + RPC_INVALID_PARAMETER, + "Couldn't open file " + temppath.utf8string() + " for writing."); + } + NodeContext& node = EnsureAnyNodeContext(request.context); UniValue result = CreateUTXOSnapshot( node, node.chainman->ActiveChainstate(), afile, path, temppath); diff --git a/src/test/evo_assetlocks_tests.cpp b/src/test/evo_assetlocks_tests.cpp index 02d2353d34a91..c2179479c5e42 100644 --- a/src/test/evo_assetlocks_tests.cpp +++ b/src/test/evo_assetlocks_tests.cpp @@ -289,7 +289,7 @@ BOOST_FIXTURE_TEST_CASE(evo_assetlock, TestChain100Setup) { // OP_RETURN should not have any data CMutableTransaction txReturnData(tx); - txReturnData.vout[0].scriptPubKey = CScript() << OP_RETURN << ParseHex("abc"); + txReturnData.vout[0].scriptPubKey = CScript() << OP_RETURN << ParseHex("abcd"); BOOST_CHECK(!CheckAssetLockTx(CTransaction(txReturnData), tx_state)); BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetlocktx-non-empty-return"); diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp new file mode 100644 index 0000000000000..ee59ec6967fc7 --- /dev/null +++ b/src/test/httpserver_tests.cpp @@ -0,0 +1,38 @@ +// Copyright (c) 2012-2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include + +#include + +BOOST_FIXTURE_TEST_SUITE(httpserver_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(test_query_parameters) +{ + std::string uri {}; + + // No parameters + uri = "localhost:8080/rest/headers/someresource.json"; + BOOST_CHECK(!GetQueryParameterFromUri(uri.c_str(), "p1").has_value()); + + // Single parameter + uri = "localhost:8080/rest/endpoint/someresource.json?p1=v1"; + BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p1").value(), "v1"); + BOOST_CHECK(!GetQueryParameterFromUri(uri.c_str(), "p2").has_value()); + + // Multiple parameters + uri = "/rest/endpoint/someresource.json?p1=v1&p2=v2"; + BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p1").value(), "v1"); + BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p2").value(), "v2"); + + // If the query string contains duplicate keys, the first value is returned + uri = "/rest/endpoint/someresource.json?p1=v1&p1=v2"; + BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p1").value(), "v1"); + + // Invalid query string syntax is the same as not having parameters + uri = "/rest/endpoint/someresource.json&p1=v1&p2=v2"; + BOOST_CHECK(!GetQueryParameterFromUri(uri.c_str(), "p1").has_value()); +} +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 8447a5e927da3..f16dfa7fabff8 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -386,7 +386,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // next->pprev = prev; // next->nHeight = prev->nHeight + 1; // next->BuildSkip(); - // m_node.chainman->ActiveChain().SetTip(next); + // m_node.chainman->ActiveChain().SetTip(*next); // } //BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); // // Extend to a 210000-long block chain. @@ -398,7 +398,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // next->pprev = prev; // next->nHeight = prev->nHeight + 1; // next->BuildSkip(); - // m_node.chainman->ActiveChain().SetTip(next); + // m_node.chainman->ActiveChain().SetTip(*next); // } //BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); @@ -423,7 +423,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // // Delete the dummy blocks again. // while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) { // CBlockIndex* del = m_node.chainman->ActiveChain().Tip(); - // m_node.chainman->ActiveChain().SetTip(del->pprev); + // m_node.chainman->ActiveChain().SetTip(*Assert(del->pprev)); // m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(del->pprev->GetBlockHash()); // delete del->phashBlock; // delete del; diff --git a/src/test/rest_tests.cpp b/src/test/rest_tests.cpp new file mode 100644 index 0000000000000..20dfe4b41a703 --- /dev/null +++ b/src/test/rest_tests.cpp @@ -0,0 +1,48 @@ +// Copyright (c) 2012-2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include + +#include + +#include + +BOOST_FIXTURE_TEST_SUITE(rest_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(test_query_string) +{ + std::string param; + RESTResponseFormat rf; + // No query string + rf = ParseDataFormat(param, "/rest/endpoint/someresource.json"); + BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource"); + BOOST_CHECK_EQUAL(rf, RESTResponseFormat::JSON); + + // Query string with single parameter + rf = ParseDataFormat(param, "/rest/endpoint/someresource.bin?p1=v1"); + BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource"); + BOOST_CHECK_EQUAL(rf, RESTResponseFormat::BINARY); + + // Query string with multiple parameters + rf = ParseDataFormat(param, "/rest/endpoint/someresource.hex?p1=v1&p2=v2"); + BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource"); + BOOST_CHECK_EQUAL(rf, RESTResponseFormat::HEX); + + // Incorrectly formed query string will not be handled + rf = ParseDataFormat(param, "/rest/endpoint/someresource.json&p1=v1"); + BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource.json&p1=v1"); + BOOST_CHECK_EQUAL(rf, RESTResponseFormat::UNDEF); + + // Omitted data format with query string should return UNDEF and hide query string + rf = ParseDataFormat(param, "/rest/endpoint/someresource?p1=v1"); + BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource"); + BOOST_CHECK_EQUAL(rf, RESTResponseFormat::UNDEF); + + // Data format specified after query string + rf = ParseDataFormat(param, "/rest/endpoint/someresource?p1=v1.json"); + BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource"); + BOOST_CHECK_EQUAL(rf, RESTResponseFormat::UNDEF); +} +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/skiplist_tests.cpp b/src/test/skiplist_tests.cpp index 7092c7fce5ba9..07724a97d06d2 100644 --- a/src/test/skiplist_tests.cpp +++ b/src/test/skiplist_tests.cpp @@ -72,7 +72,7 @@ BOOST_AUTO_TEST_CASE(getlocator_test) // Build a CChain for the main branch. CChain chain; - chain.SetTip(&vBlocksMain.back()); + chain.SetTip(vBlocksMain.back()); // Test 100 random starting points for locators. for (int n=0; n<100; n++) { @@ -128,7 +128,7 @@ BOOST_AUTO_TEST_CASE(findearliestatleast_test) // Build a CChain for the main branch. CChain chain; - chain.SetTip(&vBlocksMain.back()); + chain.SetTip(vBlocksMain.back()); // Verify that FindEarliestAtLeast is correct. for (unsigned int i=0; i<10000; ++i) { @@ -155,7 +155,7 @@ BOOST_AUTO_TEST_CASE(findearliestatleast_edge_test) } CChain chain; - chain.SetTip(&blocks.back()); + chain.SetTip(blocks.back()); BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(50, 0)->nHeight, 0); BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(100, 0)->nHeight, 0); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 50ed481bea03a..844e61a109b5d 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -145,26 +145,52 @@ BOOST_AUTO_TEST_CASE(parse_hex) // Basic test vector result = ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"); BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); + result = TryParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f").value(); + BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); // Spaces between bytes must be supported result = ParseHex("12 34 56 78"); BOOST_CHECK(result.size() == 4 && result[0] == 0x12 && result[1] == 0x34 && result[2] == 0x56 && result[3] == 0x78); + result = TryParseHex("12 34 56 78").value(); + BOOST_CHECK(result.size() == 4 && result[0] == 0x12 && result[1] == 0x34 && result[2] == 0x56 && result[3] == 0x78); // Leading space must be supported (used in BerkeleyEnvironment::Salvage) result = ParseHex(" 89 34 56 78"); BOOST_CHECK(result.size() == 4 && result[0] == 0x89 && result[1] == 0x34 && result[2] == 0x56 && result[3] == 0x78); + result = TryParseHex(" 89 34 56 78").value(); + BOOST_CHECK(result.size() == 4 && result[0] == 0x89 && result[1] == 0x34 && result[2] == 0x56 && result[3] == 0x78); + + // Mixed case and spaces are supported + result = ParseHex(" Ff aA "); + BOOST_CHECK(result.size() == 2 && result[0] == 0xff && result[1] == 0xaa); + result = TryParseHex(" Ff aA ").value(); + BOOST_CHECK(result.size() == 2 && result[0] == 0xff && result[1] == 0xaa); - // Embedded null is treated as end + // Empty string is supported + result = ParseHex(""); + BOOST_CHECK(result.size() == 0); + result = TryParseHex("").value(); + BOOST_CHECK(result.size() == 0); + + // Spaces between nibbles is treated as invalid + BOOST_CHECK_EQUAL(ParseHex("AAF F").size(), 0); + BOOST_CHECK(!TryParseHex("AAF F").has_value()); + + // Embedded null is treated as invalid const std::string with_embedded_null{" 11 "s " \0 " " 22 "s}; BOOST_CHECK_EQUAL(with_embedded_null.size(), 11); - result = ParseHex(with_embedded_null); - BOOST_CHECK(result.size() == 1 && result[0] == 0x11); + BOOST_CHECK_EQUAL(ParseHex(with_embedded_null).size(), 0); + BOOST_CHECK(!TryParseHex(with_embedded_null).has_value()); + + // Non-hex is treated as invalid + BOOST_CHECK_EQUAL(ParseHex("1234 invalid 1234").size(), 0); + BOOST_CHECK(!TryParseHex("1234 invalid 1234").has_value()); - // Stop parsing at invalid value - result = ParseHex("1234 invalid 1234"); - BOOST_CHECK(result.size() == 2 && result[0] == 0x12 && result[1] == 0x34); + // Truncated input is treated as invalid + BOOST_CHECK_EQUAL(ParseHex("12 3").size(), 0); + BOOST_CHECK(!TryParseHex("12 3").has_value()); } BOOST_AUTO_TEST_CASE(util_HexStr) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index e647cd0f9c2f3..a69c770ac1b9d 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -79,24 +79,25 @@ bool IsHexNumber(std::string_view str) } template -std::vector ParseHex(std::string_view str) +std::optional> TryParseHex(std::string_view str) { std::vector vch; auto it = str.begin(); - while (it != str.end() && it + 1 != str.end()) { + while (it != str.end()) { if (IsSpace(*it)) { ++it; continue; } auto c1 = HexDigit(*(it++)); + if (it == str.end()) return std::nullopt; auto c2 = HexDigit(*(it++)); - if (c1 < 0 || c2 < 0) break; + if (c1 < 0 || c2 < 0) return std::nullopt; vch.push_back(Byte(c1 << 4) | Byte(c2)); } return vch; } -template std::vector ParseHex(std::string_view); -template std::vector ParseHex(std::string_view); +template std::optional> TryParseHex(std::string_view); +template std::optional> TryParseHex(std::string_view); bool SplitHostPort(std::string_view in, uint16_t& portOut, std::string& hostOut) { diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 75b531ecc4e63..e6de59e3a1a52 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -58,9 +58,15 @@ enum class ByteUnit : uint64_t { * @return A new string without unsafe chars */ std::string SanitizeString(std::string_view str, int rule = SAFE_CHARS_DEFAULT); -/** Parse the hex string into bytes (uint8_t or std::byte). Ignores whitespace. */ +/** Parse the hex string into bytes (uint8_t or std::byte). Ignores whitespace. Returns nullopt on invalid input. */ +template +std::optional> TryParseHex(std::string_view str); +/** Like TryParseHex, but returns an empty vector on invalid input. */ template -std::vector ParseHex(std::string_view str); +std::vector ParseHex(std::string_view hex_str) +{ + return TryParseHex(hex_str).value_or(std::vector{}); +} signed char HexDigit(char c); /* Returns true if each character in str is a hex character, and has an even * number of hex digits.*/ diff --git a/src/validation.cpp b/src/validation.cpp index 823c2ca5d3548..a2a71eeaddce2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2922,6 +2922,7 @@ bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTr CBlockIndex *pindexDelete = m_chain.Tip(); assert(pindexDelete); + assert(pindexDelete->pprev); // Read block from disk. std::shared_ptr pblock = std::make_shared(); CBlock& block = *pblock; @@ -2972,7 +2973,7 @@ bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTr } } - m_chain.SetTip(pindexDelete->pprev); + m_chain.SetTip(*pindexDelete->pprev); UpdateTip(pindexDelete->pprev); // Let wallets know transactions went from 1-confirmed to @@ -3103,7 +3104,7 @@ bool CChainState::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew disconnectpool.removeForBlock(blockConnecting.vtx); } // Update m_chain & related variables. - m_chain.SetTip(pindexNew); + m_chain.SetTip(*pindexNew); UpdateTip(pindexNew); int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; @@ -4448,7 +4449,7 @@ bool CChainState::LoadChainTip() if (!pindex) { return false; } - m_chain.SetTip(pindex); + m_chain.SetTip(*pindex); PruneBlockIndexCandidates(); tip = m_chain.Tip(); @@ -5827,7 +5828,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( return false; } - snapshot_chainstate.m_chain.SetTip(snapshot_start_block); + snapshot_chainstate.m_chain.SetTip(*snapshot_start_block); // The remainder of this function requires modifying data protected by cs_main. LOCK(::cs_main); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 71ed77e58de46..3696a786e0fdc 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1067,12 +1067,13 @@ std::optional CreateTransaction( } std::optional txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, error2, tmp_cc, fee_calc_out, sign, nExtraPayloadSize); + // if fee of this alternative one is within the range of the max fee, we use this one + const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped->fee + wallet.m_max_aps_fee) : false}; + TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, txr_grouped.has_value(), + txr_grouped.has_value() ? txr_grouped->fee : 0, txr_grouped.has_value() ? txr_grouped->change_pos : 0); if (txr_grouped) { - // if fee of this alternative one is within the range of the max fee, we use this one - const bool use_aps = txr_grouped->fee <= txr_ungrouped->fee + wallet.m_max_aps_fee; wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", txr_ungrouped->fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped"); - TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, true, txr_grouped->fee, txr_grouped->change_pos); if (use_aps) return txr_grouped; } } diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index 39f9346f3ef90..e76064573ac92 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -15,13 +15,13 @@ namespace wallet { -static void AddCoin(const CAmount& value, int n_input, int n_input_bytes, int locktime, std::vector& coins) +static void AddCoin(const CAmount& value, int n_input, int n_input_bytes, int locktime, std::vector& coins, CFeeRate fee_rate) { CMutableTransaction tx; tx.vout.resize(n_input + 1); tx.vout[n_input].nValue = value; tx.nLockTime = locktime; // all transactions get different hashes - coins.emplace_back(COutPoint(tx.GetHash(), n_input), tx.vout.at(n_input), /*depth=*/0, n_input_bytes, /*spendable=*/true, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/true); + coins.emplace_back(COutPoint(tx.GetHash(), n_input), tx.vout.at(n_input), /*depth=*/0, n_input_bytes, /*spendable=*/true, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/true, fee_rate); } // Randomly distribute coins to instances of OutputGroup @@ -71,7 +71,7 @@ FUZZ_TARGET(coinselection) if (total_balance + amount >= MAX_MONEY) { break; } - AddCoin(amount, n_input, n_input_bytes, ++next_locktime, utxo_pool); + AddCoin(amount, n_input, n_input_bytes, ++next_locktime, utxo_pool, coin_params.m_effective_feerate); total_balance += amount; } diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 5808dfbffef4f..f402a626a9813 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1235,13 +1235,36 @@ std::unique_ptr CreateDummyWalletDatabase() } /** Return object for accessing temporary in-memory database. */ -std::unique_ptr CreateMockWalletDatabase() +std::unique_ptr CreateMockWalletDatabase(DatabaseOptions& options) { - DatabaseOptions options; + + std::optional format; + if (options.require_format) format = options.require_format; + if (!format) { +#ifdef USE_BDB + format = DatabaseFormat::BERKELEY; +#endif #ifdef USE_SQLITE - return std::make_unique("", "", options, true); -#elif defined(USE_BDB) + format = DatabaseFormat::SQLITE; +#endif + } + + if (format == DatabaseFormat::SQLITE) { +#ifdef USE_SQLITE + return std::make_unique(":memory:", "", options, true); +#endif + assert(false); + } + +#ifdef USE_BDB return std::make_unique(std::make_shared(), "", options); #endif + assert(false); +} + +std::unique_ptr CreateMockWalletDatabase() +{ + DatabaseOptions options; + return CreateMockWalletDatabase(options); } } // namespace wallet diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 43cd4885acc5c..574cea38061c4 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -267,6 +267,7 @@ bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, st std::unique_ptr CreateDummyWalletDatabase(); /** Return object for accessing temporary in-memory database. */ +std::unique_ptr CreateMockWalletDatabase(DatabaseOptions& options); std::unique_ptr CreateMockWalletDatabase(); } // namespace wallet diff --git a/test/functional/feature_index_prune.py b/test/functional/feature_index_prune.py index a699990c45e24..1f39a0dd327b7 100755 --- a/test/functional/feature_index_prune.py +++ b/test/functional/feature_index_prune.py @@ -78,7 +78,7 @@ def run_test(self): pruneheight_new = node.pruneblockchain(400) # the prune heights used here and below are magic numbers that are determined by the # thresholds at which block files wrap, so they depend on disk serialization and default block file size. - assert_equal(pruneheight_new, 368) + assert_equal(pruneheight_new, 367) self.log.info("check if we can access the tips blockfilter and coinstats when we have pruned some blocks") tip = self.nodes[0].getbestblockhash() @@ -113,7 +113,7 @@ def run_test(self): self.log.info("prune exactly up to the indices best blocks while the indices are disabled") for i in range(3): pruneheight_2 = self.nodes[i].pruneblockchain(1000) - assert_equal(pruneheight_2, 736) + assert_equal(pruneheight_2, 735) # Restart the nodes again with the indices activated self.restart_node(i, extra_args=self.extra_args[i], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) @@ -148,7 +148,7 @@ def run_test(self): for node in self.nodes[:2]: with node.assert_debug_log(['limited pruning to height 2489']): pruneheight_new = node.pruneblockchain(2500) - assert_equal(pruneheight_new, 2208) + assert_equal(pruneheight_new, 2207) self.log.info("ensure that prune locks don't prevent indices from failing in a reorg scenario") with self.nodes[0].assert_debug_log(['basic block filter index prune lock moved back to 2480']): diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index 8c6e1e3124370..a58ea8c514d9d 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -313,7 +313,7 @@ def height(index): def prune(index): ret = node.pruneblockchain(height=height(index)) - assert_equal(ret, node.getblockchaininfo()['pruneheight']) + assert_equal(ret + 1, node.getblockchaininfo()['pruneheight']) def has_block(index): return os.path.isfile(os.path.join(self.nodes[node_number].datadir, self.chain, "blocks", f"blk{index:05}.dat")) diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 2e36aa2174843..5f63718c4bba2 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -10,6 +10,7 @@ from io import BytesIO import json from struct import pack, unpack +import typing import urllib.parse @@ -57,14 +58,21 @@ def set_test_params(self): args.append("-whitelist=noban@127.0.0.1") self.supports_cli = False - def test_rest_request(self, uri, http_method='GET', req_type=ReqType.JSON, body='', status=200, ret_type=RetType.JSON): + def test_rest_request( + self, + uri: str, + http_method: str = 'GET', + req_type: ReqType = ReqType.JSON, + body: str = '', + status: int = 200, + ret_type: RetType = RetType.JSON, + query_params: typing.Dict[str, typing.Any] = None, + ) -> typing.Union[http.client.HTTPResponse, bytes, str, None]: rest_uri = '/rest' + uri - if req_type == ReqType.JSON: - rest_uri += '.json' - elif req_type == ReqType.BIN: - rest_uri += '.bin' - elif req_type == ReqType.HEX: - rest_uri += '.hex' + if req_type in ReqType: + rest_uri += f'.{req_type.name.lower()}' + if query_params: + rest_uri += f'?{urllib.parse.urlencode(query_params)}' conn = http.client.HTTPConnection(self.url.hostname, self.url.port) self.log.debug(f'{http_method} {rest_uri} {body}') @@ -83,6 +91,8 @@ def test_rest_request(self, uri, http_method='GET', req_type=ReqType.JSON, body= elif ret_type == RetType.JSON: return json.loads(resp.read().decode('utf-8'), parse_float=Decimal) + return None + def run_test(self): self.url = urllib.parse.urlparse(self.nodes[0].url) self.wallet = MiniWallet(self.nodes[0]) @@ -209,16 +219,16 @@ def run_test(self): self.generate(self.nodes[0], 1) # generate block to not affect upcoming tests - self.log.info("Test the /block, /blockhashbyheight and /headers URIs") + self.log.info("Test the /block, /blockhashbyheight, /headers, and /blockfilterheaders URIs") bb_hash = self.nodes[0].getbestblockhash() # Check result if block does not exists - assert_equal(self.test_rest_request(f"/headers/1/{UNKNOWN_PARAM}"), []) + assert_equal(self.test_rest_request(f"/headers/{UNKNOWN_PARAM}", query_params={"count": 1}), []) self.test_rest_request(f"/block/{UNKNOWN_PARAM}", status=404, ret_type=RetType.OBJ) # Check result if block is not in the active chain self.nodes[0].invalidateblock(bb_hash) - assert_equal(self.test_rest_request(f'/headers/1/{bb_hash}'), []) + assert_equal(self.test_rest_request(f'/headers/{bb_hash}', query_params={'count': 1}), []) self.test_rest_request(f'/block/{bb_hash}') self.nodes[0].reconsiderblock(bb_hash) @@ -228,7 +238,7 @@ def run_test(self): response_bytes = response.read() # Compare with block header - response_header = self.test_rest_request(f"/headers/1/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ) + response_header = self.test_rest_request(f"/headers/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ, query_params={"count": 1}) assert_equal(int(response_header.getheader('content-length')), BLOCK_HEADER_SIZE) response_header_bytes = response_header.read() assert_equal(response_bytes[:BLOCK_HEADER_SIZE], response_header_bytes) @@ -240,7 +250,7 @@ def run_test(self): assert_equal(response_bytes.hex().encode(), response_hex_bytes) # Compare with hex block header - response_header_hex = self.test_rest_request(f"/headers/1/{bb_hash}", req_type=ReqType.HEX, ret_type=RetType.OBJ) + response_header_hex = self.test_rest_request(f"/headers/{bb_hash}", req_type=ReqType.HEX, ret_type=RetType.OBJ, query_params={"count": 1}) assert_greater_than(int(response_header_hex.getheader('content-length')), BLOCK_HEADER_SIZE*2) response_header_hex_bytes = response_header_hex.read(BLOCK_HEADER_SIZE*2) assert_equal(response_bytes[:BLOCK_HEADER_SIZE].hex().encode(), response_header_hex_bytes) @@ -267,7 +277,7 @@ def run_test(self): self.test_rest_request("/blockhashbyheight/", ret_type=RetType.OBJ, status=400) # Compare with json block header - json_obj = self.test_rest_request(f"/headers/1/{bb_hash}") + json_obj = self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 1}) assert_equal(len(json_obj), 1) # ensure that there is one header in the json response assert_equal(json_obj[0]['hash'], bb_hash) # request/response hash should be the same @@ -278,9 +288,14 @@ def run_test(self): # See if we can get 5 headers in one response self.generate(self.nodes[1], 5) - json_obj = self.test_rest_request(f"/headers/5/{bb_hash}") + expected_filter = { + 'txindex': {'synced': True, 'best_block_height': 208}, + 'basic block filter index': {'synced': True, 'best_block_height': 208}, + } + self.wait_until(lambda: self.nodes[0].getindexinfo() == expected_filter) + json_obj = self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 5}) assert_equal(len(json_obj), 5) # now we should have 5 header objects - json_obj = self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}") + json_obj = self.test_rest_request(f"/blockfilterheaders/basic/{bb_hash}", query_params={"count": 5}) first_filter_header = json_obj[0] assert_equal(len(json_obj), 5) # now we should have 5 filter header objects json_obj = self.test_rest_request(f"/blockfilter/basic/{bb_hash}") @@ -290,11 +305,17 @@ def run_test(self): assert_equal(first_filter_header, rpc_blockfilter['header']) assert_equal(json_obj['filter'], rpc_blockfilter['filter']) + # Test blockfilterheaders with an invalid hash and filtertype + resp = self.test_rest_request(f"/blockfilterheaders/{INVALID_PARAM}/{bb_hash}", ret_type=RetType.OBJ, status=400) + assert_equal(resp.read().decode('utf-8').rstrip(), f"Unknown filtertype {INVALID_PARAM}") + resp = self.test_rest_request(f"/blockfilterheaders/basic/{INVALID_PARAM}", ret_type=RetType.OBJ, status=400) + assert_equal(resp.read().decode('utf-8').rstrip(), f"Invalid hash: {INVALID_PARAM}") + # Test number parsing for num in ['5a', '-5', '0', '2001', '99999999999999999999999999999999999']: assert_equal( bytes(f'Header count is invalid or out of acceptable range (1-2000): {num}\r\n', 'ascii'), - self.test_rest_request(f"/headers/{num}/{bb_hash}", ret_type=RetType.BYTES, status=400), + self.test_rest_request(f"/headers/{bb_hash}", ret_type=RetType.BYTES, status=400, query_params={"count": num}), ) self.log.info("Test tx inclusion in the /mempool and /block URIs") @@ -362,6 +383,11 @@ def run_test(self): blockchain_info = self.nodes[0].getblockchaininfo() assert_equal(blockchain_info, json_obj) + # Test compatibility of deprecated and newer endpoints + self.log.info("Test compatibility of deprecated and newer endpoints") + assert_equal(self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 1}), self.test_rest_request(f"/headers/1/{bb_hash}")) + assert_equal(self.test_rest_request(f"/blockfilterheaders/basic/{bb_hash}", query_params={"count": 1}), self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}")) + if __name__ == '__main__': RESTTest().main() diff --git a/test/functional/mempool_datacarrier.py b/test/functional/mempool_datacarrier.py new file mode 100755 index 0000000000000..13df564a378f9 --- /dev/null +++ b/test/functional/mempool_datacarrier.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020-2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test datacarrier functionality""" +from test_framework.messages import ( + CTxOut, + MAX_OP_RETURN_RELAY, +) +from test_framework.script import ( + CScript, + OP_RETURN, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.test_node import TestNode +from test_framework.util import ( + assert_raises_rpc_error, + random_bytes, +) +from test_framework.wallet import MiniWallet + + +class DataCarrierTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 3 + self.extra_args = [ + [], + ["-datacarrier=0"], + ["-datacarrier=1", f"-datacarriersize={MAX_OP_RETURN_RELAY - 1}"] + ] + + def test_null_data_transaction(self, node: TestNode, data: bytes, success: bool) -> None: + tx = self.wallet.create_self_transfer(fee_rate=0)["tx"] + tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN, data]))) + tx.vout[0].nValue -= tx.get_vsize() # simply pay 1sat/vbyte fee + + tx_hex = tx.serialize().hex() + + if success: + self.wallet.sendrawtransaction(from_node=node, tx_hex=tx_hex) + assert tx.rehash() in node.getrawmempool(True), f'{tx_hex} not in mempool' + else: + assert_raises_rpc_error(-26, "scriptpubkey", self.wallet.sendrawtransaction, from_node=node, tx_hex=tx_hex) + + def run_test(self): + self.wallet = MiniWallet(self.nodes[0]) + self.wallet.rescan_utxos() + + # By default, only 80 bytes are used for data (+1 for OP_RETURN, +2 for the pushdata opcodes). + default_size_data = random_bytes(MAX_OP_RETURN_RELAY - 3) + too_long_data = random_bytes(MAX_OP_RETURN_RELAY - 2) + small_data = random_bytes(MAX_OP_RETURN_RELAY - 4) + + self.log.info("Testing null data transaction with default -datacarrier and -datacarriersize values.") + self.test_null_data_transaction(node=self.nodes[0], data=default_size_data, success=True) + + self.log.info("Testing a null data transaction larger than allowed by the default -datacarriersize value.") + self.test_null_data_transaction(node=self.nodes[0], data=too_long_data, success=False) + + self.log.info("Testing a null data transaction with -datacarrier=false.") + self.test_null_data_transaction(node=self.nodes[1], data=default_size_data, success=False) + + self.log.info("Testing a null data transaction with a size larger than accepted by -datacarriersize.") + self.test_null_data_transaction(node=self.nodes[2], data=default_size_data, success=False) + + self.log.info("Testing a null data transaction with a size smaller than accepted by -datacarriersize.") + self.test_null_data_transaction(node=self.nodes[2], data=small_data, success=True) + + +if __name__ == '__main__': + DataCarrierTest().main() diff --git a/test/functional/mempool_package_onemore.py b/test/functional/mempool_package_onemore.py index 15ebf39a15ce9..9abb1c6dcf126 100755 --- a/test/functional/mempool_package_onemore.py +++ b/test/functional/mempool_package_onemore.py @@ -7,6 +7,9 @@ size. """ +from test_framework.messages import ( + DEFAULT_ANCESTOR_LIMIT, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -15,10 +18,6 @@ from test_framework.wallet import MiniWallet -MAX_ANCESTORS = 25 -MAX_DESCENDANTS = 25 - - class MempoolPackagesTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 @@ -34,19 +33,19 @@ def run_test(self): self.wallet = MiniWallet(self.nodes[0]) self.wallet.rescan_utxos() - # MAX_ANCESTORS transactions off a confirmed tx should be fine + # DEFAULT_ANCESTOR_LIMIT transactions off a confirmed tx should be fine chain = [] utxo = self.wallet.get_utxo() for _ in range(4): utxo, utxo2 = self.chain_tx([utxo], num_outputs=2) chain.append(utxo2) - for _ in range(MAX_ANCESTORS - 4): + for _ in range(DEFAULT_ANCESTOR_LIMIT - 4): utxo, = self.chain_tx([utxo]) chain.append(utxo) second_chain, = self.chain_tx([self.wallet.get_utxo()]) - # Check mempool has MAX_ANCESTORS + 1 transactions in it - assert_equal(len(self.nodes[0].getrawmempool()), MAX_ANCESTORS + 1) + # Check mempool has DEFAULT_ANCESTOR_LIMIT + 1 transactions in it + assert_equal(len(self.nodes[0].getrawmempool()), DEFAULT_ANCESTOR_LIMIT + 1) # Adding one more transaction on to the chain should fail. assert_raises_rpc_error(-26, "too-long-mempool-chain, too many unconfirmed ancestors [limit: 25]", self.chain_tx, [utxo]) @@ -63,7 +62,7 @@ def run_test(self): self.chain_tx([second_chain]) # Finally, check that we added two transactions - assert_equal(len(self.nodes[0].getrawmempool()), MAX_ANCESTORS + 3) + assert_equal(len(self.nodes[0].getrawmempool()), DEFAULT_ANCESTOR_LIMIT + 3) if __name__ == '__main__': diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index cbb91db21be5b..0a9d227aadeb5 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -7,7 +7,11 @@ from decimal import Decimal from test_framework.blocktools import COINBASE_MATURITY -from test_framework.messages import COIN +from test_framework.messages import ( + COIN, + DEFAULT_ANCESTOR_LIMIT, + DEFAULT_DESCENDANT_LIMIT, +) from test_framework.p2p import P2PTxInvStore from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -16,13 +20,12 @@ chain_transaction, ) -# default limits -MAX_ANCESTORS = 25 -MAX_DESCENDANTS = 25 + # custom limits for node1 -MAX_ANCESTORS_CUSTOM = 5 -MAX_DESCENDANTS_CUSTOM = 10 -assert MAX_DESCENDANTS_CUSTOM >= MAX_ANCESTORS_CUSTOM +CUSTOM_ANCESTOR_LIMIT = 5 +CUSTOM_DESCENDANT_LIMIT = 10 +assert CUSTOM_DESCENDANT_LIMIT >= CUSTOM_ANCESTOR_LIMIT + class MempoolPackagesTest(BitcoinTestFramework): def set_test_params(self): @@ -34,8 +37,8 @@ def set_test_params(self): ], [ "-maxorphantxsize=1000", - "-limitancestorcount={}".format(MAX_ANCESTORS_CUSTOM), - "-limitdescendantcount={}".format(MAX_DESCENDANTS_CUSTOM), + "-limitancestorcount={}".format(CUSTOM_ANCESTOR_LIMIT), + "-limitdescendantcount={}".format(CUSTOM_DESCENDANT_LIMIT), ], ] @@ -55,11 +58,11 @@ def run_test(self): assert 'ancestorfees' not in utxo[0] fee = Decimal("0.0001") - # MAX_ANCESTORS transactions off a confirmed tx should be fine + # DEFAULT_ANCESTOR_LIMIT transactions off a confirmed tx should be fine chain = [] ancestor_vsize = 0 ancestor_fees = Decimal(0) - for i in range(MAX_ANCESTORS): + for i in range(DEFAULT_ANCESTOR_LIMIT): (txid, sent_value) = chain_transaction(self.nodes[0], [txid], [0], value, fee, 1) value = sent_value chain.append(txid) @@ -77,16 +80,16 @@ def run_test(self): # Otherwise, getrawmempool may be inconsistent with getmempoolentry if unbroadcast changes in between peer_inv_store.wait_for_broadcast(chain) - # Check mempool has MAX_ANCESTORS transactions in it, and descendant and ancestor + # Check mempool has DEFAULT_ANCESTOR_LIMIT transactions in it, and descendant and ancestor # count and fees should look correct mempool = self.nodes[0].getrawmempool(True) - assert_equal(len(mempool), MAX_ANCESTORS) + assert_equal(len(mempool), DEFAULT_ANCESTOR_LIMIT) descendant_count = 1 descendant_fees = 0 descendant_vsize = 0 assert_equal(ancestor_vsize, sum([mempool[tx]['vsize'] for tx in mempool])) - ancestor_count = MAX_ANCESTORS + ancestor_count = DEFAULT_ANCESTOR_LIMIT assert_equal(ancestor_fees, sum([mempool[tx]['fees']['base'] for tx in mempool])) descendants = [] @@ -203,9 +206,9 @@ def run_test(self): # Check that node1's mempool is as expected (-> custom ancestor limit) mempool0 = self.nodes[0].getrawmempool(False) mempool1 = self.nodes[1].getrawmempool(False) - assert_equal(len(mempool1), MAX_ANCESTORS_CUSTOM) + assert_equal(len(mempool1), CUSTOM_ANCESTOR_LIMIT) assert set(mempool1).issubset(set(mempool0)) - for tx in chain[:MAX_ANCESTORS_CUSTOM]: + for tx in chain[:CUSTOM_ANCESTOR_LIMIT]: assert tx in mempool1 entry0 = self.nodes[0].getmempoolentry(tx) entry1 = self.nodes[1].getmempoolentry(tx) @@ -230,7 +233,7 @@ def run_test(self): # Sign and send up to MAX_DESCENDANT transactions chained off the parent tx chain = [] # save sent txs for the purpose of checking node1's mempool later (see below) - for _ in range(MAX_DESCENDANTS - 1): + for _ in range(DEFAULT_DESCENDANT_LIMIT - 1): utxo = transaction_package.pop(0) (txid, sent_value) = chain_transaction(self.nodes[0], [utxo['txid']], [utxo['vout']], utxo['amount'], fee, 10) chain.append(txid) @@ -240,7 +243,7 @@ def run_test(self): transaction_package.append({'txid': txid, 'vout': j, 'amount': sent_value}) mempool = self.nodes[0].getrawmempool(True) - assert_equal(mempool[parent_transaction]['descendantcount'], MAX_DESCENDANTS) + assert_equal(mempool[parent_transaction]['descendantcount'], DEFAULT_DESCENDANT_LIMIT) assert_equal(sorted(mempool[parent_transaction]['spentby']), sorted(tx_children)) for child in tx_children: @@ -255,14 +258,14 @@ def run_test(self): # - parent tx for descendant test # - txs chained off parent tx (-> custom descendant limit) self.wait_until(lambda: len(self.nodes[1].getrawmempool()) == - MAX_ANCESTORS_CUSTOM + 1 + MAX_DESCENDANTS_CUSTOM, timeout=10) + CUSTOM_ANCESTOR_LIMIT + 1 + CUSTOM_DESCENDANT_LIMIT, timeout=10) mempool0 = self.nodes[0].getrawmempool(False) mempool1 = self.nodes[1].getrawmempool(False) assert set(mempool1).issubset(set(mempool0)) assert parent_transaction in mempool1 - for tx in chain[:MAX_DESCENDANTS_CUSTOM]: + for tx in chain[:CUSTOM_DESCENDANT_LIMIT]: assert tx in mempool1 - for tx in chain[MAX_DESCENDANTS_CUSTOM:]: + for tx in chain[CUSTOM_DESCENDANT_LIMIT:]: assert tx not in mempool1 for tx in mempool1: entry0 = self.nodes[0].getmempoolentry(tx) diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index a4651e1f05450..6214768034a88 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -49,9 +49,13 @@ def run_test(self): out['txoutset_hash'], 'b2d7429106c96f5ab831843d5c96ba131ca8793111d0a0e30e7d7d8b4841e6cc') assert_equal(out['nchaintx'], 101) - # Specifying a path to an existing file will fail. + # Specifying a path to an existing or invalid file will fail. assert_raises_rpc_error( -8, '{} already exists'.format(FILENAME), node.dumptxoutset, FILENAME) + invalid_path = str(Path(node.datadir) / "invalid" / "path") + assert_raises_rpc_error( + -8, "Couldn't open file {}.incomplete for writing".format(invalid_path), node.dumptxoutset, invalid_path) + if __name__ == '__main__': DumptxoutsetTest().main() diff --git a/test/functional/test_framework/address.py b/test/functional/test_framework/address.py index 31813e6cd7394..1b217b04f4edd 100644 --- a/test/functional/test_framework/address.py +++ b/test/functional/test_framework/address.py @@ -12,7 +12,6 @@ import unittest from .script import hash160, hash256, CScript -from .util import assert_equal # Note unlike in bitcoin, this address isn't bech32 since we don't (at this time) support bech32. ADDRESS_BCRT1_UNSPENDABLE = 'yVg3NBUHNEhgDceqwVUjsZHreC5PBHnUo9' @@ -20,7 +19,7 @@ ADDRESS_BCRT1_P2SH_OP_TRUE = '8zJctvfrzGZ5s1zQ3kagwyW1DsPYSQ4V2P' -chars = '123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz' +b58chars = '123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz' def byte_to_base58(b, version): @@ -29,10 +28,10 @@ def byte_to_base58(b, version): b += hash256(b)[:4] # append checksum value = int.from_bytes(b, 'big') while value > 0: - result = chars[value % 58] + result + result = b58chars[value % 58] + result value //= 58 while b[0] == 0: - result = chars[0] + result + result = b58chars[0] + result b = b[1:] return result @@ -46,8 +45,8 @@ def base58_to_byte(s): n = 0 for c in s: n *= 58 - assert c in chars - digit = chars.index(c) + assert c in b58chars + digit = b58chars.index(c) n += digit h = '%x' % n if len(h) % 2: @@ -55,14 +54,14 @@ def base58_to_byte(s): res = n.to_bytes((n.bit_length() + 7) // 8, 'big') pad = 0 for c in s: - if c == chars[0]: + if c == b58chars[0]: pad += 1 else: break res = b'\x00' * pad + res - # Assert if the checksum is invalid - assert_equal(hash256(res[:-4])[:4], res[-4:]) + if hash256(res[:-4])[:4] != res[-4:]: + raise ValueError('Invalid Base58Check checksum') return res[1:-4], int(res[0]) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 5892c7c0d1ac2..d55010faeed44 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -65,6 +65,12 @@ FILTER_TYPE_BASIC = 0 +DEFAULT_ANCESTOR_LIMIT = 25 # default max number of in-mempool ancestors +DEFAULT_DESCENDANT_LIMIT = 25 # default max number of in-mempool descendants + +# Default setting for -datacarriersize. 80 bytes of data, +1 for OP_RETURN, +2 for the pushdata opcodes. +MAX_OP_RETURN_RELAY = 83 + MAGIC_BYTES = { "mainnet": b"\xbf\x0c\x6b\xbd", # mainnet "testnet3": b"\xce\xe2\xca\xff", # testnet3 diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 66b5bb231c7d6..194f44054c8f4 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -374,6 +374,7 @@ 'feature_unsupported_utxo_db.py', 'feature_logging.py', 'feature_anchors.py', + 'mempool_datacarrier.py', 'feature_coinstatsindex.py', 'wallet_orphanedreward.py', 'wallet_timelock.py', diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py index 47652d9a1e1f3..f8ba2cab73188 100755 --- a/test/functional/wallet_createwallet.py +++ b/test/functional/wallet_createwallet.py @@ -29,7 +29,7 @@ def run_test(self): self.log.info("Run createwallet with invalid parameters.") # Run createwallet with invalid parameters. This must not prevent a new wallet with the same name from being created with the correct parameters. assert_raises_rpc_error(-4, "Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.", - self.nodes[0].createwallet, wallet_name='w0', descriptors=True, disable_private_keys=True, passphrase="passphrase") + self.nodes[0].createwallet, wallet_name='w0', disable_private_keys=True, passphrase="passphrase") self.nodes[0].createwallet(wallet_name='w0') w0 = node.get_wallet_rpc('w0') diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py index 1fab78d9a1cfe..6da3f236c89be 100755 --- a/test/functional/wallet_listreceivedby.py +++ b/test/functional/wallet_listreceivedby.py @@ -27,9 +27,6 @@ def skip_test_if_missing_module(self): self.skip_if_no_cli() def run_test(self): - # Generate block to get out of IBD - self.generate(self.nodes[0], 1) - # save the number of coinbase reward addresses so far num_cb_reward_addresses = len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True)) @@ -178,7 +175,7 @@ def run_test(self): address = self.nodes[0].getnewaddress(label) reward = Decimal("464.28571429") - self.generatetoaddress(self.nodes[0], 1, address, sync_fun=self.no_op) + self.generatetoaddress(self.nodes[0], 1, address) hash = self.nodes[0].getbestblockhash() self.log.info("getreceivedbyaddress returns nothing with defaults") @@ -218,7 +215,7 @@ def run_test(self): {"label": label, "amount": reward}) self.log.info("Generate 100 more blocks") - self.generate(self.nodes[0], COINBASE_MATURITY, sync_fun=self.no_op) + self.generate(self.nodes[0], COINBASE_MATURITY) self.log.info("getreceivedbyaddress returns reward with defaults") balance = self.nodes[0].getreceivedbyaddress(address)