diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1d6521a3f452d..9d8fdb4a16ee7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -358,7 +358,7 @@ jobs: ./src/univalue/unitester.exe - name: Run benchmarks - run: ./bin/bench_bitcoin.exe -sanity-check -priority-level=high + run: ./bin/bench_bitcoin.exe -sanity-check - name: Adjust paths in test/config.ini shell: pwsh diff --git a/CMakeLists.txt b/CMakeLists.txt index 99c36f8621a52..b272658177f25 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -154,7 +154,7 @@ endif() cmake_dependent_option(WITH_DBUS "Enable DBus support." ON "CMAKE_SYSTEM_NAME STREQUAL \"Linux\" AND BUILD_GUI" OFF) option(ENABLE_IPC "Build multiprocess bitcoin-node and bitcoin-gui executables in addition to monolithic bitcoind and bitcoin-qt executables. Requires libmultiprocess library. Experimental." OFF) -cmake_dependent_option(WITH_EXTERNAL_LIBMULTIPROCESS "Build with external libmultiprocess library instead of with local git subtree when ENABLE_IPC is enabled. This is not normally recommended, but can be useful when cross-compiling or making changes to the upstream project." OFF "ENABLE_IPC" OFF) +cmake_dependent_option(WITH_EXTERNAL_LIBMULTIPROCESS "Build with external libmultiprocess library instead of with local git subtree when ENABLE_IPC is enabled. This is not normally recommended, but can be useful for developing libmultiprocess itself." OFF "ENABLE_IPC" OFF) if(ENABLE_IPC AND WITH_EXTERNAL_LIBMULTIPROCESS) find_package(Libmultiprocess REQUIRED COMPONENTS Lib) find_package(LibmultiprocessNative REQUIRED COMPONENTS Bin diff --git a/ci/lint/04_install.sh b/ci/lint/04_install.sh index 9ef1f37c73885..c38a173dc480d 100755 --- a/ci/lint/04_install.sh +++ b/ci/lint/04_install.sh @@ -12,10 +12,11 @@ pushd "/" ${CI_RETRY_EXE} apt-get update # Lint dependencies: +# - cargo (used to run the lint tests) # - curl/xz-utils (to install shellcheck) # - git (used in many lint scripts) # - gpg (used by verify-commits) -${CI_RETRY_EXE} apt-get install -y curl xz-utils git gpg +${CI_RETRY_EXE} apt-get install -y cargo curl xz-utils git gpg PYTHON_PATH="/python_build" if [ ! -d "${PYTHON_PATH}/bin" ]; then @@ -35,17 +36,6 @@ export PATH="${PYTHON_PATH}/bin:${PATH}" command -v python3 python3 --version -export LINT_RUNNER_PATH="/lint_test_runner" -if [ ! -d "${LINT_RUNNER_PATH}" ]; then - ${CI_RETRY_EXE} apt-get install -y cargo - ( - cd "/test/lint/test_runner" || exit 1 - cargo build - mkdir -p "${LINT_RUNNER_PATH}" - mv target/debug/test_runner "${LINT_RUNNER_PATH}" - ) -fi - ${CI_RETRY_EXE} pip3 install \ codespell==2.2.6 \ lief==0.13.2 \ diff --git a/ci/lint/06_script.sh b/ci/lint/06_script.sh index 7e27197024e1d..6d637c2a438b6 100755 --- a/ci/lint/06_script.sh +++ b/ci/lint/06_script.sh @@ -16,7 +16,7 @@ if [ -n "$CIRRUS_PR" ]; then fi fi -RUST_BACKTRACE=1 "${LINT_RUNNER_PATH}/test_runner" +RUST_BACKTRACE=1 cargo run --manifest-path "./test/lint/test_runner/Cargo.toml" if [ "$CIRRUS_REPO_FULL_NAME" = "bitcoin/bitcoin" ] && [ "$CIRRUS_PR" = "" ] ; then # Sanity check only the last few commits to get notified of missing sigs, diff --git a/ci/lint/container-entrypoint.sh b/ci/lint/container-entrypoint.sh index c8519a39129cf..84e60be2917aa 100755 --- a/ci/lint/container-entrypoint.sh +++ b/ci/lint/container-entrypoint.sh @@ -11,7 +11,6 @@ export LC_ALL=C git config --global --add safe.directory /bitcoin export PATH="/python_build/bin:${PATH}" -export LINT_RUNNER_PATH="/lint_test_runner" if [ -z "$1" ]; then bash -ic "./ci/lint/06_script.sh" diff --git a/ci/lint_imagefile b/ci/lint_imagefile index c05f2108f7aa8..9da3747e08386 100644 --- a/ci/lint_imagefile +++ b/ci/lint_imagefile @@ -4,7 +4,7 @@ # See test/lint/README.md for usage. -FROM mirror.gcr.io/debian:bookworm +FROM mirror.gcr.io/ubuntu:24.04 ENV DEBIAN_FRONTEND=noninteractive ENV LC_ALL=C.UTF-8 @@ -13,7 +13,6 @@ COPY ./ci/retry/retry /ci_retry COPY ./.python-version /.python-version COPY ./ci/lint/container-entrypoint.sh /entrypoint.sh COPY ./ci/lint/04_install.sh /install.sh -COPY ./test/lint/test_runner /test/lint/test_runner RUN /install.sh && \ echo 'alias lint="./ci/lint/06_script.sh"' >> ~/.bashrc && \ diff --git a/ci/lint_run.sh b/ci/lint_run.sh index 319cdc214acef..6327c3c456104 100755 --- a/ci/lint_run.sh +++ b/ci/lint_run.sh @@ -9,5 +9,4 @@ set -o errexit -o pipefail -o xtrace # Only used in .cirrus.yml. Refer to test/lint/README.md on how to run locally. export PATH="/python_build/bin:${PATH}" -export LINT_RUNNER_PATH="/lint_test_runner" ./ci/lint/06_script.sh diff --git a/ci/test/00_setup_env_arm.sh b/ci/test/00_setup_env_arm.sh index dfeb72216cef2..45dabb8f383d9 100755 --- a/ci/test/00_setup_env_arm.sh +++ b/ci/test/00_setup_env_arm.sh @@ -10,7 +10,7 @@ export HOST=arm-linux-gnueabihf export DPKG_ADD_ARCH="armhf" export PACKAGES="python3-zmq g++-arm-linux-gnueabihf busybox libc6:armhf libstdc++6:armhf libfontconfig1:armhf libxcb1:armhf" export CONTAINER_NAME=ci_arm_linux -export CI_IMAGE_NAME_TAG="mirror.gcr.io/ubuntu:noble" # Check that https://packages.ubuntu.com/noble/g++-arm-linux-gnueabihf (version 13.3, similar to guix) can cross-compile +export CI_IMAGE_NAME_TAG="mirror.gcr.io/ubuntu:24.04" # Check that https://packages.ubuntu.com/noble/g++-arm-linux-gnueabihf (version 13.x, similar to guix) can cross-compile export CI_IMAGE_PLATFORM="linux/arm64" export USE_BUSY_BOX=true export RUN_UNIT_TESTS=true diff --git a/ci/test/00_setup_env_win64.sh b/ci/test/00_setup_env_win64.sh index ba67af264f89e..d1c5e5a7b2cea 100755 --- a/ci/test/00_setup_env_win64.sh +++ b/ci/test/00_setup_env_win64.sh @@ -7,7 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_win64 -export CI_IMAGE_NAME_TAG="mirror.gcr.io/ubuntu:noble" # Check that g++-mingw-w64-x86-64-posix (version 13.2, similar to guix) can cross-compile +export CI_IMAGE_NAME_TAG="mirror.gcr.io/ubuntu:24.04" # Check that https://packages.ubuntu.com/noble/g++-mingw-w64-x86-64-posix (version 13.x, similar to guix) can cross-compile export CI_IMAGE_PLATFORM="linux/amd64" export HOST=x86_64-w64-mingw32 export PACKAGES="g++-mingw-w64-x86-64-posix nsis" diff --git a/doc/multiprocess.md b/doc/multiprocess.md index 90dfb73fecd7e..33c5a44aa829f 100644 --- a/doc/multiprocess.md +++ b/doc/multiprocess.md @@ -35,6 +35,10 @@ The `cmake` build will pick up settings and library locations from the depends d When cross-compiling and not using depends, native code generation tools from [libmultiprocess](https://github.com/bitcoin-core/libmultiprocess) and [Cap'n Proto](https://capnproto.org/) are required. They can be passed to the cmake build by specifying `-DMPGEN_EXECUTABLE=/path/to/mpgen -DCAPNP_EXECUTABLE=/path/to/capnp -DCAPNPC_CXX_EXECUTABLE=/path/to/capnpc-c++` options. +### External libmultiprocess installation + +By default when `-DENABLE_IPC=ON` is enabled, the libmultiprocess sources at [../src/ipc/libmultiprocess/](../src/ipc/libmultiprocess/) are built as part of the bitcoin cmake build, but alternately an external [libmultiprocess](https://github.com/bitcoin-core/libmultiprocess/) cmake package can be used instead by following its [installation instructions](https://github.com/bitcoin-core/libmultiprocess/blob/master/doc/install.md) and specifying `-DWITH_EXTERNAL_LIBMULTIPROCESS=ON` to the bitcoin build, so it will use the external package instead of the sources. This can be useful when making changes to the upstream project. If libmultiprocess is not installed in a default system location it is possible to specify the [`CMAKE_PREFIX_PATH`](https://cmake.org/cmake/help/latest/envvar/CMAKE_PREFIX_PATH.html) environment variable to point to the installation prefix where libmultiprocess is installed. + ## Usage `bitcoin-node` is a drop-in replacement for `bitcoind`, and `bitcoin-gui` is a drop-in replacement for `bitcoin-qt`, and there are no differences in use or external behavior between the new and old executables. But internally after [#10102](https://github.com/bitcoin/bitcoin/pull/10102), `bitcoin-gui` will spawn a `bitcoin-node` process to run P2P and RPC code, communicating with it across a socket pair, and `bitcoin-node` will spawn `bitcoin-wallet` to run wallet code, also communicating over a socket pair. This will let node, wallet, and GUI code run in separate address spaces for better isolation, and allow future improvements like being able to start and stop components independently on different machines and environments. diff --git a/doc/zmq.md b/doc/zmq.md index 0a74d6eef97b8..c04f6db21b767 100644 --- a/doc/zmq.md +++ b/doc/zmq.md @@ -87,40 +87,69 @@ For instance: -zmqpubrawtx=ipc:///tmp/bitcoind.tx.raw \ -zmqpubhashtxhwm=10000 -Each PUB notification has a topic and body, where the header -corresponds to the notification type. For instance, for the -notification `-zmqpubhashtx` the topic is `hashtx` (no null -terminator). These options can also be provided in bitcoin.conf. +Notification types correspond to message topics (details in next section). For instance, +for the notification `-zmqpubhashtx` the topic is `hashtx`. These options can also be +provided in bitcoin.conf. -The topics are: +### Message format -`sequence`: the body is structured as the following based on the type of message: +All ZMQ messages share the same structure with three parts: _topic_ string, +message _body_, and _message sequence number_: - <32-byte hash>C : Blockhash connected - <32-byte hash>D : Blockhash disconnected - <32-byte hash>R<8-byte LE uint> : Transactionhash removed from mempool for non-block inclusion reason - <32-byte hash>A<8-byte LE uint> : Transactionhash added mempool + | topic | body | message sequence number | + |-----------+------------------------------------------------------+--------------------------| + | rawtx | | <4-byte LE uint> | + | hashtx | | <4-byte LE uint> | + | rawblock | | <4-byte LE uint> | + | hashblock | | <4-byte LE uint> | + | sequence | C | <4-byte LE uint> | + | sequence | D | <4-byte LE uint> | + | sequence | R<8-byte LE uint> | <4-byte LE uint> | + | sequence | A<8-byte LE uint> | <4-byte LE uint> | -Where the 8-byte uints correspond to the mempool sequence number. +where: -`rawtx`: Notifies about all transactions, both when they are added to mempool or when a new block arrives. This means a transaction could be published multiple times. First, when it enters the mempool and then again in each block that includes it. The messages are ZMQ multipart messages with three parts. The first part is the topic (`rawtx`), the second part is the serialized transaction, and the last part is a sequence number (representing the message count to detect lost messages). + - message sequence number represents message count to detect lost messages, distinct for each topic + - all transaction and block hashes are in _reversed byte order_ (i. e. with bytes + produced by hashing function reversed), the same format as the RPC interface and block + explorers use to display transaction and block hashes - | rawtx | | +#### rawtx -`hashtx`: Notifies about all transactions, both when they are added to mempool or when a new block arrives. This means a transaction could be published multiple times. First, when it enters the mempool and then again in each block that includes it. The messages are ZMQ multipart messages with three parts. The first part is the topic (`hashtx`), the second part is the 32-byte transaction hash, and the last part is a sequence number (representing the message count to detect lost messages). +Notifies about all transactions, both when they are added to mempool or when a new block +arrives. This means a transaction could be published multiple times: first when it enters +mempool and then again in each block that includes it. The body part of the message is the +serialized transaction. - | hashtx | <32-byte transaction hash in Little Endian> | +#### hashtx +Notifies about all transactions, both when they are added to mempool or when a new block +arrives. This means a transaction could be published multiple times: first when it enters +mempool and then again in each block that includes it. The body part of the mesage is the +32-byte transaction hash in reversed byte order. -`rawblock`: Notifies when the chain tip is updated. When assumeutxo is in use, this notification will not be issued for historical blocks connected to the background validation chainstate. Messages are ZMQ multipart messages with three parts. The first part is the topic (`rawblock`), the second part is the serialized block, and the last part is a sequence number (representing the message count to detect lost messages). +#### rawblock - | rawblock | | +Notifies when the chain tip is updated. When assumeutxo is in use, this notification will +not be issued for historical blocks connected to the background validation chainstate. The +body part of the message is the serialized block. -`hashblock`: Notifies when the chain tip is updated. When assumeutxo is in use, this notification will not be issued for historical blocks connected to the background validation chainstate. Messages are ZMQ multipart messages with three parts. The first part is the topic (`hashblock`), the second part is the 32-byte block hash, and the last part is a sequence number (representing the message count to detect lost messages). +#### hashblock - | hashblock | <32-byte block hash in Little Endian> | +Notifies when the chain tip is updated. When assumeutxo is in use, this notification will +not be issued for historical blocks connected to the background validation chainstate. The +body part of the message is the 32-byte block hash in reversed byte order. -**_NOTE:_** Note that the 32-byte hashes are in Little Endian and not in the Big Endian format that the RPC interface and block explorers use to display transaction and block hashes. +#### sequence + +The 8-byte LE uints correspond to _mempool sequence number_ and the types of bodies are: + + - `C` : block with this hash connected + - `D` : block with this hash disconnected + - `R` : transaction with this hash removed from mempool for non-block inclusion reason + - `A` : transaction with this hash added to mempool + +### Implementing ZMQ client ZeroMQ endpoint specifiers for TCP (and others) are documented in the [ZeroMQ API](http://api.zeromq.org/4-0:_start). @@ -138,7 +167,7 @@ operating system configuration and must be configured prior to connection establ For example, when running on GNU/Linux, one might use the following to lower the keepalive setting to 10 minutes: -sudo sysctl -w net.ipv4.tcp_keepalive_time=600 + sudo sysctl -w net.ipv4.tcp_keepalive_time=600 Setting the keepalive values appropriately for your operating environment may improve connectivity in situations where long-lived connections are silently diff --git a/src/bench/readwriteblock.cpp b/src/bench/readwriteblock.cpp index cdf86185ae9ab..10434b15d200e 100644 --- a/src/bench/readwriteblock.cpp +++ b/src/bench/readwriteblock.cpp @@ -27,7 +27,7 @@ static CBlock CreateTestBlock() return block; } -static void SaveBlockBench(benchmark::Bench& bench) +static void WriteBlockBench(benchmark::Bench& bench) { const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; auto& blockman{testing_setup->m_node.chainman->m_blockman}; @@ -63,6 +63,6 @@ static void ReadRawBlockBench(benchmark::Bench& bench) }); } -BENCHMARK(SaveBlockBench, benchmark::PriorityLevel::HIGH); +BENCHMARK(WriteBlockBench, benchmark::PriorityLevel::HIGH); BENCHMARK(ReadBlockBench, benchmark::PriorityLevel::HIGH); BENCHMARK(ReadRawBlockBench, benchmark::PriorityLevel::HIGH); diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp index 5305833131a05..ca28ab0448789 100644 --- a/src/bench/wallet_migration.cpp +++ b/src/bench/wallet_migration.cpp @@ -32,6 +32,7 @@ static void WalletMigration(benchmark::Bench& bench) std::unique_ptr wallet = std::make_unique(test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase()); wallet->chainStateFlushed(ChainstateRole::NORMAL, CBlockLocator{}); LegacyDataSPKM* legacy_spkm = wallet->GetOrCreateLegacyDataSPKM(); + WalletBatch batch{wallet->GetDatabase()}; // Add watch-only addresses std::vector scripts_watch_only; @@ -42,6 +43,7 @@ static void WalletMigration(benchmark::Bench& bench) const CScript& script = scripts_watch_only.emplace_back(GetScriptForDestination(dest)); assert(legacy_spkm->LoadWatchOnly(script)); assert(wallet->SetAddressBook(dest, strprintf("watch_%d", w), /*purpose=*/std::nullopt)); + batch.WriteWatchOnly(script, CKeyMetadata()); } // Generate transactions and local addresses @@ -58,6 +60,7 @@ static void WalletMigration(benchmark::Bench& bench) mtx.vout.emplace_back(COIN, scripts_watch_only.at(j % NUM_WATCH_ONLY_ADDR)); mtx.vin.resize(2); wallet->AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, /*rescanning_old_block=*/true); + batch.WriteKey(pubkey, key.GetPrivKey(), CKeyMetadata()); } bench.epochs(/*numEpochs=*/1).run([&context, &wallet] { diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index fe44aa8a3f20b..3d73459bf6c4f 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -529,7 +529,7 @@ bool BlockManager::LoadBlockIndexDB(const std::optional& snapshot_block } for (std::set::iterator it = setBlkDataFiles.begin(); it != setBlkDataFiles.end(); it++) { FlatFilePos pos(*it, 0); - if (OpenBlockFile(pos, true).IsNull()) { + if (OpenBlockFile(pos, /*fReadOnly=*/true).IsNull()) { return false; } } @@ -660,27 +660,30 @@ bool BlockManager::ReadBlockUndo(CBlockUndo& blockundo, const CBlockIndex& index const FlatFilePos pos{WITH_LOCK(::cs_main, return index.GetUndoPos())}; // Open history file to read - AutoFile filein{OpenUndoFile(pos, true)}; - if (filein.IsNull()) { - LogError("OpenUndoFile failed for %s", pos.ToString()); + AutoFile file{OpenUndoFile(pos, true)}; + if (file.IsNull()) { + LogError("OpenUndoFile failed for %s while reading block undo", pos.ToString()); return false; } + BufferedReader filein{std::move(file)}; - // Read block - uint256 hashChecksum; - HashVerifier verifier{filein}; // Use HashVerifier as reserializing may lose data, c.f. commit d342424301013ec47dc146a4beb49d5c9319d80a try { + // Read block + HashVerifier verifier{filein}; // Use HashVerifier, as reserializing may lose data, c.f. commit d3424243 + verifier << index.pprev->GetBlockHash(); verifier >> blockundo; + + uint256 hashChecksum; filein >> hashChecksum; - } catch (const std::exception& e) { - LogError("%s: Deserialize or I/O error - %s at %s\n", __func__, e.what(), pos.ToString()); - return false; - } - // Verify checksum - if (hashChecksum != verifier.GetHash()) { - LogError("%s: Checksum mismatch at %s\n", __func__, pos.ToString()); + // Verify checksum + if (hashChecksum != verifier.GetHash()) { + LogError("Checksum mismatch at %s while reading block undo", pos.ToString()); + return false; + } + } catch (const std::exception& e) { + LogError("Deserialize or I/O error - %s at %s while reading block undo", e.what(), pos.ToString()); return false; } @@ -931,29 +934,34 @@ bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationSt // Write undo information to disk if (block.GetUndoPos().IsNull()) { FlatFilePos pos; - const unsigned int blockundo_size{static_cast(GetSerializeSize(blockundo))}; + const auto blockundo_size{static_cast(GetSerializeSize(blockundo))}; if (!FindUndoPos(state, block.nFile, pos, blockundo_size + UNDO_DATA_DISK_OVERHEAD)) { - LogError("FindUndoPos failed"); + LogError("FindUndoPos failed for %s while writing block undo", pos.ToString()); return false; } - // Open history file to append - AutoFile fileout{OpenUndoFile(pos)}; - if (fileout.IsNull()) { - LogError("OpenUndoFile failed"); - return FatalError(m_opts.notifications, state, _("Failed to write undo data.")); - } - // Write index header - fileout << GetParams().MessageStart() << blockundo_size; - // Write undo data - pos.nPos += BLOCK_SERIALIZATION_HEADER_SIZE; - fileout << blockundo; + { + // Open history file to append + AutoFile file{OpenUndoFile(pos)}; + if (file.IsNull()) { + LogError("OpenUndoFile failed for %s while writing block undo", pos.ToString()); + return FatalError(m_opts.notifications, state, _("Failed to write undo data.")); + } + BufferedWriter fileout{file}; + + // Write index header + fileout << GetParams().MessageStart() << blockundo_size; + pos.nPos += STORAGE_HEADER_BYTES; + { + // Calculate checksum + HashWriter hasher{}; + hasher << block.pprev->GetBlockHash() << blockundo; + // Write undo data & checksum + fileout << blockundo << hasher.GetHash(); + } - // Calculate & write checksum - HashWriter hasher{}; - hasher << block.pprev->GetBlockHash(); - hasher << blockundo; - fileout << hasher.GetHash(); + fileout.flush(); // Make sure `AutoFile`/`BufferedWriter` go out of scope before we call `FlushUndoFile` + } // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order) // we want to flush the rev (undo) file once we've written the last block, which is indicated by the last height @@ -986,29 +994,28 @@ bool BlockManager::ReadBlock(CBlock& block, const FlatFilePos& pos) const block.SetNull(); // Open history file to read - AutoFile filein{OpenBlockFile(pos, true)}; - if (filein.IsNull()) { - LogError("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString()); + std::vector block_data; + if (!ReadRawBlock(block_data, pos)) { return false; } - // Read block try { - filein >> TX_WITH_WITNESS(block); + // Read block + SpanReader{block_data} >> TX_WITH_WITNESS(block); } catch (const std::exception& e) { - LogError("%s: Deserialize or I/O error - %s at %s\n", __func__, e.what(), pos.ToString()); + LogError("Deserialize or I/O error - %s at %s while reading block", e.what(), pos.ToString()); return false; } // Check the header if (!CheckProofOfWork(block.GetHash(), block.nBits, GetConsensus())) { - LogError("%s: Errors in block header at %s\n", __func__, pos.ToString()); + LogError("Errors in block header at %s while reading block", pos.ToString()); return false; } // Signet only: check block solution if (GetConsensus().signet_blocks && !CheckSignetBlockSolution(block, GetConsensus())) { - LogError("%s: Errors in block solution at %s\n", __func__, pos.ToString()); + LogError("Errors in block solution at %s while reading block", pos.ToString()); return false; } @@ -1023,7 +1030,7 @@ bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const return false; } if (block.GetHash() != index.GetBlockHash()) { - LogError("%s: GetHash() doesn't match index for %s at %s\n", __func__, index.ToString(), block_pos.ToString()); + LogError("GetHash() doesn't match index for %s at %s while reading block", index.ToString(), block_pos.ToString()); return false; } return true; @@ -1031,17 +1038,16 @@ bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos& pos) const { - FlatFilePos hpos = pos; - // If nPos is less than 8 the pos is null and we don't have the block data - // Return early to prevent undefined behavior of unsigned int underflow - if (hpos.nPos < 8) { - LogError("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString()); + if (pos.nPos < STORAGE_HEADER_BYTES) { + // If nPos is less than STORAGE_HEADER_BYTES, we can't read the header that precedes the block data + // This would cause an unsigned integer underflow when trying to position the file cursor + // This can happen after pruning or default constructed positions + LogError("Failed for %s while reading raw block storage header", pos.ToString()); return false; } - hpos.nPos -= 8; // Seek back 8 bytes for meta header - AutoFile filein{OpenBlockFile(hpos, true)}; + AutoFile filein{OpenBlockFile({pos.nFile, pos.nPos - STORAGE_HEADER_BYTES}, /*fReadOnly=*/true)}; if (filein.IsNull()) { - LogError("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString()); + LogError("OpenBlockFile failed for %s while reading raw block", pos.ToString()); return false; } @@ -1052,22 +1058,21 @@ bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos& filein >> blk_start >> blk_size; if (blk_start != GetParams().MessageStart()) { - LogError("%s: Block magic mismatch for %s: %s versus expected %s\n", __func__, pos.ToString(), - HexStr(blk_start), - HexStr(GetParams().MessageStart())); + LogError("Block magic mismatch for %s: %s versus expected %s while reading raw block", + pos.ToString(), HexStr(blk_start), HexStr(GetParams().MessageStart())); return false; } if (blk_size > MAX_SIZE) { - LogError("%s: Block data is larger than maximum deserialization size for %s: %s versus %s\n", __func__, pos.ToString(), - blk_size, MAX_SIZE); + LogError("Block data is larger than maximum deserialization size for %s: %s versus %s while reading raw block", + pos.ToString(), blk_size, MAX_SIZE); return false; } block.resize(blk_size); // Zeroing of memory is intentional here filein.read(MakeWritableByteSpan(block)); } catch (const std::exception& e) { - LogError("%s: Read from block file failed: %s for %s\n", __func__, e.what(), pos.ToString()); + LogError("Read from block file failed: %s for %s while reading raw block", e.what(), pos.ToString()); return false; } @@ -1077,22 +1082,23 @@ bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos& FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight) { const unsigned int block_size{static_cast(GetSerializeSize(TX_WITH_WITNESS(block)))}; - FlatFilePos pos{FindNextBlockPos(block_size + BLOCK_SERIALIZATION_HEADER_SIZE, nHeight, block.GetBlockTime())}; + FlatFilePos pos{FindNextBlockPos(block_size + STORAGE_HEADER_BYTES, nHeight, block.GetBlockTime())}; if (pos.IsNull()) { - LogError("FindNextBlockPos failed"); + LogError("FindNextBlockPos failed for %s while writing block", pos.ToString()); return FlatFilePos(); } - AutoFile fileout{OpenBlockFile(pos)}; - if (fileout.IsNull()) { - LogError("OpenBlockFile failed"); + AutoFile file{OpenBlockFile(pos, /*fReadOnly=*/false)}; + if (file.IsNull()) { + LogError("OpenBlockFile failed for %s while writing block", pos.ToString()); m_opts.notifications.fatalError(_("Failed to write block.")); return FlatFilePos(); } + BufferedWriter fileout{file}; // Write index header fileout << GetParams().MessageStart() << block_size; + pos.nPos += STORAGE_HEADER_BYTES; // Write block - pos.nPos += BLOCK_SERIALIZATION_HEADER_SIZE; fileout << TX_WITH_WITNESS(block); return pos; } @@ -1201,7 +1207,7 @@ void ImportBlocks(ChainstateManager& chainman, std::span import_ if (!fs::exists(chainman.m_blockman.GetBlockPosFilename(pos))) { break; // No block files left to reindex } - AutoFile file{chainman.m_blockman.OpenBlockFile(pos, true)}; + AutoFile file{chainman.m_blockman.OpenBlockFile(pos, /*fReadOnly=*/true)}; if (file.IsNull()) { break; // This error is logged in OpenBlockFile } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 665c2ccd83436..324f8e6860516 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -75,10 +75,10 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB /** Size of header written by WriteBlock before a serialized CBlock (8 bytes) */ -static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE{std::tuple_size_v + sizeof(unsigned int)}; +static constexpr uint32_t STORAGE_HEADER_BYTES{std::tuple_size_v + sizeof(unsigned int)}; /** Total overhead when writing undo data: header (8 bytes) plus checksum (32 bytes) */ -static constexpr size_t UNDO_DATA_DISK_OVERHEAD{BLOCK_SERIALIZATION_HEADER_SIZE + uint256::size()}; +static constexpr uint32_t UNDO_DATA_DISK_OVERHEAD{STORAGE_HEADER_BYTES + uint256::size()}; // Because validation code takes pointers to the map's CBlockIndex objects, if // we ever switch to another associative container, we need to either use a @@ -164,7 +164,7 @@ class BlockManager * blockfile info, and checks if there is enough disk space to save the block. * * The nAddSize argument passed to this function should include not just the size of the serialized CBlock, but also the size of - * separator fields (BLOCK_SERIALIZATION_HEADER_SIZE). + * separator fields (STORAGE_HEADER_BYTES). */ [[nodiscard]] FlatFilePos FindNextBlockPos(unsigned int nAddSize, unsigned int nHeight, uint64_t nTime); [[nodiscard]] bool FlushChainstateBlockFile(int tip_height); @@ -400,7 +400,7 @@ class BlockManager void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Open a block file (blk?????.dat) */ - AutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false) const; + AutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly) const; /** Translation to a filesystem path */ fs::path GetBlockPosFilename(const FlatFilePos& pos) const; diff --git a/src/random.cpp b/src/random.cpp index 5da053f74e4a6..fa3d8ec3f1e07 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -41,9 +41,6 @@ #ifdef HAVE_SYSCTL_ARND #include #endif -#if defined(HAVE_STRONG_GETAUXVAL) && defined(__aarch64__) -#include -#endif namespace { @@ -189,62 +186,6 @@ uint64_t GetRdSeed() noexcept #endif } -#elif defined(__aarch64__) && defined(HWCAP2_RNG) - -bool g_rndr_supported = false; - -void InitHardwareRand() -{ - if (getauxval(AT_HWCAP2) & HWCAP2_RNG) { - g_rndr_supported = true; - } -} - -void ReportHardwareRand() -{ - // This must be done in a separate function, as InitHardwareRand() may be indirectly called - // from global constructors, before logging is initialized. - if (g_rndr_supported) { - LogPrintf("Using RNDR and RNDRRS as additional entropy sources\n"); - } -} - -/** Read 64 bits of entropy using rndr. - * - * Must only be called when RNDR is supported. - */ -uint64_t GetRNDR() noexcept -{ - uint8_t ok = 0; - uint64_t r1; - do { - // https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDR--Random-Number - __asm__ volatile("mrs %0, s3_3_c2_c4_0; cset %w1, ne;" - : "=r"(r1), "=r"(ok)::"cc"); - if (ok) break; - __asm__ volatile("yield"); - } while (true); - return r1; -} - -/** Read 64 bits of entropy using rndrrs. - * - * Must only be called when RNDRRS is supported. - */ -uint64_t GetRNDRRS() noexcept -{ - uint8_t ok = 0; - uint64_t r1; - do { - // https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDRRS--Reseeded-Random-Number - __asm__ volatile("mrs %0, s3_3_c2_c4_1; cset %w1, ne;" - : "=r"(r1), "=r"(ok)::"cc"); - if (ok) break; - __asm__ volatile("yield"); - } while (true); - return r1; -} - #else /* Access to other hardware random number generators could be added here later, * assuming it is sufficiently fast (in the order of a few hundred CPU cycles). @@ -263,12 +204,6 @@ void SeedHardwareFast(CSHA512& hasher) noexcept { hasher.Write((const unsigned char*)&out, sizeof(out)); return; } -#elif defined(__aarch64__) && defined(HWCAP2_RNG) - if (g_rndr_supported) { - uint64_t out = GetRNDR(); - hasher.Write((const unsigned char*)&out, sizeof(out)); - return; - } #endif } @@ -294,14 +229,6 @@ void SeedHardwareSlow(CSHA512& hasher) noexcept { } return; } -#elif defined(__aarch64__) && defined(HWCAP2_RNG) - if (g_rndr_supported) { - for (int i = 0; i < 4; ++i) { - uint64_t out = GetRNDRRS(); - hasher.Write((const unsigned char*)&out, sizeof(out)); - } - return; - } #endif } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 950c7e0400481..14cebebdc071e 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -150,7 +150,7 @@ static std::vector CreateTxDoc() }, {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { - {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"}, + {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data that becomes a part of an OP_RETURN output"}, }, }, }, @@ -1550,13 +1550,15 @@ static RPCHelpMan createpsbt() { return RPCHelpMan{"createpsbt", "\nCreates a transaction in the Partially Signed Transaction format.\n" - "Implements the Creator role.\n", + "Implements the Creator role.\n" + "Note that the transaction's inputs are not signed, and\n" + "it is not stored in the wallet or transmitted to the network.\n", CreateTxDoc(), RPCResult{ RPCResult::Type::STR, "", "The resulting raw transaction (base64-encoded string)" }, RPCExamples{ - HelpExampleCli("createpsbt", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" \"[{\\\"data\\\":\\\"00010203\\\"}]\"") + HelpExampleCli("createpsbt", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" \"[{\\\"address\\\":0.01}]\"") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { diff --git a/src/serialize.h b/src/serialize.h index 98851056bdb71..b2f1ccd9cb1c9 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -253,14 +253,14 @@ template concept CharNotInt8 = std::same_as && !std::same_as; template void Serialize(Stream&, V) = delete; // char serialization forbidden. Use uint8_t or int8_t -template void Serialize(Stream& s, std::byte a) { ser_writedata8(s, uint8_t(a)); } -template inline void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, a); } +template void Serialize(Stream& s, std::byte a) { ser_writedata8(s, static_cast(a)); } +template inline void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, static_cast(a)); } template inline void Serialize(Stream& s, uint8_t a ) { ser_writedata8(s, a); } -template inline void Serialize(Stream& s, int16_t a ) { ser_writedata16(s, a); } +template inline void Serialize(Stream& s, int16_t a ) { ser_writedata16(s, static_cast(a)); } template inline void Serialize(Stream& s, uint16_t a) { ser_writedata16(s, a); } -template inline void Serialize(Stream& s, int32_t a ) { ser_writedata32(s, a); } +template inline void Serialize(Stream& s, int32_t a ) { ser_writedata32(s, static_cast(a)); } template inline void Serialize(Stream& s, uint32_t a) { ser_writedata32(s, a); } -template inline void Serialize(Stream& s, int64_t a ) { ser_writedata64(s, a); } +template inline void Serialize(Stream& s, int64_t a ) { ser_writedata64(s, static_cast(a)); } template inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); } template void Serialize(Stream& s, const B (&a)[N]) { s.write(MakeByteSpan(a)); } template void Serialize(Stream& s, const std::array& a) { s.write(MakeByteSpan(a)); } @@ -269,13 +269,13 @@ template void Serialize(Stream& s, std::span s template void Unserialize(Stream&, V) = delete; // char serialization forbidden. Use uint8_t or int8_t template void Unserialize(Stream& s, std::byte& a) { a = std::byte{ser_readdata8(s)}; } -template inline void Unserialize(Stream& s, int8_t& a ) { a = ser_readdata8(s); } +template inline void Unserialize(Stream& s, int8_t& a ) { a = static_cast(ser_readdata8(s)); } template inline void Unserialize(Stream& s, uint8_t& a ) { a = ser_readdata8(s); } -template inline void Unserialize(Stream& s, int16_t& a ) { a = ser_readdata16(s); } +template inline void Unserialize(Stream& s, int16_t& a ) { a = static_cast(ser_readdata16(s)); } template inline void Unserialize(Stream& s, uint16_t& a) { a = ser_readdata16(s); } -template inline void Unserialize(Stream& s, int32_t& a ) { a = ser_readdata32(s); } +template inline void Unserialize(Stream& s, int32_t& a ) { a = static_cast(ser_readdata32(s)); } template inline void Unserialize(Stream& s, uint32_t& a) { a = ser_readdata32(s); } -template inline void Unserialize(Stream& s, int64_t& a ) { a = ser_readdata64(s); } +template inline void Unserialize(Stream& s, int64_t& a ) { a = static_cast(ser_readdata64(s)); } template inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); } template void Unserialize(Stream& s, B (&a)[N]) { s.read(MakeWritableByteSpan(a)); } template void Unserialize(Stream& s, std::array& a) { s.read(MakeWritableByteSpan(a)); } diff --git a/src/streams.cpp b/src/streams.cpp index d82824ee5830c..19c2b47445274 100644 --- a/src/streams.cpp +++ b/src/streams.cpp @@ -87,21 +87,29 @@ void AutoFile::write(std::span src) } if (m_position.has_value()) *m_position += src.size(); } else { - if (!m_position.has_value()) throw std::ios_base::failure("AutoFile::write: position unknown"); std::array buf; - while (src.size() > 0) { + while (src.size()) { auto buf_now{std::span{buf}.first(std::min(src.size(), buf.size()))}; - std::copy(src.begin(), src.begin() + buf_now.size(), buf_now.begin()); - util::Xor(buf_now, m_xor, *m_position); - if (std::fwrite(buf_now.data(), 1, buf_now.size(), m_file) != buf_now.size()) { - throw std::ios_base::failure{"XorFile::write: failed"}; - } + std::copy_n(src.begin(), buf_now.size(), buf_now.begin()); + write_buffer(buf_now); src = src.subspan(buf_now.size()); - *m_position += buf_now.size(); } } } +void AutoFile::write_buffer(std::span src) +{ + if (!m_file) throw std::ios_base::failure("AutoFile::write_buffer: file handle is nullptr"); + if (m_xor.size()) { + if (!m_position) throw std::ios_base::failure("AutoFile::write_buffer: obfuscation position unknown"); + util::Xor(src, m_xor, *m_position); // obfuscate in-place + } + if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) { + throw std::ios_base::failure("AutoFile::write_buffer: write failed"); + } + if (m_position) *m_position += src.size(); +} + bool AutoFile::Commit() { return ::FileCommit(m_file); diff --git a/src/streams.h b/src/streams.h index 20bdaf2c060ad..1ebcff3671fd2 100644 --- a/src/streams.h +++ b/src/streams.h @@ -445,6 +445,9 @@ class AutoFile /** Wrapper around TruncateFile(). */ bool Truncate(unsigned size); + //! Write a mutable buffer more efficiently than write(), obfuscating the buffer in-place. + void write_buffer(std::span src); + // // Stream subset // @@ -467,6 +470,8 @@ class AutoFile } }; +using DataBuffer = std::vector; + /** Wrapper around an AutoFile& that implements a ring buffer to * deserialize from. It guarantees the ability to rewind a given number of bytes. * @@ -481,7 +486,7 @@ class BufferedFile uint64_t m_read_pos{0}; //!< how many bytes have been read from this uint64_t nReadLimit; //!< up to which position we're allowed to read uint64_t nRewind; //!< how many bytes we guarantee to rewind - std::vector vchBuf; //!< the buffer + DataBuffer vchBuf; //! read data from the source to fill the buffer bool Fill() { @@ -523,7 +528,7 @@ class BufferedFile } public: - BufferedFile(AutoFile& file, uint64_t nBufSize, uint64_t nRewindIn) + BufferedFile(AutoFile& file LIFETIMEBOUND, uint64_t nBufSize, uint64_t nRewindIn) : m_src{file}, nReadLimit{std::numeric_limits::max()}, nRewind{nRewindIn}, vchBuf(nBufSize, std::byte{0}) { if (nRewindIn >= nBufSize) @@ -614,4 +619,87 @@ class BufferedFile } }; +/** + * Wrapper that buffers reads from an underlying stream. + * Requires underlying stream to support read() and detail_fread() calls + * to support fixed-size and variable-sized reads, respectively. + */ +template +class BufferedReader +{ + S& m_src; + DataBuffer m_buf; + size_t m_buf_pos; + +public: + //! Requires stream ownership to prevent leaving the stream at an unexpected position after buffered reads. + explicit BufferedReader(S&& stream LIFETIMEBOUND, size_t size = 1 << 16) + requires std::is_rvalue_reference_v + : m_src{stream}, m_buf(size), m_buf_pos{size} {} + + void read(std::span dst) + { + if (const auto available{std::min(dst.size(), m_buf.size() - m_buf_pos)}) { + std::copy_n(m_buf.begin() + m_buf_pos, available, dst.begin()); + m_buf_pos += available; + dst = dst.subspan(available); + } + if (dst.size()) { + assert(m_buf_pos == m_buf.size()); + m_src.read(dst); + + m_buf_pos = 0; + m_buf.resize(m_src.detail_fread(m_buf)); + } + } + + template + BufferedReader& operator>>(T&& obj) + { + Unserialize(*this, obj); + return *this; + } +}; + +/** + * Wrapper that buffers writes to an underlying stream. + * Requires underlying stream to support write_buffer() method + * for efficient buffer flushing and obfuscation. + */ +template +class BufferedWriter +{ + S& m_dst; + DataBuffer m_buf; + size_t m_buf_pos{0}; + +public: + explicit BufferedWriter(S& stream LIFETIMEBOUND, size_t size = 1 << 16) : m_dst{stream}, m_buf(size) {} + + ~BufferedWriter() { flush(); } + + void flush() + { + if (m_buf_pos) m_dst.write_buffer(std::span{m_buf}.first(m_buf_pos)); + m_buf_pos = 0; + } + + void write(std::span src) + { + while (const auto available{std::min(src.size(), m_buf.size() - m_buf_pos)}) { + std::copy_n(src.begin(), available, m_buf.begin() + m_buf_pos); + m_buf_pos += available; + if (m_buf_pos == m_buf.size()) flush(); + src = src.subspan(available); + } + } + + template + BufferedWriter& operator<<(const T& obj) + { + Serialize(*this, obj); + return *this; + } +}; + #endif // BITCOIN_STREAMS_H diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 8f8cce687f536..49e49b2d536df 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -17,7 +17,7 @@ #include #include -using node::BLOCK_SERIALIZATION_HEADER_SIZE; +using node::STORAGE_HEADER_BYTES; using node::BlockManager; using node::KernelNotifications; using node::MAX_BLOCKFILE_SIZE; @@ -40,12 +40,12 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) }; BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; // simulate adding a genesis block normally - BOOST_CHECK_EQUAL(blockman.WriteBlock(params->GenesisBlock(), 0).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); + BOOST_CHECK_EQUAL(blockman.WriteBlock(params->GenesisBlock(), 0).nPos, STORAGE_HEADER_BYTES); // simulate what happens during reindex // simulate a well-formed genesis block being found at offset 8 in the blk00000.dat file // the block is found at offset 8 because there is an 8 byte serialization header // consisting of 4 magic bytes + 4 length bytes before each block in a well-formed blk file. - const FlatFilePos pos{0, BLOCK_SERIALIZATION_HEADER_SIZE}; + const FlatFilePos pos{0, STORAGE_HEADER_BYTES}; blockman.UpdateBlockInfo(params->GenesisBlock(), 0, pos); // now simulate what happens after reindex for the first new block processed // the actual block contents don't matter, just that it's a block. @@ -54,7 +54,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) // 8 bytes (for serialization header) + 285 (for serialized genesis block) = 293 // add another 8 bytes for the second block's serialization header and we get 293 + 8 = 301 FlatFilePos actual{blockman.WriteBlock(params->GenesisBlock(), 1)}; - BOOST_CHECK_EQUAL(actual.nPos, BLOCK_SERIALIZATION_HEADER_SIZE + ::GetSerializeSize(TX_WITH_WITNESS(params->GenesisBlock())) + BLOCK_SERIALIZATION_HEADER_SIZE); + BOOST_CHECK_EQUAL(actual.nPos, STORAGE_HEADER_BYTES + ::GetSerializeSize(TX_WITH_WITNESS(params->GenesisBlock())) + STORAGE_HEADER_BYTES); } BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain100Setup) @@ -172,19 +172,19 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) FlatFilePos pos2{blockman.WriteBlock(block2, /*nHeight=*/2)}; // Two blocks in the file - BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); + BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + STORAGE_HEADER_BYTES) * 2); // First two blocks are written as expected // Errors are expected because block data is junk, thrown AFTER successful read CBlock read_block; BOOST_CHECK_EQUAL(read_block.nVersion, 0); { - ASSERT_DEBUG_LOG("ReadBlock: Errors in block header"); + ASSERT_DEBUG_LOG("Errors in block header"); BOOST_CHECK(!blockman.ReadBlock(read_block, pos1)); BOOST_CHECK_EQUAL(read_block.nVersion, 1); } { - ASSERT_DEBUG_LOG("ReadBlock: Errors in block header"); + ASSERT_DEBUG_LOG("Errors in block header"); BOOST_CHECK(!blockman.ReadBlock(read_block, pos2)); BOOST_CHECK_EQUAL(read_block.nVersion, 2); } @@ -199,7 +199,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) // Metadata is updated... BOOST_CHECK_EQUAL(block_data->nBlocks, 3); // ...but there are still only two blocks in the file - BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); + BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + STORAGE_HEADER_BYTES) * 2); // Block 2 was not overwritten: blockman.ReadBlock(read_block, pos2); diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index 2942740395274..cda7725a1367a 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2022 The Bitcoin Core developers +// Copyright (c) 2020-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -33,6 +33,12 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) bool good_data{true}; CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES}; + + uint32_t current_height{0}; + const auto advance_height{ + [&] { current_height = fuzzed_data_provider.ConsumeIntegralInRange(current_height, 1 << 30); }, + }; + advance_height(); LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000) { CallOneOf( @@ -44,7 +50,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) return; } const CTransaction tx{*mtx}; - const CTxMemPoolEntry& entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx); + const auto entry{ConsumeTxMemPoolEntry(fuzzed_data_provider, tx, current_height)}; const auto tx_submitted_in_package = fuzzed_data_provider.ConsumeBool(); const auto tx_has_mempool_parents = fuzzed_data_provider.ConsumeBool(); const auto tx_info = NewMempoolTransactionInfo(entry.GetSharedTx(), entry.GetFee(), @@ -68,14 +74,15 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) break; } const CTransaction tx{*mtx}; - mempool_entries.emplace_back(CTxMemPoolEntry::ExplicitCopy, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx)); + mempool_entries.emplace_back(CTxMemPoolEntry::ExplicitCopy, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx, current_height)); } std::vector txs; txs.reserve(mempool_entries.size()); for (const CTxMemPoolEntry& mempool_entry : mempool_entries) { txs.emplace_back(mempool_entry); } - block_policy_estimator.processBlock(txs, fuzzed_data_provider.ConsumeIntegral()); + advance_height(); + block_policy_estimator.processBlock(txs, current_height); }, [&] { (void)block_policy_estimator.removeTx(ConsumeUInt256(fuzzed_data_provider)); diff --git a/src/test/fuzz/util/mempool.cpp b/src/test/fuzz/util/mempool.cpp index 8e7499a860df5..a6a28f9400659 100644 --- a/src/test/fuzz/util/mempool.cpp +++ b/src/test/fuzz/util/mempool.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2022 The Bitcoin Core developers +// Copyright (c) 2022-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -14,7 +14,7 @@ #include #include -CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept +CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx, uint32_t max_height) noexcept { // Avoid: // policy/feerate.cpp:28:34: runtime error: signed integer overflow: 34873208148477500 * 1000 cannot be represented in type 'long' @@ -24,7 +24,7 @@ CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, assert(MoneyRange(fee)); const int64_t time = fuzzed_data_provider.ConsumeIntegral(); const uint64_t entry_sequence{fuzzed_data_provider.ConsumeIntegral()}; - const unsigned int entry_height = fuzzed_data_provider.ConsumeIntegral(); + const auto entry_height{fuzzed_data_provider.ConsumeIntegralInRange(0, max_height)}; const bool spends_coinbase = fuzzed_data_provider.ConsumeBool(); const unsigned int sig_op_cost = fuzzed_data_provider.ConsumeIntegralInRange(0, MAX_BLOCK_SIGOPS_COST); return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, entry_sequence, spends_coinbase, sig_op_cost, {}}; diff --git a/src/test/fuzz/util/mempool.h b/src/test/fuzz/util/mempool.h index 31b578dc4b8ec..948e936c75076 100644 --- a/src/test/fuzz/util/mempool.h +++ b/src/test/fuzz/util/mempool.h @@ -1,4 +1,4 @@ -// Copyright (c) 2022 The Bitcoin Core developers +// Copyright (c) 2022-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -21,6 +21,6 @@ class DummyChainState final : public Chainstate } }; -[[nodiscard]] CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept; +[[nodiscard]] CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx, uint32_t max_height=std::numeric_limits::max()) noexcept; #endif // BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 1a44e66932cce..c7b5cd353e00f 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -2,6 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include +#include #include #include #include @@ -553,6 +555,146 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) fs::remove(streams_test_filename); } +BOOST_AUTO_TEST_CASE(buffered_reader_matches_autofile_random_content) +{ + const size_t file_size{1 + m_rng.randrange(1 << 17)}; + const size_t buf_size{1 + m_rng.randrange(file_size)}; + const FlatFilePos pos{0, 0}; + + const FlatFileSeq test_file{m_args.GetDataDirBase(), "buffered_file_test_random", node::BLOCKFILE_CHUNK_SIZE}; + const std::vector obfuscation{m_rng.randbytes(8)}; + + // Write out the file with random content + { + AutoFile{test_file.Open(pos, /*read_only=*/false), obfuscation}.write(m_rng.randbytes(file_size)); + } + BOOST_CHECK_EQUAL(fs::file_size(test_file.FileName(pos)), file_size); + + { + AutoFile direct_file{test_file.Open(pos, /*read_only=*/true), obfuscation}; + + AutoFile buffered_file{test_file.Open(pos, /*read_only=*/true), obfuscation}; + BufferedReader buffered_reader{std::move(buffered_file), buf_size}; + + for (size_t total_read{0}; total_read < file_size;) { + const size_t read{Assert(std::min(1 + m_rng.randrange(m_rng.randbool() ? buf_size : 2 * buf_size), file_size - total_read))}; + + DataBuffer direct_file_buffer{read}; + direct_file.read(direct_file_buffer); + + DataBuffer buffered_buffer{read}; + buffered_reader.read(buffered_buffer); + + BOOST_CHECK_EQUAL_COLLECTIONS( + direct_file_buffer.begin(), direct_file_buffer.end(), + buffered_buffer.begin(), buffered_buffer.end() + ); + + total_read += read; + } + + { + DataBuffer excess_byte{1}; + BOOST_CHECK_EXCEPTION(direct_file.read(excess_byte), std::ios_base::failure, HasReason{"end of file"}); + } + + { + DataBuffer excess_byte{1}; + BOOST_CHECK_EXCEPTION(buffered_reader.read(excess_byte), std::ios_base::failure, HasReason{"end of file"}); + } + } + + fs::remove(test_file.FileName(pos)); +} + +BOOST_AUTO_TEST_CASE(buffered_writer_matches_autofile_random_content) +{ + const size_t file_size{1 + m_rng.randrange(1 << 17)}; + const size_t buf_size{1 + m_rng.randrange(file_size)}; + const FlatFilePos pos{0, 0}; + + const FlatFileSeq test_buffered{m_args.GetDataDirBase(), "buffered_write_test", node::BLOCKFILE_CHUNK_SIZE}; + const FlatFileSeq test_direct{m_args.GetDataDirBase(), "direct_write_test", node::BLOCKFILE_CHUNK_SIZE}; + const std::vector obfuscation{m_rng.randbytes(8)}; + + { + DataBuffer test_data{m_rng.randbytes(file_size)}; + + AutoFile direct_file{test_direct.Open(pos, /*read_only=*/false), obfuscation}; + + AutoFile buffered_file{test_buffered.Open(pos, /*read_only=*/false), obfuscation}; + BufferedWriter buffered{buffered_file, buf_size}; + + for (size_t total_written{0}; total_written < file_size;) { + const size_t write_size{Assert(std::min(1 + m_rng.randrange(m_rng.randbool() ? buf_size : 2 * buf_size), file_size - total_written))}; + + auto current_span = std::span{test_data}.subspan(total_written, write_size); + direct_file.write(current_span); + buffered.write(current_span); + + total_written += write_size; + } + // Destructors of AutoFile and BufferedWriter will flush/close here + } + + // Compare the resulting files + DataBuffer direct_result{file_size}; + { + AutoFile verify_direct{test_direct.Open(pos, /*read_only=*/true), obfuscation}; + verify_direct.read(direct_result); + + DataBuffer excess_byte{1}; + BOOST_CHECK_EXCEPTION(verify_direct.read(excess_byte), std::ios_base::failure, HasReason{"end of file"}); + } + + DataBuffer buffered_result{file_size}; + { + AutoFile verify_buffered{test_buffered.Open(pos, /*read_only=*/true), obfuscation}; + verify_buffered.read(buffered_result); + + DataBuffer excess_byte{1}; + BOOST_CHECK_EXCEPTION(verify_buffered.read(excess_byte), std::ios_base::failure, HasReason{"end of file"}); + } + + BOOST_CHECK_EQUAL_COLLECTIONS( + direct_result.begin(), direct_result.end(), + buffered_result.begin(), buffered_result.end() + ); + + fs::remove(test_direct.FileName(pos)); + fs::remove(test_buffered.FileName(pos)); +} + +BOOST_AUTO_TEST_CASE(buffered_writer_reader) +{ + const uint32_t v1{m_rng.rand32()}, v2{m_rng.rand32()}, v3{m_rng.rand32()}; + const fs::path test_file{m_args.GetDataDirBase() / "test_buffered_write_read.bin"}; + + // Write out the values through a precisely sized BufferedWriter + { + AutoFile file{fsbridge::fopen(test_file, "w+b")}; + BufferedWriter f(file, sizeof(v1) + sizeof(v2) + sizeof(v3)); + f << v1 << v2; + f.write(std::as_bytes(std::span{&v3, 1})); + } + // Read back and verify using BufferedReader + { + uint32_t _v1{0}, _v2{0}, _v3{0}; + AutoFile file{fsbridge::fopen(test_file, "rb")}; + BufferedReader f(std::move(file), sizeof(v1) + sizeof(v2) + sizeof(v3)); + f >> _v1 >> _v2; + f.read(std::as_writable_bytes(std::span{&_v3, 1})); + BOOST_CHECK_EQUAL(_v1, v1); + BOOST_CHECK_EQUAL(_v2, v2); + BOOST_CHECK_EQUAL(_v3, v3); + + DataBuffer excess_byte{1}; + BOOST_CHECK_EXCEPTION(f.read(excess_byte), std::ios_base::failure, HasReason{"end of file"}); + } + + fs::remove(test_file); +} + BOOST_AUTO_TEST_CASE(streams_hashed) { DataStream stream{}; diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 8e6747e363006..b89b373741856 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -983,7 +983,7 @@ static std::vector OutputsDoc() }, {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { - {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"}, + {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data that becomes a part of an OP_RETURN output"}, }, }, }; @@ -1734,8 +1734,10 @@ RPCHelpMan walletcreatefundedpsbt() } }, RPCExamples{ - "\nCreate a transaction with no inputs\n" - + HelpExampleCli("walletcreatefundedpsbt", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" \"[{\\\"data\\\":\\\"00010203\\\"}]\"") + "\nCreate a PSBT with automatically picked inputs that sends 0.5 BTC to an address and has a fee rate of 2 sat/vB:\n" + + HelpExampleCli("walletcreatefundedpsbt", "\"[]\" \"[{\\\"" + EXAMPLE_ADDRESS[0] + "\\\":0.5}]\" 0 \"{\\\"add_inputs\\\":true,\\\"fee_rate\\\":2}\"") + + "\nCreate the same PSBT as the above one instead using named arguments:\n" + + HelpExampleCli("-named walletcreatefundedpsbt", "outputs=\"[{\\\"" + EXAMPLE_ADDRESS[0] + "\\\":0.5}]\" add_inputs=true fee_rate=2") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { diff --git a/test/functional/rpc_whitelist.py b/test/functional/rpc_whitelist.py index ad6af4c9648b4..a35c89fadd2de 100755 --- a/test/functional/rpc_whitelist.py +++ b/test/functional/rpc_whitelist.py @@ -26,7 +26,7 @@ def rpccall(node, user, method): def get_permissions(whitelist): - return [perm for perm in whitelist.replace(" ", "").split(",") if perm] + return [perm for perm in whitelist.split(",") if perm] class RPCWhitelistTest(BitcoinTestFramework): @@ -56,7 +56,7 @@ def run_test(self): # Testing the same permission twice ["strangedude5", "d12c6e962d47a454f962eb41225e6ec8$2dd39635b155536d3c1a2e95d05feff87d5ba55f2d5ff975e6e997a836b717c9", ":getblockcount,getblockcount", "s7R4nG3R7H1nGZ"], # Test non-whitelisted user - ["strangedude6", "ab02e4fb22ef4ab004cca217a49ee8d2$90dd09b08edd12d552d9d8a5ada838dcef2ac587789fa7e9c47f5990e80cdf93", None, "password123"] + ["strangedude6", "67e5583538958883291f6917883eca64$8a866953ef9c5b7d078a62c64754a4eb74f47c2c17821eb4237021d7ef44f991", None, "N4SziYbHmhC1"] ] # These commands shouldn't be allowed for any user to test failures self.never_allowed = ["getnetworkinfo"] @@ -74,7 +74,7 @@ def run_test(self): for user in self.users: for permission in self.never_allowed: - self.log.info("[" + user[0] + "]: Testing a non permitted permission (" + permission + ")") + self.log.info(f"[{user[0]}]: Testing a non permitted permission ({permission})") assert_equal(403, rpccall(self.nodes[0], user, permission).status) # Now test the strange users for permission in self.never_allowed: @@ -91,7 +91,7 @@ def run_test(self): assert_equal(200, rpccall(self.nodes[0], self.strange_users[4], "getblockcount").status) self.test_users_permissions() - self.test_rpcwhitelistdefault_0_no_permissions() + self.test_rpcwhitelistdefault_permissions(0, 200) # Replace file configurations self.nodes[0].replace_in_config([("rpcwhitelistdefault=0", "rpcwhitelistdefault=1")]) @@ -99,9 +99,16 @@ def run_test(self): f.write("rpcwhitelist=__cookie__:getblockcount,getblockchaininfo,getmempoolinfo,stop\n") self.restart_node(0) - # Test rpcwhitelistdefault=1 self.test_users_permissions() - self.test_rpcwhitelistdefault_1_no_permissions() + self.test_rpcwhitelistdefault_permissions(1, 403) + + # Ensure that not specifying -rpcwhitelistdefault is the same as + # specifying -rpcwhitelistdefault=1. Only explicitly whitelisted users + # should be allowed. + self.nodes[0].replace_in_config([("rpcwhitelistdefault=1", "")]) + self.restart_node(0) + self.test_users_permissions() + self.test_rpcwhitelistdefault_permissions(1, 403) def test_users_permissions(self): """ @@ -113,32 +120,23 @@ def test_users_permissions(self): for user in self.users: permissions = get_permissions(user[2]) for permission in permissions: - self.log.info("[" + user[0] + "]: Testing whitelisted user permission (" + permission + ")") + self.log.info(f"[{user[0]}]: Testing whitelisted user permission ({permission})") assert_equal(200, rpccall(self.nodes[0], user, permission).status) - self.log.info("[" + user[0] + "]: Testing non-permitted permission: getblockchaininfo") + self.log.info(f"[{user[0]}]: Testing non-permitted permission: getblockchaininfo") assert_equal(403, rpccall(self.nodes[0], user, "getblockchaininfo").status) - def test_rpcwhitelistdefault_0_no_permissions(self): + def test_rpcwhitelistdefault_permissions(self, default_value, expected_status): """ - * rpcwhitelistdefault=0 + * rpcwhitelistdefault={default_value} * No Permissions defined - Expected result: * strangedude6 (not whitelisted) can access any method + Expected result: strangedude6 (not whitelisted) access is determined by default_value + When default_value=0: expects 200 + When default_value=1: expects 403 """ - unrestricted_user = self.strange_users[6] - for permission in ["getbestblockhash", "getblockchaininfo"]: - self.log.info("[" + unrestricted_user[0] + "]: Testing unrestricted user permission (" + permission + ")") - assert_equal(200, rpccall(self.nodes[0], unrestricted_user, permission).status) - - def test_rpcwhitelistdefault_1_no_permissions(self): - """ - * rpcwhitelistdefault=1 - * No Permissions defined - Expected result: * strangedude6 (not whitelisted) can not access any method - """ - + user = self.strange_users[6] # strangedude6 for permission in ["getbestblockhash", "getblockchaininfo"]: - self.log.info("[" + self.strange_users[6][0] + "]: Testing rpcwhitelistdefault=1 no specified permission (" + permission + ")") - assert_equal(403, rpccall(self.nodes[0], self.strange_users[6], permission).status) + self.log.info(f"[{user[0]}]: Testing rpcwhitelistdefault={default_value} no specified permission ({permission})") + assert_equal(expected_status, rpccall(self.nodes[0], user, permission).status) if __name__ == "__main__": diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 52f6bbe283849..7416e08a1fd39 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -919,6 +919,8 @@ def send_cli(self, clicommand=None, *args, **kwargs): # Ignore cli_stdout, raise with cli_stderr raise subprocess.CalledProcessError(returncode, p_args, output=cli_stderr) try: + if not cli_stdout.strip(): + return None return json.loads(cli_stdout, parse_float=decimal.Decimal) except (json.JSONDecodeError, decimal.InvalidOperation): return cli_stdout.rstrip("\n") diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index a92b16b1d9c3c..45b8c93c75cbe 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -44,36 +44,25 @@ unsigned-integer-overflow:arith_uint256.h unsigned-integer-overflow:CBloomFilter::Hash unsigned-integer-overflow:CRollingBloomFilter::insert unsigned-integer-overflow:RollingBloomHash -unsigned-integer-overflow:CCoinsViewCache::AddCoin -unsigned-integer-overflow:CCoinsViewCache::BatchWrite -unsigned-integer-overflow:CCoinsViewCache::DynamicMemoryUsage -unsigned-integer-overflow:CCoinsViewCache::SpendCoin -unsigned-integer-overflow:CCoinsViewCache::Uncache unsigned-integer-overflow:CompressAmount unsigned-integer-overflow:DecompressAmount unsigned-integer-overflow:crypto/ unsigned-integer-overflow:MurmurHash3 -unsigned-integer-overflow:CBlockPolicyEstimator::processBlockTx unsigned-integer-overflow:TxConfirmStats::EstimateMedianVal -unsigned-integer-overflow:prevector.h unsigned-integer-overflow:InsecureRandomContext::rand64 unsigned-integer-overflow:InsecureRandomContext::SplitMix64 unsigned-integer-overflow:bitset_detail::PopCount -implicit-integer-sign-change:CBlockPolicyEstimator::processBlockTx implicit-integer-sign-change:SetStdinEcho implicit-integer-sign-change:compressor.h implicit-integer-sign-change:crypto/ implicit-integer-sign-change:TxConfirmStats::removeTx -implicit-integer-sign-change:prevector.h implicit-integer-sign-change:verify_flags implicit-integer-sign-change:EvalScript -implicit-integer-sign-change:serialize.h implicit-signed-integer-truncation:crypto/ implicit-unsigned-integer-truncation:crypto/ shift-base:arith_uint256.cpp shift-base:crypto/ shift-base:streams.h shift-base:FormatHDKeypath -shift-base:InsecureRandomContext::rand64 shift-base:RandomMixin<*>::randbits shift-base:RandomMixin<*>::randbits<*>