👋 MarcoFalke's pull request is ready for review: "ci: Use TSan new runtime (llvm-16, take 3)"
(https://github.com/bitcoin/bitcoin/pull/27298)
(https://github.com/bitcoin/bitcoin/pull/27298)
💬 jnewbery commented on pull request "net: #27257 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27324#discussion_r1150334044)
This is absolutely fine. I would have gone for something like "For performance reasons, once we've deserialized the bytes into a CNetMessage, we avoid re-allocating and copying the message as we pass it up the stack for processing. Delete the copy ctor/assignment to avoid accidentally introducing a copy operation.", which is more-or-less the same as what you came up with.
(https://github.com/bitcoin/bitcoin/pull/27324#discussion_r1150334044)
This is absolutely fine. I would have gone for something like "For performance reasons, once we've deserialized the bytes into a CNetMessage, we avoid re-allocating and copying the message as we pass it up the stack for processing. Delete the copy ctor/assignment to avoid accidentally introducing a copy operation.", which is more-or-less the same as what you came up with.
💬 jnewbery commented on pull request "net: #27257 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27324#issuecomment-1486557249)
utACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829
(https://github.com/bitcoin/bitcoin/pull/27324#issuecomment-1486557249)
utACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829
👍 fanquake approved a pull request: "fuzz: Remove legacy int parse fuzz tests"
(https://github.com/bitcoin/bitcoin/pull/27344)
ACK faf8dc496e761a15956f8226d727f4bbab8dff82
(https://github.com/bitcoin/bitcoin/pull/27344)
ACK faf8dc496e761a15956f8226d727f4bbab8dff82
👍 dergoegge approved a pull request: "fuzz: Remove legacy int parse fuzz tests"
(https://github.com/bitcoin/bitcoin/pull/27344)
ACK faf8dc496e761a15956f8226d727f4bbab8dff82
(https://github.com/bitcoin/bitcoin/pull/27344)
ACK faf8dc496e761a15956f8226d727f4bbab8dff82
📝 TheCharlatan opened a pull request: "refactor (tidy): Fixes after enable-debug configure"
(https://github.com/bitcoin/bitcoin/pull/27353)
This diff can be reproduced by running (tested with clang-14 and clang-15):
```
./autogen.sh && CC=clang CXX=clang++ ./configure --enable-suppress-external-warnings --enable-debug
bear --config src/.bear-tidy-config -- make -j $(nproc)
cd ./src/ && run-clang-tidy -quiet -fix -j $(nproc)
```
This pull request is set to draft, since I still don't know why `--enable-debug` allows tidy to catch this additional lint (performance-for-range-copy). Any pointers would be appreciated. The same l
...
(https://github.com/bitcoin/bitcoin/pull/27353)
This diff can be reproduced by running (tested with clang-14 and clang-15):
```
./autogen.sh && CC=clang CXX=clang++ ./configure --enable-suppress-external-warnings --enable-debug
bear --config src/.bear-tidy-config -- make -j $(nproc)
cd ./src/ && run-clang-tidy -quiet -fix -j $(nproc)
```
This pull request is set to draft, since I still don't know why `--enable-debug` allows tidy to catch this additional lint (performance-for-range-copy). Any pointers would be appreciated. The same l
...
✅ fanquake closed an issue: "test_bitcoin: ./chain.h:261: uint256 CBlockIndex::GetBlockHash() const: Assertion `phashBlock != nullptr' failed."
(https://github.com/bitcoin/bitcoin/issues/27320)
(https://github.com/bitcoin/bitcoin/issues/27320)
🚀 fanquake merged a pull request: "test: fix intermittent failure in ChainStateManager tests"
(https://github.com/bitcoin/bitcoin/pull/27348)
(https://github.com/bitcoin/bitcoin/pull/27348)
💬 ismaelsadeeq commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150368780)
Yes.
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150368780)
Yes.
💬 ismaelsadeeq commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150370139)
Thanks.
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150370139)
Thanks.
👍 fanquake approved a pull request: "ci: Use TSan new runtime (llvm-16, take 3)"
(https://github.com/bitcoin/bitcoin/pull/27298)
ACK faf4aca15a319a26aaec7127455f6db97c7039cc - I still see [this](https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1480041089) failure on aarch64, but that isn't really a regression, as running this tests was already broken for me. I'll open a separate issue, and we can follow up.
(https://github.com/bitcoin/bitcoin/pull/27298)
ACK faf4aca15a319a26aaec7127455f6db97c7039cc - I still see [this](https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1480041089) failure on aarch64, but that isn't really a regression, as running this tests was already broken for me. I'll open a separate issue, and we can follow up.
💬 glozow commented on pull request "test: refactor: dedup mempool_package_limits.py subtests via decorator":
(https://github.com/bitcoin/bitcoin/pull/27350#discussion_r1150385623)
Ah nice. But might as well just make it this?
```suggestion
high_fee = target_weight * 10
```
(https://github.com/bitcoin/bitcoin/pull/27350#discussion_r1150385623)
Ah nice. But might as well just make it this?
```suggestion
high_fee = target_weight * 10
```
💬 glozow commented on pull request "test: refactor: dedup mempool_package_limits.py subtests via decorator":
(https://github.com/bitcoin/bitcoin/pull/27350#discussion_r1150382620)
(1) I think it could be useful to clarify that the subtest modifies mempool and creates the txns
(2) I prefer to reserve the term "submit" for sendraw and submitpackage
```suggestion
# 2) run the subtest, which may submit some transaction(s) to the mempool and create a list of hex transactions
# 3) testmempoolaccept the package hex and check that it fails with the error "package-mempool-limits" for each tx
# 4) after mining a block, clearing the pre-submitted transactions from mempool, su
...
(https://github.com/bitcoin/bitcoin/pull/27350#discussion_r1150382620)
(1) I think it could be useful to clarify that the subtest modifies mempool and creates the txns
(2) I prefer to reserve the term "submit" for sendraw and submitpackage
```suggestion
# 2) run the subtest, which may submit some transaction(s) to the mempool and create a list of hex transactions
# 3) testmempoolaccept the package hex and check that it fails with the error "package-mempool-limits" for each tx
# 4) after mining a block, clearing the pre-submitted transactions from mempool, su
...
💬 fanquake commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1486632813)
The failure here is #27316.
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1486632813)
The failure here is #27316.
💬 fanquake commented on issue "wallet_create_tx.py "Not solvable pre-selected input" exception":
(https://github.com/bitcoin/bitcoin/issues/27316#issuecomment-1486633439)
Also seen here https://cirrus-ci.com/task/4836577990410240
(https://github.com/bitcoin/bitcoin/issues/27316#issuecomment-1486633439)
Also seen here https://cirrus-ci.com/task/4836577990410240
🚀 fanquake merged a pull request: "net: #27257 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/27324)
(https://github.com/bitcoin/bitcoin/pull/27324)
🚀 fanquake merged a pull request: "fuzz: Remove legacy int parse fuzz tests"
(https://github.com/bitcoin/bitcoin/pull/27344)
(https://github.com/bitcoin/bitcoin/pull/27344)
💬 fanquake commented on issue "-Wmaybe-uninitialized warnings under LTO":
(https://github.com/bitcoin/bitcoin/issues/27343#issuecomment-1486680561)
> Fuzz ones removed in https://github.com/bitcoin/bitcoin/pull/27344
Thanks. Retested at 8d31d769b7b504104f162a03c94bbc95dec7d849 and updated the op to drop those (other than in float).
(https://github.com/bitcoin/bitcoin/issues/27343#issuecomment-1486680561)
> Fuzz ones removed in https://github.com/bitcoin/bitcoin/pull/27344
Thanks. Retested at 8d31d769b7b504104f162a03c94bbc95dec7d849 and updated the op to drop those (other than in float).
💬 MarcoFalke commented on issue "-Wmaybe-uninitialized warnings under LTO":
(https://github.com/bitcoin/bitcoin/issues/27343#issuecomment-1486706789)
Yeah, that is a false positive. Not sure if they are going to fix it, but if you want to work around, you can try something like:
```diff
diff --git a/src/test/fuzz/float.cpp b/src/test/fuzz/float.cpp
index 8714ab8a04..57f8d4cec8 100644
--- a/src/test/fuzz/float.cpp
+++ b/src/test/fuzz/float.cpp
@@ -7,11 +7,13 @@
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
#include <util/serfloat.h>
+#include <util/check.h>
#include <version.h>
#include <cassert>
#include <c
...
(https://github.com/bitcoin/bitcoin/issues/27343#issuecomment-1486706789)
Yeah, that is a false positive. Not sure if they are going to fix it, but if you want to work around, you can try something like:
```diff
diff --git a/src/test/fuzz/float.cpp b/src/test/fuzz/float.cpp
index 8714ab8a04..57f8d4cec8 100644
--- a/src/test/fuzz/float.cpp
+++ b/src/test/fuzz/float.cpp
@@ -7,11 +7,13 @@
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
#include <util/serfloat.h>
+#include <util/check.h>
#include <version.h>
#include <cassert>
#include <c
...
⚠️ MarcoFalke opened an issue: "wallet: Data race in GetOrCreateLegacyScriptPubKeyMan vs IsMine"
(https://github.com/bitcoin/bitcoin/issues/27354)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
tsan error
### Expected behaviour
no tsan error
### Steps to reproduce
I presume the error is intermittent, but the basic idea is that the validation interface thread (scheduler) may connect blocks and sync wallet transactions, thus call `IsMine` and access `m_spk_managers`. At the same time, the main thread may call `GetOrCreateLegacyScriptPubKeyMan`, also accessing `m_spk_managers`,
...
(https://github.com/bitcoin/bitcoin/issues/27354)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
tsan error
### Expected behaviour
no tsan error
### Steps to reproduce
I presume the error is intermittent, but the basic idea is that the validation interface thread (scheduler) may connect blocks and sync wallet transactions, thus call `IsMine` and access `m_spk_managers`. At the same time, the main thread may call `GetOrCreateLegacyScriptPubKeyMan`, also accessing `m_spk_managers`,
...