Made every signed/unsigned conversion in the serialization helpers explicit so the UBSan `implicit-sign-change` check passes and the `serialize.h` suppression can be dropped.
For consistency, a few other simple changes were also applied to the serialization helpers:
* remove redundant `inline` on function templates;
* unify formatting to make the differences between similar methods obvious.
The original recursive `FindChallenges` explores the Miniscript node tree using depth-first search. Specifically, it performs a pre-order traversal (processing the node's data, then recursively visiting children from left-to-right). This recursion uses the call stack, which can lead to stack overflows on platforms with limited stack space, particularly noticeable in Windows debug builds.
This change replaces the recursive implementation with an iterative version using an explicit stack. The iterative version also performs a depth-first search and processes the node's data before exploring children (preserving pre-order characteristics), although the children are explored in right-to-left order due to the LIFO nature of the explicit stack.
Critically, both versions collect challenges into a `std::set`, which automatically deduplicates and sorts elements. This ensures that not only the final result, but the actual state of the set at any equivalent point in traversal remains identical, despite the difference in insertion order.
This iterative approach is an alternative to increasing the default stack size (as proposed in #32349) and directly addresses the stack overflow issue reported in #32341 by avoiding deep recursion.
The change is done in two commits:
* add a new iterative `FindChallenges` method and rename the old method to `*_recursive` (to simplify removal in the next commit), asserting that its result matches the original;
* Remove the original recursive implementation.
This approach avoids needing to suppress `misc-no-recursion` warnings and provides a portable, low-risk fix.
Using a `std::set` is necessary for deduplication, matching the original function's behavior. An experiment using an `std::vector` showed duplicate challenges being added, confirming the need for the set:
Example failure with vector:
Recursive (set):
(6, 9070746)
(6, 19532513)
(6, 3343376967)
Iterative (vector attempt):
(6, 19532513)
(6, 9070746)
(6, 3343376967)
(6, 9070746) // Duplicate
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
The migration benchmark crashes if run more than once, because of `std::move(wallet)` and leaves subsequent iterations in an undefined state - avoiding `UndefinedBehaviorSanitizer` null‑dereference error.
`MigrateLegacyToDescriptor` returns both a spendable descriptor wallet and a watch‑only wallet.
If these remain attached, their files stay open and on Windows this can hang CI when removing the test directory.
By constructing them via `MakeWalletLoader` (which owns the `WalletContext`), both wallets are automatically unloaded when the loader is destroyed at the end.
This ensures no lingering handles or resource leaks when running the benchmark on CI with `-sanity-check`.
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
The obfuscation (XOR) operations are currently done byte-by-byte during serialization. Buffering the reads will enable batching the obfuscation operations later.
Different operating systems handle file caching differently, so reading larger batches (and processing them from memory) is measurably faster, likely because of fewer native fread calls and reduced lock contention.
Note that `ReadRawBlock` doesn't need buffering since it already reads the whole block directly.
Unlike `ReadBlockUndo`, the new `ReadBlock` implementation delegates to `ReadRawBlock`, which uses more memory than a buffered alternative but results in slightly simpler code and a small performance increase (~0.4%). This approach also clearly documents that `ReadRawBlock` is a logical subset of `ReadBlock` functionality.
The current implementation, which iterates over a fixed-size buffer, provides a more general alternative to Cory Fields' solution of reading the entire block size in advance.
Buffer sizes were selected based on benchmarking to ensure the buffered reader produces performance similar to reading the whole block into memory. Smaller buffers were slower, while larger ones showed diminishing returns.
------
> macOS Sequoia 15.3.1
> C++ compiler .......................... Clang 19.1.7
> cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ && cmake --build build -j$(nproc) && build/bin/bench_bitcoin -filter='ReadBlockBench' -min-time=10000
Before:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 2,271,441.67 | 440.25 | 0.1% | 11.00 | `ReadBlockBench`
After:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1,738,971.29 | 575.05 | 0.2% | 10.97 | `ReadBlockBench`
------
> Ubuntu 24.04.2 LTS
> C++ compiler .......................... GNU 13.3.0
> cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ && cmake --build build -j$(nproc) && build/bin/bench_bitcoin -filter='ReadBlockBench' -min-time=20000
Before:
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 6,895,987.11 | 145.01 | 0.0% | 71,055,269.86 | 23,977,374.37 | 2.963 | 5,074,828.78 | 0.4% | 22.00 | `ReadBlockBench`
After:
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 5,771,882.71 | 173.25 | 0.0% | 65,741,889.82 | 20,453,232.33 | 3.214 | 3,971,321.75 | 0.3% | 22.01 | `ReadBlockBench`
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: Martin Leitner-Ankerl <martin.ankerl@gmail.com>
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
Made every OpenBlockFile#fReadOnly value explicit.
Replaced hard-coded values in ReadRawBlock with STORAGE_HEADER_BYTES.
Changed `STORAGE_HEADER_BYTES` and `UNDO_DATA_DISK_OVERHEAD` to `uint32_t` to avoid casts.
Also added `LIFETIMEBOUND` to the `AutoFile` parameter of `BufferedFile`, which stores a reference to the underlying `AutoFile`, allowing Clang to emit warnings if the referenced `AutoFile` might be destroyed while `BufferedFile` still exists.
Without this attribute, code with lifetime violations wouldn't trigger compiler warnings.
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Renames the constant to be less verbose and better reflect its purpose:
it represents the size of the storage header that precedes serialized block data on disk,
not to be confused with a block's own header.
-BEGIN VERIFY SCRIPT-
git grep -q "STORAGE_HEADER_BYTES" $(git ls-files) && echo "Error: Target name STORAGE_HEADER_BYTES already exists in the codebase" && exit 1
sed -i 's/BLOCK_SERIALIZATION_HEADER_SIZE/STORAGE_HEADER_BYTES/g' $(git grep -l 'BLOCK_SERIALIZATION_HEADER_SIZE')
-END VERIFY SCRIPT-
Reorganized error handling in block-related operations by grouping related operations together within the same scope.
In `ReadBlockUndo()` and `ReadBlock()`, moved all deserialization operations, comments and checksum verification inside a single try/catch block for cleaner error handling.
In `WriteBlockUndo()`, consolidated hash calculation and data writing operations within a common block to better express their logical relationship.
Remove the manual batch size estimation logic (`SizeEstimate()` method and `size_estimate` member) from `CDBBatch`.
Size is now determined solely by the `ApproximateSize()` method introduced in the previous commit, which delegates to the native LevelDB function.
The manual calculation is no longer necessary as LevelDB now provides this functionality directly, and the previous commit verified that the native function's results matched the manual estimation.
Assertions comparing the two methods are removed from `txdb.cpp`.
Co-authored-by: Wladimir J. van der Laan <laanwj@protonmail.com>
Serialized batch size can be queried via the underlying LevelDB implementation calling the native `leveldb::WriteBatch::ApproximateSize()`.
The previous manual calculation was added in https://github.com/bitcoin/bitcoin/pull/10195/commits/e66dbde6d14cb5f253b3bf8850a98f4fda2d9f49 as part of https://github.com/bitcoin/bitcoin/pull/10195. At that time (April 2017), the version of LevelDB used by Bitcoin Core (and even the latest source) lacked a native function for this. LevelDB added this capability in https://github.com/google/leveldb/commit/69e2bd224b7f11e021527cb95bab18f1ee6e1b3b, merged later that year.
The old manual estimation method (`SizeEstimate()`) is kept temporarily in this commit, and assertions are added in `txdb.cpp` to verify its results against `ApproximateSize()` during batch writes. This ensures the native function behaves as expected before removing the manual calculation in the subsequent commit.
The initialization of the manual `size_estimate` in `CDBBatch::Clear()` is corrected from `0` to `kHeader` (LevelDB's fixed batch header size).
This aligns the manual estimate with LevelDB's actual size immediately after clearing, fixing discrepancies that would otherwise be caught by tests in the next commit (e.g., `coins_tests`, `validation_chainstatemanager_tests`).
https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1817851827 introduced an unsequenced operations with side-effects - which is undefined behavior, i.e. the right hand side can be evaluated before the left hand side, which happens to mutate it.
Tried:
```
clang++ --analyze -std=c++20 -I./src -I./src/test -I./src/test/fuzz src/test/fuzz/base_encode_decode.cpp src/psbt.cpp
```
but it didn't warn about UB.
Grepped for similar ones, but could find any other one in the codebase:
> grep -rnE --include='*.cpp' --include='*.h' '\b(\w+)\(([^)]*\b(\w+)\b[^)]*)\)\s*==\s*\3\.' .
```
./src/test/arith_uint256_tests.cpp:373: BOOST_CHECK(R1L.GetHex() == R1L.ToString());
./src/test/arith_uint256_tests.cpp:374: BOOST_CHECK(R2L.GetHex() == R2L.ToString());
./src/test/arith_uint256_tests.cpp:375: BOOST_CHECK(OneL.GetHex() == OneL.ToString());
./src/test/arith_uint256_tests.cpp:376: BOOST_CHECK(MaxL.GetHex() == MaxL.ToString());
./src/test/fuzz/cluster_linearize.cpp:565: assert(depgraph.FeeRate(best_anc.transactions) == best_anc.feerate);
./src/test/fuzz/cluster_linearize.cpp:646: assert(depgraph.FeeRate(found.transactions) == found.feerate);
./src/test/fuzz/cluster_linearize.cpp:765: assert(depgraph.FeeRate(chunk_info.transactions) == chunk_info.feerate);
./src/test/fuzz/base_encode_decode.cpp:95: assert(DecodeBase64PSBT(psbt, random_string, error) == error.empty());
./src/test/fuzz/key.cpp:102: assert(pubkey.data() == pubkey.begin());
./src/test/skiplist_tests.cpp:42: BOOST_CHECK(vIndex[from].GetAncestor(0) == vIndex.data());
./src/script/signingprovider.cpp:535: ComputeTapbranchHash(node.sub[1]->hash, node.sub[1]->hash) == node.hash) {
./src/pubkey.h:78: return vch.size() > 0 && GetLen(vch[0]) == vch.size();
./src/cluster_linearize.h:881: Assume(elem.inc.feerate.IsEmpty() == elem.pot_feerate.IsEmpty());
```
Hodlinator deduced the UB on Windows in https://github.com/bitcoin/bitcoin/issues/32135#issuecomment-2751723855
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
In Base58 fuzz the two roundtrips are merged now, the new `decode_input` switches between a completely random input and a valid encoded one, to make sure the decoding passes more often.
The `max_ret_len` can also exceed the original length now and is being validated more thoroughly.
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Co-authored-by: marcofleon <marleo23@proton.me>
They seem to cause timeouts:
> Issue 397734700: bitcoin-core:base58check_encode_decode: Timeout in base58check_encode_decode
The `encoded_string.empty()` check was corrected here to `decoded.empty()` to make sure the `(0, decoded.size() - 1)` range is always valid.
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Co-authored-by: marcofleon <marleo23@proton.me>
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
The use of e.g. `std::underlying_type_t<T>` replaces the older `typename std::underlying_type<T>::type`.
The `_t` helper alias template (such as `std::underlying_type_t<T>`) introduced in C++14 offers a cleaner and more concise way to extract the type directly.
See https://en.cppreference.com/w/cpp/types/underlying_type for details.
-BEGIN VERIFY SCRIPT-
sed -i -E 's/(typename )?(std::[a-z_]+)(<[^<>]+>)::type\b/\2_t\3/g' $(git grep -l '::type' ./src ':(exclude)src/bench/nanobench.h' ':(exclude)src/leveldb' ':(exclude)src/minisketch' ':(exclude)src/span.h' ':(exclude)src/sync.h')
-END VERIFY SCRIPT-
When the behavior was changes in a previous commit (caching `GetSerializeSize` and avoiding `AutoFile.tell`), (static)asserts were added to make sure the behavior was kept - to make sure reviewers and CI validates it.
We can safely remove them now.
Logs were also slightly modernized since they were trivial to do.
Co-authored-by: Anthony Towns <aj@erisian.com.au>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
For consistency `UNDO_DATA_DISK_OVERHEAD` was also extracted to avoid the constant's ambiguity.
Asserts were added to help with the review - they are removed in the next commit.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Example logs before the change:
```
2025-01-07T11:58:33Z Verification progress: 99%
2025-01-07T11:58:33Z Verification: No coin database inconsistencies in last 6 blocks (18905 transactions)
2025-01-07T11:58:33Z block index 31892ms
2025-01-07T11:58:33Z Setting NODE_NETWORK on non-prune mode
2025-01-07T11:58:33Z block tree size = 878086
2025-01-07T11:58:33Z nBestHeight = 878085
```
Removed redundant duration as well since it can be recovered from the timestamps.
Co-authored-by: TheCharlatan <seb.kung@gmail.com>
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Similarly, `WriteBlockToDisk` wasn't really extracting a meaningful subset of the `SaveBlockToDisk` functionality, it's tied closely to the only caller (needs the header size twice, recalculated block serializes size, returns multiple branches, mutates parameter).
The inlined code should only differ in these parts (modernization will be done in other commits):
* renamed `blockPos` to `pos` in `SaveBlockToDisk` to match the parameter name;
* changed `return false` to `return FlatFilePos()`.
Also removed remaining references to `SaveBlockToDisk`.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
`UndoWriteToDisk` wasn't really extracting a meaningful subset of the `WriteUndoDataForBlock` functionality, it's tied closely to the only caller (needs the header size twice, recalculated undo serializes size, returns multiple branches, modifies parameter, needs documentation).
The inlined code should only differ in these parts (modernization will be done in other commits):
* renamed `_pos` to `pos` in `WriteUndoDataForBlock` to match the parameter name;
* inlined `hashBlock` parameter usage into `hasher << block.pprev->GetBlockHash()`;
* changed `return false` to `return FatalError`;
* capitalize comment.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Different values are used for max_ret_len throughout the codebase (e.g., 21, 34, 78).
Theoretically, negative and zero values are also permitted. Let's stress-test those as well.
Co-authored-by: brunoerg <brunoely.gc@gmail.com>
This commit introduces symmetric encode-decode roundtrips for all bases.
Minor refactors were also included:
• Split each base into a separate fuzz target.
• Added symmetric encode-decode roundtrip tests for all bases.
• Removed trim testing for encoded_string, as Base58 does not use whitespace padding.
• Made comparisons stricter by removing unnecessary lowercase conversions for bases that have mixed-case alphabets.
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Setting a large `-dbcache` size postpones the index writes until the coins cache size exceeds the specified limit.
This causes the final flush after manual termination to seemingly hang forever (e.g. tens of minutes for 20 GiB);
Now that the `dbcache` upper cap has been lifted, this will become even more apparent, so a warning will be shown when large UTXO sets are flushed (currently >1 GiB), such as:
> 2024-12-18T18:25:03Z Flushed fee estimates to fee_estimates.dat.
> 2024-12-18T18:25:03Z [warning] Flushing large (1 GiB) UTXO set to disk, it may take several minutes
> 2024-12-18T18:25:09Z Shutdown: done
Note that the related BCLog::BENCH units were also converted to `KiB` from `kB` to unify the bases.
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
Use character literals instead of integer hex values (i.e. `'\x5b','\x0a', ...` instead of `0x5b, 0x0a, ...`) for generated headers.
This avoids C++11 narrowing warnings in a more concise way than using explicit char casts.
Extra whitespace is also removed between elements for brevity.
The `ccoins_add` and `ccoins_write` tests check the actual exception error messages now instead of just that they fail for the given parameters.
This enables us testing different exceptions in a more fine-grained way in later changes.
We don't need so much access to the internals of CCoinsCacheEntry, since many tests are just exercising invalid combinations this way.
This implies that `AddFlags` has private access now.
CCoinsCacheEntry provided general access to its internal flags state, even though in reality it could only be clean, fresh, dirty or fresh|dirty.
After it got dirtied we couldn't set the state back to clean by AddFlags(0) - tests were explicitly checking against that.
This commit cleans up the public interface to make this distinction cleaner and invalid behavior impossible instead of just checked by tests.
This includes the removal of redundant `inline` qualifiers (we're inside a struct).
Also renamed `self` to `pair` to simplify the upcoming commits.
Also modernized `EmplaceCoinInternalDANGER` since it was already modified.
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
* Since the main LIMITED_WHILE stated `outpoints.size() < 200'000`, I've presized outpoints accordingly.
* `tx_mut.vin` and `tx_mut.vout` weren't caught by the clang-tidy, but addressed them anyway.
- Modify `SipHash_32b` benchmark to use `FastRandomContext` for generating initial values.
- Cycle through and modify each byte of the `uint256` value to ensure no part of it can be optimized away.
The lack of "recursion" (where the method call overwrites the used inputs partially) and the systematic modification of each input byte makes the benchmark usage more reliable and thorough.
Also separated the roundtrip testing from the random string decoding for clarity
Note that while BIP 173 claims:
```
The human-readable part, which is intended to convey the type of data, or anything else that is relevant to the reader. This part MUST contain 1 to 83 US-ASCII characters, with each character having a value in the range [33-126]. HRP validity may be further restricted by specific applications.
```
bech32::Encode rejects uppercase letters.
Tested the performance with:
> time cmake -DJSON_SOURCE_PATH=src/secp256k1/src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.json -DHEADER_PATH=build/after/ecdsa_secp256k1_sha256_bitcoin_test.json -P cmake/script/GenerateHeaderFromJson.cmake
Before:
> 3.57s user 6.01s system 94% cpu 10.136 total
After:
> 0.17s user 0.01s system 98% cpu 0.187 total
Replaced multiple file writes with a single string template write.
The raw content is first grouped into 8 byte chunks, followed by another regex replace which wraps them in `std::byte`.
Tested the output with `diff -w` and they're the same - only whitespace differences because slightly different source formatting.
Tested the performance with:
> time cmake -DRAW_SOURCE_PATH=src/bench/data/block413567.raw -DHEADER_PATH=build/after/block413567.raw.h -DRAW_NAMESPACE=benchmark::data -P cmake/script/GenerateHeaderFromRaw.cmake
Before:
> 15.41s user 23.06s system 97% cpu 39.593 total
After:
> 0.77s user 0.06s system 97% cpu 0.849 total
Extracted existing serialization to append size & data in separate private methods to clarify that it does more than just a simple data insertion.
* the C style casts were changed to static_cast
* `unsigned char` and `uint8_t` were changed to value_type for forward compatibility
* `data + sizeof(data)` was changed to `std::cend`
* data insertion (in AppendData) relies on pointer arithmetic now to enable both `std::span<const value_type>` and `std::span<const std::byte>` operators
* use uint32_t for data size instead of size_t
* used span instead of raw pointers in the new methods
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
When compiling with GCC 12.2, both `-Warray-bounds` and `-Wstringop-overflow` warnings were triggered in the `prevector::insert` method during CScript prevector operations.
GCC incorrectly assumed that operator new could modify the state of class members, leading to false positives during the memmove operation.
Following the approach in https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=cca06f0d6d76b0, we introduced local copies for the destination pointer in memmove operations. This prevents GCC from misinterpreting memory manipulation as unsafe.
A minimal reproducer triggering this issue in GCC 12.2 and passing in GCC 12.3 can be found at https://godbolt.org/z/8r9TKKoxv.
-------
Full error (with changes from the next commit as well):
```
In file included from /ci_container_base/src/script/script.h:11,
from /ci_container_base/src/primitives/transaction.h:11,
from /ci_container_base/src/primitives/block.h:9,
from /ci_container_base/src/kernel/chainparams.h:11,
from /ci_container_base/src/kernel/chainparams.cpp:6:
In member function ‘void prevector<N, T, Size, Diff>::fill(T*, InputIterator, InputIterator) [with InputIterator = const unsigned char*; unsigned int N = 28; T = unsigned char; Size = unsigned int; Diff = int]’,
inlined from ‘void prevector<N, T, Size, Diff>::insert(iterator, InputIterator, InputIterator) [with InputIterator = const unsigned char*; unsigned int N = 28; T = unsigned char; Size = unsigned int; Diff = int]’ at /ci_container_base/src/prevector.h:395:13,
inlined from ‘void CScript::AppendData(const prevector<28, unsigned char>::value_type*, size_t)’ at /ci_container_base/src/script/script.h:439:15,
inlined from ‘CScript& CScript::operator<<(std::span<const std::byte>)’ at /ci_container_base/src/script/script.h:496:17,
inlined from ‘CBlock CreateGenesisBlock(uint32_t, uint32_t, uint32_t, int32_t, const CAmount&)’ at /ci_container_base/src/kernel/chainparams.cpp:76:54:
/ci_container_base/src/prevector.h:216:13: error: writing 65 bytes into a region of size 32 [-Werror=stringop-overflow=]
216 | new(static_cast<void*>(dst)) T(*first);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/ci_container_base/src/kernel/chainparams.cpp: In function ‘CBlock CreateGenesisBlock(uint32_t, uint32_t, uint32_t, int32_t, const CAmount&)’:
/ci_container_base/src/kernel/chainparams.cpp:76:49: note: destination object ‘<anonymous>’ of size 32
76 | const CScript genesisOutputScript = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
| ^
In file included from /usr/lib/gcc/x86_64-w64-mingw32/12-posix/include/c++/cstring:42,
from /ci_container_base/src/crypto/common.h:11,
from /ci_container_base/src/uint256.h:9,
from /ci_container_base/src/consensus/params.h:9,
from /ci_container_base/src/kernel/chainparams.h:9:
In function ‘void* memmove(void*, const void*, size_t)’,
inlined from ‘void prevector<N, T, Size, Diff>::insert(iterator, InputIterator, InputIterator) [with InputIterator = const unsigned char*; unsigned int N = 28; T = unsigned char; Size = unsigned int; Diff = int]’ at /ci_container_base/src/prevector.h:393:16,
inlined from ‘void CScript::AppendData(const prevector<28, unsigned char>::value_type*, size_t)’ at /ci_container_base/src/script/script.h:439:15,
inlined from ‘CScript& CScript::operator<<(std::span<const std::byte>)’ at /ci_container_base/src/script/script.h:496:17,
inlined from ‘CBlock CreateGenesisBlock(uint32_t, uint32_t, uint32_t, int32_t, const CAmount&)’ at /ci_container_base/src/kernel/chainparams.cpp:76:54:
/usr/share/mingw-w64/include/string.h:214:33: warning: ‘void* __builtin_memmove(void*, const void*, long long unsigned int)’ offset [65, 35] is out of the bounds [0, 32] of object ‘<anonymous>’ with type ‘CScript’ [-Warray-bounds]
214 | return __builtin___memmove_chk(__dst, __src, __n, __mingw_bos(__dst, 0));
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/ci_container_base/src/kernel/chainparams.cpp: In function ‘CBlock CreateGenesisBlock(uint32_t, uint32_t, uint32_t, int32_t, const CAmount&)’:
/ci_container_base/src/kernel/chainparams.cpp:76:49: note: ‘<anonymous>’ declared here
76 | const CScript genesisOutputScript = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
| ^
```
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Example error before:
> unknown location:0: fatal error: in "validation_chainstatemanager_tests/chainstatemanager_args": std::bad_optional_access: bad_optional_access
test/validation_chainstatemanager_tests.cpp:781: last checkpoint
after:
> test/validation_chainstatemanager_tests.cpp:801: error: in "validation_chainstatemanager_tests/chainstatemanager_args": check set_opts({"-assumevalid=0"}).assumed_valid_block == uint256::ZERO has failed [std::nullopt != 0000000000000000000000000000000000000000000000000000000000000000]
Also added extra minimum_chainwork test to make it symmetric with assumevalid
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Enhanced efficiency and readability of CCoinsViewCache::FetchCoin by replacing separate find() and emplace() calls with a single try_emplace(), reducing map lookups and potential insertions.
AddToBlock was called repeatedly from `addPackageTxs` where the constant value of `printpriority` is recalculated every time.
Since its behavior was changed in 400b151, I've named the variable accordingly.
This showed up during profiling of AssembleBlock, fetching it once in the constructor results in a measurable speed increase for many iterations.
> ./src/bench/bench_bitcoin --filter='AssembleBlock' --min-time=1000
before:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 155,558.97 | 6,428.43 | 0.1% | 1.10 | `AssembleBlock`
after:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 148,083.68 | 6,752.94 | 0.1% | 1.10 | `AssembleBlock`
Co-authored-by: furszy <mfurszy@protonmail.com>
Lőrinc