refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors #30765

l0rinc · 2024-08-30T12:48:02Z

Replace _hex_v_u8 for CScript appends to _hex, to skip vector conversion before serializing to the prevector in CScript.

To enable both unsigned char and std::byte values, I've extracted the existing serialization to append the size & data in separate private methods to clarify that it does more than just a simple data insertion.

There were also discussion on eliminating the operators here completely to obviate when we're serializing fixed-size collections as raw bytes, and when we're prefixing them with their size - should also be done in a separate PR.

DrahtBot · 2024-08-30T12:48:06Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky, hodlinator, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#28710 (Remove the legacy wallet and BDB dependency by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

src/script/script.h

DrahtBot · 2024-09-06T15:30:50Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/29778092690}

Hints

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

src/script/script.h

ryanofsky

Code review e6c02b9

ryanofsky · 2024-09-09T12:52:25Z

src/prevector.h

+        // Passing dst variable with explicit type instead of temporary expression
+        // calms down GCC under some circumstances.


In commit "refactor: Make code more tolerant of constexpr std::arrays" (c4c102d)

Sorry I didn't read the previous threads which might explain this, but this is a very vague comment which should be improved. It's is not clear looking at the new code what "temporary expression" or "explicit type" the comment is referring to, what "calms down GCC" could mean or what "some circumstances" could be. Would suggest writing a more understandable comment like // Passing dst to memmove instead of (ptr + count) prevents GCC error/warning "<whatever the error/warning is>"

Related to #30765 (comment), will see what I can do to document it better. I guess the explanation should rather go to the commit message instead of the code, since the code itself isn't doing anything special.

Dropped the commit after the suggested simplification

Got the error again, trying to create a godbolt reproducer to understand it better

Error

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::PushData(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::PushData(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; | ^ cc1plus: all warnings being treated as errors make[2]: *** [src/CMakeFiles/bitcoin_common.dir/build.make:422: src/CMakeFiles/bitcoin_common.dir/kernel/chainparams.cpp.obj] Error 1 make[1]: *** [CMakeFiles/Makefile2:864: src/CMakeFiles/bitcoin_common.dir/all] Error 2 make: *** [Makefile:146: all] Error 2

This error was reported many times for GCC 12.2, managed to reproduce the error via godbolt (it's fixed in GCC 12.3) and on a local Dockerfile:

Dockerfile

FROM docker.io/amd64/debian:bookworm # Update and install dependencies RUN apt update && apt install -y \ build-essential \ cmake \ git \ curl \ wget \ pkg-config \ bsdmainutils \ python3 \ libevent-dev \ libboost-system-dev \ libboost-filesystem-dev \ libboost-chrono-dev \ libboost-test-dev \ libboost-thread-dev \ libminiupnpc-dev \ libzmq3-dev \ libsqlite3-dev \ libdb-dev \ libdb++-dev \ libssl-dev \ libprotobuf-dev \ protobuf-compiler \ libqrencode-dev \ libqt5gui5 \ libqt5core5a \ libqt5dbus5 \ qttools5-dev \ qttools5-dev-tools \ libseccomp-dev \ libcap-dev \ nsis \ g++-mingw-w64-x86-64-posix \ wine-binfmt \ wine64 \ file RUN gcc --version && cmake --version COPY . /bitcoin WORKDIR /bitcoin RUN git clean -fdx RUN cmake -B build -DREDUCE_EXPORTS=ON -DBUILD_GUI_TESTS=OFF -DCMAKE_CXX_FLAGS='-Werror' RUN cmake --build build -j8

Fixed in a39f57f (#30765), added all related info to the commit message, let me know if more info is still needed.
Thanks @hodlinator and @ryanofsky for the reviews and suggestions!

src/script/script.h

src/test/util_tests.cpp

src/span.h

ryanofsky · 2024-09-09T13:50:55Z

src/script/script.h

@@ -498,6 +498,11 @@ class CScript : public CScriptBase
        return *this;
    }

+    CScript& operator<<(const std::span<const std::byte> b) LIFETIMEBOUND
+    {
+        return *this << BytesToUInt8s(b);


In commit "Add std::byte span overload to CScript" (e6c02b9)

If code is moving away from char and uint8_t and towards std::byte, it would seem better for the uint8_t overload to call the std::byte overload (with UInt8sToBytes) instead of vice versa so the uint8_t overload could be dropped in the future without having to change this code again.

Yes, that's what I tried at first, but I don't yet see how that would be possible without changing prevector, as explained here

Yes, that's what I tried at first, but I don't yet see how that would be possible without changing prevector, as explained here

Thanks that's helpful to know, and it's clear why changing prevector could fix this but not clear why it would be necessary to change prevector to fix that as opposed to just passing a start and end pointer of the type it is expecting

Because we were casting back and forth this way. But I think I found a good enough generalization with your help, we don't need to connect the two operators this way.

DrahtBot · 2024-09-10T09:08:09Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/29897924592}

Hints

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

ryanofsky

Code review ACK 48b67d8. This looks great! PR is much simpler now, and it is nice to see the GCC workaround expanded and documented so well.

Would suggest a few changes, implemented in diff below:

Replacing PushDataSize size_t parameter with uint32_t parameter to document fact that it can't accept larger sizes, and to call WriteLE32 without a type conversion.
Replacing PushData const value_type* data, size_t size parameters with std::span<const value_type> data because using span is more idiomatic and self-documenting.
Fixing grammar in operator<< comment so it doesn't sound like std::byte is preferred for backwards compatibility.
Making the char operator call the std::byte operator to make it smaller, deduplicate code, and make it obvious the operators are equivalent.

Suggested changes:

diff

--- a/src/script/script.h
+++ b/src/script/script.h
@@ -414,7 +414,7 @@ bool GetScriptOp(CScriptBase::const_iterator& pc, CScriptBase::const_iterator en
 class CScript : public CScriptBase
 {
 private:
-    inline void PushDataSize(const size_t size)
+    inline void PushDataSize(const uint32_t size)
     {
         if (size < OP_PUSHDATA1) {
             insert(end(), static_cast<value_type>(size));
@@ -434,9 +434,9 @@ private:
         }
     }
 
-    void PushData(const value_type* data, size_t size)
+    void PushData(std::span<const value_type> data)
     {
-        insert(end(), data, data + size);
+        insert(end(), data.begin(), data.end());
     }
 
 protected:
@@ -493,16 +493,14 @@ public:
     CScript& operator<<(std::span<const std::byte> b) LIFETIMEBOUND
     {
         PushDataSize(b.size());
-        PushData(reinterpret_cast<const value_type*>(b.data()), b.size());
+        PushData({reinterpret_cast<const value_type*>(b.data()), b.size()});
         return *this;
     }
 
-    // For backwards compatibility, please prefer std::byte over uint8_t in new code
+    // For backwards compatibility. Please prefer std::byte over uint8_t in new code.
     CScript& operator<<(std::span<const value_type> b) LIFETIMEBOUND
     {
-        PushDataSize(b.size());
-        PushData(b.data(), b.size());
-        return *this;
+        return *this << std::as_bytes(b);
     }
 
     bool GetOp(const_iterator& pc, opcodetype& opcodeRet, std::vector<unsigned char>& vchRet) const

l0rinc · 2024-09-10T22:20:28Z

Replacing PushDataSize size_t parameter with uint32_t parameter

Good observation, thanks.
I was being overly cautious with modifying that code.

Making the char operator call the std::byte operator to make it smaller, deduplicate code, and make it obvious the operators are equivalent.

My goal was to avoid extra casting and wrapping, especially in performance-critical areas like this.

Right now, we avoid intermediary vectors, but with this change, we'd wrap the data in a std::span, recast via std::as_bytes, only to unwrap it again during insertion.
While std::span is lightweight, these extra steps feel unnecessary here, especially in the case of operator<<(std::span<const value_type>), where we're recasting and rewrapping data we already have in its final form, only to unwrap it again during insertion.
It feels like overkill for something that could be done directly - it's why I've extracted the size and data insertion. I don’t see that as repetition (especially compared to the insertion methods in prevector).

I don't like raw pointers either, so if you insist, I'll change it to span, but I want to make sure you understand why I prefer the current version.

hodlinator

ACK e6a5ab7

git range-diff master 4574e45 e6a5ab7

`a39f57f`

Nice work on the GCC bug!
Wish the example error in the commit message didn't come from code that only becomes possible in later commit. Maybe worth at least noting that in the message?

`e6a5ab7`

Commit message:
"Replace _hex_v_u8 CScript appends to _hex"
->
"Replace _hex_v_u8 CScript appends with _hex"

src/script/script.h

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>

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>

This will skip vector conversion before serializing to the prevector in CScript.

ryanofsky

Code review ACK 5e190cd. Looks good!

Since last review, just renamed push methods to append, and applied a few other suggested tweaks

maflcko · 2024-09-11T17:15:04Z

src/script/script.h

+        AppendDataSize(b.size());
+        AppendData({reinterpret_cast<const value_type*>(b.data()), b.size()});


Is there a reason to extract those function? I think it would be better to not extract them, because:

The diff would be smaller

They are only used and called once

There shouldn't be a reason for them to be called from somewhere else internally, because operator<< is available internally and externally

If someone wanted to use them externally (for whatever reason), they'd have to be moved again, because they are private right now.

Seems easier to extract them and possibly make them public when there is need, instead of repeatedly changing the code.

Not reuse, but to obviate that the method does two separate things: complex logic for how to serialize a number, followed by inserting bytes - so basically a separation of concerns.

And also #30765 (comment) indicated that operators may not be here to stay, so it makes sense to unburden them early.

hodlinator

re-ACK 5e190cd

git range-diff master e6a5ab7 5e190cd

Disagree with AppendData/AppendDataSize names over PushData/PushDataSize, but will let it go.
Nice switch back from pointer arithmetic to span in AppendData.
Touched up commit messages per my latest review, cheers!
Implements legacy operator<< in terms of the other. I agree with l0rinc's reservations against the added casting uint8_t->std::byte->uint8_t until CScript::value_type becomes std::byte, but shouldn't cause any runtime overhead.

achow101 · 2024-09-20T17:33:59Z

The PR is still in a draft phase since there isn't an overall agreement about the exact direction.

Is this still the case? If not, please update the OP.

l0rinc · 2024-09-20T18:47:59Z

Is this still the case? If not, please update the OP.

Removed, thank you.

achow101 · 2024-09-20T19:02:17Z

ACK 5e190cd

…47757ea3b 1047757ea3b kernel: Add pure kernel bitcoin-chainstate c568fdf75fd kernel: Add block index utility functions to C header 0f1da1dcba5 kernel: Add function to read block undo data from disk to C header 45af559c9f6 kernel: Add functions to read block from disk to C header 2a7f8a8240c kernel: Add function for copying block data to C header b19f5336c03 kernel: Add functions for the block validation state to C header 9c0ffa913f4 kernel: Add validation interface to C header a93318c6152 kernel: Add interrupt function to C header 51053f33720 kernel: Add import blocks function to C header 6b0ada2af42 kernel: Add chainstate load options for in-memory dbs in C header 34427bfa9c7 kernel: Add options for reindexing in C header ca57311c969 kernel: Add block validation to C header 44156d84838 Kernel: Add chainstate loading to kernel C header 2cee46cdcc1 kernel: Add chainstate manager object to C header 7102c7ae45e kernel: Add notifications context option to C header ed628a2a3c4 kerenl: Add chain params context option to C header 27643297ff7 kernel: Add kernel library context object 2ba22cf3f90 kernel: Add logging to kernel library C header 873874c03e9 kernel: Introduce initial kernel C header API d94adc7270b Merge bitcoin/bitcoin#29702: fees: Remove CLIENT_VERSION serialization 7290bc61c00 Merge bitcoin/bitcoin#31078: build: Fix kernel static lib component install 68f29b24907 Merge bitcoin/bitcoin#31141: doc: Make list of targets in depends README consistent e9b95665eea Merge bitcoin/bitcoin#31046: init: Some small chainstate load improvements b8c821cc1ea Merge bitcoin/bitcoin#30724: test: add test for specifying custom pidfile via `-pid` a0c9595810c doc: Make list of targets in depends README consistent fa1c5cc9df1 fees: Log non-fatal errors as [warning], instead of info-level ffe4261cb06 Merge bitcoin/bitcoin#30935: ci: Approximate MAKEJOBS in image build phase 28ce159bc32 Merge bitcoin/bitcoin#30183: rpc: net: follow-ups for #30062 684873931b3 Merge bitcoin/bitcoin#26334: Add Signet and testnet4 launch shortcuts for Windows 9b0e2598089 Merge bitcoin/bitcoin#31121: guix: Enable CET for `glibc` package d9f8dc64534 Merge bitcoin/bitcoin#31097: validation: Improve input script check error reporting a16917fb598 rpc, net: improve `mapped_as` doc for getrawaddrman/getpeerinfo 563c4d29268 Merge bitcoin/bitcoin#31105: Update libmultiprocess library 0e9f20625a1 Merge bitcoin/bitcoin#31063: lint: commit-script-check.sh: echo to stderr e8f72aefd20 Merge bitcoin/bitcoin#29877: tracing: explicitly cast block_connected duration to nanoseconds 86e2a6b749c [test] A non-standard transaction which is also consensus-invalid should return the consensus error 4d3da08d1b9 guix: Enable CET for `glibc` package a38603456e9 Merge bitcoin/bitcoin#31100: doc: remove dependency install instructions from win docs 90b405516f7 Update libmultiprocess library 479715e9db0 Merge bitcoin/bitcoin#30996: doc: update signet documentation related to build directories 99e041f86fd Merge bitcoin/bitcoin#31099: doc: drop macOS LLVM install instructions 21e2f06a1cc Merge bitcoin/bitcoin#31067: test: Print CompletedProcess object on error 184f12c1542 doc: remove dependency install instructions from win docs dea9fb9a8b8 Merge bitcoin/bitcoin#30093: optimization: reserve memory allocation for transaction inputs/outputs 79aa8280b2e doc: drop LLVM install instructions 2123c94448e Merge bitcoin/bitcoin#30527: Bump python minimum supported version to 3.10 538ccaed004 Merge bitcoin/bitcoin#31048: build: Bump minimum supported macOS to 13.0 f859ff8a4e9 [validation] Improve script check error reporting ddddbac9c10 fees: Pin required version to 149900 fa5126adcb1 fees: Pin "version that wrote" to 0 0ca1d1bf69c Merge bitcoin/bitcoin#31092: doc: fuzz: remove Honggfuzz NetDriver instructions d823ba6e20b doc: fuzz: remove Honggfuzz NetDriver instructions 15563d3388e Merge bitcoin/bitcoin#30859: doc: cmake: prepend "build" to functional/test_runner.py 2ac5ba24bf0 Merge bitcoin/bitcoin#31083: doc: add doxygen for m_args in tests a0e089a71dc build: Bump minimum supported macOS to 13.0 1fe1b3ba8e9 doc: doxygen comment for m_args usage in tests 82e16e69832 cmake: Refactor install kernel dependencies 42e62779873 build: Add static libraries to Kernel install component e64b2f1a16e doc: cmake: prepend and explain "build/" where needed 48cf3da6360 Merge bitcoin/bitcoin#30970: build: Add missing USDT header dependency to kernel d8b835cf18c Merge bitcoin/bitcoin#31070: contrib: fix typos in check-deps.sh da8824ba301 Fix typos in check-deps.sh fa43c4f93ca test: Print CompletedProcess object on error 489e5aa3a29 Merge bitcoin/bitcoin#30857: cluster mempool: extend DepGraph functionality 9f45062b9b0 Merge bitcoin/bitcoin#30937: build: scripted-diff: drop config/ subdir for bitcoin-config.h 882f736d0a6 doc: lint: correct outdated comment (s/Makefile.am/CMakeLists.txt/) 1786be7b4a5 scripted-diff: drop config/ subdir for bitcoin-config.h, rename to bitcoin-build-config.h 0c2c3bb3f5c Merge bitcoin/bitcoin#30955: Mining interface: getCoinbaseMerklePath() and submitSolution() 9909a34d794 Merge bitcoin/bitcoin#30992: doc: update IBD requirements in doc/README.md fac6cfe5ac0 lint: commit-script-check.sh: echo to stderr 5fb94550638 Merge bitcoin/bitcoin#31058: refactor: include the proper header rather than forward-declaring RemovalReasonToString e569eb8d917 Merge bitcoin/bitcoin#30885: scripted-diff: Modernize nLocalServices naming 31cc5006c3d init: Return fatal failure on snapshot validation failure 8f1246e8338 init: Improve chainstate init db error messages 5837e3463fe Merge bitcoin/bitcoin#30967: refactor: Replace g_genesis_wait_cv with m_tip_block_cv ca2e4ba352c refactor: include the proper header rather than forward-declaring RemovalReasonToString a9f6a57b691 Merge bitcoin/bitcoin#30920: test: Remove 0.16.3 test from wallet_backwards_compatibility.py fa71bedf860 ci: Approximate MAKEJOBS in image build phase 03696bb1bd5 Merge bitcoin/bitcoin#31045: ci: Add missing -DWERROR=ON to test-each-commit 56093565bbe Merge bitcoin/bitcoin#31018: test: Treat exclude list warning as failure in CI bb47b5a6576 Merge bitcoin/bitcoin#31038: test: Fix copy-paste in wallet/test/db_tests ostream operator 3fecf36c7b3 Merge bitcoin/bitcoin#31056: ci: Double ctest timeout 3c4a9419dbe Merge bitcoin/bitcoin#31013: depends: For mingw cross compile use -gcc-posix to prevent library conflict 5d5cc021ce3 Merge bitcoin/bitcoin#31051: test: remove unused code from `script_tests` caf44e500eb Merge bitcoin/bitcoin#31008: depends: Print ready-to-use `--toolchain` option for CMake invocation fa5ebc99207 ci: Double ctest timeout 0b3ec8c59b2 clusterlin: remove Cluster type 1c24c625105 clusterlin: merge two DepGraph fuzz tests into simulation test 0606e66fdbb clusterlin: add DepGraph::RemoveTransactions and support for holes in DepGraph 75b5d42419e clusterlin: make DepGraph::AddDependency support multiple dependencies at once abf50649d13 clusterlin: simplify DepGraphFormatter::Ser eaab55ffc81 clusterlin: rework DepGraphFormatter::Unser 5901cf7100a clusterlin: abstract out DepGraph::GetReduced{Parents,Children} e0287bc4b2d test: remove unused code from script_tests 62e45167221 Merge bitcoin/bitcoin#31026: ci: set a ctest test timeout of 1200 (20 minutes) cd093049dda init: Remove incorrect comment about shutdown condition 635e9f85d76 init: Remove misleading log line when user chooses not to retry fa1cffacae6 ci: Install missing nproc in macos task faf7a2bccc7 ci: Add missing -DWERROR=ON to test-each-commit 56aad83307e ci: set a ctest timeout of 1200 (20 minutes) 1b707146717 Merge bitcoin-core/gui#840: qt6: Handle different signatures of `QANEF::nativeEventFilter` 720ce880a35 init: Improve comment describing chainstate load retry behaviour baea842ff18 init: Remove unneeded argument for mempool_opts checks ec58dfe8f74 Merge bitcoin/bitcoin#31010: cmake: Avoid hardcoding Qt's major version in Find module / variable names 5fe6878b5f7 Merge bitcoin-core/gui#836: Fix display issues for IPv6 proxy setup in Options Dialog (UI only, no functionality impact) f50557f5d36 test: Fix copy-paste in db_tests ostream operator 5ea335a97f8 Merge bitcoin/bitcoin#30793: rpc: add getorphantxs 76e2e8aabd8 Merge bitcoin/bitcoin#31035: doc: Archive 28.0 release notes f019fcec412 doc: Archive 28.0 release notes 80761afced1 qt6: Handle different signatures of `QANEF::nativeEventFilter` 51c698161b5 Merge bitcoin-core/gui#837: qt6: Fix linking when configured with `-DENABLE_WALLET=OFF` 4be785b3e33 Merge bitcoin-core/gui#839: qt6, test: Handle deprecated code f117f3f7473 Merge bitcoin-core/gui#838: qt6: Handle deprecated `QLocale::nativeCountryName` 5625840c11d qt6, test: Handle deprecated `QVERIFY_EXCEPTION_THROWN` 772928a13c2 Merge bitcoin/bitcoin#30982: docs: Add instructions on how to self-sign bitcoin-core binaries for macOS 27709f51ee0 docs: Add instructions on how to self-sign bitcoin-core binaries for macOS cfb59da4b3b Merge bitcoin/bitcoin#30980: fuzz: fix bug in p2p_headers_presync harness dda2613239b Merge bitcoin/bitcoin#30929: log: Enforce trailing newline e0ae9c14c4e Merge bitcoin/bitcoin#31011: refactor: move util/pcp and util/netif to common/ 98c1536852d test: add getorphantxs tests 93f48fceb7d test: add tx_in_orphanage() 34a9c10e8cd rpc: add getorphantxs f511ff3654d refactor: move verbosity parsing to rpc/util 36a6d4b0078 doc: update IBD requirements in doc/README.md fa6d14eacb2 test: Treat exclude list warning as failure in CI 6a370435526 Merge bitcoin/bitcoin#31007: doc: add testnet4 section header for config file 70910eb2ecb Merge bitcoin/bitcoin#31016: test: add missing sync to feature_fee_estimation.py 532491faf1a net: add GetOrphanTransactions() to PeerManager 91b65adff2a refactor: add OrphanTxBase for external use a1576edab35 test: add missing sync to feature_fee_estimation.py ae56b3230b2 depends: For mingw cross compile use -gcc-posix to prevent library conflict fd38711217c ci: make CI job fail when check-deps.sh script fails d51edecddcb common: move pcp.cpp and netif.cpp files from util to common library since they depend on netaddress.cpp 61cdb1c9d83 doc: add testnet4 section header for config file deacf3c7cd6 cmake: Avoid hardcoding Qt's major version in Find module 605926da0ab depends: Print ready-to-use `--toolchain` option for CMake invocation fa2b7d8d6b3 Remove redundant unterminated-logprintf tidy check bbbb2e43ee9 log: Enforce trailing newline, Remove redundant m_started_new_line fa22e5c430a refactor: Remove dead code that assumed tip == nullptr fa2e4439652 refactor: Replace g_genesis_wait_cv with m_tip_block_cv fa7f52af1a4 refactor: Use wait_for predicate to check for interrupt 5ca28ef28bc refactor: Split up NodeContext shutdown_signal and shutdown_request fad8e7fba7b bugfix: Mark m_tip_block_cv as guarded by m_tip_block_mutex fa18586c29d refactor: Add missing GUARDED_BY(m_tip_block_mutex) fa4c0750331 doc: Clarify waitTipChanged docs fc642c33ef2 Merge bitcoin/bitcoin#30718: test: switch MiniWallet padding unit from weight to vsize d7f956a309e Merge bitcoin/bitcoin#30968: init: Remove retry for loop c33eb2360e2 Merge bitcoin/bitcoin#30043: net: Replace libnatpmp with built-in PCP+NATPMP implementation f3c74c4a7e1 Merge bitcoin/bitcoin#30989: guix: Drop no longer needed `PATH` modification 5c7cacf649a ci: Remove natpmp build option and libnatpmp dependency 7e7ec984da5 doc: Remove mention of natpmp build options 061c3e32a26 depends: Drop natpmp and associated option from depends 20a18bf6aa3 build: Drop libnatpmp from build system 7b04709862f qt: Changes for built-in PCP+NAT-PMP 52f8ef66c61 net: Replace libnatpmp with built-in NATPMP+PCP implementation in mapport 97c97177cdb net: Add PCP and NATPMP implementation cb750b4b405 qt6, test: Use `qWarning()` instead of `QWARN()` macro 9123a286e97 qt6: Handle deprecated `QLocale::nativeCountryName` 940edd6ac24 test: refactor: introduce and use `TRUC_CHILD_MAX_VSIZE` constant c16ae717689 test: switch MiniWallet padding unit from weight to vsize a647d4400d5 doc: update signet documentation related to build directories f1daa80521e guix: Drop no longer needed `PATH` modification d812cf11896 Merge bitcoin/bitcoin#30879: test: re-bucket long-running tests 18d4c43cab4 Merge bitcoin/bitcoin#30921: test: generalize HasReason and use it in FailFmtWithError d7fcc91416a Merge bitcoin/bitcoin#30974: ci: Inline PACKAGE_MANAGER_INSTALL 29d00a1cee1 Merge bitcoin/bitcoin#30940: depends: Fix build with `MULTIPROCESS=1` in Guix environment 89a8e9b732f Merge bitcoin/bitcoin#30979: contrib: Update asmap link in seeds readme fafd1a0f648 ci: Inline PACKAGE_MANAGER_INSTALL 36ad9516dbd Merge bitcoin/bitcoin#30981: ci: add timestamps to cirrus jobs fa7c2838a5f Merge bitcoin/bitcoin#30948: test: Add missing sync_mempools() to fill_mempool() f951f1fab25 ci: add timestamps to cirrus jobs f158993fd55 contrib: Update asmap link in seeds readme d5af7d28f47 Merge bitcoin/bitcoin#30976: depends, doc: Drop package-specific note about CMake a7498cc7e26 Fix bug in p2p_headers_presync harness 4cf84b344de depends, doc: No need to specify general requirement e13da501db9 Merge bitcoin/bitcoin#30973: doc: fix `loadtxoutset` example 513b7136c79 Merge bitcoin/bitcoin#30961: ci: add `LLVM_SYMBOLIZER_PATH` to Valgrind fuzz job 286725168ae doc: fix loadtxoutset example 525e9dcba0b Add submitSolution to BlockTemplate interface 47b4875ef05 Add getCoinbaseMerklePath() to Mining interface 63d6ad7c89c Move BlockMerkleBranch back to merkle.{h,cpp} 65f6e7078b1 Merge bitcoin/bitcoin#30510: multiprocess: Add IPC wrapper for Mining interface da612cea032 Merge bitcoin/bitcoin#30962: validation: Disable CheckForkWarningConditions for background chainstate e9d60af9889 refactor: Replace init retry for loop with if statement c1d8870ea41 refactor: Move most of init retry for loop to a function ccd10fdb97f build: Add missing USDT header dependency to kernel 781c01f5806 init: Check mempool arguments in AppInitParameterInteractions 39219fe145e Merge bitcoin/bitcoin#30946: doc: correct the zmq automatic build info 06e7e836329 doc: correct the zmq automatic build info a9773b6215e Merge bitcoin/bitcoin#30963: doc: Adjust links in OSS-Fuzz section fa6c1946d23 doc: Adjust links in OSS-Fuzz section c0a0c72b4d6 validation: Disable CheckForkWarningConditions for background chainstate c1832584bfd ci: add LLVM_SYMBOLIZER_PATH to Valgrind fuzz job 393f323bd60 Merge bitcoin/bitcoin#30952: test: Use shell builtins in run_command test case faf801515f8 test: Add missing sync_mempools() to fill_mempool() fa48be6f023 test: Refactor fill_mempool to extract send_batch helper 1a332817665 doc: multiprocess documentation improvements 90a5786bba4 Merge bitcoin/bitcoin#30678: wallet: Write best block to disk before backup d043950ba24 multiprocess: Add serialization code for BlockValidationState 33c2eee285e multiprocess: Add IPC wrapper for Mining interface 06882f84017 multiprocess: Add serialization code for vector<char> 095286f790a multiprocess: Add serialization code for CTransaction 69dfeb18761 multiprocess: update common-types.h to use C++20 concepts 206c6e78eec build: Make bitcoin_ipc_test depend on bitcoin_ipc 070e6a32d5f depends: Update libmultiprocess library for cmake headers target dabc74e86c3 Merge bitcoin/bitcoin#30409: Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block 7bd3ee62f6d test: Use shell builtins in run_command test case f5a2000579b test: re-bucket long-running tests 06b4c339e89 depends: Fix reproducibility when building with `MULTIPROCESS=1` 04e4d52420a test: add test for specifying custom pidfile via `-pid` b832ffe0446 refactor: introduce default pid file name constant in tests d8e3afc3352 depends: Fix build with `MULTIPROCESS=1` in Guix environment f20fe33e94c test: Add basic balance coverage to wallet_assumeutxo.py d72df63d169 net: Use GetLocalAddresses in Discover e02030432b7 net: Add netif utility 754e4254388 crypto: Add missing WriteBE16 function 33adc7521cc Merge bitcoin/bitcoin#30765: refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors 0894748316c Merge bitcoin/bitcoin#30918: fuzz: Add check in `p2p_headers_presync` that chain work never exceeds minimum work f57a6754ed6 Merge bitcoin/bitcoin#30826: fuzz: reduce number of iterations in `crypto_aeadchacha20poly1305` target 48c20dbd86c Merge bitcoin/bitcoin#30794: interpreter: use int32_t instead of int type for risczero compile 4148e60909e Merge bitcoin/bitcoin#30679: fix: handle invalid `-rpcbind` port earlier a8a2628b7a9 Merge bitcoin/bitcoin#30828: interfaces: #30697 follow ups 0d81b3ddedc Merge bitcoin/bitcoin#30568: addrman: change internal id counting to int64_t c985a34b9c3 Merge bitcoin/bitcoin#26990: cli: Improve error message on multiwallet cli-side commands 037b101e808 test: Add coverage for best block locator write in wallet_backup 31c0df03890 wallet: migration, write best locator before unloading wallet 7e3dbe4180c wallet: Write best block to disk before backup 79f20fa1b1e Merge bitcoin/bitcoin#30561: refactor: move `SignSignature` helpers to test utils 284bd17309a add check that chainwork doesn't exceed minimum work 9aa5d1c3fcd add clarification in comment 197aa249551 Merge bitcoin/bitcoin#30856: build: drop obj/ subdirectory for generated build.h 7025942687f build: drop superfluous `HAVE_BUILD_INFO` define 0dd662510c5 build: drop obj/ subdir for generated build.h, rename to bitcoin-build-info.h 84cd6478c42 Merge bitcoin/bitcoin#30927: Follow-up after AutoFile position caching: remove unused code caac06f784c streams: reorder/document functions 67a3d590768 streams: remove unused code 2db926f49c8 Merge bitcoin/bitcoin#30889: log: Use ConstevalFormatString fee4cba4847 gui: Fix proxy details display in Options Dialog 9ba56884f62 Merge bitcoin/bitcoin#30869: ci: Print inner env, Make ccache config more flexible 6c3c619b35c test: generalize HasReason and use it in FailFmtWithError ab0b5706b25 Merge bitcoin/bitcoin#30875: doc: fixed inconsistencies in documentation between autotools to cmake change fd08fded63a Merge bitcoin/bitcoin#30639: ci: Use clang-19 in msan tasks 5be34bacf6d qt: Fix linking when configured with `-DENABLE_WALLET=OFF` a9964c04447 doc: Updating docs from autotools to cmake fae44c83da9 test: Remove 0.16.3 test from wallet_backwards_compatibility.py 69409bc6e55 Merge bitcoin/bitcoin#30908: doc: remove Eclipser fuzzing documentation 6b97882ab53 Merge bitcoin/bitcoin#30915: ci: Use `ninja` to build in macOS native CI job e6994efe08b fix: increase rpcbind check robustness d38e3aed89e fix: handle invalid rpcbind port earlier 83b67f2e6d5 refactor: move host/port checking 73c243965ab test: add tests for invalid rpcbind ports 54227e681a4 rpc, cli: improve error message on multiwallet mode 735436df8ce Remove outdated Eclipser fuzzing documentation ccccb67851b ci: Use clang-19 in msan tasks facbcd4cef8 log: Use ConstevalFormatString d01b85bfecb ci: Use `ninja` to build in macOS native CI job 6fc46927971 Merge bitcoin/bitcoin#29624: doc: update NeedsRedownload() and nStatus comment 2a0949f0977 Merge bitcoin/bitcoin#30888: build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake bdbc90f29ac Merge bitcoin/bitcoin#30902: Remove Autotools packages from CI (and depends doc) a95e742b692 Merge bitcoin/bitcoin#30913: ci: Use macos-14 GHA image (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0) 225718eda89 Merge bitcoin/bitcoin#30438: guix: (explicitly) build Linux GCC with `--enable-cet` fab932b4211 ci: Remove incorrectly hardcoded HOST in mac_native task af9f9878934 doc: update NeedsRedownload() comment fa8f35d7865 ci: Use macos-14 GHA image 7942951e3fc Remove unused g_best_block e3a560ca68d rpc: use waitTipChanged for longpoll 460687a09c2 Remove unused CRPCSignals dca923150e3 Replace RPCNotifyBlockChange with waitTipChanged() 2a40ee11219 rpc: check for negative timeout arg in waitfor* de7c855b3af rpc: recommend -rpcclienttimeout=0 for waitfor* 77ec072925a rpc: fix waitfornewblock description 285fe9fb51c rpc: add test for waitforblock and waitfornewblock b94b27cf05c Add waitTipChanged to Mining interface 7eccdaf1608 node: Track last block that received a blockTip notification ebb8215f236 Rename getTipHash() to getTip() and return BlockRef 89a8f74bbbb refactor: rename BlockKey to BlockRef 9f1aa88d4d9 Merge bitcoin/bitcoin#30884: streams: cache file position within AutoFile 06329eb1348 Merge bitcoin/bitcoin#29436: net: call `Select` with reachable networks in `ThreadOpenConnections` e983ed41d9f Merge bitcoin/bitcoin#30410: rpc, rest: Improve block rpc error handling, check header before attempting to read block data. fce9e065c16 Merge bitcoin/bitcoin#28358: Drop -dbcache limit 8d000b85dd4 Merge bitcoin/bitcoin#30868: refactor: add clang-tidy `modernize-use-starts-ends-with` check 3f66642820b Merge bitcoin/bitcoin#30440: Have createNewBlock() return a BlockTemplate interface 2bf721e76a5 Merge bitcoin/bitcoin#30661: fuzz: Test headers pre-sync through p2p c38e9993de7 Merge bitcoin/bitcoin#30286: cluster mempool: optimized candidate search fa99e4521b6 ci: Allow CCACHE_DIR bind mount 37679b856ce Merge bitcoin/bitcoin#30899: qt: Translations update fc7b507e9a5 tidy: add clang-tidy `modernize-use-starts-ends-with` check 7a8a6a06676 doc: Fix comment in `contrib/devtools/check-deps.sh` script 712d105e093 depends, doc: Do not install Autotools packages b786449e663 ci: Do not install Autotools packages a240e150e83 streams: remove AutoFile::Get() entirely ae052957614 qt: Translations update 6a1aa510e31 rpc: check block index before reading block / undo data 6cbf2e5f819 rpc: Improve gettxoutproof error when only header is available. 69fc867ea19 test: add coverage to getblock and getblockstats 5290cbd5850 rpc: Improve getblock / getblockstats error when only header is available. e5b537bbdfb rest: improve error when only header of a block is available. e624a9bef16 streams: cache file position within AutoFile 89bf11b8072 guix: build Linux GCC with --enable-cet a93c171faa7 Drop unneeded nullptr check from CreateNewBlock() dd87b6dff35 Have createNewBlock return BlockTemplate interface fae9b60c4ff test: Use LogPrintStr to test m_log_sourcelocations 33381ea530a scripted-diff: Modernize nLocalServices to m_local_services 2a581144f28 build: Minimize I/O operations in GenerateHeaderFromJson.cmake aa003d1568b build: Minimize I/O operations in GenerateHeaderFromRaw.cmake 9ad2fe7e69e clusterlin: only start/use search when enough iterations left bd044356edb clusterlin: improve heuristic to decide split transaction (optimization) 71f26293988 clusterlin: include topological pot subsets automatically (optimization) e20fda77a2d clusterlin: reduce computation of unnecessary pot sets (optimization) 6060a948caf clusterlin bench: add example hard cluster benchmarks 2965fbf203f clusterlin: track upper bound potential set for work items (optimization) 9e43e4ce109 clusterlin: use feerate-sorted depgraph in SearchCandidateFinder b80e6dfe780 clusterlin: add reordering support for DepGraph 85a285a3061 clusterlin: separate initial search entries per component (optimization) e4faea9ca79 clusterlin bench: have low/high iter benchmarks instead of per-iter fa39b1ca638 doc: move-only logging warning fa252da0b9c ci: Remove hardcoded CCACHE_DIR in cirrus fa146904e19 ci: Bump default CCACHE_MAXSIZE to 500M aaaa7cf8bad cirrus: Drop CCACHE_NOHASHDIR fa7ca182a9b ci: Print inner env 84663291275 chain: simplify `deleteRwSettings` code and improve it's doc f8d91f49c70 chain: dont check for null settings value in `overwriteRwSetting` df601993f2d chain: ensure `updateRwSetting` doesn't update to a null settings 5e190cd11f6 Replace CScript _hex_v_u8 appends with _hex cac846c2fbf Allow CScript's operator<< to accept spans, not just vectors c78d8ff4cb8 prevector: avoid GCC bogus warnings in insert method e4e3b44e9cc net: call `Select` with reachable networks in `ThreadOpenConnections` 829becd990b addrman: change `Select` to support multiple networks f698636ec86 net: add `All()` in `ReachableNets` a97f43d63a6 fuzz: Add harness for p2p headers sync c8e2eeeffb4 chain: uniformly use `SettingsAction` enum in settings methods f482d0e366a fuzz: reduce number of iterations in `crypto_aeadchacha20poly1305` target 1e9e735670f chain: move new settings safely in `overwriteRwSetting` 1c409004c80 test: remove wallet context from `write_wallet_settings_concurrently` 58499b00d0a refactor: move `SignSignature` helpers to test utils cd0edf26c07 tracing: cast block_connected duration to nanoseconds cfd03de965a Add Testnet4 launch shortcut for Windows 77b2923f871 Add Signet launch shortcut for Windows bc52cda1f3c fix use int32_t instead of int type for risczero compile with (-march=rv32i, -mabi=ilp32) a0eaa4749fe Add FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION in PoW check a3f6f5acd89 build: Automatically define FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION for fuzz builds 0c02d4b2bdb net_processing: Make MAX_HEADERS_RESULTS a PeerManager option 51f7668d31e addrman: change nid_type from int to int64_t 051ba3290e3 addrman, refactor: introduce user-defined type for internal nId bdad0243be8 rpc, net: getrawaddrman "mapped_as" follow-ups fa1b139d17d Bump python minimum supported version to 3.10 ec585f11c38 Reserve space for transaction inputs in CreateTransactionInternal c76aaaf9003 Reserve space for transaction outputs in CreateTransactionInternal bb3b980dfd9 validation: drop maximum -dbcache REVERT: e70e527ef21 kernel: Add pure kernel bitcoin-chainstate REVERT: ed9a8a54d3c kernel: Add block index utility functions to C header REVERT: 6338dd45b55 kernel: Add function to read block undo data from disk to C header REVERT: a2ac0c1e7c9 kernel: Add functions to read block from disk to C header REVERT: 170060c3372 kernel: Add function for copying block data to C header REVERT: 0f8c00bba07 kernel: Add functions for the block validation state to C header REVERT: 41dba7d2603 kernel: Add validation interface to C header REVERT: 877cf01f22c kernel: Add interrupt function to C header REVERT: f77c2b90422 kernel: Add import blocks function to C header REVERT: 254e17dbeab kernel: Add chainstate load options for in-memory dbs in C header REVERT: 8baa06d318f kernel: Add options for reindexing in C header REVERT: 0243ed8a200 kernel: Add block validation to C header REVERT: 36fbab87e9e Kernel: Add chainstate loading to kernel C header REVERT: b249e93f295 kernel: Add chainstate manager object to C header REVERT: 2546745a393 kernel: Add notifications context option to C header REVERT: 2578746e87f kerenl: Add chain params context option to C header REVERT: 21107de0ca7 kernel: Add kernel library context object REVERT: 83cc65c4911 kernel: Add logging to kernel library C header REVERT: 2f47169f91e kernel: Introduce initial kernel C header API git-subtree-dir: libbitcoinkernel-sys/bitcoin git-subtree-split: 1047757ea3b4b78b51d7338ea44e2123851143fe

…ray-bounds` and `-Wdangling-reference`, re-enable `-Werror` for GCC 40caa7d revert: don't build using `-Werror` when using GCC (Kittywhiskers Van Gogh) 96e494c build: constrain `-Wstringop-over{read,flow}` suppression to GCC only (Kittywhiskers Van Gogh) 6bff03b build: don't error on `-Wattributes` with GCC <14 (Kittywhiskers Van Gogh) 9aab60b build: don't error on `-Warray-bounds` and `-Wdangling-reference` with GCC (Kittywhiskers Van Gogh) 18fcb26 fix: resolve `-Wunused-function` warning (Kittywhiskers Van Gogh) 7a65315 fix: resolve `-Wmaybe-uninitialized` warnings (Kittywhiskers Van Gogh) b208018 fix: resolve `-Wignored-qualifiers` warning (Kittywhiskers Van Gogh) 7dd0daa fix: resolve `-Woverloaded-virtual` warning (Kittywhiskers Van Gogh) c587838 refactor: use `GetTransactionBlock()` as narrow `GetTransaction()` proxy (Kittywhiskers Van Gogh) d9828a9 refactor: move `ToJson` definitions relied by `TxToUniv` to source file (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Depends on #6637 * Depends on #6638 * `TxToUniv()` is a function defined in `core_write.cpp`, which is a part of `libbitcoin_common` that relies on `ToJson()` definitions in various classes (`CCbTx`, `CProRegTx` and others) to print out of contents of transaction payload. The problem is, those various classes have their source files a part of `libbitcoin_server`, so when binaries like `dash-tx` (which do not use `libbitcoin_server`) need to be linked, the necessary `ToJson()` implementations, were they defined in source files, wouldn't be found and this would result in a linking error. This is worked around by defining them in the header instead but this creates a new problem where these headers are included by `core_write` but constructing the UniValue objects returned by `ToJson()` needs something from `core_write` (i.e. there is a circular dependency). This was worked around in [dash#6229](#6229) ([commit](b330318)) with redefinition, which was fine but is now verboten with `-Wredundant-decls` enforcement. * To work around this, all `ToJson()` definitions from those headers relied on by `TxToUniv()` have been moved to their own source file (`evo/core_write.cpp`) and included in `libbitcoin_common`. This takes care of the linking issue and avoids circular dependencies. * `GetTransaction()` is extensively used in Dash-specific code and a redefinition in `validation.cpp` is used to reduce the amount of reported circular dependencies, removing that circular dependency in order to satisfy `-Wredundant-decls` results in the following (see below). <details> <summary>Circular dependencies:</summary> Removed: ``` "evo/deterministicmns -> validation -> txmempool -> evo/deterministicmns" ``` Added: ``` "node/transaction -> validation -> node/transaction" "evo/deterministicmns -> node/transaction -> net_processing -> evo/deterministicmns" "evo/deterministicmns -> node/transaction -> node/blockstorage -> evo/deterministicmns" "evo/deterministicmns -> node/transaction -> node/context -> evo/deterministicmns" "evo/deterministicmns -> node/transaction -> txmempool -> evo/deterministicmns" "evo/chainhelper -> llmq/chainlocks -> node/transaction -> node/context -> evo/chainhelper" "evo/deterministicmns -> node/transaction -> net_processing -> evo/mnauth -> evo/deterministicmns" "evo/deterministicmns -> node/transaction -> node/context -> governance/governance -> evo/deterministicmns" "evo/deterministicmns -> node/transaction -> net_processing -> llmq/dkgsession -> evo/deterministicmns" "evo/deterministicmns -> node/transaction -> net_processing -> llmq/dkgsessionmgr -> evo/deterministicmns" "evo/deterministicmns -> node/transaction -> net_processing -> llmq/quorums -> evo/deterministicmns" "evo/deterministicmns -> node/transaction -> node/blockstorage -> masternode/node -> evo/deterministicmns" "governance/governance -> governance/object -> node/transaction -> node/context -> governance/governance" "llmq/chainlocks -> node/transaction -> node/context -> llmq/context -> llmq/chainlocks" "llmq/context -> llmq/instantsend -> node/transaction -> node/context -> llmq/context" "coinjoin/context -> coinjoin/server -> evo/deterministicmns -> node/transaction -> node/context -> coinjoin/context" "evo/cbtx -> evo/deterministicmns -> node/transaction -> node/context -> evo/creditpool -> evo/cbtx" "evo/deterministicmns -> node/transaction -> net_processing -> llmq/dkgsession -> llmq/debug -> evo/deterministicmns" "evo/deterministicmns -> node/transaction -> node/context -> evo/mnhftx -> llmq/signing -> llmq/signing_shares -> evo/deterministicmns" ``` Diff: **+18** </details> To avoid this, alongside some header adjustments, a proxy to `GetTransaction()` has been introduced in `evo/chainhelper.cpp`, `GetTransactionBlock()`. This _reduces_ the number of circular dependencies (see below). <details> <summary>Circular dependencies:</summary> Removed: ``` "consensus/tx_verify -> evo/assetlocktx -> validation -> consensus/tx_verify" "consensus/tx_verify -> evo/assetlocktx -> validation -> txmempool -> consensus/tx_verify" "evo/assetlocktx -> validation -> txmempool -> evo/assetlocktx" "evo/deterministicmns -> validation -> evo/deterministicmns" "evo/deterministicmns -> validation -> txmempool -> evo/deterministicmns" ``` Added: ``` "evo/chainhelper -> llmq/chainlocks -> evo/chainhelper" "evo/chainhelper -> llmq/instantsend -> evo/chainhelper" "evo/chainhelper -> evo/specialtxman -> evo/deterministicmns -> evo/chainhelper" "evo/chainhelper -> node/transaction -> node/context -> evo/chainhelper" "evo/deterministicmns -> llmq/commitment -> validation -> evo/deterministicmns" "consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> consensus/tx_verify" "evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> evo/assetlocktx" "evo/chainhelper -> masternode/payments -> governance/classes -> governance/object -> evo/chainhelper" "evo/deterministicmns -> llmq/commitment -> validation -> txmempool -> evo/deterministicmns" "consensus/tx_verify -> evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> consensus/tx_verify" ``` Diff: **+5** </details> To differentiate it from `GetTransaction()`, since all Dash-specific invocations either rely on the transaction index or the mempool, we can safely trim down the number of arguments and add a fast-fail. * Even with `-Werror` enabled, we have to downgrade `-Warray-bounds` to warnings (i.e. use `-Wno-error=array-bounds`) as there are two sources of the warning, `immer` (see [arximboldi/immer#223](arximboldi/immer#223)) and serialization code (fixed with [bitcoin#30765](bitcoin#30765)). As due to the former, we need to suppress it anyways, the latter has not been backported in the interest of brevity. * Similar to the above, we have to downgrade `-Wdangling-reference` as it is caused by UniValue-adjacent code that is fixed by [bitcoin#27605](bitcoin#27605) but would require an out-of-order backport that pulls in multiple dependent PRs in order to be complete (or alternatively, backport it as-is and then make future backporting annoying). The simpler solution to downgrade the warning was opted for instead. * We need to annotate `CHDPubKey::nVersion` with `[[maybe_unused]]` to avoid upsetting Clang due to `-Wunused-private-field`. GCC 11 doesn't emit a warning for that variable and therefore, finds the annotation unnecessary, triggering an error due to `-Wattributes` ([build](https://gitlab.com/dashpay/dash/-/jobs/9777475240#L1581)). Since both Clang 18 and GCC 14 agree with the `-Wunused-private-field` assessment, we are better off downgrading `-Wattributes` to a warning for GCC 11. ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: knst: ACK 40caa7d UdjinM6: utACK 40caa7d Tree-SHA512: 4fb383f4e2142db24ffedffd023cc075804c07e499abfcfc5708047d64fd981599cdb7479d92f501bedede6d94b76cc16f77f1372dd5bce7e1906ddb374679c3

DrahtBot added the Refactoring label Aug 30, 2024

l0rinc force-pushed the l0rinc/CScript-vector-and-array branch from 176d42b to 873fe22 Compare August 31, 2024 14:20

l0rinc marked this pull request as ready for review August 31, 2024 14:21

maflcko reviewed Sep 2, 2024

View reviewed changes

src/script/script.h Outdated Show resolved Hide resolved

l0rinc marked this pull request as draft September 3, 2024 09:57

l0rinc force-pushed the l0rinc/CScript-vector-and-array branch from 873fe22 to ab875e1 Compare September 6, 2024 10:58

l0rinc changed the title ~~refactor: Extend CScript with operator<< for std::array~~ refactor: Generalize CScript's std::vector push to std::span to accept std::array, too Sep 6, 2024

l0rinc changed the title ~~refactor: Generalize CScript's std::vector push to std::span to accept std::array, too~~ refactor: Generalize CScript's std::vector push to accept std::array, too Sep 6, 2024

l0rinc force-pushed the l0rinc/CScript-vector-and-array branch from ab875e1 to 14080bb Compare September 6, 2024 15:30

DrahtBot added the CI failed label Sep 6, 2024

maflcko reviewed Sep 6, 2024

View reviewed changes

src/script/script.h Outdated Show resolved Hide resolved

l0rinc force-pushed the l0rinc/CScript-vector-and-array branch 5 times, most recently from 18870ac to eb82bb8 Compare September 8, 2024 09:32

l0rinc changed the title ~~refactor: Generalize CScript's std::vector push to accept std::array, too~~ refactor: Generalize CScript's std::vector push to accept std::array Sep 8, 2024

l0rinc force-pushed the l0rinc/CScript-vector-and-array branch 2 times, most recently from c21b0f7 to ec05c0a Compare September 9, 2024 09:37

l0rinc marked this pull request as ready for review September 9, 2024 09:40

l0rinc force-pushed the l0rinc/CScript-vector-and-array branch 2 times, most recently from d0c739a to e6c02b9 Compare September 9, 2024 09:50

DrahtBot removed the CI failed label Sep 9, 2024

ryanofsky reviewed Sep 9, 2024

View reviewed changes

DrahtBot added the CI failed label Sep 10, 2024

l0rinc force-pushed the l0rinc/CScript-vector-and-array branch from 1802ed2 to 48b67d8 Compare September 10, 2024 18:12

ryanofsky approved these changes Sep 10, 2024

View reviewed changes

DrahtBot requested a review from hodlinator September 10, 2024 20:15

l0rinc force-pushed the l0rinc/CScript-vector-and-array branch from 48b67d8 to e6a5ab7 Compare September 10, 2024 22:20

hodlinator approved these changes Sep 11, 2024

View reviewed changes

DrahtBot requested a review from ryanofsky September 11, 2024 15:01

l0rinc marked this pull request as ready for review September 11, 2024 15:05

maflcko reviewed Sep 11, 2024

View reviewed changes

src/script/script.h Outdated Show resolved Hide resolved

l0rinc and others added 3 commits September 11, 2024 17:41

Replace CScript _hex_v_u8 appends with _hex

5e190cd

This will skip vector conversion before serializing to the prevector in CScript.

l0rinc force-pushed the l0rinc/CScript-vector-and-array branch from e6a5ab7 to 5e190cd Compare September 11, 2024 15:41

ryanofsky approved these changes Sep 11, 2024

View reviewed changes

DrahtBot requested a review from hodlinator September 11, 2024 16:20

maflcko reviewed Sep 11, 2024

View reviewed changes

hodlinator approved these changes Sep 11, 2024

View reviewed changes

DrahtBot removed the CI failed label Sep 13, 2024

achow101 merged commit 33adc75 into bitcoin:master Sep 20, 2024
16 checks passed

l0rinc deleted the l0rinc/CScript-vector-and-array branch September 20, 2024 19:18

davidgumberg mentioned this pull request Oct 10, 2024

Don't zero-after-free DataStream: Faster IBD on some configurations #30987

Closed

kwvg mentioned this pull request Apr 21, 2025

fix: resolve compiler warnings, suppress errors on -Warray-bounds and -Wdangling-reference, re-enable -Werror for GCC dashpay/dash#6639

Merged

5 tasks

hodlinator mentioned this pull request Aug 8, 2025

test,refactor: extract script template helpers & widen sigop count coverage #32729

Open

		// Passing dst variable with explicit type instead of temporary expression
		// calms down GCC under some circumstances.

		AppendDataSize(b.size());
		AppendData({reinterpret_cast<const value_type*>(b.data()), b.size()});

refactor: Allow CScript's operator<< to accept spans, not just vectors #30765

refactor: Allow CScript's operator<< to accept spans, not just vectors #30765

Uh oh!

Conversation

l0rinc commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

Uh oh!

DrahtBot commented Sep 6, 2024

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

l0rinc Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

l0rinc Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Sep 10, 2024

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

l0rinc commented Sep 10, 2024

Uh oh!

hodlinator left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hodlinator left a comment

Choose a reason for hiding this comment

Uh oh!

achow101 commented Sep 20, 2024

Uh oh!

l0rinc commented Sep 20, 2024

Uh oh!

achow101 commented Sep 20, 2024

Uh oh!

Uh oh!

Uh oh!

refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors #30765

refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors #30765

l0rinc commented Aug 30, 2024 •

edited

Loading

DrahtBot commented Aug 30, 2024 •

edited

Loading

l0rinc Sep 10, 2024 •

edited

Loading

l0rinc Sep 10, 2024 •

edited

Loading