💬 MarcoFalke commented on pull request "depends: fix compiling bdb with clang-16":
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1508365659)
Unrelated: Interesting, that probably also means we can't switch to C++20 with depends, as long as bdb is around, assuming that depends is compiled with the same flags as Bitcoin Core.

💬 vasild commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1166722759)
It begins its life with 16 bytes pre-allocated (without using `new` or `malloc()`), but this is just an optimization which can be ignored. It is supposed to behave like `std::vector`.

Something like this should work to store `std::string` in it (untested):

```cpp
std::string path = ...;
m_addr.assign(path.begin(), path.end());
```

💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1166745046)
Looking into `GetGroup()` I saw it returns a C++ `std::vector<unsigned char>`. We can't pass C++ `std::vector`'s in the tracepoints as the eBPF code, written in C, won't be able to read it. We can call `data()` and pass this in the tracepoint as pointer to unsigned chars (we do this with e.g. the raw P2P messages too). To read from this pointer, we also need to pass the of the length of underlying array.

I'll give this a shot in a separate branch.

💬 real-or-random commented on pull request "Update src/secp256k1 subtree to release v0.3.1":
(https://github.com/bitcoin/bitcoin/pull/27445#issuecomment-1508419009)
@sipa https://github.com/bitcoin-core/secp256k1/pull/1276 has been merged, can you update this PR to include the current secp256k1 master?

💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1166768178)
An alternative is to pass `GetMappedAS()` which is an `uint32_t` that will be !=0 in an ASMap future. A net group similar to Bitcoin Core's can be calculated from the peer address by consumers that need it.

💬 fanquake commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1508445672)
> Hmm, then my guess that it could be the timeout was probably wrong. Could you run the test in isolation with -- -printtoconsole and post the log?

With the branch in #27462 (which just fixes BDB compilation), if i run the TSAN CI, it fails as it has been. However if I exec into the container after it fails, and run `src/test/test_bitcoin --catch_system_errors=no -l test_suite -t blockfilter_index_tests`, this issue does not reproduce.

💬 MarcoFalke commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1508450185)
Can you also try with `LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" ./src/test/...`

💬 glozow commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#discussion_r1166795553)
We can't because these get rejected and `info()` only works on transactions that make it into the mempool.

💬 glozow commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1508455396)
@ajtowns thanks for the feedback, I'm not really sure where to draw the line between useful and not useful but I've made a tracking issue #27463. I've marked places where I think the outcome is meaningful (e.g. "after this, we could expose on p2p and it would be safe").

💬 achow101 commented on pull request "verify-commits: error and exit cleanly when git is too old.":
(https://github.com/bitcoin/bitcoin/pull/27461#issuecomment-1508503086)
ACK 1fefcf27edcac7ebb87c1d3c68bcd9870e3ae78a

🚀 achow101 merged a pull request: "verify-commits: error and exit cleanly when git is too old."
(https://github.com/bitcoin/bitcoin/pull/27461)

💬 MarcoFalke commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1508505587)
I also can *not* reproduce with:

```
time bash -c 'RUN_UNIT_TESTS_SEQUENTIAL="true" RUN_UNIT_TESTS="false" RUN_FUNCTIONAL_TESTS=false CCACHE_SIZE=500M MAKEJOBS="-j9" FILE_ENV="./ci/test/00_setup_env_native_tsan.sh" ./ci/test_run_all.sh'
```
and diff:

```diff
diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh
index f7dcbcee5..899c99119 100755
--- a/ci/test/06_script_b.sh
+++ b/ci/test/06_script_b.sh
@@ -29,7 +29,7 @@ if [ "$RUN_UNIT_TESTS" = "true" ]; then
fi

if [ "
...

💬 instagibbs commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#issuecomment-1508506500)
concept ACK, will review

📝 MarcoFalke opened a pull request: "fuzz: re-enable prioritisetransaction RPC"
(https://github.com/bitcoin/bitcoin/pull/27464)
The linked issue seems fixed, so it should be fine to re-enable

💬 instagibbs commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1166889886)
I'd like the test to load against two mempool files with differing txn subsets at least, showing that "additional" txs are loaded even if they weren't there originally. Even better if they're not strict subsets.

💬 MarcoFalke commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1508629562)
All I run into here is a bdb issue, which can be worked around with:

```diff
diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan
index d33199127..35c6a5f95 100644
--- a/test/sanitizer_suppressions/tsan
+++ b/test/sanitizer_suppressions/tsan
@@ -23,7 +23,7 @@ race:src/qt/test/*
deadlock:src/qt/test/*

# External libraries
-deadlock:libdb
+deadlock:__db_pthread_mutex_lock
race:libzmq

# Intermittent issues

💬 instagibbs commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1508641506)
Very helpful. I think this helps define "sprints" that can be achieved to hit next milestones.

💬 MarcoFalke commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1166919624)
unrelated nit: If you retouch, you could also remove `__func__` for `-signetchallenge`

💬 fanquake commented on issue "qa: Intermittent failure in `feature_fee_estimation.py`":
(https://github.com/bitcoin/bitcoin/issues/23165#issuecomment-1508710573)
https://cirrus-ci.com/task/6110951150190592?logs=ci#L3312:
```bash
node1 2023-04-14T11:01:41.617514Z [http] [httpserver.cpp:257] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:49238
node1 2023-04-14T11:01:41.617673Z [httpworker.2] [rpc/request.cpp:180] [parse] [rpc] ThreadRPCServer method=getmempoolinfo user=__cookie__
test 2023-04-14T11:01:41.618000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):

...

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1166947724)
thx, done