diff options
73 files changed, 1032 insertions, 594 deletions
diff --git a/.cirrus.yml b/.cirrus.yml index f15afca005..703d28557e 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -93,7 +93,7 @@ task: QTBASEDIR: 'C:\Qt5.12.11_x64_static_vs2019_160900' x64_NATIVE_TOOLS: '"C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Auxiliary\Build\vcvars64.bat"' IgnoreWarnIntDirInTempDetected: 'true' - EXCLUDE_TESTS: 'feature_addrman.py,feature_bip68_sequence.py,feature_fee_estimation.py,mining_prioritisetransaction.py,p2p_getaddr_caching.py,p2p_invalid_locator.py,p2p_invalid_tx.py,rpc_misc.py,rpc_net.py,wallet_avoidreuse.py,wallet_descriptor.py,wallet_groups.py,wallet_keypool.py' + EXCLUDE_TESTS: 'feature_addrman.py,feature_asmap.py' merge_script: - git config --global user.email "ci@ci.ci" - git config --global user.name "ci" @@ -147,6 +147,10 @@ task: - python test\util\test_runner.py - python test\util\rpcauth-test.py functional_tests_script: + # Increase the dynamic port range to the maximum allowed value to mitigate "OSError: [WinError 10048] Only one usage of each socket address (protocol/network address/port) is normally permitted". + # See: https://docs.microsoft.com/en-us/biztalk/technical-guides/settings-that-can-be-modified-to-improve-network-performance + - netsh int ipv4 set dynamicport tcp start=1025 num=64511 + - netsh int ipv6 set dynamicport tcp start=1025 num=64511 # TODO enable '--extended' and drop '--exclude'. - python test\functional\test_runner.py --nocleanup --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 --exclude %EXCLUDE_TESTS% --failfast diff --git a/configure.ac b/configure.ac index 0dc480e6c1..e30159f91b 100644 --- a/configure.ac +++ b/configure.ac @@ -568,13 +568,17 @@ AX_CHECK_COMPILE_FLAG([-march=armv8-a+crc+crypto],[[ARM_CRC_CXXFLAGS="-march=arm TEMP_CXXFLAGS="$CXXFLAGS" CXXFLAGS="$CXXFLAGS $ARM_CRC_CXXFLAGS" -AC_MSG_CHECKING(for ARM CRC32 intrinsics) +AC_MSG_CHECKING(for AArch64 CRC32 intrinsics) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ #include <arm_acle.h> #include <arm_neon.h> ]],[[ +#ifdef __aarch64__ __crc32cb(0, 0); __crc32ch(0, 0); __crc32cw(0, 0); __crc32cd(0, 0); vmull_p64(0, 0); +#else +#error "crc32c library does not support hardware acceleration on 32-bit ARM" +#endif ]])], [ AC_MSG_RESULT(yes); enable_arm_crc=yes; ], [ AC_MSG_RESULT(no)] diff --git a/doc/release-notes-23065.md b/doc/release-notes-23065.md new file mode 100644 index 0000000000..6ec002b2df --- /dev/null +++ b/doc/release-notes-23065.md @@ -0,0 +1,15 @@ +Notable changes +=============== + +Updated RPCs +------------ + +- `lockunspent` now optionally takes a third parameter, `persistent`, which +causes the lock to be written persistently to the wallet database. This +allows UTXOs to remain locked even after node restarts or crashes. + +GUI changes +----------- + +- UTXOs which are locked via the GUI are now stored persistently in the +wallet database, so are not lost on node shutdown or crash.
\ No newline at end of file diff --git a/doc/release-notes.md b/doc/release-notes.md index a57ddb85ca..c6b43379d3 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -119,10 +119,8 @@ Tests ----- - For the `regtest` network the activation heights of several softforks were - changed. - * BIP 34 (blockheight in coinbase) from 500 to 2 (#16333) - * BIP 66 (DERSIG) from 1251 to 102 (#22632) - * BIP 65 (CLTV) from 1351 to 111 (#21862) + set to block height 1. They can be changed by the runtime setting + `-testactivationheight=name@height`. (#22818) Credits ======= diff --git a/src/Makefile.test_util.include b/src/Makefile.test_util.include index 85e50ebf70..0a3b99e7d2 100644 --- a/src/Makefile.test_util.include +++ b/src/Makefile.test_util.include @@ -9,6 +9,7 @@ EXTRA_LIBRARIES += \ TEST_UTIL_H = \ test/util/blockfilter.h \ + test/util/chainstate.h \ test/util/logging.h \ test/util/mining.h \ test/util/net.h \ diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index e5dd571a4c..bebf86a09d 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -103,41 +103,33 @@ static void AddrManGetAddr(benchmark::Bench& bench) }); } -static void AddrManGood(benchmark::Bench& bench) +static void AddrManAddThenGood(benchmark::Bench& bench) { - /* Create many CAddrMan objects - one to be modified at each loop iteration. - * This is necessary because the CAddrMan::Good() method modifies the - * object, affecting the timing of subsequent calls to the same method and - * we want to do the same amount of work in every loop iteration. */ - - bench.epochs(5).epochIterations(1); - const uint64_t addrman_count{bench.epochs() * bench.epochIterations()}; - Assert(addrman_count == 5U); - - std::vector<std::unique_ptr<CAddrMan>> addrmans(addrman_count); - for (size_t i{0}; i < addrman_count; ++i) { - addrmans[i] = std::make_unique<CAddrMan>(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0); - FillAddrMan(*addrmans[i]); - } - auto markSomeAsGood = [](CAddrMan& addrman) { for (size_t source_i = 0; source_i < NUM_SOURCES; ++source_i) { for (size_t addr_i = 0; addr_i < NUM_ADDRESSES_PER_SOURCE; ++addr_i) { - if (addr_i % 32 == 0) { - addrman.Good(g_addresses[source_i][addr_i]); - } + addrman.Good(g_addresses[source_i][addr_i]); } } }; - uint64_t i = 0; + CreateAddresses(); + bench.run([&] { - markSomeAsGood(*addrmans.at(i)); - ++i; + // To make the benchmark independent of the number of evaluations, we always prepare a new addrman. + // This is necessary because CAddrMan::Good() method modifies the object, affecting the timing of subsequent calls + // to the same method and we want to do the same amount of work in every loop iteration. + // + // This has some overhead (exactly the result of AddrManAdd benchmark), but that overhead is constant so improvements in + // CAddrMan::Good() will still be noticeable. + CAddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0); + AddAddressesToAddrMan(addrman); + + markSomeAsGood(addrman); }); } BENCHMARK(AddrManAdd); BENCHMARK(AddrManSelect); BENCHMARK(AddrManGetAddr); -BENCHMARK(AddrManGood); +BENCHMARK(AddrManAddThenGood); diff --git a/src/bench/bech32.cpp b/src/bench/bech32.cpp index 8e10862a37..bc3685818e 100644 --- a/src/bench/bech32.cpp +++ b/src/bench/bech32.cpp @@ -3,7 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <bench/bench.h> -#include <bench/nanobench.h> #include <bech32.h> #include <util/strencodings.h> diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp index 012057e792..030bc43396 100644 --- a/src/bench/bench.cpp +++ b/src/bench/bench.cpp @@ -4,11 +4,18 @@ #include <bench/bench.h> -#include <chainparams.h> #include <test/util/setup_common.h> -#include <validation.h> +#include <chrono> +#include <fstream> +#include <functional> +#include <iostream> +#include <map> #include <regex> +#include <string> +#include <vector> + +using namespace std::chrono_literals; const std::function<void(const std::string&)> G_TEST_LOG_FUN{}; @@ -61,6 +68,12 @@ void benchmark::BenchRunner::RunAll(const Args& args) Bench bench; bench.name(p.first); + if (args.min_time > 0ms) { + // convert to nanos before dividing to reduce rounding errors + std::chrono::nanoseconds min_time_ns = args.min_time; + bench.minEpochTime(min_time_ns / bench.epochs()); + } + if (args.asymptote.empty()) { p.second(bench); } else { diff --git a/src/bench/bench.h b/src/bench/bench.h index c4fcd80e33..b0e4006ee3 100644 --- a/src/bench/bench.h +++ b/src/bench/bench.h @@ -41,11 +41,12 @@ using ankerl::nanobench::Bench; typedef std::function<void(Bench&)> BenchFunction; struct Args { - std::string regex_filter; bool is_list_only; + std::chrono::milliseconds min_time; std::vector<double> asymptote; std::string output_csv; std::string output_json; + std::string regex_filter; }; class BenchRunner diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index aab777cac1..33a9ea80ed 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -4,21 +4,28 @@ #include <bench/bench.h> +#include <clientversion.h> #include <crypto/sha256.h> #include <util/strencodings.h> #include <util/system.h> -#include <memory> +#include <chrono> +#include <cstdint> +#include <iostream> +#include <sstream> +#include <vector> static const char* DEFAULT_BENCH_FILTER = ".*"; +static constexpr int64_t DEFAULT_MIN_TIME_MS{10}; static void SetupBenchArgs(ArgsManager& argsman) { SetupHelpOptions(argsman); - argsman.AddArg("-asymptote=n1,n2,n3,...", "Test asymptotic growth of the runtime of an algorithm, if supported by the benchmark", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-asymptote=<n1,n2,n3,...>", "Test asymptotic growth of the runtime of an algorithm, if supported by the benchmark", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-filter=<regex>", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-list", "List benchmarks without executing them", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-list", "List benchmarks without executing them", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); + argsman.AddArg("-min_time=<milliseconds>", strprintf("Minimum runtime per benchmark, in milliseconds (default: %d)", DEFAULT_MIN_TIME_MS), ArgsManager::ALLOW_INT, OptionsCategory::OPTIONS); argsman.AddArg("-output_csv=<output.csv>", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-output_json=<output.json>", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); } @@ -48,17 +55,62 @@ int main(int argc, char** argv) } if (HelpRequested(argsman)) { - std::cout << argsman.GetHelpMessage(); + std::cout << "Usage: bench_bitcoin [options]\n" + "\n" + << argsman.GetHelpMessage() + << "Description:\n" + "\n" + " bench_bitcoin executes microbenchmarks. The quality of the benchmark results\n" + " highly depend on the stability of the machine. It can sometimes be difficult\n" + " to get stable, repeatable results, so here are a few tips:\n" + "\n" + " * Use pyperf [1] to disable frequency scaling, turbo boost etc. For best\n" + " results, use CPU pinning and CPU isolation (see [2]).\n" + "\n" + " * Each call of run() should do exactly the same work. E.g. inserting into\n" + " a std::vector doesn't do that as it will reallocate on certain calls. Make\n" + " sure each run has exactly the same preconditions.\n" + "\n" + " * If results are still not reliable, increase runtime with e.g.\n" + " -min_time=5000 to let a benchmark run for at least 5 seconds.\n" + "\n" + " * bench_bitcoin uses nanobench [3] for which there is extensive\n" + " documentation available online.\n" + "\n" + "Environment Variables:\n" + "\n" + " To attach a profiler you can run a benchmark in endless mode. This can be\n" + " done with the environment variable NANOBENCH_ENDLESS. E.g. like so:\n" + "\n" + " NANOBENCH_ENDLESS=MuHash ./bench_bitcoin -filter=MuHash\n" + "\n" + " In rare cases it can be useful to suppress stability warnings. This can be\n" + " done with the environment variable NANOBENCH_SUPPRESS_WARNINGS, e.g:\n" + "\n" + " NANOBENCH_SUPPRESS_WARNINGS=1 ./bench_bitcoin\n" + "\n" + "Notes:\n" + "\n" + " 1. pyperf\n" + " https://github.com/psf/pyperf\n" + "\n" + " 2. CPU pinning & isolation\n" + " https://pyperf.readthedocs.io/en/latest/system.html\n" + "\n" + " 3. nanobench\n" + " https://github.com/martinus/nanobench\n" + "\n"; return EXIT_SUCCESS; } benchmark::Args args; - args.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER); - args.is_list_only = argsman.GetBoolArg("-list", false); args.asymptote = parseAsymptote(argsman.GetArg("-asymptote", "")); + args.is_list_only = argsman.GetBoolArg("-list", false); + args.min_time = std::chrono::milliseconds(argsman.GetArg("-min_time", DEFAULT_MIN_TIME_MS)); args.output_csv = argsman.GetArg("-output_csv", ""); args.output_json = argsman.GetArg("-output_json", ""); + args.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER); benchmark::BenchRunner::RunAll(args); diff --git a/src/bench/crypto_hash.cpp b/src/bench/crypto_hash.cpp index 30fe11be6b..d36e504bfc 100644 --- a/src/bench/crypto_hash.cpp +++ b/src/bench/crypto_hash.cpp @@ -110,9 +110,9 @@ static void MuHash(benchmark::Bench& bench) { MuHash3072 acc; unsigned char key[32] = {0}; - int i = 0; + uint32_t i = 0; bench.run([&] { - key[0] = ++i; + key[0] = ++i & 0xFF; acc *= MuHash3072(key); }); } @@ -134,10 +134,6 @@ static void MuHashDiv(benchmark::Bench& bench) FastRandomContext rng(true); MuHash3072 muhash{rng.randbytes(32)}; - for (size_t i = 0; i < bench.epochIterations(); ++i) { - acc *= muhash; - } - bench.run([&] { acc /= muhash; }); diff --git a/src/bench/nanobench.h b/src/bench/nanobench.h index 030d6ebf6a..27df08fb69 100644 --- a/src/bench/nanobench.h +++ b/src/bench/nanobench.h @@ -33,7 +33,7 @@ // see https://semver.org/ #define ANKERL_NANOBENCH_VERSION_MAJOR 4 // incompatible API changes #define ANKERL_NANOBENCH_VERSION_MINOR 3 // backwards-compatible changes -#define ANKERL_NANOBENCH_VERSION_PATCH 4 // backwards-compatible bug fixes +#define ANKERL_NANOBENCH_VERSION_PATCH 6 // backwards-compatible bug fixes /////////////////////////////////////////////////////////////////////////////////////////////////// // public facing api - as minimal as possible @@ -88,13 +88,15 @@ } while (0) #endif -#if defined(__linux__) && defined(PERF_EVENT_IOC_ID) && defined(PERF_COUNT_HW_REF_CPU_CYCLES) && defined(PERF_FLAG_FD_CLOEXEC) && \ - !defined(ANKERL_NANOBENCH_DISABLE_PERF_COUNTERS) -// only enable perf counters on kernel 3.14 which seems to have all the necessary defines. The three PERF_... defines are not in -// kernel 2.6.32 (all others are). -# define ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS() 1 -#else -# define ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS() 0 +#define ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS() 0 +#if defined(__linux__) && !defined(ANKERL_NANOBENCH_DISABLE_PERF_COUNTERS) +# include <linux/version.h> +# if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 14, 0) +// PERF_COUNT_HW_REF_CPU_CYCLES only available since kernel 3.3 +// PERF_FLAG_FD_CLOEXEC since kernel 3.14 +# undef ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS +# define ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS() 1 +# endif #endif #if defined(__clang__) @@ -2210,20 +2212,20 @@ struct IterationLogic::Impl { columns.emplace_back(10, 1, "err%", "%", rErrorMedian * 100.0); double rInsMedian = -1.0; - if (mResult.has(Result::Measure::instructions)) { + if (mBench.performanceCounters() && mResult.has(Result::Measure::instructions)) { rInsMedian = mResult.median(Result::Measure::instructions); columns.emplace_back(18, 2, "ins/" + mBench.unit(), "", rInsMedian / mBench.batch()); } double rCycMedian = -1.0; - if (mResult.has(Result::Measure::cpucycles)) { + if (mBench.performanceCounters() && mResult.has(Result::Measure::cpucycles)) { rCycMedian = mResult.median(Result::Measure::cpucycles); columns.emplace_back(18, 2, "cyc/" + mBench.unit(), "", rCycMedian / mBench.batch()); } if (rInsMedian > 0.0 && rCycMedian > 0.0) { columns.emplace_back(9, 3, "IPC", "", rCycMedian <= 0.0 ? 0.0 : rInsMedian / rCycMedian); } - if (mResult.has(Result::Measure::branchinstructions)) { + if (mBench.performanceCounters() && mResult.has(Result::Measure::branchinstructions)) { double rBraMedian = mResult.median(Result::Measure::branchinstructions); columns.emplace_back(17, 2, "bra/" + mBench.unit(), "", rBraMedian / mBench.batch()); if (mResult.has(Result::Measure::branchmisses)) { @@ -2402,6 +2404,14 @@ public: return (a + divisor / 2) / divisor; } + ANKERL_NANOBENCH_NO_SANITIZE("integer", "undefined") + static inline uint32_t mix(uint32_t x) noexcept { + x ^= x << 13; + x ^= x >> 17; + x ^= x << 5; + return x; + } + template <typename Op> ANKERL_NANOBENCH_NO_SANITIZE("integer", "undefined") void calibrate(Op&& op) { @@ -2441,15 +2451,10 @@ public: uint64_t const numIters = 100000U + (std::random_device{}() & 3); uint64_t n = numIters; uint32_t x = 1234567; - auto fn = [&]() { - x ^= x << 13; - x ^= x >> 17; - x ^= x << 5; - }; beginMeasure(); while (n-- > 0) { - fn(); + x = mix(x); } endMeasure(); detail::doNotOptimizeAway(x); @@ -2459,8 +2464,8 @@ public: beginMeasure(); while (n-- > 0) { // we now run *twice* so we can easily calculate the overhead - fn(); - fn(); + x = mix(x); + x = mix(x); } endMeasure(); detail::doNotOptimizeAway(x); diff --git a/src/bench/peer_eviction.cpp b/src/bench/peer_eviction.cpp index 46fd9d999e..8429f18613 100644 --- a/src/bench/peer_eviction.cpp +++ b/src/bench/peer_eviction.cpp @@ -20,19 +20,17 @@ static void EvictionProtectionCommon( { using Candidates = std::vector<NodeEvictionCandidate>; FastRandomContext random_context{true}; - bench.warmup(100).epochIterations(1100); Candidates candidates{GetRandomNodeEvictionCandidates(num_candidates, random_context)}; for (auto& c : candidates) { candidate_setup_fn(c); } - std::vector<Candidates> copies{ - static_cast<size_t>(bench.epochs() * bench.epochIterations()), candidates}; - size_t i{0}; + bench.run([&] { - ProtectEvictionCandidatesByRatio(copies.at(i)); - ++i; + // creating a copy has an overhead of about 3%, so it does not influence the benchmark results much. + auto copy = candidates; + ProtectEvictionCandidatesByRatio(copy); }); } diff --git a/src/bench/rollingbloom.cpp b/src/bench/rollingbloom.cpp index 997ab56549..28167767db 100644 --- a/src/bench/rollingbloom.cpp +++ b/src/bench/rollingbloom.cpp @@ -13,16 +13,16 @@ static void RollingBloom(benchmark::Bench& bench) uint32_t count = 0; bench.run([&] { count++; - data[0] = count; - data[1] = count >> 8; - data[2] = count >> 16; - data[3] = count >> 24; + data[0] = count & 0xFF; + data[1] = (count >> 8) & 0xFF; + data[2] = (count >> 16) & 0xFF; + data[3] = (count >> 24) & 0xFF; filter.insert(data); - data[0] = count >> 24; - data[1] = count >> 16; - data[2] = count >> 8; - data[3] = count; + data[0] = (count >> 24) & 0xFF; + data[1] = (count >> 16) & 0xFF; + data[2] = (count >> 8) & 0xFF; + data[3] = count & 0xFF; filter.contains(data); }); } diff --git a/src/chain.h b/src/chain.h index 84a3a4e1e7..365a7f79b6 100644 --- a/src/chain.h +++ b/src/chain.h @@ -126,7 +126,15 @@ enum BlockStatus: uint32_t { BLOCK_FAILED_CHILD = 64, //!< descends from failed block BLOCK_FAILED_MASK = BLOCK_FAILED_VALID | BLOCK_FAILED_CHILD, - BLOCK_OPT_WITNESS = 128, //!< block data in blk*.data was received with a witness-enforcing client + BLOCK_OPT_WITNESS = 128, //!< block data in blk*.dat was received with a witness-enforcing client + + /** + * If set, this indicates that the block index entry is assumed-valid. + * Certain diagnostics will be skipped in e.g. CheckBlockIndex(). + * It almost certainly means that the block's full validation is pending + * on a background chainstate. See `doc/assumeutxo.md`. + */ + BLOCK_ASSUMED_VALID = 256, }; /** The block chain is a tree shaped structure starting with the @@ -300,14 +308,24 @@ public: return ((nStatus & BLOCK_VALID_MASK) >= nUpTo); } + //! @returns true if the block is assumed-valid; this means it is queued to be + //! validated by a background chainstate. + bool IsAssumedValid() const { return nStatus & BLOCK_ASSUMED_VALID; } + //! Raise the validity level of this block index entry. //! Returns true if the validity was changed. bool RaiseValidity(enum BlockStatus nUpTo) { assert(!(nUpTo & ~BLOCK_VALID_MASK)); // Only validity flags allowed. - if (nStatus & BLOCK_FAILED_MASK) - return false; + if (nStatus & BLOCK_FAILED_MASK) return false; + if ((nStatus & BLOCK_VALID_MASK) < nUpTo) { + // If this block had been marked assumed-valid and we're raising + // its validity to a certain point, there is no longer an assumption. + if (nStatus & BLOCK_ASSUMED_VALID && nUpTo >= BLOCK_VALID_SCRIPTS) { + nStatus &= ~BLOCK_ASSUMED_VALID; + } + nStatus = (nStatus & ~BLOCK_VALID_MASK) | nUpTo; return true; } diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 4cc37560a3..fdee64c529 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -390,12 +390,12 @@ public: consensus.signet_challenge.clear(); consensus.nSubsidyHalvingInterval = 150; consensus.BIP16Exception = uint256(); - consensus.BIP34Height = 2; // BIP34 activated on regtest (Block at height 1 not enforced for testing purposes) + consensus.BIP34Height = 1; // Always active unless overridden consensus.BIP34Hash = uint256(); - consensus.BIP65Height = 111; // BIP65 activated on regtest (Block at height 110 and earlier not enforced for testing purposes) - consensus.BIP66Height = 102; // BIP66 activated on regtest (Block at height 101 and earlier not enforced for testing purposes) - consensus.CSVHeight = 432; // CSV activated on regtest (Used in rpc activation tests) - consensus.SegwitHeight = 0; // SEGWIT is always activated on regtest unless overridden + consensus.BIP65Height = 1; // Always active unless overridden + consensus.BIP66Height = 1; // Always active unless overridden + consensus.CSVHeight = 1; // Always active unless overridden + consensus.SegwitHeight = 1; // Always active unless overridden consensus.MinBIP9WarningHeight = 0; consensus.powLimit = uint256S("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks @@ -487,15 +487,38 @@ public: void UpdateActivationParametersFromArgs(const ArgsManager& args); }; -void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args) +static void MaybeUpdateHeights(const ArgsManager& args, Consensus::Params& consensus) { - if (args.IsArgSet("-segwitheight")) { - int64_t height = args.GetArg("-segwitheight", consensus.SegwitHeight); - if (height < 0 || height >= std::numeric_limits<int>::max()) { - throw std::runtime_error(strprintf("Activation height %ld for segwit is out of valid range.", height)); + for (const std::string& arg : args.GetArgs("-testactivationheight")) { + const auto found{arg.find('@')}; + if (found == std::string::npos) { + throw std::runtime_error(strprintf("Invalid format (%s) for -testactivationheight=name@height.", arg)); + } + const auto name{arg.substr(0, found)}; + const auto value{arg.substr(found + 1)}; + int32_t height; + if (!ParseInt32(value, &height) || height < 0 || height >= std::numeric_limits<int>::max()) { + throw std::runtime_error(strprintf("Invalid height value (%s) for -testactivationheight=name@height.", arg)); + } + if (name == "segwit") { + consensus.SegwitHeight = int{height}; + } else if (name == "bip34") { + consensus.BIP34Height = int{height}; + } else if (name == "dersig") { + consensus.BIP66Height = int{height}; + } else if (name == "cltv") { + consensus.BIP65Height = int{height}; + } else if (name == "csv") { + consensus.CSVHeight = int{height}; + } else { + throw std::runtime_error(strprintf("Invalid name (%s) for -testactivationheight=name@height.", arg)); } - consensus.SegwitHeight = static_cast<int>(height); } +} + +void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args) +{ + MaybeUpdateHeights(args, consensus); if (!args.IsArgSet("-vbparams")) return; diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index 79c1bc25bc..dc484f5c03 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -20,7 +20,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman) argsman.AddArg("-chain=<chain>", "Use the chain <chain> (default: main). Allowed values: main, test, signet, regtest", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-regtest", "Enter regression test mode, which uses a special chain in which blocks can be solved instantly. " "This is intended for regression testing tools and app development. Equivalent to -chain=regtest.", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); - argsman.AddArg("-segwitheight=<n>", "Set the activation height of segwit. (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-testactivationheight=name@height.", "Set the activation height of 'name' (segwit, bip34, dersig, cltv, csv). (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-testnet", "Use the test chain. Equivalent to -chain=test.", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-vbparams=deployment:start:end[:min_activation_height]", "Use given start/end times and min_activation_height for specified version bits deployment (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-signet", "Use the signet chain. Equivalent to -chain=signet. Note that the network is defined by the -signetchallenge parameter", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index a85db04b8b..6766e0510f 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -122,10 +122,10 @@ public: virtual bool displayAddress(const CTxDestination& dest) = 0; //! Lock coin. - virtual void lockCoin(const COutPoint& output) = 0; + virtual bool lockCoin(const COutPoint& output, const bool write_to_db) = 0; //! Unlock coin. - virtual void unlockCoin(const COutPoint& output) = 0; + virtual bool unlockCoin(const COutPoint& output) = 0; //! Return whether coin is locked. virtual bool isLockedCoin(const COutPoint& output) = 0; diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 15527afb8a..7ac2e22006 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -48,17 +48,19 @@ RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx) } std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx, - CTxMemPool& pool, - const CTxMemPool::setEntries& iters_conflicting, - CTxMemPool::setEntries& all_conflicts) + CTxMemPool& pool, + const CTxMemPool::setEntries& iters_conflicting, + CTxMemPool::setEntries& all_conflicts) { AssertLockHeld(pool.cs); const uint256 txid = tx.GetHash(); uint64_t nConflictingCount = 0; for (const auto& mi : iters_conflicting) { nConflictingCount += mi->GetCountWithDescendants(); - // This potentially overestimates the number of actual descendants but we just want to be - // conservative to avoid doing too much work. + // BIP125 Rule #5: don't consider replacing more than MAX_BIP125_REPLACEMENT_CANDIDATES + // entries from the mempool. This potentially overestimates the number of actual + // descendants (i.e. if multiple conflicts share a descendant, it will be counted multiple + // times), but we just want to be conservative to avoid doing too much work. if (nConflictingCount > MAX_BIP125_REPLACEMENT_CANDIDATES) { return strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", txid.ToString(), @@ -66,8 +68,7 @@ std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx, MAX_BIP125_REPLACEMENT_CANDIDATES); } } - // If not too many to replace, then calculate the set of - // transactions that would have to be evicted + // Calculate the set of all transactions that would have to be evicted. for (CTxMemPool::txiter it : iters_conflicting) { pool.CalculateDescendants(it, all_conflicts); } @@ -81,19 +82,19 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, AssertLockHeld(pool.cs); std::set<uint256> parents_of_conflicts; for (const auto& mi : iters_conflicting) { - for (const CTxIn &txin : mi->GetTx().vin) { + for (const CTxIn& txin : mi->GetTx().vin) { parents_of_conflicts.insert(txin.prevout.hash); } } for (unsigned int j = 0; j < tx.vin.size(); j++) { - // We don't want to accept replacements that require low feerate junk to be mined first. - // Ideally we'd keep track of the ancestor feerates and make the decision based on that, but - // for now requiring all new inputs to be confirmed works. + // BIP125 Rule #2: We don't want to accept replacements that require low feerate junk to be + // mined first. Ideally we'd keep track of the ancestor feerates and make the decision + // based on that, but for now requiring all new inputs to be confirmed works. // // Note that if you relax this to make RBF a little more useful, this may break the - // CalculateMempoolAncestors RBF relaxation, above. See the comment above the first - // CalculateMempoolAncestors call for more info. + // CalculateMempoolAncestors RBF relaxation which subtracts the conflict count/size from the + // descendant limit. if (!parents_of_conflicts.count(tx.vin[j].prevout.hash)) { // Rather than check the UTXO set - potentially expensive - it's cheaper to just check // if the new input refers to a tx that's in the mempool. @@ -111,7 +112,7 @@ std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& const uint256& txid) { for (CTxMemPool::txiter ancestorIt : ancestors) { - const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); + const uint256& hashAncestor = ancestorIt->GetTx().GetHash(); if (direct_conflicts.count(hashAncestor)) { return strprintf("%s spends conflicting transaction %s", txid.ToString(), @@ -150,24 +151,26 @@ std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& i std::optional<std::string> PaysForRBF(CAmount original_fees, CAmount replacement_fees, size_t replacement_vsize, + CFeeRate relay_fee, const uint256& txid) { - // The replacement must pay greater fees than the transactions it - // replaces - if we did the bandwidth used by those conflicting - // transactions would not be paid for. + // BIP125 Rule #3: The replacement fees must be greater than or equal to fees of the + // transactions it replaces, otherwise the bandwidth used by those conflicting transactions + // would not be paid for. if (replacement_fees < original_fees) { return strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", txid.ToString(), FormatMoney(replacement_fees), FormatMoney(original_fees)); } - // Finally in addition to paying more fees than the conflicts the - // new transaction must pay for its own bandwidth. + // BIP125 Rule #4: The new transaction must pay for its own bandwidth. Otherwise, we have a DoS + // vector where attackers can cause a transaction to be replaced (and relayed) repeatedly by + // increasing the fee by tiny amounts. CAmount additional_fees = replacement_fees - original_fees; - if (additional_fees < ::incrementalRelayFee.GetFee(replacement_vsize)) { + if (additional_fees < relay_fee.GetFee(replacement_vsize)) { return strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", txid.ToString(), FormatMoney(additional_fees), - FormatMoney(::incrementalRelayFee.GetFee(replacement_vsize))); + FormatMoney(relay_fee.GetFee(replacement_vsize))); } return std::nullopt; } diff --git a/src/policy/rbf.h b/src/policy/rbf.h index 56468a09b2..be8c2e5b8b 100644 --- a/src/policy/rbf.h +++ b/src/policy/rbf.h @@ -5,7 +5,12 @@ #ifndef BITCOIN_POLICY_RBF_H #define BITCOIN_POLICY_RBF_H +#include <primitives/transaction.h> #include <txmempool.h> +#include <uint256.h> + +#include <optional> +#include <string> /** Maximum number of transactions that can be replaced by BIP125 RBF (Rule #5). This includes all * mempool conflicts and their descendants. */ @@ -48,14 +53,14 @@ RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx); std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx, CTxMemPool& pool, const CTxMemPool::setEntries& iters_conflicting, CTxMemPool::setEntries& all_conflicts) - EXCLUSIVE_LOCKS_REQUIRED(pool.cs); + EXCLUSIVE_LOCKS_REQUIRED(pool.cs); /** BIP125 Rule #2: "The replacement transaction may only include an unconfirmed input if that input * was included in one of the original transactions." * @returns error message if Rule #2 is broken, otherwise std::nullopt. */ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTxMemPool& pool, const CTxMemPool::setEntries& iters_conflicting) - EXCLUSIVE_LOCKS_REQUIRED(pool.cs); + EXCLUSIVE_LOCKS_REQUIRED(pool.cs); /** Check the intersection between two sets of transactions (a set of mempool entries and a set of * txids) to make sure they are disjoint. @@ -84,12 +89,14 @@ std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& i * @param[in] original_fees Total modified fees of original transaction(s). * @param[in] replacement_fees Total modified fees of replacement transaction(s). * @param[in] replacement_vsize Total virtual size of replacement transaction(s). + * @param[in] relay_fee The node's minimum feerate for transaction relay. * @param[in] txid Transaction ID, included in the error message if violation occurs. * @returns error string if fees are insufficient, otherwise std::nullopt. */ std::optional<std::string> PaysForRBF(CAmount original_fees, CAmount replacement_fees, size_t replacement_vsize, + CFeeRate relay_fee, const uint256& txid); #endif // BITCOIN_POLICY_RBF_H diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index d2a9365890..86dbd05b1a 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -241,7 +241,7 @@ void CoinControlDialog::lockCoin() contextMenuItem->setCheckState(COLUMN_CHECKBOX, Qt::Unchecked); COutPoint outpt(uint256S(contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), contextMenuItem->data(COLUMN_ADDRESS, VOutRole).toUInt()); - model->wallet().lockCoin(outpt); + model->wallet().lockCoin(outpt, /* write_to_db = */ true); contextMenuItem->setDisabled(true); contextMenuItem->setIcon(COLUMN_CHECKBOX, platformStyle->SingleColorIcon(":/icons/lock_closed")); updateLabelLocked(); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 909019d796..08d21e0f71 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1115,11 +1115,11 @@ static RPCHelpMan gettxoutsetinfo() {RPCResult::Type::NUM, "bogosize", "Database-independent, meaningless metric indicating the UTXO set size"}, {RPCResult::Type::STR_HEX, "hash_serialized_2", /* optional */ true, "The serialized hash (only present if 'hash_serialized_2' hash_type is chosen)"}, {RPCResult::Type::STR_HEX, "muhash", /* optional */ true, "The serialized hash (only present if 'muhash' hash_type is chosen)"}, - {RPCResult::Type::NUM, "transactions", "The number of transactions with unspent outputs (not available when coinstatsindex is used)"}, - {RPCResult::Type::NUM, "disk_size", "The estimated size of the chainstate on disk (not available when coinstatsindex is used)"}, + {RPCResult::Type::NUM, "transactions", /* optional */ true, "The number of transactions with unspent outputs (not available when coinstatsindex is used)"}, + {RPCResult::Type::NUM, "disk_size", /* optional */ true, "The estimated size of the chainstate on disk (not available when coinstatsindex is used)"}, {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount of coins in the UTXO set"}, - {RPCResult::Type::STR_AMOUNT, "total_unspendable_amount", "The total amount of coins permanently excluded from the UTXO set (only available if coinstatsindex is used)"}, - {RPCResult::Type::OBJ, "block_info", "Info on amounts in the block at this block height (only available if coinstatsindex is used)", + {RPCResult::Type::STR_AMOUNT, "total_unspendable_amount", /* optional */ true, "The total amount of coins permanently excluded from the UTXO set (only available if coinstatsindex is used)"}, + {RPCResult::Type::OBJ, "block_info", /* optional */ true, "Info on amounts in the block at this block height (only available if coinstatsindex is used)", { {RPCResult::Type::STR_AMOUNT, "prevout_spent", "Total amount of all prevouts spent in this block"}, {RPCResult::Type::STR_AMOUNT, "coinbase", "Coinbase subsidy amount of this block"}, @@ -1436,32 +1436,32 @@ 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", "lowest-height complete block stored (only present if pruning is enabled)"}, - {RPCResult::Type::BOOL, "automatic_pruning", "whether automatic pruning is enabled (only present if pruning is enabled)"}, - {RPCResult::Type::NUM, "prune_target_size", "the target size used by pruning (only present if automatic pruning is enabled)"}, + {RPCResult::Type::NUM, "pruneheight", /* optional */ true, "lowest-height complete block stored (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_DYN, "softforks", "status of softforks", { {RPCResult::Type::OBJ, "xxxx", "name of the softfork", { {RPCResult::Type::STR, "type", "one of \"buried\", \"bip9\""}, - {RPCResult::Type::OBJ, "bip9", "status of bip9 softforks (only for \"bip9\" type)", + {RPCResult::Type::OBJ, "bip9", /* optional */ true, "status of bip9 softforks (only for \"bip9\" type)", { {RPCResult::Type::STR, "status", "one of \"defined\", \"started\", \"locked_in\", \"active\", \"failed\""}, - {RPCResult::Type::NUM, "bit", "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" and \"locked_in\" status)"}, + {RPCResult::Type::NUM, "bit", /* optional */ true, "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" and \"locked_in\" status)"}, {RPCResult::Type::NUM_TIME, "start_time", "the minimum median time past of a block at which the bit gains its meaning"}, {RPCResult::Type::NUM_TIME, "timeout", "the median time past of a block at which the deployment is considered failed if not yet locked in"}, {RPCResult::Type::NUM, "since", "height of the first block to which the status applies"}, {RPCResult::Type::NUM, "min_activation_height", "minimum height of blocks for which the rules may be enforced"}, - {RPCResult::Type::OBJ, "statistics", "numeric statistics about signalling for a softfork (only for \"started\" and \"locked_in\" status)", + {RPCResult::Type::OBJ, "statistics", /* optional */ true, "numeric statistics about signalling for a softfork (only for \"started\" and \"locked_in\" status)", { {RPCResult::Type::NUM, "period", "the length in blocks of the signalling period"}, - {RPCResult::Type::NUM, "threshold", "the number of blocks with the version bit set required to activate the feature (only for \"started\" status)"}, + {RPCResult::Type::NUM, "threshold", /* optional */ true, "the number of blocks with the version bit set required to activate the feature (only for \"started\" status)"}, {RPCResult::Type::NUM, "elapsed", "the number of blocks elapsed since the beginning of the current period"}, {RPCResult::Type::NUM, "count", "the number of blocks with the version bit set in the current period"}, - {RPCResult::Type::BOOL, "possible", "returns false if there are not enough blocks left in this period to pass activation threshold (only for \"started\" status)"}, + {RPCResult::Type::BOOL, "possible", /* optional */ true, "returns false if there are not enough blocks left in this period to pass activation threshold (only for \"started\" status)"}, }}, }}, - {RPCResult::Type::NUM, "height", "height of the first block which the rules are or will be enforced (only for \"buried\" type, or \"bip9\" type with \"active\" status)"}, + {RPCResult::Type::NUM, "height", /* optional */ true, "height of the first block which the rules are or will be enforced (only for \"buried\" type, or \"bip9\" type with \"active\" status)"}, {RPCResult::Type::BOOL, "active", "true if the rules are enforced for the mempool and the next block"}, }}, }}, @@ -1971,11 +1971,11 @@ static RPCHelpMan getblockstats() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::NUM, "avgfee", "Average fee in the block"}, - {RPCResult::Type::NUM, "avgfeerate", "Average feerate (in satoshis per virtual byte)"}, - {RPCResult::Type::NUM, "avgtxsize", "Average transaction size"}, - {RPCResult::Type::STR_HEX, "blockhash", "The block hash (to check for potential reorgs)"}, - {RPCResult::Type::ARR_FIXED, "feerate_percentiles", "Feerates at the 10th, 25th, 50th, 75th, and 90th percentile weight unit (in satoshis per virtual byte)", + {RPCResult::Type::NUM, "avgfee", /* optional */ true, "Average fee in the block"}, + {RPCResult::Type::NUM, "avgfeerate", /* optional */ true, "Average feerate (in satoshis per virtual byte)"}, + {RPCResult::Type::NUM, "avgtxsize", /* optional */ true, "Average transaction size"}, + {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "The block hash (to check for potential reorgs)"}, + {RPCResult::Type::ARR_FIXED, "feerate_percentiles", /* optional */ true, "Feerates at the 10th, 25th, 50th, 75th, and 90th percentile weight unit (in satoshis per virtual byte)", { {RPCResult::Type::NUM, "10th_percentile_feerate", "The 10th percentile feerate"}, {RPCResult::Type::NUM, "25th_percentile_feerate", "The 25th percentile feerate"}, @@ -1983,30 +1983,30 @@ static RPCHelpMan getblockstats() {RPCResult::Type::NUM, "75th_percentile_feerate", "The 75th percentile feerate"}, {RPCResult::Type::NUM, "90th_percentile_feerate", "The 90th percentile feerate"}, }}, - {RPCResult::Type::NUM, "height", "The height of the block"}, - {RPCResult::Type::NUM, "ins", "The number of inputs (excluding coinbase)"}, - {RPCResult::Type::NUM, "maxfee", "Maximum fee in the block"}, - {RPCResult::Type::NUM, "maxfeerate", "Maximum feerate (in satoshis per virtual byte)"}, - {RPCResult::Type::NUM, "maxtxsize", "Maximum transaction size"}, - {RPCResult::Type::NUM, "medianfee", "Truncated median fee in the block"}, - {RPCResult::Type::NUM, "mediantime", "The block median time past"}, - {RPCResult::Type::NUM, "mediantxsize", "Truncated median transaction size"}, - {RPCResult::Type::NUM, "minfee", "Minimum fee in the block"}, - {RPCResult::Type::NUM, "minfeerate", "Minimum feerate (in satoshis per virtual byte)"}, - {RPCResult::Type::NUM, "mintxsize", "Minimum transaction size"}, - {RPCResult::Type::NUM, "outs", "The number of outputs"}, - {RPCResult::Type::NUM, "subsidy", "The block subsidy"}, - {RPCResult::Type::NUM, "swtotal_size", "Total size of all segwit transactions"}, - {RPCResult::Type::NUM, "swtotal_weight", "Total weight of all segwit transactions"}, - {RPCResult::Type::NUM, "swtxs", "The number of segwit transactions"}, - {RPCResult::Type::NUM, "time", "The block time"}, - {RPCResult::Type::NUM, "total_out", "Total amount in all outputs (excluding coinbase and thus reward [ie subsidy + totalfee])"}, - {RPCResult::Type::NUM, "total_size", "Total size of all non-coinbase transactions"}, - {RPCResult::Type::NUM, "total_weight", "Total weight of all non-coinbase transactions"}, - {RPCResult::Type::NUM, "totalfee", "The fee total"}, - {RPCResult::Type::NUM, "txs", "The number of transactions (including coinbase)"}, - {RPCResult::Type::NUM, "utxo_increase", "The increase/decrease in the number of unspent outputs"}, - {RPCResult::Type::NUM, "utxo_size_inc", "The increase/decrease in size for the utxo index (not discounting op_return and similar)"}, + {RPCResult::Type::NUM, "height", /* optional */ true, "The height of the block"}, + {RPCResult::Type::NUM, "ins", /* optional */ true, "The number of inputs (excluding coinbase)"}, + {RPCResult::Type::NUM, "maxfee", /* optional */ true, "Maximum fee in the block"}, + {RPCResult::Type::NUM, "maxfeerate", /* optional */ true, "Maximum feerate (in satoshis per virtual byte)"}, + {RPCResult::Type::NUM, "maxtxsize", /* optional */ true, "Maximum transaction size"}, + {RPCResult::Type::NUM, "medianfee", /* optional */ true, "Truncated median fee in the block"}, + {RPCResult::Type::NUM, "mediantime", /* optional */ true, "The block median time past"}, + {RPCResult::Type::NUM, "mediantxsize", /* optional */ true, "Truncated median transaction size"}, + {RPCResult::Type::NUM, "minfee", /* optional */ true, "Minimum fee in the block"}, + {RPCResult::Type::NUM, "minfeerate", /* optional */ true, "Minimum feerate (in satoshis per virtual byte)"}, + {RPCResult::Type::NUM, "mintxsize", /* optional */ true, "Minimum transaction size"}, + {RPCResult::Type::NUM, "outs", /* optional */ true, "The number of outputs"}, + {RPCResult::Type::NUM, "subsidy", /* optional */ true, "The block subsidy"}, + {RPCResult::Type::NUM, "swtotal_size", /* optional */ true, "Total size of all segwit transactions"}, + {RPCResult::Type::NUM, "swtotal_weight", /* optional */ true, "Total weight of all segwit transactions"}, + {RPCResult::Type::NUM, "swtxs", /* optional */ true, "The number of segwit transactions"}, + {RPCResult::Type::NUM, "time", /* optional */ true, "The block time"}, + {RPCResult::Type::NUM, "total_out", /* optional */ true, "Total amount in all outputs (excluding coinbase and thus reward [ie subsidy + totalfee])"}, + {RPCResult::Type::NUM, "total_size", /* optional */ true, "Total size of all non-coinbase transactions"}, + {RPCResult::Type::NUM, "total_weight", /* optional */ true, "Total weight of all non-coinbase transactions"}, + {RPCResult::Type::NUM, "totalfee", /* optional */ true, "The fee total"}, + {RPCResult::Type::NUM, "txs", /* optional */ true, "The number of transactions (including coinbase)"}, + {RPCResult::Type::NUM, "utxo_increase", /* optional */ true, "The increase/decrease in the number of unspent outputs"}, + {RPCResult::Type::NUM, "utxo_size_inc", /* optional */ true, "The increase/decrease in size for the utxo index (not discounting op_return and similar)"}, }}, RPCExamples{ HelpExampleCli("getblockstats", R"('"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09"' '["minfeerate","avgfeerate"]')") + diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index d6943e066a..93e49cb9a8 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -131,6 +131,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "gettxoutsetinfo", 2, "use_index"}, { "lockunspent", 0, "unlock" }, { "lockunspent", 1, "transactions" }, + { "lockunspent", 2, "persistent" }, { "send", 0, "outputs" }, { "send", 1, "conf_target" }, { "send", 3, "fee_rate"}, diff --git a/src/rpc/external_signer.cpp b/src/rpc/external_signer.cpp index 6ec2b1a07f..60ec15e904 100644 --- a/src/rpc/external_signer.cpp +++ b/src/rpc/external_signer.cpp @@ -24,8 +24,11 @@ static RPCHelpMan enumeratesigners() { {RPCResult::Type::ARR, "signers", /* optional */ false, "", { - {RPCResult::Type::STR_HEX, "masterkeyfingerprint", "Master key fingerprint"}, - {RPCResult::Type::STR, "name", "Device name"}, + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "fingerprint", "Master key fingerprint"}, + {RPCResult::Type::STR, "name", "Device name"}, + }}, }, } } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 27cbb3a702..eb2d5b9c5f 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -553,6 +553,10 @@ static RPCHelpMan getblocktemplate() { {RPCResult::Type::NUM, "rulename", "identifies the bit number as indicating acceptance and readiness for the named softfork rule"}, }}, + {RPCResult::Type::ARR, "capabilities", "", + { + {RPCResult::Type::STR, "value", "A supported feature, for example 'proposal'"}, + }}, {RPCResult::Type::NUM, "vbrequired", "bit mask of versionbits the server requires set in submissions"}, {RPCResult::Type::STR, "previousblockhash", "The hash of current highest block"}, {RPCResult::Type::ARR, "transactions", "contents of non-coinbase transactions that should be included in the next block", @@ -586,11 +590,12 @@ static RPCHelpMan getblocktemplate() {RPCResult::Type::STR_HEX, "noncerange", "A range of valid nonces"}, {RPCResult::Type::NUM, "sigoplimit", "limit of sigops in blocks"}, {RPCResult::Type::NUM, "sizelimit", "limit of block size"}, - {RPCResult::Type::NUM, "weightlimit", "limit of block weight"}, + {RPCResult::Type::NUM, "weightlimit", /* optional */ true, "limit of block weight"}, {RPCResult::Type::NUM_TIME, "curtime", "current timestamp in " + UNIX_EPOCH_TIME}, {RPCResult::Type::STR, "bits", "compressed target of next block"}, {RPCResult::Type::NUM, "height", "The height of the next block"}, - {RPCResult::Type::STR, "default_witness_commitment", /* optional */ true, "a valid witness commitment for the unmodified block template"}, + {RPCResult::Type::STR_HEX, "signet_challenge", /* optional */ true, "Only on signet"}, + {RPCResult::Type::STR_HEX, "default_witness_commitment", /* optional */ true, "a valid witness commitment for the unmodified block template"}, }}, }, RPCExamples{ diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 14b0e5a984..dff2129980 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -44,10 +44,10 @@ static RPCHelpMan validateaddress() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::BOOL, "isvalid", "If the address is valid or not"}, - {RPCResult::Type::STR, "address", "The bitcoin address validated"}, - {RPCResult::Type::STR_HEX, "scriptPubKey", "The hex-encoded scriptPubKey generated by the address"}, - {RPCResult::Type::BOOL, "isscript", "If the key is a script"}, - {RPCResult::Type::BOOL, "iswitness", "If the address is a witness address"}, + {RPCResult::Type::STR, "address", /* optional */ true, "The bitcoin address validated"}, + {RPCResult::Type::STR_HEX, "scriptPubKey", /* optional */ true, "The hex-encoded scriptPubKey generated by the address"}, + {RPCResult::Type::BOOL, "isscript", /* optional */ true, "If the key is a script"}, + {RPCResult::Type::BOOL, "iswitness", /* optional */ true, "If the address is a witness address"}, {RPCResult::Type::NUM, "witness_version", /* optional */ true, "The version number of the witness program"}, {RPCResult::Type::STR_HEX, "witness_program", /* optional */ true, "The hex value of the witness program"}, {RPCResult::Type::STR, "error", /* optional */ true, "Error message, if any"}, @@ -717,7 +717,7 @@ static RPCHelpMan getindexinfo() {"index_name", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Filter results for an index with a specific name."}, }, RPCResult{ - RPCResult::Type::OBJ, "", "", { + RPCResult::Type::OBJ_DYN, "", "", { { RPCResult::Type::OBJ, "name", "The name of the index", { diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 227eec722f..a9bee33c5d 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -116,10 +116,10 @@ static RPCHelpMan getpeerinfo() { {RPCResult::Type::NUM, "id", "Peer index"}, {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"}, - {RPCResult::Type::STR, "addrbind", "(ip:port) Bind address of the connection to the peer"}, - {RPCResult::Type::STR, "addrlocal", "(ip:port) Local address as reported by the peer"}, + {RPCResult::Type::STR, "addrbind", /* optional */ true, "(ip:port) Bind address of the connection to the peer"}, + {RPCResult::Type::STR, "addrlocal", /* optional */ true, "(ip:port) Local address as reported by the peer"}, {RPCResult::Type::STR, "network", "Network (" + Join(GetNetworkNames(/* append_unroutable */ true), ", ") + ")"}, - {RPCResult::Type::NUM, "mapped_as", "The AS in the BGP route to the peer used for diversifying\n" + {RPCResult::Type::NUM, "mapped_as", /* optional */ true, "The AS in the BGP route to the peer used for diversifying\n" "peer selection (only available if the asmap config flag is set)"}, {RPCResult::Type::STR_HEX, "services", "The services offered"}, {RPCResult::Type::ARR, "servicesnames", "the services offered, in human-readable form", @@ -135,9 +135,9 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::NUM, "bytesrecv", "The total bytes received"}, {RPCResult::Type::NUM_TIME, "conntime", "The " + UNIX_EPOCH_TIME + " of the connection"}, {RPCResult::Type::NUM, "timeoffset", "The time offset in seconds"}, - {RPCResult::Type::NUM, "pingtime", "ping time (if available)"}, - {RPCResult::Type::NUM, "minping", "minimum observed ping time (if any at all)"}, - {RPCResult::Type::NUM, "pingwait", "ping wait (if non-zero)"}, + {RPCResult::Type::NUM, "pingtime", /* optional */ true, "ping time (if available)"}, + {RPCResult::Type::NUM, "minping", /* optional */ true, "minimum observed ping time (if any at all)"}, + {RPCResult::Type::NUM, "pingwait", /* optional */ true, "ping wait (if non-zero)"}, {RPCResult::Type::NUM, "version", "The peer version, such as 70001"}, {RPCResult::Type::STR, "subver", "The string version"}, {RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"}, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 00e77d89e5..737abf6ca9 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -94,7 +94,7 @@ static RPCHelpMan getrawtransaction() RPCResult{"if verbose is set to true", RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "in_active_chain", "Whether specified block is in the active chain or not (only present with explicit \"blockhash\" argument)"}, + {RPCResult::Type::BOOL, "in_active_chain", /* optional */ true, "Whether specified block is in the active chain or not (only present with explicit \"blockhash\" argument)"}, {RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"}, {RPCResult::Type::STR_HEX, "txid", "The transaction id (same as provided)"}, {RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"}, @@ -115,7 +115,7 @@ static RPCHelpMan getrawtransaction() {RPCResult::Type::STR_HEX, "hex", "hex"}, }}, {RPCResult::Type::NUM, "sequence", "The script sequence number"}, - {RPCResult::Type::ARR, "txinwitness", "", + {RPCResult::Type::ARR, "txinwitness", /* optional */ true, "", { {RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"}, }}, @@ -141,10 +141,10 @@ static RPCHelpMan getrawtransaction() }}, }}, }}, - {RPCResult::Type::STR_HEX, "blockhash", "the block hash"}, - {RPCResult::Type::NUM, "confirmations", "The confirmations"}, - {RPCResult::Type::NUM_TIME, "blocktime", "The block time expressed in " + UNIX_EPOCH_TIME}, - {RPCResult::Type::NUM, "time", "Same as \"blocktime\""}, + {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "the block hash"}, + {RPCResult::Type::NUM, "confirmations", /* optional */ true, "The confirmations"}, + {RPCResult::Type::NUM_TIME, "blocktime", /* optional */ true, "The block time expressed in " + UNIX_EPOCH_TIME}, + {RPCResult::Type::NUM, "time", /* optional */ true, "Same as \"blocktime\""}, } }, }, @@ -470,14 +470,15 @@ static RPCHelpMan decoderawtransaction() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR_HEX, "txid", "The transaction id"}, - {RPCResult::Type::NUM, "vout", "The output number"}, - {RPCResult::Type::OBJ, "scriptSig", "The script", + {RPCResult::Type::STR_HEX, "coinbase", /* optional */ true, ""}, + {RPCResult::Type::STR_HEX, "txid", /* optional */ true, "The transaction id"}, + {RPCResult::Type::NUM, "vout", /* optional */ true, "The output number"}, + {RPCResult::Type::OBJ, "scriptSig", /* optional */ true, "The script", { {RPCResult::Type::STR, "asm", "asm"}, {RPCResult::Type::STR_HEX, "hex", "hex"}, }}, - {RPCResult::Type::ARR, "txinwitness", "", + {RPCResult::Type::ARR, "txinwitness", /* optional */ true, "", { {RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"}, }}, @@ -559,8 +560,8 @@ static RPCHelpMan decodescript() { {RPCResult::Type::STR, "address", "bitcoin address"}, }}, - {RPCResult::Type::STR, "p2sh", "address of P2SH script wrapping this redeem script (not returned if the script is already a P2SH)"}, - {RPCResult::Type::OBJ, "segwit", "Result of a witness script public key wrapping this redeem script (not returned if the script is a P2SH or witness)", + {RPCResult::Type::STR, "p2sh", /* optional */ true, "address of P2SH script wrapping this redeem script (not returned if the script is already a P2SH)"}, + {RPCResult::Type::OBJ, "segwit", /* optional */ true, "Result of a witness script public key wrapping this redeem script (not returned if the script is a P2SH or witness)", { {RPCResult::Type::STR, "asm", "String representation of the script public key"}, {RPCResult::Type::STR_HEX, "hex", "Hex string of the script public key"}, @@ -772,6 +773,10 @@ static RPCHelpMan signrawtransactionwithkey() { {RPCResult::Type::STR_HEX, "txid", "The hash of the referenced, previous transaction"}, {RPCResult::Type::NUM, "vout", "The index of the output to spent and used as input"}, + {RPCResult::Type::ARR, "witness", "", + { + {RPCResult::Type::STR_HEX, "witness", ""}, + }}, {RPCResult::Type::STR_HEX, "scriptSig", "The hex-encoded signature script"}, {RPCResult::Type::NUM, "sequence", "Script sequence number"}, {RPCResult::Type::STR, "error", "Verification or signing error related to the input"}, @@ -909,15 +914,15 @@ static RPCHelpMan testmempoolaccept() { {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, {RPCResult::Type::STR_HEX, "wtxid", "The transaction witness hash in hex"}, - {RPCResult::Type::STR, "package-error", "Package validation error, if any (only possible if rawtxs had more than 1 transaction)."}, - {RPCResult::Type::BOOL, "allowed", "Whether this tx would be accepted to the mempool and pass client-specified maxfeerate." + {RPCResult::Type::STR, "package-error", /* optional */ true, "Package validation error, if any (only possible if rawtxs had more than 1 transaction)."}, + {RPCResult::Type::BOOL, "allowed", /* optional */ true, "Whether this tx would be accepted to the mempool and pass client-specified maxfeerate. " "If not present, the tx was not fully validated due to a failure in another tx in the list."}, - {RPCResult::Type::NUM, "vsize", "Virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted (only present when 'allowed' is true)"}, - {RPCResult::Type::OBJ, "fees", "Transaction fees (only present if 'allowed' is true)", + {RPCResult::Type::NUM, "vsize", /* optional */ true, "Virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted (only present when 'allowed' is true)"}, + {RPCResult::Type::OBJ, "fees", /* optional */ true, "Transaction fees (only present if 'allowed' is true)", { {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, }}, - {RPCResult::Type::STR, "reject-reason", "Rejection string (only present when 'allowed' is false)"}, + {RPCResult::Type::STR, "reject-reason", /* optional */ true, "Rejection string (only present when 'allowed' is false)"}, }}, } }, @@ -1056,7 +1061,7 @@ static RPCHelpMan decodepsbt() {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR_HEX, "hex", "The hex"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - {RPCResult::Type::STR, "address"," Bitcoin address if there is one"}, + {RPCResult::Type::STR, "address", /*optional=*/true, "Bitcoin address if there is one"}, }}, }}, {RPCResult::Type::OBJ_DYN, "partial_signatures", /* optional */ true, "", @@ -1078,22 +1083,23 @@ static RPCHelpMan decodepsbt() }}, {RPCResult::Type::ARR, "bip32_derivs", /* optional */ true, "", { - {RPCResult::Type::OBJ, "pubkey", /* optional */ true, "The public key with the derivation path as the value.", + {RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR, "pubkey", "The public key with the derivation path as the value."}, {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, {RPCResult::Type::STR, "path", "The path"}, }}, }}, - {RPCResult::Type::OBJ, "final_scriptsig", /* optional */ true, "", + {RPCResult::Type::OBJ, "final_scriptSig", /* optional */ true, "", { {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR, "hex", "The hex"}, }}, - {RPCResult::Type::ARR, "final_scriptwitness", "", + {RPCResult::Type::ARR, "final_scriptwitness", /* optional */ true, "", { {RPCResult::Type::STR_HEX, "", "hex-encoded witness data (if any)"}, }}, - {RPCResult::Type::OBJ_DYN, "unknown", "The unknown global fields", + {RPCResult::Type::OBJ_DYN, "unknown", /* optional */ true, "The unknown global fields", { {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, }}, @@ -1124,7 +1130,7 @@ static RPCHelpMan decodepsbt() {RPCResult::Type::STR, "path", "The path"}, }}, }}, - {RPCResult::Type::OBJ_DYN, "unknown", "The unknown global fields", + {RPCResult::Type::OBJ_DYN, "unknown", /* optional */ true, "The unknown global fields", { {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, }}, @@ -1398,8 +1404,8 @@ static RPCHelpMan finalizepsbt() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "psbt", "The base64-encoded partially signed transaction if not extracted"}, - {RPCResult::Type::STR_HEX, "hex", "The hex-encoded network transaction if extracted"}, + {RPCResult::Type::STR, "psbt", /* optional */ true, "The base64-encoded partially signed transaction if not extracted"}, + {RPCResult::Type::STR_HEX, "hex", /* optional */ true, "The hex-encoded network transaction if extracted"}, {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, } }, @@ -1791,7 +1797,7 @@ static RPCHelpMan analyzepsbt() RPCResult { RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::ARR, "inputs", "", + {RPCResult::Type::ARR, "inputs", /* optional */ true, "", { {RPCResult::Type::OBJ, "", "", { diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index 6f2bc081c6..8d2a06f11a 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -4,6 +4,7 @@ #include <chainparams.h> #include <consensus/validation.h> +#include <node/utxo_snapshot.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 1924ea55b1..afb3ad0cfd 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -13,6 +13,11 @@ #include <boost/test/unit_test.hpp> +struct Dersig100Setup : public TestChain100Setup { + Dersig100Setup() + : TestChain100Setup{{"-testactivationheight=dersig@102"}} {} +}; + bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, @@ -20,7 +25,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, BOOST_AUTO_TEST_SUITE(txvalidationcache_tests) -BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) +BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, Dersig100Setup) { // Make sure skipping validation of transactions that were // validated going into the memory pool does not allow @@ -153,7 +158,7 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail } } -BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) +BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup) { // Test that passing CheckInputScripts with one set of script flags doesn't imply // that we would pass again with a different set of flags. diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h new file mode 100644 index 0000000000..81ea4c38f5 --- /dev/null +++ b/src/test/util/chainstate.h @@ -0,0 +1,54 @@ +// Copyright (c) 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. +// +#ifndef BITCOIN_TEST_UTIL_CHAINSTATE_H +#define BITCOIN_TEST_UTIL_CHAINSTATE_H + +#include <clientversion.h> +#include <fs.h> +#include <node/context.h> +#include <node/utxo_snapshot.h> +#include <rpc/blockchain.h> +#include <validation.h> + +#include <univalue.h> + +#include <boost/test/unit_test.hpp> + +const auto NoMalleation = [](CAutoFile& file, SnapshotMetadata& meta){}; + +/** + * Create and activate a UTXO snapshot, optionally providing a function to + * malleate the snapshot. + */ +template<typename F = decltype(NoMalleation)> +static bool +CreateAndActivateUTXOSnapshot(NodeContext& node, const fs::path root, F malleation = NoMalleation) +{ + // Write out a snapshot to the test's tempdir. + // + int height; + WITH_LOCK(::cs_main, height = node.chainman->ActiveHeight()); + fs::path snapshot_path = root / tfm::format("test_snapshot.%d.dat", height); + FILE* outfile{fsbridge::fopen(snapshot_path, "wb")}; + CAutoFile auto_outfile{outfile, SER_DISK, CLIENT_VERSION}; + + UniValue result = CreateUTXOSnapshot(node, node.chainman->ActiveChainstate(), auto_outfile); + BOOST_TEST_MESSAGE( + "Wrote UTXO snapshot to " << snapshot_path.make_preferred().string() << ": " << result.write()); + + // Read the written snapshot in and then activate it. + // + FILE* infile{fsbridge::fopen(snapshot_path, "rb")}; + CAutoFile auto_infile{infile, SER_DISK, CLIENT_VERSION}; + SnapshotMetadata metadata; + auto_infile >> metadata; + + malleation(auto_infile, metadata); + + return node.chainman->ActivateSnapshot(auto_infile, metadata, /*in_memory*/ true); +} + + +#endif // BITCOIN_TEST_UTIL_CHAINSTATE_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index cabc4b3b49..97e614379c 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -205,7 +205,8 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const } } -TestChain100Setup::TestChain100Setup() +TestChain100Setup::TestChain100Setup(const std::vector<const char*>& extra_args) + : TestingSetup{CBaseChainParams::REGTEST, extra_args} { SetMockTime(1598887952); constexpr std::array<unsigned char, 32> vchKey = { @@ -234,11 +235,14 @@ void TestChain100Setup::mineBlocks(int num_blocks) } } -CBlock TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey) +CBlock TestChain100Setup::CreateBlock( + const std::vector<CMutableTransaction>& txns, + const CScript& scriptPubKey, + CChainState& chainstate) { const CChainParams& chainparams = Params(); CTxMemPool empty_pool; - CBlock block = BlockAssembler(m_node.chainman->ActiveChainstate(), empty_pool, chainparams).CreateNewBlock(scriptPubKey)->block; + CBlock block = BlockAssembler(chainstate, empty_pool, chainparams).CreateNewBlock(scriptPubKey)->block; Assert(block.vtx.size() == 1); for (const CMutableTransaction& tx : txns) { @@ -248,6 +252,20 @@ CBlock TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransa while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce; + return block; +} + +CBlock TestChain100Setup::CreateAndProcessBlock( + const std::vector<CMutableTransaction>& txns, + const CScript& scriptPubKey, + CChainState* chainstate) +{ + if (!chainstate) { + chainstate = &Assert(m_node.chainman)->ActiveChainstate(); + } + + const CChainParams& chainparams = Params(); + const CBlock block = this->CreateBlock(txns, scriptPubKey, *chainstate); std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(block); Assert(m_node.chainman)->ProcessNewBlock(chainparams, shared_pblock, true, nullptr); @@ -304,11 +322,6 @@ CMutableTransaction TestChain100Setup::CreateValidMempoolTransaction(CTransactio return mempool_txn; } -TestChain100Setup::~TestChain100Setup() -{ - gArgs.ForceSetArg("-segwitheight", "0"); -} - CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) const { return FromTx(MakeTransactionRef(tx)); diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index 5d12dc2323..7518cdb042 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -113,15 +113,26 @@ class CScript; /** * Testing fixture that pre-creates a 100-block REGTEST-mode block chain */ -struct TestChain100Setup : public RegTestingSetup { - TestChain100Setup(); +struct TestChain100Setup : public TestingSetup { + TestChain100Setup(const std::vector<const char*>& extra_args = {}); /** * Create a new block with just given transactions, coinbase paying to * scriptPubKey, and try to add it to the current chain. + * If no chainstate is specified, default to the active. */ CBlock CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns, - const CScript& scriptPubKey); + const CScript& scriptPubKey, + CChainState* chainstate = nullptr); + + /** + * Create a new block with just given transactions, coinbase paying to + * scriptPubKey. + */ + CBlock CreateBlock( + const std::vector<CMutableTransaction>& txns, + const CScript& scriptPubKey, + CChainState& chainstate); //! Mine a series of new blocks on the active chain. void mineBlocks(int num_blocks); @@ -145,8 +156,6 @@ struct TestChain100Setup : public RegTestingSetup { CAmount output_amount = CAmount(1 * COIN), bool submit = true); - ~TestChain100Setup(); - std::vector<CTransactionRef> m_coinbase_txns; // For convenience, coinbase transactions CKey coinbaseKey; // private/public key needed to spend coinbase transactions }; diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 315ef22599..726c9ebbb8 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -2,10 +2,13 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. // +#include <chainparams.h> #include <random.h> #include <uint256.h> #include <consensus/validation.h> #include <sync.h> +#include <rpc/blockchain.h> +#include <test/util/chainstate.h> #include <test/util/setup_common.h> #include <validation.h> @@ -73,4 +76,77 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) WITH_LOCK(::cs_main, manager.Unload()); } +//! Test UpdateTip behavior for both active and background chainstates. +//! +//! When run on the background chainstate, UpdateTip should do a subset +//! of what it does for the active chainstate. +BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) +{ + ChainstateManager& chainman = *Assert(m_node.chainman); + uint256 curr_tip = ::g_best_block; + + // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can + // be found. + mineBlocks(10); + + // After adding some blocks to the tip, best block should have changed. + BOOST_CHECK(::g_best_block != curr_tip); + + BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root)); + + // Ensure our active chain is the snapshot chainstate. + BOOST_CHECK(chainman.IsSnapshotActive()); + + curr_tip = ::g_best_block; + + // Mine a new block on top of the activated snapshot chainstate. + mineBlocks(1); // Defined in TestChain100Setup. + + // After adding some blocks to the snapshot tip, best block should have changed. + BOOST_CHECK(::g_best_block != curr_tip); + + curr_tip = ::g_best_block; + + CChainState* background_cs; + + BOOST_CHECK_EQUAL(chainman.GetAll().size(), 2); + for (CChainState* cs : chainman.GetAll()) { + if (cs != &chainman.ActiveChainstate()) { + background_cs = cs; + } + } + BOOST_CHECK(background_cs); + + // Create a block to append to the validation chain. + std::vector<CMutableTransaction> noTxns; + CScript scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; + CBlock validation_block = this->CreateBlock(noTxns, scriptPubKey, *background_cs); + auto pblock = std::make_shared<const CBlock>(validation_block); + BlockValidationState state; + CBlockIndex* pindex = nullptr; + const CChainParams& chainparams = Params(); + bool newblock = false; + + // TODO: much of this is inlined from ProcessNewBlock(); just reuse PNB() + // once it is changed to support multiple chainstates. + { + LOCK(::cs_main); + bool checked = CheckBlock(*pblock, state, chainparams.GetConsensus()); + BOOST_CHECK(checked); + bool accepted = background_cs->AcceptBlock( + pblock, state, &pindex, true, nullptr, &newblock); + BOOST_CHECK(accepted); + } + // UpdateTip is called here + bool block_added = background_cs->ActivateBestChain(state, pblock); + + // Ensure tip is as expected + BOOST_CHECK_EQUAL(background_cs->m_chain.Tip()->GetBlockHash(), validation_block.GetHash()); + + // g_best_block should be unchanged after adding a block to the background + // validation chain. + BOOST_CHECK(block_added); + BOOST_CHECK_EQUAL(curr_tip, ::g_best_block); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 0bd378631b..be9e05a65e 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -8,13 +8,13 @@ #include <random.h> #include <rpc/blockchain.h> #include <sync.h> +#include <test/util/chainstate.h> #include <test/util/setup_common.h> #include <uint256.h> #include <validation.h> #include <validationinterface.h> #include <tinyformat.h> -#include <univalue.h> #include <vector> @@ -44,7 +44,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) BOOST_CHECK(!manager.IsSnapshotActive()); BOOST_CHECK(!manager.IsSnapshotValidated()); - BOOST_CHECK(!manager.IsBackgroundIBD(&c1)); auto all = manager.GetAll(); BOOST_CHECK_EQUAL_COLLECTIONS(all.begin(), all.end(), chainstates.begin(), chainstates.end()); @@ -57,9 +56,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) auto exp_tip = c1.m_chain.Tip(); BOOST_CHECK_EQUAL(active_tip, exp_tip); - auto& validated_cs = manager.ValidatedChainstate(); - BOOST_CHECK_EQUAL(&validated_cs, &c1); - BOOST_CHECK(!manager.SnapshotBlockhash().has_value()); // Create a snapshot-based chainstate. @@ -81,8 +77,8 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) BOOST_CHECK(manager.IsSnapshotActive()); BOOST_CHECK(!manager.IsSnapshotValidated()); - BOOST_CHECK(manager.IsBackgroundIBD(&c1)); - BOOST_CHECK(!manager.IsBackgroundIBD(&c2)); + BOOST_CHECK_EQUAL(&c2, &manager.ActiveChainstate()); + BOOST_CHECK(&c1 != &manager.ActiveChainstate()); auto all2 = manager.GetAll(); BOOST_CHECK_EQUAL_COLLECTIONS(all2.begin(), all2.end(), chainstates.begin(), chainstates.end()); @@ -99,16 +95,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // CCoinsViewCache instances. BOOST_CHECK(exp_tip != exp_tip2); - auto& validated_cs2 = manager.ValidatedChainstate(); - BOOST_CHECK_EQUAL(&validated_cs2, &c1); - - auto& validated_chain = manager.ValidatedChain(); - BOOST_CHECK_EQUAL(&validated_chain, &c1.m_chain); - - auto validated_tip = manager.ValidatedTip(); - exp_tip = c1.m_chain.Tip(); - BOOST_CHECK_EQUAL(validated_tip, exp_tip); - // Let scheduler events finish running to avoid accessing memory that is going to be unloaded SyncWithValidationInterfaceQueue(); @@ -168,36 +154,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) BOOST_CHECK_CLOSE(c2.m_coinsdb_cache_size_bytes, max_cache * 0.95, 1); } -auto NoMalleation = [](CAutoFile& file, SnapshotMetadata& meta){}; - -template<typename F = decltype(NoMalleation)> -static bool -CreateAndActivateUTXOSnapshot(NodeContext& node, const fs::path root, F malleation = NoMalleation) -{ - // Write out a snapshot to the test's tempdir. - // - int height; - WITH_LOCK(::cs_main, height = node.chainman->ActiveHeight()); - fs::path snapshot_path = root / tfm::format("test_snapshot.%d.dat", height); - FILE* outfile{fsbridge::fopen(snapshot_path, "wb")}; - CAutoFile auto_outfile{outfile, SER_DISK, CLIENT_VERSION}; - - UniValue result = CreateUTXOSnapshot(node, node.chainman->ActiveChainstate(), auto_outfile); - BOOST_TEST_MESSAGE( - "Wrote UTXO snapshot to " << snapshot_path.make_preferred().string() << ": " << result.write()); - - // Read the written snapshot in and then activate it. - // - FILE* infile{fsbridge::fopen(snapshot_path, "rb")}; - CAutoFile auto_infile{infile, SER_DISK, CLIENT_VERSION}; - SnapshotMetadata metadata; - auto_infile >> metadata; - - malleation(auto_infile, metadata); - - return node.chainman->ActivateSnapshot(auto_infile, metadata, /*in_memory*/ true); -} - //! Test basic snapshot activation. BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) { @@ -321,27 +277,27 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) { LOCK(::cs_main); size_t coins_in_active{0}; - size_t coins_in_ibd{0}; - size_t coins_missing_ibd{0}; + size_t coins_in_background{0}; + size_t coins_missing_from_background{0}; for (CChainState* chainstate : chainman.GetAll()) { BOOST_TEST_MESSAGE("Checking coins in " << chainstate->ToString()); CCoinsViewCache& coinscache = chainstate->CoinsTip(); - bool is_ibd = chainman.IsBackgroundIBD(chainstate); + bool is_background = chainstate != &chainman.ActiveChainstate(); for (CTransactionRef& txn : m_coinbase_txns) { COutPoint op{txn->GetHash(), 0}; if (coinscache.HaveCoin(op)) { - (is_ibd ? coins_in_ibd : coins_in_active)++; - } else if (is_ibd) { - coins_missing_ibd++; + (is_background ? coins_in_background : coins_in_active)++; + } else if (is_background) { + coins_missing_from_background++; } } } BOOST_CHECK_EQUAL(coins_in_active, initial_total_coins + new_coins); - BOOST_CHECK_EQUAL(coins_in_ibd, initial_total_coins); - BOOST_CHECK_EQUAL(coins_missing_ibd, new_coins); + BOOST_CHECK_EQUAL(coins_in_background, initial_total_coins); + BOOST_CHECK_EQUAL(coins_missing_from_background, new_coins); } // Snapshot should refuse to load after one has already loaded. diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index 22aafcaa6c..9136c497ea 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -9,7 +9,7 @@ #include <boost/test/unit_test.hpp> -BOOST_FIXTURE_TEST_SUITE(validation_flush_tests, BasicTestingSetup) +BOOST_FIXTURE_TEST_SUITE(validation_flush_tests, ChainTestingSetup) //! Test utilities for detecting when we need to flush the coins cache based //! on estimated memory usage. @@ -20,7 +20,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) { CTxMemPool mempool; BlockManager blockman{}; - CChainState chainstate{&mempool, blockman}; + CChainState chainstate{&mempool, blockman, *Assert(m_node.chainman)}; chainstate.InitCoinsDB(/*cache_size_bytes*/ 1 << 10, /*in_memory*/ true, /*should_wipe*/ false); WITH_LOCK(::cs_main, chainstate.InitCoinsCache(1 << 10)); diff --git a/src/timedata.cpp b/src/timedata.cpp index 354092752d..f53fbe231b 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -11,6 +11,7 @@ #include <netaddress.h> #include <node/ui_interface.h> #include <sync.h> +#include <tinyformat.h> #include <util/system.h> #include <util/translation.h> #include <warnings.h> @@ -98,11 +99,12 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) } if (LogAcceptCategory(BCLog::NET)) { + std::string log_message{"time data samples: "}; for (const int64_t n : vSorted) { - LogPrint(BCLog::NET, "%+d ", n); /* Continued */ + log_message += strprintf("%+d ", n); } - LogPrint(BCLog::NET, "| "); /* Continued */ - LogPrint(BCLog::NET, "nTimeOffset = %+d (%+d minutes)\n", nTimeOffset, nTimeOffset / 60); + log_message += strprintf("| median offset = %+d (%+d minutes)", nTimeOffset, nTimeOffset / 60); + LogPrint(BCLog::NET, "%s\n", log_message); } } } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 3cf62f3c0e..5a93f30c8a 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -21,23 +21,23 @@ #include <cmath> #include <optional> -CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, - int64_t _nTime, unsigned int _entryHeight, - bool _spendsCoinbase, int64_t _sigOpsCost, LockPoints lp) - : tx(_tx), nFee(_nFee), nTxWeight(GetTransactionWeight(*tx)), nUsageSize(RecursiveDynamicUsage(tx)), nTime(_nTime), entryHeight(_entryHeight), - spendsCoinbase(_spendsCoinbase), sigOpCost(_sigOpsCost), lockPoints(lp) -{ - nCountWithDescendants = 1; - nSizeWithDescendants = GetTxSize(); - nModFeesWithDescendants = nFee; - - feeDelta = 0; - - nCountWithAncestors = 1; - nSizeWithAncestors = GetTxSize(); - nModFeesWithAncestors = nFee; - nSigOpCostWithAncestors = sigOpCost; -} +CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, + int64_t time, unsigned int entry_height, + bool spends_coinbase, int64_t sigops_cost, LockPoints lp) + : tx{tx}, + nFee{fee}, + nTxWeight(GetTransactionWeight(*tx)), + nUsageSize{RecursiveDynamicUsage(tx)}, + nTime{time}, + entryHeight{entry_height}, + spendsCoinbase{spends_coinbase}, + sigOpCost{sigops_cost}, + lockPoints{lp}, + nSizeWithDescendants{GetTxSize()}, + nModFeesWithDescendants{nFee}, + nSizeWithAncestors{GetTxSize()}, + nModFeesWithAncestors{nFee}, + nSigOpCostWithAncestors{sigOpCost} {} void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta) { diff --git a/src/txmempool.h b/src/txmempool.h index d1308aeeed..27ee0628a7 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -37,19 +37,16 @@ extern RecursiveMutex cs_main; /** Fake height value used in Coin to signify they are only in the memory pool (since 0.8) */ static const uint32_t MEMPOOL_HEIGHT = 0x7FFFFFFF; -struct LockPoints -{ +struct LockPoints { // Will be set to the blockchain height and median time past // values that would be necessary to satisfy all relative locktime // constraints (BIP68) of this tx given our view of block chain history - int height; - int64_t time; + int height{0}; + int64_t time{0}; // As long as the current chain descends from the highest height block // containing one of the inputs used in the calculation, then the cached // values are still valid even after a reorg. - CBlockIndex* maxInputBlock; - - LockPoints() : height(0), time(0), maxInputBlock(nullptr) { } + CBlockIndex* maxInputBlock{nullptr}; }; struct CompareIteratorByHash { @@ -98,27 +95,27 @@ private: const unsigned int entryHeight; //!< Chain height when entering the mempool const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase const int64_t sigOpCost; //!< Total sigop cost - int64_t feeDelta; //!< Used for determining the priority of the transaction for mining in a block + int64_t feeDelta{0}; //!< Used for determining the priority of the transaction for mining in a block LockPoints lockPoints; //!< Track the height and time at which tx was final // Information about descendants of this transaction that are in the // mempool; if we remove this transaction we must remove all of these // descendants as well. - uint64_t nCountWithDescendants; //!< number of descendant transactions + uint64_t nCountWithDescendants{1}; //!< number of descendant transactions uint64_t nSizeWithDescendants; //!< ... and size CAmount nModFeesWithDescendants; //!< ... and total fees (all including us) // Analogous statistics for ancestor transactions - uint64_t nCountWithAncestors; + uint64_t nCountWithAncestors{1}; uint64_t nSizeWithAncestors; CAmount nModFeesWithAncestors; int64_t nSigOpCostWithAncestors; public: - CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, - int64_t _nTime, unsigned int _entryHeight, - bool spendsCoinbase, - int64_t nSigOpsCost, LockPoints lp); + CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, + int64_t time, unsigned int entry_height, + bool spends_coinbase, + int64_t sigops_cost, LockPoints lp); const CTransaction& GetTx() const { return *this->tx; } CTransactionRef GetSharedTx() const { return this->tx; } diff --git a/src/util/rbf.h b/src/util/rbf.h index 6d44a2cb83..aa522d8bfb 100644 --- a/src/util/rbf.h +++ b/src/util/rbf.h @@ -9,7 +9,7 @@ class CTransaction; -static const uint32_t MAX_BIP125_RBF_SEQUENCE = 0xfffffffd; +static constexpr uint32_t MAX_BIP125_RBF_SEQUENCE{0xfffffffd}; /** Check whether the sequence numbers on this transaction are signaling opt-in to replace-by-fee, * according to BIP 125. Allow opt-out of transaction replacement by setting nSequence > @@ -17,7 +17,7 @@ static const uint32_t MAX_BIP125_RBF_SEQUENCE = 0xfffffffd; * * SEQUENCE_FINAL-1 is picked to still allow use of nLockTime by non-replaceable transactions. All * inputs rather than just one is for the sake of multi-party protocols, where we don't want a single -* party to be able to disable replacement. */ -bool SignalsOptInRBF(const CTransaction &tx); +* party to be able to disable replacement by opting out in their own input. */ +bool SignalsOptInRBF(const CTransaction& tx); #endif // BITCOIN_UTIL_RBF_H diff --git a/src/validation.cpp b/src/validation.cpp index ed336a177f..a5842187e5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -24,6 +24,7 @@ #include <node/blockstorage.h> #include <node/coinstats.h> #include <node/ui_interface.h> +#include <node/utxo_snapshot.h> #include <policy/policy.h> #include <policy/rbf.h> #include <policy/settings.h> @@ -771,39 +772,43 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // pathological case by making sure setConflicts and setAncestors don't // intersect. if (const auto err_string{EntriesAndTxidsDisjoint(setAncestors, setConflicts, hash)}) { + // We classify this as a consensus error because a transaction depending on something it + // conflicts with would be inconsistent. return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", *err_string); } - // If we don't hold the lock allConflicting might be incomplete; the - // subsequent RemoveStaged() and addUnchecked() calls don't guarantee - // mempool consistency for us. fReplacementTransaction = setConflicts.size(); - if (fReplacementTransaction) - { + if (fReplacementTransaction) { CFeeRate newFeeRate(nModifiedFees, nSize); + // It's possible that the replacement pays more fees than its direct conflicts but not more + // than all conflicts (i.e. the direct conflicts have high-fee descendants). However, if the + // replacement doesn't pay more fees than its direct conflicts, then we can be sure it's not + // more economically rational to mine. Before we go digging through the mempool for all + // transactions that would need to be removed (direct conflicts and all descendants), check + // that the replacement transaction pays more than its direct conflicts. if (const auto err_string{PaysMoreThanConflicts(setIterConflicting, newFeeRate, hash)}) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); } - // Calculate all conflicting entries and enforce Rule #5. + // Calculate all conflicting entries and enforce BIP125 Rule #5. if (const auto err_string{GetEntriesForConflicts(tx, m_pool, setIterConflicting, allConflicting)}) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too many potential replacements", *err_string); } - // Enforce Rule #2. + // Enforce BIP125 Rule #2. if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, setIterConflicting)}) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "replacement-adds-unconfirmed", *err_string); } - // Check if it's economically rational to mine this transaction rather - // than the ones it replaces. Enforce Rules #3 and #4. + // Check if it's economically rational to mine this transaction rather than the ones it + // replaces and pays for its own relay fees. Enforce BIP125 Rules #3 and #4. for (CTxMemPool::txiter it : allConflicting) { nConflictingFees += it->GetModifiedFee(); nConflictingSize += it->GetTxSize(); } - if (const auto err_string{PaysForRBF(nConflictingFees, nModifiedFees, nSize, hash)}) { + if (const auto err_string{PaysForRBF(nConflictingFees, nModifiedFees, nSize, ::incrementalRelayFee, hash)}) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); } } @@ -1090,10 +1095,15 @@ void CoinsViews::InitCache() m_cacheview = std::make_unique<CCoinsViewCache>(&m_catcherview); } -CChainState::CChainState(CTxMemPool* mempool, BlockManager& blockman, std::optional<uint256> from_snapshot_blockhash) +CChainState::CChainState( + CTxMemPool* mempool, + BlockManager& blockman, + ChainstateManager& chainman, + std::optional<uint256> from_snapshot_blockhash) : m_mempool(mempool), m_params(::Params()), m_blockman(blockman), + m_chainman(chainman), m_from_snapshot_blockhash(from_snapshot_blockhash) {} void CChainState::InitCoinsDB( @@ -1567,7 +1577,6 @@ static int64_t nTimeForks = 0; static int64_t nTimeVerify = 0; static int64_t nTimeConnect = 0; static int64_t nTimeIndex = 0; -static int64_t nTimeCallbacks = 0; static int64_t nTimeTotal = 0; static int64_t nBlocksTotal = 0; @@ -1872,9 +1881,6 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, int64_t nTime5 = GetTimeMicros(); nTimeIndex += nTime5 - nTime4; LogPrint(BCLog::BENCH, " - Index writing: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5 - nTime4), nTimeIndex * MICRO, nTimeIndex * MILLI / nBlocksTotal); - int64_t nTime6 = GetTimeMicros(); nTimeCallbacks += nTime6 - nTime5; - LogPrint(BCLog::BENCH, " - Callbacks: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime6 - nTime5), nTimeCallbacks * MICRO, nTimeCallbacks * MILLI / nBlocksTotal); - TRACE7(validation, block_connected, block.GetHash().ToString().c_str(), pindex->nHeight, @@ -2089,8 +2095,42 @@ static void AppendWarning(bilingual_str& res, const bilingual_str& warn) res += warn; } +static void UpdateTipLog( + const CCoinsViewCache& coins_tip, + const CBlockIndex* tip, + const CChainParams& params, + const std::string& func_name, + const std::string& prefix, + const std::string& warning_messages) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +{ + + AssertLockHeld(::cs_main); + LogPrintf("%s%s: new best=%s height=%d version=0x%08x log2_work=%f tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n", + prefix, func_name, + tip->GetBlockHash().ToString(), tip->nHeight, tip->nVersion, + log(tip->nChainWork.getdouble()) / log(2.0), (unsigned long)tip->nChainTx, + FormatISO8601DateTime(tip->GetBlockTime()), + GuessVerificationProgress(params.TxData(), tip), + coins_tip.DynamicMemoryUsage() * (1.0 / (1 << 20)), + coins_tip.GetCacheSize(), + !warning_messages.empty() ? strprintf(" warning='%s'", warning_messages) : ""); +} + void CChainState::UpdateTip(const CBlockIndex* pindexNew) { + const auto& coins_tip = this->CoinsTip(); + + // The remainder of the function isn't relevant if we are not acting on + // the active chainstate, so return if need be. + if (this != &m_chainman.ActiveChainstate()) { + // Only log every so often so that we don't bury log messages at the tip. + constexpr int BACKGROUND_LOG_INTERVAL = 2000; + if (pindexNew->nHeight % BACKGROUND_LOG_INTERVAL == 0) { + UpdateTipLog(coins_tip, pindexNew, m_params, __func__, "[background validation] ", ""); + } + return; + } + // New best block if (m_mempool) { m_mempool->AddTransactionsUpdated(1); @@ -2118,12 +2158,7 @@ void CChainState::UpdateTip(const CBlockIndex* pindexNew) } } } - LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%f tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n", __func__, - pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, pindexNew->nVersion, - log(pindexNew->nChainWork.getdouble())/log(2.0), (unsigned long)pindexNew->nChainTx, - FormatISO8601DateTime(pindexNew->GetBlockTime()), - GuessVerificationProgress(m_params.TxData(), pindexNew), this->CoinsTip().DynamicMemoryUsage() * (1.0 / (1<<20)), this->CoinsTip().GetCacheSize(), - !warning_messages.empty() ? strprintf(" warning='%s'", warning_messages.original) : ""); + UpdateTipLog(coins_tip, pindexNew, m_params, __func__, "", warning_messages.original); } /** Disconnect m_chain's tip. @@ -3620,7 +3655,9 @@ bool BlockManager::LoadBlockIndex( pindex->nStatus |= BLOCK_FAILED_CHILD; setDirtyBlockIndex.insert(pindex); } - if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr)) { + if (pindex->IsAssumedValid() || + (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && + (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr))) { block_index_candidates.insert(pindex); } if (pindex->nStatus & BLOCK_FAILED_MASK && (!pindexBestInvalid || pindex->nChainWork > pindexBestInvalid->nChainWork)) @@ -4199,12 +4236,33 @@ void CChainState::CheckBlockIndex() while (pindex != nullptr) { nNodes++; if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex; - if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA)) pindexFirstMissing = pindex; + // Assumed-valid index entries will not have data since we haven't downloaded the + // full block yet. + if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA) && !pindex->IsAssumedValid()) { + pindexFirstMissing = pindex; + } if (pindexFirstNeverProcessed == nullptr && pindex->nTx == 0) pindexFirstNeverProcessed = pindex; if (pindex->pprev != nullptr && pindexFirstNotTreeValid == nullptr && (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TREE) pindexFirstNotTreeValid = pindex; - if (pindex->pprev != nullptr && pindexFirstNotTransactionsValid == nullptr && (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS) pindexFirstNotTransactionsValid = pindex; - if (pindex->pprev != nullptr && pindexFirstNotChainValid == nullptr && (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_CHAIN) pindexFirstNotChainValid = pindex; - if (pindex->pprev != nullptr && pindexFirstNotScriptsValid == nullptr && (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_SCRIPTS) pindexFirstNotScriptsValid = pindex; + + if (pindex->pprev != nullptr && !pindex->IsAssumedValid()) { + // Skip validity flag checks for BLOCK_ASSUMED_VALID index entries, since these + // *_VALID_MASK flags will not be present for index entries we are temporarily assuming + // valid. + if (pindexFirstNotTransactionsValid == nullptr && + (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS) { + pindexFirstNotTransactionsValid = pindex; + } + + if (pindexFirstNotChainValid == nullptr && + (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_CHAIN) { + pindexFirstNotChainValid = pindex; + } + + if (pindexFirstNotScriptsValid == nullptr && + (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_SCRIPTS) { + pindexFirstNotScriptsValid = pindex; + } + } // Begin: actual consistency checks. if (pindex->pprev == nullptr) { @@ -4215,7 +4273,9 @@ void CChainState::CheckBlockIndex() if (!pindex->HaveTxsDownloaded()) assert(pindex->nSequenceId <= 0); // nSequenceId can't be set positive for blocks that aren't linked (negative is used for preciousblock) // VALID_TRANSACTIONS is equivalent to nTx > 0 for all nodes (whether or not pruning has occurred). // HAVE_DATA is only equivalent to nTx > 0 (or VALID_TRANSACTIONS) if no pruning has occurred. - if (!fHavePruned) { + // Unless these indexes are assumed valid and pending block download on a + // background chainstate. + if (!fHavePruned && !pindex->IsAssumedValid()) { // If we've never pruned, then HAVE_DATA should be equivalent to nTx > 0 assert(!(pindex->nStatus & BLOCK_HAVE_DATA) == (pindex->nTx == 0)); assert(pindexFirstMissing == pindexFirstNeverProcessed); @@ -4224,7 +4284,16 @@ void CChainState::CheckBlockIndex() if (pindex->nStatus & BLOCK_HAVE_DATA) assert(pindex->nTx > 0); } if (pindex->nStatus & BLOCK_HAVE_UNDO) assert(pindex->nStatus & BLOCK_HAVE_DATA); - assert(((pindex->nStatus & BLOCK_VALID_MASK) >= BLOCK_VALID_TRANSACTIONS) == (pindex->nTx > 0)); // This is pruning-independent. + if (pindex->IsAssumedValid()) { + // Assumed-valid blocks should have some nTx value. + assert(pindex->nTx > 0); + // Assumed-valid blocks should connect to the main chain. + assert((pindex->nStatus & BLOCK_VALID_MASK) >= BLOCK_VALID_TREE); + } else { + // Otherwise there should only be an nTx value if we have + // actually seen a block's transactions. + assert(((pindex->nStatus & BLOCK_VALID_MASK) >= BLOCK_VALID_TRANSACTIONS) == (pindex->nTx > 0)); // This is pruning-independent. + } // All parents having had data (at some point) is equivalent to all parents being VALID_TRANSACTIONS, which is equivalent to HaveTxsDownloaded(). assert((pindexFirstNeverProcessed == nullptr) == pindex->HaveTxsDownloaded()); assert((pindexFirstNotTransactionsValid == nullptr) == pindex->HaveTxsDownloaded()); @@ -4241,11 +4310,17 @@ void CChainState::CheckBlockIndex() } if (!CBlockIndexWorkComparator()(pindex, m_chain.Tip()) && pindexFirstNeverProcessed == nullptr) { if (pindexFirstInvalid == nullptr) { + const bool is_active = this == &m_chainman.ActiveChainstate(); + // If this block sorts at least as good as the current tip and // is valid and we have all data for its parents, it must be in // setBlockIndexCandidates. m_chain.Tip() must also be there // even if some data has been pruned. - if (pindexFirstMissing == nullptr || pindex == m_chain.Tip()) { + // + // Don't perform this check for the background chainstate since + // its setBlockIndexCandidates shouldn't have some entries (i.e. those past the + // snapshot block) which do exist in the block index for the active chainstate. + if (is_active && (pindexFirstMissing == nullptr || pindex == m_chain.Tip())) { assert(setBlockIndexCandidates.count(pindex)); } // If some parent is missing, then it could be that this block was in @@ -4580,7 +4655,7 @@ CChainState& ChainstateManager::InitializeChainstate( if (to_modify) { throw std::logic_error("should not be overwriting a chainstate"); } - to_modify.reset(new CChainState(mempool, m_blockman, snapshot_blockhash)); + to_modify.reset(new CChainState(mempool, m_blockman, *this, snapshot_blockhash)); // Snapshot chainstates and initial IBD chaintates always become active. if (is_snapshot || (!is_snapshot && !m_active_chainstate)) { @@ -4649,8 +4724,9 @@ bool ChainstateManager::ActivateSnapshot( static_cast<size_t>(current_coinsdb_cache_size * IBD_CACHE_PERC)); } - auto snapshot_chainstate = WITH_LOCK(::cs_main, return std::make_unique<CChainState>( - /* mempool */ nullptr, m_blockman, base_blockhash)); + auto snapshot_chainstate = WITH_LOCK(::cs_main, + return std::make_unique<CChainState>( + /* mempool */ nullptr, m_blockman, *this, base_blockhash)); { LOCK(::cs_main); @@ -4853,11 +4929,25 @@ bool ChainstateManager::PopulateAndValidateSnapshot( // Fake nChainTx so that GuessVerificationProgress reports accurately index->nChainTx = index->pprev ? index->pprev->nChainTx + index->nTx : 1; + // Mark unvalidated block index entries beneath the snapshot base block as assumed-valid. + if (!index->IsValid(BLOCK_VALID_SCRIPTS)) { + // This flag will be removed once the block is fully validated by a + // background chainstate. + index->nStatus |= BLOCK_ASSUMED_VALID; + } + // Fake BLOCK_OPT_WITNESS so that CChainState::NeedsRedownload() // won't ask to rewind the entire assumed-valid chain on startup. if (index->pprev && DeploymentActiveAt(*index, ::Params().GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { index->nStatus |= BLOCK_OPT_WITNESS; } + + setDirtyBlockIndex.insert(index); + // Changes to the block index will be flushed to disk after this call + // returns in `ActivateSnapshot()`, when `MaybeRebalanceCaches()` is + // called, since we've added a snapshot chainstate and therefore will + // have to downsize the IBD chainstate, which will result in a call to + // `FlushStateToDisk(ALWAYS)`. } assert(index); @@ -4882,22 +4972,6 @@ bool ChainstateManager::IsSnapshotActive() const return m_snapshot_chainstate && m_active_chainstate == m_snapshot_chainstate.get(); } -CChainState& ChainstateManager::ValidatedChainstate() const -{ - LOCK(::cs_main); - if (m_snapshot_chainstate && IsSnapshotValidated()) { - return *m_snapshot_chainstate.get(); - } - assert(m_ibd_chainstate); - return *m_ibd_chainstate.get(); -} - -bool ChainstateManager::IsBackgroundIBD(CChainState* chainstate) const -{ - LOCK(::cs_main); - return (m_snapshot_chainstate && chainstate == m_ibd_chainstate.get()); -} - void ChainstateManager::Unload() { for (CChainState* chainstate : this->GetAll()) { diff --git a/src/validation.h b/src/validation.h index 078b988052..b2282828ce 100644 --- a/src/validation.h +++ b/src/validation.h @@ -14,16 +14,10 @@ #include <arith_uint256.h> #include <attributes.h> #include <chain.h> -#include <coins.h> -#include <consensus/validation.h> -#include <crypto/common.h> // for ReadLE64 #include <fs.h> -#include <node/utxo_snapshot.h> #include <policy/feerate.h> #include <policy/packages.h> -#include <protocol.h> // For CMessageHeader::MessageStartChars #include <script/script_error.h> -#include <serialize.h> #include <sync.h> #include <txdb.h> #include <txmempool.h> // For CTxMemPool::cs @@ -44,18 +38,13 @@ #include <vector> class CChainState; -class BlockValidationState; class CBlockTreeDB; -class CBlockUndo; class CChainParams; struct CCheckpointData; -class CInv; -class CConnman; -class CScriptCheck; class CTxMemPool; class ChainstateManager; +class SnapshotMetadata; struct ChainTxData; - struct DisconnectedBlockTransactions; struct PrecomputedTransactionData; struct LockPoints; @@ -111,6 +100,7 @@ extern RecursiveMutex cs_main; typedef std::unordered_map<uint256, CBlockIndex*, BlockHasher> BlockMap; extern Mutex g_best_block_mutex; extern std::condition_variable g_best_block_cv; +/** Used to notify getblocktemplate RPC of new tips. */ extern uint256 g_best_block; /** Whether there are dedicated script-checking threads running. * False indicates all script checking is done on the main threadMessageHandler thread. @@ -595,9 +585,15 @@ public: //! CChainState instances. BlockManager& m_blockman; + //! The chainstate manager that owns this chainstate. The reference is + //! necessary so that this instance can check whether it is the active + //! chainstate within deeply nested method calls. + ChainstateManager& m_chainman; + explicit CChainState( CTxMemPool* mempool, BlockManager& blockman, + ChainstateManager& chainman, std::optional<uint256> from_snapshot_blockhash = std::nullopt); /** @@ -634,9 +630,10 @@ public: const std::optional<uint256> m_from_snapshot_blockhash; /** - * The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and - * as good as our current tip or better. Entries may be failed, though, and pruning nodes may be - * missing the data for the block. + * The set of all CBlockIndex entries with either BLOCK_VALID_TRANSACTIONS (for + * itself and all ancestors) *or* BLOCK_ASSUMED_VALID (if using background + * chainstates) and as good as our current tip or better. Entries may be failed, + * though, and pruning nodes may be missing the data for the block. */ std::set<CBlockIndex*, CBlockIndexWorkComparator> setBlockIndexCandidates; @@ -848,12 +845,6 @@ private: * *Background IBD chainstate*: an IBD chainstate for which the * IBD process is happening in the background while use of the * active (snapshot) chainstate allows the rest of the system to function. - * - * *Validated chainstate*: the most-work chainstate which has been validated - * locally via initial block download. This will be the snapshot chainstate - * if a snapshot was loaded and all blocks up to the snapshot starting point - * have been downloaded and validated (via background validation), otherwise - * it will be the IBD chainstate. */ class ChainstateManager { @@ -972,19 +963,6 @@ public: //! Is there a snapshot in use and has it been fully validated? bool IsSnapshotValidated() const { return m_snapshot_validated; } - //! @returns true if this chainstate is being used to validate an active - //! snapshot in the background. - bool IsBackgroundIBD(CChainState* chainstate) const; - - //! Return the most-work chainstate that has been fully validated. - //! - //! During background validation of a snapshot, this is the IBD chain. After - //! background validation has completed, this is the snapshot chain. - CChainState& ValidatedChainstate() const; - - CChain& ValidatedChain() const { return ValidatedChainstate().m_chain; } - CBlockIndex* ValidatedTip() const { return ValidatedChain().Tip(); } - /** * Process an incoming block. This only returns after the best known valid * block is made active. Note that it does not, however, guarantee that the diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 9a8c1e3c02..d9fc6de79b 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -214,15 +214,17 @@ public: LOCK(m_wallet->cs_wallet); return m_wallet->DisplayAddress(dest); } - void lockCoin(const COutPoint& output) override + bool lockCoin(const COutPoint& output, const bool write_to_db) override { LOCK(m_wallet->cs_wallet); - return m_wallet->LockCoin(output); + std::unique_ptr<WalletBatch> batch = write_to_db ? std::make_unique<WalletBatch>(m_wallet->GetDatabase()) : nullptr; + return m_wallet->LockCoin(output, batch.get()); } - void unlockCoin(const COutPoint& output) override + bool unlockCoin(const COutPoint& output) override { LOCK(m_wallet->cs_wallet); - return m_wallet->UnlockCoin(output); + std::unique_ptr<WalletBatch> batch = std::make_unique<WalletBatch>(m_wallet->GetDatabase()); + return m_wallet->UnlockCoin(output, batch.get()); } bool isLockedCoin(const COutPoint& output) override { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7d194ae262..c101f6560d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -472,7 +472,7 @@ static RPCHelpMan sendtoaddress() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, - {RPCResult::Type::STR, "fee reason", "The transaction fee reason."} + {RPCResult::Type::STR, "fee_reason", "The transaction fee reason."} }, }, }, @@ -899,7 +899,7 @@ static RPCHelpMan sendmany() { {RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n" "the number of addresses."}, - {RPCResult::Type::STR, "fee reason", "The transaction fee reason."} + {RPCResult::Type::STR, "fee_reason", "The transaction fee reason."} }, }, }, @@ -1213,7 +1213,7 @@ static RPCHelpMan listreceivedbyaddress() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction"}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR, "address", "The receiving address"}, {RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " received by the address"}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations of the most recent transaction included"}, @@ -1261,7 +1261,7 @@ static RPCHelpMan listreceivedbylabel() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction"}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR_AMOUNT, "amount", "The total amount received by addresses with this label"}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations of the most recent transaction included"}, {RPCResult::Type::STR, "label", "The label of the receiving address. The default label is \"\""}, @@ -1388,20 +1388,24 @@ static const std::vector<RPCResult> TransactionDescriptionString() { return{{RPCResult::Type::NUM, "confirmations", "The number of confirmations for the transaction. Negative confirmations means the\n" "transaction conflicted that many blocks ago."}, - {RPCResult::Type::BOOL, "generated", "Only present if transaction only input is a coinbase one."}, - {RPCResult::Type::BOOL, "trusted", "Only present if we consider transaction to be trusted and so safe to spend from."}, - {RPCResult::Type::STR_HEX, "blockhash", "The block hash containing the transaction."}, - {RPCResult::Type::NUM, "blockheight", "The block height containing the transaction."}, - {RPCResult::Type::NUM, "blockindex", "The index of the transaction in the block that includes it."}, - {RPCResult::Type::NUM_TIME, "blocktime", "The block time expressed in " + UNIX_EPOCH_TIME + "."}, + {RPCResult::Type::BOOL, "generated", /* optional */ true, "Only present if transaction only input is a coinbase one."}, + {RPCResult::Type::BOOL, "trusted", /* optional */ true, "Only present if we consider transaction to be trusted and so safe to spend from."}, + {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "The block hash containing the transaction."}, + {RPCResult::Type::NUM, "blockheight", /* optional */ true, "The block height containing the transaction."}, + {RPCResult::Type::NUM, "blockindex", /* optional */ true, "The index of the transaction in the block that includes it."}, + {RPCResult::Type::NUM_TIME, "blocktime", /* optional */ true, "The block time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, {RPCResult::Type::ARR, "walletconflicts", "Conflicting transaction ids.", { {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, }}, + {RPCResult::Type::STR_HEX, "replaced_by_txid", /* optional */ true, "The txid if this tx was replaced."}, + {RPCResult::Type::STR_HEX, "replaces_txid", /* optional */ true, "The txid if the tx replaces one."}, + {RPCResult::Type::STR, "comment", /* optional */ true, ""}, + {RPCResult::Type::STR, "to", /* optional */ true, "If a comment to is associated with the transaction."}, {RPCResult::Type::NUM_TIME, "time", "The transaction time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + "."}, - {RPCResult::Type::STR, "comment", "If a comment is associated with the transaction, only present if not empty."}, + {RPCResult::Type::STR, "comment", /* optional */ true, "If a comment is associated with the transaction, only present if not empty."}, {RPCResult::Type::STR, "bip125-replaceable", "(\"yes|no|unknown\") Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" "may be unknown for unconfirmed transactions not in the mempool"}}; } @@ -1423,7 +1427,7 @@ static RPCHelpMan listtransactions() { {RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>( { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction."}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction."}, {RPCResult::Type::STR, "address", "The bitcoin address of the transaction."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" @@ -1433,14 +1437,14 @@ static RPCHelpMan listtransactions() "\"orphan\" Orphaned coinbase transactions received."}, {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n" "for all other categories"}, - {RPCResult::Type::STR, "label", "A comment for the address/transaction, if any"}, + {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR_AMOUNT, "fee", "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" + {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" "'send' category of transactions."}, }, TransactionDescriptionString()), { - {RPCResult::Type::BOOL, "abandoned", "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, })}, } @@ -1537,7 +1541,7 @@ static RPCHelpMan listsinceblock() { {RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>( { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction."}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction."}, {RPCResult::Type::STR, "address", "The bitcoin address of the transaction."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" @@ -1548,18 +1552,17 @@ static RPCHelpMan listsinceblock() {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n" "for all other categories"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR_AMOUNT, "fee", "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" + {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" "'send' category of transactions."}, }, TransactionDescriptionString()), { - {RPCResult::Type::BOOL, "abandoned", "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, - {RPCResult::Type::STR, "label", "A comment for the address/transaction, if any"}, - {RPCResult::Type::STR, "to", "If a comment to is associated with the transaction."}, + {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any"}, })}, }}, - {RPCResult::Type::ARR, "removed", "<structure is the same as \"transactions\" above, only present if include_removed=true>\n" + {RPCResult::Type::ARR, "removed", /* optional */ true, "<structure is the same as \"transactions\" above, only present if include_removed=true>\n" "Note: transactions that were re-added in the active chain will appear as-is in this array, and may thus have a positive confirmation count." , {{RPCResult::Type::ELISION, "", ""},}}, {RPCResult::Type::STR_HEX, "lastblock", "The hash of the block (target_confirmations-1) from the best block on the main chain, or the genesis hash if the referenced block does not exist yet. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones"}, @@ -1672,7 +1675,7 @@ static RPCHelpMan gettransaction() RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>( { {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT}, - {RPCResult::Type::STR_AMOUNT, "fee", "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" + {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" "'send' category of transactions."}, }, TransactionDescriptionString()), @@ -1681,8 +1684,8 @@ static RPCHelpMan gettransaction() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction."}, - {RPCResult::Type::STR, "address", "The bitcoin address involved in the transaction."}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction."}, + {RPCResult::Type::STR, "address", /* optional */ true, "The bitcoin address involved in the transaction."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" "\"receive\" Non-coinbase transactions received.\n" @@ -1690,16 +1693,16 @@ static RPCHelpMan gettransaction() "\"immature\" Coinbase transactions received with 100 or fewer confirmations.\n" "\"orphan\" Orphaned coinbase transactions received."}, {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT}, - {RPCResult::Type::STR, "label", "A comment for the address/transaction, if any"}, + {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR_AMOUNT, "fee", "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" + {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" "'send' category of transactions."}, - {RPCResult::Type::BOOL, "abandoned", "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, }}, }}, {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"}, - {RPCResult::Type::OBJ, "decoded", "Optional, the decoded transaction (only present when `verbose` is passed)", + {RPCResult::Type::OBJ, "decoded", /* optional */ true, "The decoded transaction (only present when `verbose` is passed)", { {RPCResult::Type::ELISION, "", "Equivalent to the RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."}, }}, @@ -2137,8 +2140,9 @@ static RPCHelpMan lockunspent() "If no transaction outputs are specified when unlocking then all current locked transaction outputs are unlocked.\n" "A locked transaction output will not be chosen by automatic coin selection, when spending bitcoins.\n" "Manually selected coins are automatically unlocked.\n" - "Locks are stored in memory only. Nodes start with zero locked outputs, and the locked output list\n" - "is always cleared (by virtue of process exit) when a node stops or fails.\n" + "Locks are stored in memory only, unless persistent=true, in which case they will be written to the\n" + "wallet database and loaded on node start. Unwritten (persistent=false) locks are always cleared\n" + "(by virtue of process exit) when a node stops or fails. Unlocking will clear both persistent and not.\n" "Also see the listunspent call\n", { {"unlock", RPCArg::Type::BOOL, RPCArg::Optional::NO, "Whether to unlock (true) or lock (false) the specified transactions"}, @@ -2152,6 +2156,7 @@ static RPCHelpMan lockunspent() }, }, }, + {"persistent", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether to write/erase this lock in the wallet database, or keep the change in memory only. Ignored for unlocking."}, }, RPCResult{ RPCResult::Type::BOOL, "", "Whether the command was successful or not" @@ -2165,6 +2170,8 @@ static RPCHelpMan lockunspent() + HelpExampleCli("listlockunspent", "") + "\nUnlock the transaction again\n" + HelpExampleCli("lockunspent", "true \"[{\\\"txid\\\":\\\"a08e6907dbbd3d809776dbfc5d82e371b764ed838b5655e72f463568df1aadf0\\\",\\\"vout\\\":1}]\"") + + "\nLock the transaction persistently in the wallet database\n" + + HelpExampleCli("lockunspent", "false \"[{\\\"txid\\\":\\\"a08e6907dbbd3d809776dbfc5d82e371b764ed838b5655e72f463568df1aadf0\\\",\\\"vout\\\":1}]\" true") + "\nAs a JSON-RPC call\n" + HelpExampleRpc("lockunspent", "false, \"[{\\\"txid\\\":\\\"a08e6907dbbd3d809776dbfc5d82e371b764ed838b5655e72f463568df1aadf0\\\",\\\"vout\\\":1}]\"") }, @@ -2183,9 +2190,13 @@ static RPCHelpMan lockunspent() bool fUnlock = request.params[0].get_bool(); + const bool persistent{request.params[2].isNull() ? false : request.params[2].get_bool()}; + if (request.params[1].isNull()) { - if (fUnlock) - pwallet->UnlockAllCoins(); + if (fUnlock) { + if (!pwallet->UnlockAllCoins()) + throw JSONRPCError(RPC_WALLET_ERROR, "Unlocking coins failed"); + } return true; } @@ -2236,17 +2247,24 @@ static RPCHelpMan lockunspent() throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected locked output"); } - if (!fUnlock && is_locked) { + if (!fUnlock && is_locked && !persistent) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output already locked"); } outputs.push_back(outpt); } + std::unique_ptr<WalletBatch> batch = nullptr; + // Unlock is always persistent + if (fUnlock || persistent) batch = std::make_unique<WalletBatch>(pwallet->GetDatabase()); + // Atomically set (un)locked status for the outputs. for (const COutPoint& outpt : outputs) { - if (fUnlock) pwallet->UnlockCoin(outpt); - else pwallet->LockCoin(outpt); + if (fUnlock) { + if (!pwallet->UnlockCoin(outpt, batch.get())) throw JSONRPCError(RPC_WALLET_ERROR, "Unlocking coin failed"); + } else { + if (!pwallet->LockCoin(outpt, batch.get())) throw JSONRPCError(RPC_WALLET_ERROR, "Locking coin failed"); + } } return true; @@ -2362,9 +2380,9 @@ static RPCHelpMan getbalances() {RPCResult::Type::STR_AMOUNT, "trusted", "trusted balance (outputs created by the wallet or confirmed outputs)"}, {RPCResult::Type::STR_AMOUNT, "untrusted_pending", "untrusted pending balance (outputs created by others that are in the mempool)"}, {RPCResult::Type::STR_AMOUNT, "immature", "balance from immature coinbase outputs"}, - {RPCResult::Type::STR_AMOUNT, "used", "(only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)"}, + {RPCResult::Type::STR_AMOUNT, "used", /* optional */ true, "(only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)"}, }}, - {RPCResult::Type::OBJ, "watchonly", "watchonly balances (not present if wallet does not watch anything)", + {RPCResult::Type::OBJ, "watchonly", /* optional */ true, "watchonly balances (not present if wallet does not watch anything)", { {RPCResult::Type::STR_AMOUNT, "trusted", "trusted balance (outputs created by the wallet or confirmed outputs)"}, {RPCResult::Type::STR_AMOUNT, "untrusted_pending", "untrusted pending balance (outputs created by others that are in the mempool)"}, @@ -2431,9 +2449,9 @@ static RPCHelpMan getwalletinfo() {RPCResult::Type::STR_AMOUNT, "unconfirmed_balance", "DEPRECATED. Identical to getbalances().mine.untrusted_pending"}, {RPCResult::Type::STR_AMOUNT, "immature_balance", "DEPRECATED. Identical to getbalances().mine.immature"}, {RPCResult::Type::NUM, "txcount", "the total number of transactions in the wallet"}, - {RPCResult::Type::NUM_TIME, "keypoololdest", "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool. Legacy wallets only."}, + {RPCResult::Type::NUM_TIME, "keypoololdest", /* optional */ true, "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool. Legacy wallets only."}, {RPCResult::Type::NUM, "keypoolsize", "how many new keys are pre-generated (only counts external keys)"}, - {RPCResult::Type::NUM, "keypoolsize_hd_internal", "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"}, + {RPCResult::Type::NUM, "keypoolsize_hd_internal", /* optional */ true, "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"}, {RPCResult::Type::NUM_TIME, "unlocked_until", /* optional */ true, "the " + UNIX_EPOCH_TIME + " until which the wallet is unlocked for transfers, or 0 if the wallet is locked (only present for passphrase-encrypted wallets)"}, {RPCResult::Type::STR_AMOUNT, "paytxfee", "the transaction fee configuration, set in " + CURRENCY_UNIT + "/kvB"}, {RPCResult::Type::STR_HEX, "hdseedid", /* optional */ true, "the Hash160 of the HD seed (only present when HD is enabled)"}, @@ -2955,20 +2973,20 @@ static RPCHelpMan listunspent() { {RPCResult::Type::STR_HEX, "txid", "the transaction id"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR, "address", "the bitcoin address"}, - {RPCResult::Type::STR, "label", "The associated label, or \"\" for the default label"}, + {RPCResult::Type::STR, "address", /* optional */ true, "the bitcoin address"}, + {RPCResult::Type::STR, "label", /* optional */ true, "The associated label, or \"\" for the default label"}, {RPCResult::Type::STR, "scriptPubKey", "the script key"}, {RPCResult::Type::STR_AMOUNT, "amount", "the transaction output amount in " + CURRENCY_UNIT}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations"}, {RPCResult::Type::NUM, "ancestorcount", /* optional */ true, "The number of in-mempool ancestor transactions, including this one (if transaction is in the mempool)"}, {RPCResult::Type::NUM, "ancestorsize", /* optional */ true, "The virtual transaction size of in-mempool ancestors, including this one (if transaction is in the mempool)"}, {RPCResult::Type::STR_AMOUNT, "ancestorfees", /* optional */ true, "The total fees of in-mempool ancestors (including this one) with fee deltas used for mining priority in " + CURRENCY_ATOM + " (if transaction is in the mempool)"}, - {RPCResult::Type::STR_HEX, "redeemScript", "The redeemScript if scriptPubKey is P2SH"}, - {RPCResult::Type::STR, "witnessScript", "witnessScript if the scriptPubKey is P2WSH or P2SH-P2WSH"}, + {RPCResult::Type::STR_HEX, "redeemScript", /* optional */ true, "The redeemScript if scriptPubKey is P2SH"}, + {RPCResult::Type::STR, "witnessScript", /* optional */ true, "witnessScript if the scriptPubKey is P2WSH or P2SH-P2WSH"}, {RPCResult::Type::BOOL, "spendable", "Whether we have the private keys to spend this output"}, {RPCResult::Type::BOOL, "solvable", "Whether we know how to spend this output, ignoring the lack of keys"}, - {RPCResult::Type::BOOL, "reused", "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"}, - {RPCResult::Type::STR, "desc", "(only when solvable) A descriptor for spending this output"}, + {RPCResult::Type::BOOL, "reused", /* optional */ true, "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"}, + {RPCResult::Type::STR, "desc", /* optional */ true, "(only when solvable) A descriptor for spending this output"}, {RPCResult::Type::BOOL, "safe", "Whether this output is considered safe to spend. Unconfirmed transactions\n" "from outside keys and unconfirmed replacement transactions are considered unsafe\n" "and are not eligible for spending by fundrawtransaction and sendtoaddress."}, @@ -3442,6 +3460,10 @@ RPCHelpMan signrawtransactionwithwallet() { {RPCResult::Type::STR_HEX, "txid", "The hash of the referenced, previous transaction"}, {RPCResult::Type::NUM, "vout", "The index of the output to spent and used as input"}, + {RPCResult::Type::ARR, "witness", "", + { + {RPCResult::Type::STR_HEX, "witness", ""}, + }}, {RPCResult::Type::STR_HEX, "scriptSig", "The hex-encoded signature script"}, {RPCResult::Type::NUM, "sequence", "Script sequence number"}, {RPCResult::Type::STR, "error", "Verification or signing error related to the input"}, @@ -4188,9 +4210,9 @@ static RPCHelpMan send() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, - {RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of the number of addresses."}, - {RPCResult::Type::STR_HEX, "hex", "If add_to_wallet is false, the hex-encoded raw transaction with signature(s)"}, - {RPCResult::Type::STR, "psbt", "If more signatures are needed, or if add_to_wallet is false, the base64-encoded (partially) signed transaction"} + {RPCResult::Type::STR_HEX, "txid", /* optional */ true, "The transaction id for the send. Only 1 transaction is created regardless of the number of addresses."}, + {RPCResult::Type::STR_HEX, "hex", /* optional */ true, "If add_to_wallet is false, the hex-encoded raw transaction with signature(s)"}, + {RPCResult::Type::STR, "psbt", /* optional */ true, "If more signatures are needed, or if add_to_wallet is false, the base64-encoded (partially) signed transaction"} } }, RPCExamples{"" diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 70349b2455..a9610d05a4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -589,11 +589,16 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const return false; } -void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid) +void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid, WalletBatch* batch) { mapTxSpends.insert(std::make_pair(outpoint, wtxid)); - setLockedCoins.erase(outpoint); + if (batch) { + UnlockCoin(outpoint, batch); + } else { + WalletBatch temp_batch(GetDatabase()); + UnlockCoin(outpoint, &temp_batch); + } std::pair<TxSpends::iterator, TxSpends::iterator> range; range = mapTxSpends.equal_range(outpoint); @@ -601,7 +606,7 @@ void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid) } -void CWallet::AddToSpends(const uint256& wtxid) +void CWallet::AddToSpends(const uint256& wtxid, WalletBatch* batch) { auto it = mapWallet.find(wtxid); assert(it != mapWallet.end()); @@ -610,7 +615,7 @@ void CWallet::AddToSpends(const uint256& wtxid) return; for (const CTxIn& txin : thisTx.tx->vin) - AddToSpends(txin.prevout, wtxid); + AddToSpends(txin.prevout, wtxid, batch); } bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) @@ -910,7 +915,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); wtx.nTimeSmart = ComputeTimeSmart(wtx); - AddToSpends(hash); + AddToSpends(hash, &batch); } if (!fInsertedNew) @@ -2260,22 +2265,36 @@ bool CWallet::DisplayAddress(const CTxDestination& dest) return signer_spk_man->DisplayAddress(scriptPubKey, signer); } -void CWallet::LockCoin(const COutPoint& output) +bool CWallet::LockCoin(const COutPoint& output, WalletBatch* batch) { AssertLockHeld(cs_wallet); setLockedCoins.insert(output); + if (batch) { + return batch->WriteLockedUTXO(output); + } + return true; } -void CWallet::UnlockCoin(const COutPoint& output) +bool CWallet::UnlockCoin(const COutPoint& output, WalletBatch* batch) { AssertLockHeld(cs_wallet); - setLockedCoins.erase(output); + bool was_locked = setLockedCoins.erase(output); + if (batch && was_locked) { + return batch->EraseLockedUTXO(output); + } + return true; } -void CWallet::UnlockAllCoins() +bool CWallet::UnlockAllCoins() { AssertLockHeld(cs_wallet); + bool success = true; + WalletBatch batch(GetDatabase()); + for (auto it = setLockedCoins.begin(); it != setLockedCoins.end(); ++it) { + success &= batch.EraseLockedUTXO(*it); + } setLockedCoins.clear(); + return success; } bool CWallet::IsLockedCoin(uint256 hash, unsigned int n) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2dc9eff712..6b4bcf31c4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -256,8 +256,8 @@ private: */ typedef std::multimap<COutPoint, uint256> TxSpends; TxSpends mapTxSpends GUARDED_BY(cs_wallet); - void AddToSpends(const COutPoint& outpoint, const uint256& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void AddToSpends(const uint256& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void AddToSpends(const COutPoint& outpoint, const uint256& wtxid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void AddToSpends(const uint256& wtxid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Add a transaction to the wallet, or update it. pIndex and posInBlock should @@ -449,9 +449,9 @@ public: bool DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void UnlockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void UnlockAllCoins() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool LockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool UnlockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool UnlockAllCoins() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void ListLockedCoins(std::vector<COutPoint>& vOutpts) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /* diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 03464cd2c8..c697534c06 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -40,6 +40,7 @@ const std::string FLAGS{"flags"}; const std::string HDCHAIN{"hdchain"}; const std::string KEYMETA{"keymeta"}; const std::string KEY{"key"}; +const std::string LOCKED_UTXO{"lockedutxo"}; const std::string MASTER_KEY{"mkey"}; const std::string MINVERSION{"minversion"}; const std::string NAME{"name"}; @@ -284,6 +285,16 @@ bool WalletBatch::WriteDescriptorCacheItems(const uint256& desc_id, const Descri return true; } +bool WalletBatch::WriteLockedUTXO(const COutPoint& output) +{ + return WriteIC(std::make_pair(DBKeys::LOCKED_UTXO, std::make_pair(output.hash, output.n)), uint8_t{'1'}); +} + +bool WalletBatch::EraseLockedUTXO(const COutPoint& output) +{ + return EraseIC(std::make_pair(DBKeys::LOCKED_UTXO, std::make_pair(output.hash, output.n))); +} + class CWalletScanState { public: unsigned int nKeys{0}; @@ -701,6 +712,12 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, wss.m_descriptor_crypt_keys.insert(std::make_pair(std::make_pair(desc_id, pubkey.GetID()), std::make_pair(pubkey, privkey))); wss.fIsEncrypted = true; + } else if (strType == DBKeys::LOCKED_UTXO) { + uint256 hash; + uint32_t n; + ssKey >> hash; + ssKey >> n; + pwallet->LockCoin(COutPoint(hash, n)); } else if (strType != DBKeys::BESTBLOCK && strType != DBKeys::BESTBLOCK_NOMERKLE && strType != DBKeys::MINVERSION && strType != DBKeys::ACENTRY && strType != DBKeys::VERSION && strType != DBKeys::SETTINGS && diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 25c2ec5909..a549c8039c 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -65,6 +65,7 @@ extern const std::string FLAGS; extern const std::string HDCHAIN; extern const std::string KEY; extern const std::string KEYMETA; +extern const std::string LOCKED_UTXO; extern const std::string MASTER_KEY; extern const std::string MINVERSION; extern const std::string NAME; @@ -250,6 +251,9 @@ public: bool WriteDescriptorLastHardenedCache(const CExtPubKey& xpub, const uint256& desc_id, uint32_t key_exp_index); bool WriteDescriptorCacheItems(const uint256& desc_id, const DescriptorCache& cache); + bool WriteLockedUTXO(const COutPoint& output); + bool EraseLockedUTXO(const COutPoint& output); + /// Write destination data key,value tuple to database bool WriteDestData(const std::string &address, const std::string &key, const std::string &value); /// Erase destination data tuple from wallet database diff --git a/test/functional/feature_asmap.py b/test/functional/feature_asmap.py index 2dc1e3a7cb..debd87962f 100755 --- a/test/functional/feature_asmap.py +++ b/test/functional/feature_asmap.py @@ -42,8 +42,8 @@ class AsmapTest(BitcoinTestFramework): self.extra_args = [["-checkaddrman=1"]] # Do addrman checks on all operations. def fill_addrman(self, node_id): - """Add 2 tried addresses to the addrman, followed by 2 new addresses.""" - for addr, tried in [[0, True], [1, True], [2, False], [3, False]]: + """Add 1 tried address to the addrman, followed by 1 new address.""" + for addr, tried in [[0, True], [1, False]]: self.nodes[node_id].addpeeraddress(address=f"101.{addr}.0.0", tried=tried, port=8333) def test_without_asmap_arg(self): @@ -89,7 +89,7 @@ class AsmapTest(BitcoinTestFramework): self.restart_node(0, ["-asmap", "-checkaddrman=1"]) with self.node.assert_debug_log( expected_msgs=[ - "Addrman checks started: new 2, tried 2, total 4", + "Addrman checks started: new 1, tried 1, total 2", "Addrman checks completed successfully", ] ): diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py index 09cda8444a..ee2c71cd42 100755 --- a/test/functional/feature_bip68_sequence.py +++ b/test/functional/feature_bip68_sequence.py @@ -41,8 +41,14 @@ class BIP68Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 self.extra_args = [ - ["-acceptnonstdtxn=1"], - ["-acceptnonstdtxn=0"], + [ + '-testactivationheight=csv@432', + "-acceptnonstdtxn=1", + ], + [ + '-testactivationheight=csv@432', + "-acceptnonstdtxn=0", + ], ] def skip_test_if_missing_module(self): diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 777787ed32..b06ea8542b 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -82,7 +82,10 @@ class FullBlockTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True - self.extra_args = [['-acceptnonstdtxn=1']] # This is a consensus block test, we don't care about tx policy + self.extra_args = [[ + '-acceptnonstdtxn=1', # This is a consensus block test, we don't care about tx policy + '-testactivationheight=bip34@2', + ]] def run_test(self): node = self.nodes[0] # convenience reference to the node diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index 2c3ef9b88b..3dc858f5d2 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -8,7 +8,6 @@ Test that the CHECKLOCKTIMEVERIFY soft-fork activates. """ from test_framework.blocktools import ( - CLTV_HEIGHT, create_block, create_coinbase, ) @@ -76,10 +75,14 @@ def cltv_validate(tx, height): cltv_modify_tx(tx, prepend_scriptsig=scheme[0], nsequence=scheme[1], nlocktime=scheme[2]) +CLTV_HEIGHT = 111 + + class BIP65Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.extra_args = [[ + f'-testactivationheight=cltv@{CLTV_HEIGHT}', '-whitelist=noban@127.0.0.1', '-par=1', # Use only one script thread to get the exact reject reason for testing '-acceptnonstdtxn=1', # cltv_invalidate is nonstandard diff --git a/test/functional/feature_csv_activation.py b/test/functional/feature_csv_activation.py index d2b3fe45d1..5255b13bd1 100755 --- a/test/functional/feature_csv_activation.py +++ b/test/functional/feature_csv_activation.py @@ -41,7 +41,6 @@ from itertools import product import time from test_framework.blocktools import ( - CSV_ACTIVATION_HEIGHT, create_block, create_coinbase, ) @@ -89,12 +88,16 @@ def all_rlt_txs(txs): return [tx['tx'] for tx in txs] +CSV_ACTIVATION_HEIGHT = 432 + + class BIP68_112_113Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True self.extra_args = [[ '-whitelist=noban@127.0.0.1', + f'-testactivationheight=csv@{CSV_ACTIVATION_HEIGHT}', '-par=1', # Use only one script thread to get the exact reject reason for testing ]] self.supports_cli = False diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index 595d26611a..28aff1f2f9 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -8,7 +8,6 @@ Test the DERSIG soft-fork activation on regtest. """ from test_framework.blocktools import ( - DERSIG_HEIGHT, create_block, create_coinbase, ) @@ -42,10 +41,14 @@ def unDERify(tx): tx.vin[0].scriptSig = CScript(newscript) +DERSIG_HEIGHT = 102 + + class BIP66Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.extra_args = [[ + f'-testactivationheight=dersig@{DERSIG_HEIGHT}', '-whitelist=noban@127.0.0.1', '-par=1', # Use only one script thread to get the exact log msg for testing ]] @@ -83,7 +86,6 @@ class BIP66Test(BitcoinTestFramework): tip = self.nodes[0].getbestblockhash() block_time = self.nodes[0].getblockheader(tip)['mediantime'] + 1 block = create_block(int(tip, 16), create_coinbase(DERSIG_HEIGHT - 1), block_time) - block.nVersion = 2 block.vtx.append(spendtx) block.hashMerkleRoot = block.calc_merkle_root() block.rehash() @@ -110,7 +112,7 @@ class BIP66Test(BitcoinTestFramework): peer.sync_with_ping() self.log.info("Test that transactions with non-DER signatures cannot appear in a block") - block.nVersion = 3 + block.nVersion = 4 spendtx = self.create_tx(self.coinbase_txids[1]) unDERify(spendtx) @@ -139,7 +141,7 @@ class BIP66Test(BitcoinTestFramework): assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip) peer.sync_with_ping() - self.log.info("Test that a version 3 block with a DERSIG-compliant transaction is accepted") + self.log.info("Test that a block with a DERSIG-compliant transaction is accepted") block.vtx[1] = self.create_tx(self.coinbase_txids[1]) block.hashMerkleRoot = block.calc_merkle_root() block.rehash() diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py index 96984e1e64..217a38050d 100755 --- a/test/functional/feature_nulldummy.py +++ b/test/functional/feature_nulldummy.py @@ -52,7 +52,7 @@ class NULLDUMMYTest(BitcoinTestFramework): # This script tests NULLDUMMY activation, which is part of the 'segwit' deployment, so we go through # normal segwit activation here (and don't use the default always-on behaviour). self.extra_args = [[ - f'-segwitheight={COINBASE_MATURITY + 5}', + f'-testactivationheight=segwit@{COINBASE_MATURITY + 5}', '-addresstype=legacy', '-par=1', # Use only one script thread to get the exact reject reason for testing ]] diff --git a/test/functional/feature_presegwit_node_upgrade.py b/test/functional/feature_presegwit_node_upgrade.py index fd6b8620d4..aac42d4dbf 100755 --- a/test/functional/feature_presegwit_node_upgrade.py +++ b/test/functional/feature_presegwit_node_upgrade.py @@ -16,7 +16,7 @@ class SegwitUpgradeTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - self.extra_args = [["-segwitheight=10"]] + self.extra_args = [["-testactivationheight=segwit@10"]] def run_test(self): """A pre-segwit node with insufficiently validated blocks needs to redownload blocks""" @@ -37,14 +37,14 @@ class SegwitUpgradeTest(BitcoinTestFramework): # Restarting the node (with segwit activation height set to 5) should result in a shutdown # because the blockchain consists of 3 insufficiently validated blocks per segwit consensus rules. node.assert_start_raises_init_error( - extra_args=["-segwitheight=5"], + extra_args=["-testactivationheight=segwit@5"], expected_msg=": Witness data for blocks after height 5 requires " f"validation. Please restart with -reindex..{os.linesep}" "Please restart with -reindex or -reindex-chainstate to recover.", ) # As directed, the user restarts the node with -reindex - self.start_node(0, extra_args=["-reindex", "-segwitheight=5"]) + self.start_node(0, extra_args=["-reindex", "-testactivationheight=segwit@5"]) # With the segwit consensus rules, the node is able to validate only up to block 4 assert_equal(node.getblockcount(), 4) diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index 2b79b3284c..79254546f1 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -78,18 +78,18 @@ class SegWitTest(BitcoinTestFramework): [ "-acceptnonstdtxn=1", "-rpcserialversion=0", - "-segwitheight=432", + "-testactivationheight=segwit@432", "-addresstype=legacy", ], [ "-acceptnonstdtxn=1", "-rpcserialversion=1", - "-segwitheight=432", + "-testactivationheight=segwit@432", "-addresstype=legacy", ], [ "-acceptnonstdtxn=1", - "-segwitheight=432", + "-testactivationheight=segwit@432", "-addresstype=legacy", ], ] diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index c073a0e43c..71a132dca3 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -46,6 +46,7 @@ from test_framework.util import ( assert_greater_than_or_equal, assert_raises_rpc_error, ) +from test_framework.wallet import MiniWallet class MempoolPersistTest(BitcoinTestFramework): @@ -53,15 +54,26 @@ class MempoolPersistTest(BitcoinTestFramework): self.num_nodes = 3 self.extra_args = [[], ["-persistmempool=0"], []] - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - def run_test(self): + self.mini_wallet = MiniWallet(self.nodes[2]) + self.mini_wallet.rescan_utxos() + if self.is_sqlite_compiled(): + self.nodes[2].createwallet( + wallet_name="watch", + descriptors=True, + disable_private_keys=True, + load_on_startup=False, + ) + wallet_watch = self.nodes[2].get_wallet_rpc("watch") + assert_equal([{'success': True}], wallet_watch.importdescriptors([{'desc': self.mini_wallet.get_descriptor(), 'timestamp': 0}])) + self.log.debug("Send 5 transactions from node2 (to its own address)") tx_creation_time_lower = int(time.time()) for _ in range(5): - last_txid = self.nodes[2].sendtoaddress(self.nodes[2].getnewaddress(), Decimal("10")) - node2_balance = self.nodes[2].getbalance() + last_txid = self.mini_wallet.send_self_transfer(from_node=self.nodes[2])["txid"] + if self.is_sqlite_compiled(): + self.nodes[2].syncwithvalidationinterfacequeue() # Flush mempool to wallet + node2_balance = wallet_watch.getbalance() self.sync_all() tx_creation_time_higher = int(time.time()) @@ -82,16 +94,16 @@ class MempoolPersistTest(BitcoinTestFramework): assert_equal(total_fee_old, self.nodes[0].getmempoolinfo()['total_fee']) assert_equal(total_fee_old, sum(v['fees']['base'] for k, v in self.nodes[0].getrawmempool(verbose=True).items())) - tx_creation_time = self.nodes[0].getmempoolentry(txid=last_txid)['time'] + last_entry = self.nodes[0].getmempoolentry(txid=last_txid) + tx_creation_time = last_entry['time'] assert_greater_than_or_equal(tx_creation_time, tx_creation_time_lower) assert_greater_than_or_equal(tx_creation_time_higher, tx_creation_time) # disconnect nodes & make a txn that remains in the unbroadcast set. self.disconnect_nodes(0, 1) - assert(len(self.nodes[0].getpeerinfo()) == 0) - assert(len(self.nodes[0].p2ps) == 0) - self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), Decimal("12")) - self.connect_nodes(0, 2) + assert_equal(len(self.nodes[0].getpeerinfo()), 0) + assert_equal(len(self.nodes[0].p2ps), 0) + self.mini_wallet.send_self_transfer(from_node=self.nodes[0]) self.log.debug("Stop-start the nodes. Verify that node0 has the transactions in its mempool and node1 does not. Verify that node2 calculates its balance correctly after loading wallet transactions.") self.stop_nodes() @@ -111,17 +123,19 @@ class MempoolPersistTest(BitcoinTestFramework): fees = self.nodes[0].getmempoolentry(txid=last_txid)['fees'] assert_equal(fees['base'] + Decimal('0.00001000'), fees['modified']) - self.log.debug('Verify time is loaded correctly') - assert_equal(tx_creation_time, self.nodes[0].getmempoolentry(txid=last_txid)['time']) + self.log.debug('Verify all fields are loaded correctly') + assert_equal(last_entry, self.nodes[0].getmempoolentry(txid=last_txid)) # Verify accounting of mempool transactions after restart is correct - self.nodes[2].syncwithvalidationinterfacequeue() # Flush mempool to wallet - assert_equal(node2_balance, self.nodes[2].getbalance()) + if self.is_sqlite_compiled(): + self.nodes[2].loadwallet("watch") + wallet_watch = self.nodes[2].get_wallet_rpc("watch") + self.nodes[2].syncwithvalidationinterfacequeue() # Flush mempool to wallet + assert_equal(node2_balance, wallet_watch.getbalance()) - # start node0 with wallet disabled so wallet transactions don't get resubmitted self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.") self.stop_nodes() - self.start_node(0, extra_args=["-persistmempool=0", "-disablewallet"]) + self.start_node(0, extra_args=["-persistmempool=0"]) assert self.nodes[0].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[0].getrawmempool()), 0) @@ -164,18 +178,18 @@ class MempoolPersistTest(BitcoinTestFramework): # ensure node0 doesn't have any connections # make a transaction that will remain in the unbroadcast set - assert(len(node0.getpeerinfo()) == 0) - assert(len(node0.p2ps) == 0) - node0.sendtoaddress(self.nodes[1].getnewaddress(), Decimal("12")) + assert_equal(len(node0.getpeerinfo()), 0) + assert_equal(len(node0.p2ps), 0) + self.mini_wallet.send_self_transfer(from_node=node0) # shutdown, then startup with wallet disabled - self.stop_nodes() - self.start_node(0, extra_args=["-disablewallet"]) + self.restart_node(0, extra_args=["-disablewallet"]) # check that txn gets broadcast due to unbroadcast logic conn = node0.add_p2p_connection(P2PTxInvStore()) - node0.mockscheduler(16*60) # 15 min + 1 for buffer + node0.mockscheduler(16 * 60) # 15 min + 1 for buffer self.wait_until(lambda: len(conn.get_invs()) == 1) -if __name__ == '__main__': + +if __name__ == "__main__": MempoolPersistTest().main() diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index e93698b08e..15b90fa61f 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -6,22 +6,22 @@ Test addr relay """ +import random +import time + from test_framework.messages import ( CAddress, - NODE_NETWORK, - NODE_WITNESS, msg_addr, msg_getaddr, - msg_verack + msg_verack, ) from test_framework.p2p import ( P2PInterface, p2p_lock, + P2P_SERVICES, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_greater_than -import random -import time class AddrReceiver(P2PInterface): @@ -96,7 +96,7 @@ class AddrTest(BitcoinTestFramework): for i in range(num): addr = CAddress() addr.time = self.mocktime + i - addr.nServices = NODE_NETWORK | NODE_WITNESS + addr.nServices = P2P_SERVICES addr.ip = f"123.123.123.{self.counter % 256}" addr.port = 8333 + i addrs.append(addr) @@ -111,7 +111,7 @@ class AddrTest(BitcoinTestFramework): for i in range(num): addr = CAddress() addr.time = self.mocktime + i - addr.nServices = NODE_NETWORK | NODE_WITNESS + addr.nServices = P2P_SERVICES addr.ip = f"{random.randrange(128,169)}.{random.randrange(1,255)}.{random.randrange(1,255)}.{random.randrange(1,255)}" addr.port = 8333 addrs.append(addr) diff --git a/test/functional/p2p_addrfetch.py b/test/functional/p2p_addrfetch.py index 2a0f6432a9..25efd50040 100755 --- a/test/functional/p2p_addrfetch.py +++ b/test/functional/p2p_addrfetch.py @@ -8,14 +8,21 @@ Test p2p addr-fetch connections import time -from test_framework.messages import msg_addr, CAddress, NODE_NETWORK, NODE_WITNESS -from test_framework.p2p import P2PInterface, p2p_lock +from test_framework.messages import ( + CAddress, + msg_addr, +) +from test_framework.p2p import ( + P2PInterface, + p2p_lock, + P2P_SERVICES, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal ADDR = CAddress() ADDR.time = int(time.time()) -ADDR.nServices = NODE_NETWORK | NODE_WITNESS +ADDR.nServices = P2P_SERVICES ADDR.ip = "192.0.0.8" ADDR.port = 18444 diff --git a/test/functional/p2p_addrv2_relay.py b/test/functional/p2p_addrv2_relay.py index 32c1d42b1c..3833c58680 100755 --- a/test/functional/p2p_addrv2_relay.py +++ b/test/functional/p2p_addrv2_relay.py @@ -11,10 +11,11 @@ import time from test_framework.messages import ( CAddress, msg_addrv2, - NODE_NETWORK, - NODE_WITNESS, ) -from test_framework.p2p import P2PInterface +from test_framework.p2p import ( + P2PInterface, + P2P_SERVICES, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal @@ -24,7 +25,7 @@ ADDRS = [] for i in range(10): addr = CAddress() addr.time = int(time.time()) + i - addr.nServices = NODE_NETWORK | NODE_WITNESS + addr.nServices = P2P_SERVICES # Add one I2P address at an arbitrary position. if i == 5: addr.net = addr.NET_I2P diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index a71f736bd6..aa3b95fc4f 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -43,6 +43,7 @@ from test_framework.messages import ( from test_framework.p2p import ( P2PInterface, p2p_lock, + P2P_SERVICES, ) from test_framework.script import ( CScript, @@ -83,10 +84,6 @@ from test_framework.util import ( assert_raises_rpc_error, ) -# The versionbit bit used to signal activation of SegWit -VB_WITNESS_BIT = 1 -VB_TOP_BITS = 0x20000000 - MAX_SIGOP_COST = 80000 SEGWIT_HEIGHT = 120 @@ -196,8 +193,8 @@ class SegWitTest(BitcoinTestFramework): self.num_nodes = 2 # This test tests SegWit both pre and post-activation, so use the normal BIP9 activation. self.extra_args = [ - ["-acceptnonstdtxn=1", "-segwitheight={}".format(SEGWIT_HEIGHT), "-whitelist=noban@127.0.0.1"], - ["-acceptnonstdtxn=0", "-segwitheight={}".format(SEGWIT_HEIGHT)], + ["-acceptnonstdtxn=1", f"-testactivationheight=segwit@{SEGWIT_HEIGHT}", "-whitelist=noban@127.0.0.1"], + ["-acceptnonstdtxn=0", f"-testactivationheight=segwit@{SEGWIT_HEIGHT}"], ] self.supports_cli = False @@ -206,13 +203,13 @@ class SegWitTest(BitcoinTestFramework): # Helper functions - def build_next_block(self, version=4): + def build_next_block(self): """Build a block on top of node0's tip.""" tip = self.nodes[0].getbestblockhash() height = self.nodes[0].getblockcount() + 1 block_time = self.nodes[0].getblockheader(tip)["mediantime"] + 1 block = create_block(int(tip, 16), create_coinbase(height), block_time) - block.nVersion = version + block.nVersion = 4 block.rehash() return block @@ -224,14 +221,14 @@ class SegWitTest(BitcoinTestFramework): def run_test(self): # Setup the p2p connections - # self.test_node sets NODE_WITNESS|NODE_NETWORK - self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK | NODE_WITNESS) + # self.test_node sets P2P_SERVICES, i.e. NODE_WITNESS | NODE_NETWORK + self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=P2P_SERVICES) # self.old_node sets only NODE_NETWORK self.old_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK) # self.std_node is for testing node1 (fRequireStandard=true) - self.std_node = self.nodes[1].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK | NODE_WITNESS) + self.std_node = self.nodes[1].add_p2p_connection(TestP2PConn(), services=P2P_SERVICES) # self.std_wtx_node is for testing node1 with wtxid relay - self.std_wtx_node = self.nodes[1].add_p2p_connection(TestP2PConn(wtxidrelay=True), services=NODE_NETWORK | NODE_WITNESS) + self.std_wtx_node = self.nodes[1].add_p2p_connection(TestP2PConn(wtxidrelay=True), services=P2P_SERVICES) assert self.test_node.nServices & NODE_WITNESS != 0 @@ -298,7 +295,7 @@ class SegWitTest(BitcoinTestFramework): # Mine a block with an anyone-can-spend coinbase, # let it mature, then try to spend it. - block = self.build_next_block(version=1) + block = self.build_next_block() block.solve() self.test_node.send_and_ping(msg_no_witness_block(block)) # make sure the block was processed txid = block.vtx[0].sha256 @@ -336,8 +333,8 @@ class SegWitTest(BitcoinTestFramework): tx.rehash() assert tx.sha256 != tx.calc_sha256(with_witness=True) - # Construct a segwit-signaling block that includes the transaction. - block = self.build_next_block(version=(VB_TOP_BITS | (1 << VB_WITNESS_BIT))) + # Construct a block that includes the transaction. + block = self.build_next_block() self.update_witness_block_with_transactions(block, [tx]) # Sending witness data before activation is not allowed (anti-spam # rule). @@ -364,7 +361,7 @@ class SegWitTest(BitcoinTestFramework): # test_node has set NODE_WITNESS, so all getdata requests should be for # witness blocks. # Test announcing a block via inv results in a getdata, and that - # announcing a version 4 or random VB block with a header results in a getdata + # announcing a block with a header results in a getdata block1 = self.build_next_block() block1.solve() @@ -372,19 +369,13 @@ class SegWitTest(BitcoinTestFramework): assert self.test_node.last_message["getdata"].inv[0].type == blocktype test_witness_block(self.nodes[0], self.test_node, block1, True) - block2 = self.build_next_block(version=4) + block2 = self.build_next_block() block2.solve() self.test_node.announce_block_and_wait_for_getdata(block2, use_header=True) assert self.test_node.last_message["getdata"].inv[0].type == blocktype test_witness_block(self.nodes[0], self.test_node, block2, True) - block3 = self.build_next_block(version=(VB_TOP_BITS | (1 << 15))) - block3.solve() - self.test_node.announce_block_and_wait_for_getdata(block3, use_header=True) - assert self.test_node.last_message["getdata"].inv[0].type == blocktype - test_witness_block(self.nodes[0], self.test_node, block3, True) - # Check that we can getdata for witness blocks or regular blocks, # and the right thing happens. if not self.segwit_active: @@ -429,7 +420,7 @@ class SegWitTest(BitcoinTestFramework): assert_equal(rpc_details["weight"], block.get_weight()) # Upgraded node should not ask for blocks from unupgraded - block4 = self.build_next_block(version=4) + block4 = self.build_next_block() block4.solve() self.old_node.getdataset = set() @@ -2017,8 +2008,8 @@ class SegWitTest(BitcoinTestFramework): @subtest # type: ignore def test_wtxid_relay(self): # Use brand new nodes to avoid contamination from earlier tests - self.wtx_node = self.nodes[0].add_p2p_connection(TestP2PConn(wtxidrelay=True), services=NODE_NETWORK | NODE_WITNESS) - self.tx_node = self.nodes[0].add_p2p_connection(TestP2PConn(wtxidrelay=False), services=NODE_NETWORK | NODE_WITNESS) + self.wtx_node = self.nodes[0].add_p2p_connection(TestP2PConn(wtxidrelay=True), services=P2P_SERVICES) + self.tx_node = self.nodes[0].add_p2p_connection(TestP2PConn(wtxidrelay=False), services=P2P_SERVICES) # Check wtxidrelay feature negotiation message through connecting a new peer def received_wtxidrelay(): diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 0600d8b9c5..c3c6ade684 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -27,8 +27,6 @@ import subprocess from test_framework.address import ADDRESS_BCRT1_P2WSH_OP_TRUE from test_framework.blocktools import ( - CLTV_HEIGHT, - DERSIG_HEIGHT, create_block, create_coinbase, TIME_GENESIS_BLOCK, @@ -129,7 +127,29 @@ class BlockchainTest(BitcoinTestFramework): # should have exact keys assert_equal(sorted(res.keys()), keys) - self.restart_node(0, ['-stopatheight=207', '-prune=550']) + self.stop_node(0) + self.nodes[0].assert_start_raises_init_error( + extra_args=['-testactivationheight=name@2'], + expected_msg='Error: Invalid name (name@2) for -testactivationheight=name@height.', + ) + self.nodes[0].assert_start_raises_init_error( + extra_args=['-testactivationheight=bip34@-2'], + expected_msg='Error: Invalid height value (bip34@-2) for -testactivationheight=name@height.', + ) + self.nodes[0].assert_start_raises_init_error( + extra_args=['-testactivationheight='], + expected_msg='Error: Invalid format () for -testactivationheight=name@height.', + ) + self.start_node(0, extra_args=[ + '-stopatheight=207', + '-prune=550', + '-testactivationheight=bip34@2', + '-testactivationheight=dersig@3', + '-testactivationheight=cltv@4', + '-testactivationheight=csv@5', + '-testactivationheight=segwit@6', + ]) + res = self.nodes[0].getblockchaininfo() # result should have these additional pruning keys if prune=550 assert_equal(sorted(res.keys()), sorted(['pruneheight', 'automatic_pruning', 'prune_target_size'] + keys)) @@ -143,10 +163,10 @@ class BlockchainTest(BitcoinTestFramework): assert_equal(res['softforks'], { 'bip34': {'type': 'buried', 'active': True, 'height': 2}, - 'bip66': {'type': 'buried', 'active': True, 'height': DERSIG_HEIGHT}, - 'bip65': {'type': 'buried', 'active': True, 'height': CLTV_HEIGHT}, - 'csv': {'type': 'buried', 'active': False, 'height': 432}, - 'segwit': {'type': 'buried', 'active': True, 'height': 0}, + 'bip66': {'type': 'buried', 'active': True, 'height': 3}, + 'bip65': {'type': 'buried', 'active': True, 'height': 4}, + 'csv': {'type': 'buried', 'active': True, 'height': 5}, + 'segwit': {'type': 'buried', 'active': True, 'height': 6}, 'testdummy': { 'type': 'bip9', 'bip9': { diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 3fcca97cb7..0f3bbce54c 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -12,11 +12,10 @@ from itertools import product import time from test_framework.blocktools import COINBASE_MATURITY -from test_framework.p2p import P2PInterface import test_framework.messages -from test_framework.messages import ( - NODE_NETWORK, - NODE_WITNESS, +from test_framework.p2p import ( + P2PInterface, + P2P_SERVICES, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -189,7 +188,6 @@ class NetTest(BitcoinTestFramework): def test_getnodeaddresses(self): self.log.info("Test getnodeaddresses") self.nodes[0].add_p2p_connection(P2PInterface()) - services = NODE_NETWORK | NODE_WITNESS # Add an IPv6 address to the address manager. ipv6_addr = "1233:3432:2434:2343:3234:2345:6546:4534" @@ -217,7 +215,7 @@ class NetTest(BitcoinTestFramework): assert_greater_than(10000, len(node_addresses)) for a in node_addresses: assert_greater_than(a["time"], 1527811200) # 1st June 2018 - assert_equal(a["services"], services) + assert_equal(a["services"], P2P_SERVICES) assert a["address"] in imported_addrs assert_equal(a["port"], 8333) assert_equal(a["network"], "ipv4") @@ -228,7 +226,7 @@ class NetTest(BitcoinTestFramework): assert_equal(res[0]["address"], ipv6_addr) assert_equal(res[0]["network"], "ipv6") assert_equal(res[0]["port"], 8333) - assert_equal(res[0]["services"], services) + assert_equal(res[0]["services"], P2P_SERVICES) # Test for the absence of onion and I2P addresses. for network in ["onion", "i2p"]: diff --git a/test/functional/rpc_signrawtransaction.py b/test/functional/rpc_signrawtransaction.py index 8f17b29ff4..18abece253 100755 --- a/test/functional/rpc_signrawtransaction.py +++ b/test/functional/rpc_signrawtransaction.py @@ -6,7 +6,6 @@ from test_framework.blocktools import ( COINBASE_MATURITY, - CSV_ACTIVATION_HEIGHT, ) from test_framework.address import ( script_to_p2sh, @@ -18,7 +17,6 @@ from test_framework.util import ( assert_equal, assert_raises_rpc_error, find_vout_for_address, - generate_to_height, ) from test_framework.messages import ( CTxInWitness, @@ -273,7 +271,6 @@ class SignRawTransactionsTest(BitcoinTestFramework): getcontext().prec = 8 # Make sure CSV is active - generate_to_height(self, self.nodes[0], CSV_ACTIVATION_HEIGHT) assert self.nodes[0].getblockchaininfo()['softforks']['csv']['active'] # Create a P2WSH script with CSV diff --git a/test/functional/test_framework/authproxy.py b/test/functional/test_framework/authproxy.py index 81eb881234..c4ffd1fbf6 100644 --- a/test/functional/test_framework/authproxy.py +++ b/test/functional/test_framework/authproxy.py @@ -113,10 +113,8 @@ class AuthServiceProxy(): self.__conn.request(method, path, postdata, headers) return self._get_response() except OSError as e: - retry = ( - '[WinError 10053] An established connection was aborted by the software in your host machine' in str(e)) # Workaround for a bug on macOS. See https://bugs.python.org/issue33450 - retry = retry or ('[Errno 41] Protocol wrong type for socket' in str(e)) + retry = '[Errno 41] Protocol wrong type for socket' in str(e) if retry: self.__conn.close() self.__conn.request(method, path, postdata, headers) diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 25b36b6a91..6de372cd8e 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -53,11 +53,6 @@ TIME_GENESIS_BLOCK = 1296688602 # Coinbase transaction outputs can only be spent after this number of new blocks (network rule) COINBASE_MATURITY = 100 -# Soft-fork activation heights -DERSIG_HEIGHT = 102 # BIP 66 -CLTV_HEIGHT = 111 # BIP 65 -CSV_ACTIVATION_HEIGHT = 432 - # From BIP141 WITNESS_COMMITMENT_HEADER = b"\xaa\x21\xa9\xed" diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index ec563cc290..78c63b57a1 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -356,7 +356,7 @@ class P2PInterface(P2PConnection): return create_conn - def peer_accept_connection(self, *args, services=NODE_NETWORK | NODE_WITNESS, **kwargs): + def peer_accept_connection(self, *args, services=P2P_SERVICES, **kwargs): create_conn = super().peer_accept_connection(*args, **kwargs) self.peer_connect_send_version(services) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index f382e0fdb3..3aca93fab3 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -560,18 +560,19 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): self.nodes[i].process.wait(timeout) def connect_nodes(self, a, b): - def connect_nodes_helper(from_connection, node_num): - ip_port = "127.0.0.1:" + str(p2p_port(node_num)) - from_connection.addnode(ip_port, "onetry") - # poll until version handshake complete to avoid race conditions - # with transaction relaying - # See comments in net_processing: - # * Must have a version message before anything else - # * Must have a verack message before anything else - wait_until_helper(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo())) - wait_until_helper(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo())) - - connect_nodes_helper(self.nodes[a], b) + from_connection = self.nodes[a] + to_connection = self.nodes[b] + ip_port = "127.0.0.1:" + str(p2p_port(b)) + from_connection.addnode(ip_port, "onetry") + # poll until version handshake complete to avoid race conditions + # with transaction relaying + # See comments in net_processing: + # * Must have a version message before anything else + # * Must have a verack message before anything else + wait_until_helper(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo())) + wait_until_helper(lambda: all(peer['version'] != 0 for peer in to_connection.getpeerinfo())) + wait_until_helper(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo())) + wait_until_helper(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in to_connection.getpeerinfo())) def disconnect_nodes(self, a, b): def disconnect_nodes_helper(from_connection, node_num): diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index d66499dbcb..ea5c641b4a 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -560,17 +560,6 @@ def mine_large_block(test_framework, node, utxos=None): test_framework.generate(node, 1) -def generate_to_height(test_framework, node, target_height): - """Generates blocks until a given target block height has been reached. - To prevent timeouts, only up to 200 blocks are generated per RPC call. - Can be used to activate certain soft-forks (e.g. CSV, CLTV).""" - current_height = node.getblockcount() - while current_height < target_height: - nblocks = min(200, target_height - current_height) - current_height += len(test_framework.generate(node, nblocks)) - assert_equal(node.getblockcount(), target_height) - - def find_vout_for_address(node, txid, addr): """ Locate the vout index of the given transaction sending to the diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 2ffeb2d115..cea59c6d69 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -82,7 +82,7 @@ class MiniWallet: def rescan_utxos(self): """Drop all utxos and rescan the utxo set""" self._utxos = [] - res = self._test_node.scantxoutset(action="start", scanobjects=[f'raw({self._scriptPubKey.hex()})']) + res = self._test_node.scantxoutset(action="start", scanobjects=[self.get_descriptor()]) assert_equal(True, res['success']) for utxo in res['unspents']: self._utxos.append({'txid': utxo['txid'], 'vout': utxo['vout'], 'value': utxo['amount']}) @@ -110,12 +110,15 @@ class MiniWallet: def generate(self, num_blocks): """Generate blocks with coinbase outputs to the internal address, and append the outputs to the internal list""" - blocks = self._test_node.generatetodescriptor(num_blocks, f'raw({self._scriptPubKey.hex()})') + blocks = self._test_node.generatetodescriptor(num_blocks, self.get_descriptor()) for b in blocks: cb_tx = self._test_node.getblock(blockhash=b, verbosity=2)['tx'][0] self._utxos.append({'txid': cb_tx['txid'], 'vout': 0, 'value': cb_tx['vout'][0]['value']}) return blocks + def get_descriptor(self): + return self._test_node.getdescriptorinfo(f'raw({self._scriptPubKey.hex()})')['descriptor'] + def get_address(self): return self._address diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 6372e1acd7..5c464ae142 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -121,13 +121,49 @@ class WalletTest(BitcoinTestFramework): # Exercise locking of unspent outputs unspent_0 = self.nodes[2].listunspent()[0] unspent_0 = {"txid": unspent_0["txid"], "vout": unspent_0["vout"]} + # Trying to unlock an output which isn't locked should error assert_raises_rpc_error(-8, "Invalid parameter, expected locked output", self.nodes[2].lockunspent, True, [unspent_0]) + + # Locking an already-locked output should error self.nodes[2].lockunspent(False, [unspent_0]) assert_raises_rpc_error(-8, "Invalid parameter, output already locked", self.nodes[2].lockunspent, False, [unspent_0]) + + # Restarting the node should clear the lock + self.restart_node(2) + self.nodes[2].lockunspent(False, [unspent_0]) + + # Unloading and reloating the wallet should clear the lock + assert_equal(self.nodes[0].listwallets(), [self.default_wallet_name]) + self.nodes[2].unloadwallet(self.default_wallet_name) + self.nodes[2].loadwallet(self.default_wallet_name) + assert_equal(len(self.nodes[2].listlockunspent()), 0) + + # Locking non-persistently, then re-locking persistently, is allowed + self.nodes[2].lockunspent(False, [unspent_0]) + self.nodes[2].lockunspent(False, [unspent_0], True) + + # Restarting the node with the lock written to the wallet should keep the lock + self.restart_node(2) + assert_raises_rpc_error(-8, "Invalid parameter, output already locked", self.nodes[2].lockunspent, False, [unspent_0]) + + # Unloading and reloading the wallet with a persistent lock should keep the lock + self.nodes[2].unloadwallet(self.default_wallet_name) + self.nodes[2].loadwallet(self.default_wallet_name) + assert_raises_rpc_error(-8, "Invalid parameter, output already locked", self.nodes[2].lockunspent, False, [unspent_0]) + + # Locked outputs should not be used, even if they are the only available funds assert_raises_rpc_error(-6, "Insufficient funds", self.nodes[2].sendtoaddress, self.nodes[2].getnewaddress(), 20) assert_equal([unspent_0], self.nodes[2].listlockunspent()) + + # Unlocking should remove the persistent lock self.nodes[2].lockunspent(True, [unspent_0]) + self.restart_node(2) assert_equal(len(self.nodes[2].listlockunspent()), 0) + + # Reconnect node 2 after restarts + self.connect_nodes(1, 2) + self.connect_nodes(0, 2) + assert_raises_rpc_error(-8, "txid must be of length 64 (not 34, for '0000000000000000000000000000000000')", self.nodes[2].lockunspent, False, [{"txid": "0000000000000000000000000000000000", "vout": 0}]) diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 63e7c57ddb..b52e105a33 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -5,8 +5,6 @@ # names can be used. # See https://github.com/google/sanitizers/issues/1364 signed-integer-overflow:txmempool.cpp -# nLastSuccess read from peers.dat might cause an overflow in IsTerrible -signed-integer-overflow:addrman.cpp # https://github.com/bitcoin/bitcoin/pull/21798#issuecomment-829180719 signed-integer-overflow:policy/feerate.cpp |