faca9a3d5a test: Avoid intermittent issues due to async events in validationinterface_tests (MarcoFalke)
Pull request description:
Currently the tests have many issues:
* They setup the genesis block, even though it is not needed
* They queue an async `UpdatedBlockTip` even, which causes intermittent issues: https://github.com/bitcoin/bitcoin/issues/28146#issuecomment-1650064645
Fix all issues by trimming down the setup to just `ChainTestingSetup`.
ACKs for top commit:
Crypt-iQ:
tACK faca9a3d5a
Tree-SHA512: 4449040330f89bbaf5ce5b2052417c160b451c373987fdf1069596c07834ed81f0aea1506d53c7d2cd21062b27332d30679285dae194b272fd0cb9ce5ded32cf
d0c6cc4abe suppressions: note that 'type:ClassName::MethodName' should be used (fanquake)
Pull request description:
Now that the symbolizer is back in play, suppressions can once-again be targeted to functions, rather than file-wide.
ACKs for top commit:
MarcoFalke:
lgtm ACK d0c6cc4abe
hebasto:
ACK d0c6cc4abe
Tree-SHA512: fb65398eae18a6ebc5f8414275c568cf2664ab5357c2b3160f3bf285b67bc3af788225c5dba3c824c0e098627789450bec775375f52529d71c6ef700a9632d65
53c990ad34 test: fix `feature_addrman.py` on big-endian systems (Sebastian Falbesoner)
Pull request description:
The test `feature_addrman.py` currently serializes the addrdb without specifying endianness for `int`s, so the machine's native byte order is used (see https://docs.python.org/3/library/struct.html#byte-order-size-and-alignment) and the generated `peers.dat` would be invalid on big-endian systems (our internal (de)serializers always use little-endian, see `ser_{read,write}data32`). Fix this by explicitly specifying little-endian serialization via the `<` character in `struct.pack(...)`.
This is not detected by CI as we unfortunately don't run functional tests on big-endian systems there (I think we definitely should!).
ACKs for top commit:
MarcoFalke:
lgtm ACK 53c990ad34🔚
Tree-SHA512: 513af6f1f785a713e7a8ef3a57fcd3fe2520a7d537f63a9c8e1f4bdea4c2f605fd4c35001623d6b13458883dbc256f24943684ab8f224055c22bf8d8eeee5fe2
07c59eda00 Don't derive secure_allocator from std::allocator (Casey Carter)
Pull request description:
Giving the C++ Standard Committee control of the public interface of your type means they will break it. C++23 adds a new `allocate_at_least` member to `std::allocator`. Very bad things happen when, say, `std::vector` uses `allocate_at_least` from `secure_allocator`'s base to allocate memory which it then tries to free with `secure_allocator::deallocate`.
(Discovered by microsoft/STL#3712, which will be reverted by microsoft/STL#3819 before it ships.)
ACKs for top commit:
jonatack:
re-ACK 07c59eda00 no change since my previous ACK apart from squashing the commits
achow101:
ACK 07c59eda00
john-moffett:
ACK 07c59eda00 Reviewed and tested. Performance appears unaffected in my environment.
Tree-SHA512: 23606c40414d325f5605a9244d4dd50907fdf5f2fbf70f336accb3a2cb98baa8acd2972f46eab1b7fdec1d28a843a96b06083cd2d09791cda7c90ee218e5bbd5
6960c81cbf kernel: Remove Univalue from kernel library (TheCharlatan)
10eb3a9faa kernel: Split ParseSighashString (TheCharlatan)
Pull request description:
Besides the build system changes, this is a mostly move-only change for moving the few UniValue-related functions out of kernel files.
UniValue is not required by any of the kernel components and a JSON library should not need to be part of a consensus library.
ACKs for top commit:
achow101:
ACK 6960c81cbf
theuni:
Re-ACK 6960c81cbf
stickies-v:
re-ACK 6960c81cbf
Tree-SHA512: d92e4cb4e12134c94b517751bd746d39f9b8da528ec3a1c94aaedcce93274a3bae9277832e8a7c0243c13df0397ca70ae7bbb24ede200018c569f8d81103c1da
Only the combined addr:port of source and destination
must be unique. If the destination is different, the same addr:port
for the source may be used by the OS.
* The node was only used to migrate the legacy txindex. But now that it
is known to be working and that 22.x is EOL, it can be dropped.
* Also, fix a typo to properly check the txindex of node [1], not [2].
Nobody is pushing direct to guix.sigs, nor should they, as that
bypasses CI.
Use a newer example for the testing issue.
Don't duplicate the bitcoincore.org doc instructions.
faa8c1be26 fuzz: Re-enable symbolize=1 in ASAN_OPTIONS (MarcoFalke)
Pull request description:
Looks like this fixed itself somehow and is no longer reproducible?
ACKs for top commit:
fanquake:
ACK faa8c1be26
Tree-SHA512: 67d2d6349cc7485f32bebabc18869ab101ae66a778a40ff9ddb037980997e600d7c6d1e0a17a011fa2a4ba07c73594b087dd781248cb8351f2688bc4cf6e587d
Affects both secure_allocator and zero_after_free_allocator.
Giving the C++ Standard Committee control of the public interface of your type means they will break it. C++23 adds a new `allocate_at_least` member to `std::allocator`. Very bad things happen when, say, `std::vector` uses `allocate_at_least` from `secure_allocator`'s base to allocate memory which it then tries to free with `secure_allocator::deallocate`.
Drive-by: Aggressively remove facilities unnecessary since C++11 from both allocators to keep things simple.
This is to (a) avoid repeated lookups into the block index for an entry that
should never change and (b) emphasize that the snapshot base should always
exist when set and not change during the runtime of the program.
Thanks to Russ Yanofsky for suggesting this approach.
Also rewrite CheckBlockIndex() to perform tests on all chainstates.
This increases sanity-check coverage, as any place in our code where we were
invoke CheckBlockIndex() on a single chainstate will now invoke the sanity
checks on all chainstates.
This change also tightens up the checks on setBlockIndexCandidates and
mapBlocksUnlinked, to more precisely match what we aim for even in the presence
of assumed-valid blocks.
When using assumeutxo and multiple chainstates are active, the background
chainstate should consider all HAVE_DATA blocks that are ancestors of the
snapshotted block and that have more work than the tip as potential candidates.
When using assumeutxo, we only need the background chainstate to consider
blocks that are on the chain leading to the snapshotted block.
Note that this introduces the new invariant that we can only have an assumeutxo
snapshot where the snapshotted blockhash is in our block index. Unknown block
hashes that are somehow passed in will cause assertion failures when processing
new blocks.
Includes test fixes and improvements by Andrew Chow and Fabian Jahr.
This is needed for the next commit.
This also requires dropping CI_RETRY from the docker build step, which
is fine, because CI_RETRY should be called inside the build script, not
outside.
Also, fix a doc typo.
Other tests are also relying on the node1 default wallet,
which thanks to 'test_locked_wallet' is encrypted.
And can only be accessed within a specific timeframe (100ms)
set internally by the same test.
This make other tests susceptible to races. They can only
perform their operations successfully within the specified
time.
This can be seen running the test in valgrind, where other
test cases fail due the wallet re-locking itself after the
100ms.
