💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1488328390)
> Nice, this looks much simpler now. interface_usdt_mempool.py is now failing but the fix is very simple: just mine a block after each test: https://github.com/josibake/bitcoin/commit/85ec64b1eb3037e8bd9349242b05615b41b87261

Added a commit fixing it and added you as co-author, thank you! Force-pushed doing it!

💬 fanquake commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1488350147)
@glozow want to take a look at the mempool tests?

🚀 fanquake merged a pull request: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)

💬 fanquake commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1488392041)
> resulting in improved performance for functional tests.

Generally good to provide some sort of benchmark/indication of the speedup when talking about performance.

🚀 fanquake merged a pull request: "guix: use GCC tool wrappers"
(https://github.com/bitcoin/bitcoin/pull/27345)

💬 dimitaracev commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#discussion_r1151809995)
No reason, moved it more down with 5526849 .

💬 dimitaracev commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1488452250)
@MarcoFalke Thanks for the remainder, squashed them and it should be good now.

👋 MarcoFalke's pull request is ready for review: "ci: Use Cirrus CI dockerfile env"
(https://github.com/bitcoin/bitcoin/pull/27340)

📝 fanquake opened a pull request: "ci: use LLVM/clang-16 in ASAN job"
(https://github.com/bitcoin/bitcoin/pull/27360)
Similar to #27298. Working for me on `x86_64` and solves the issue I currently see with TSAN on `aarch64` with master (68828288e5d35c17848ab3da8cd231d1b69651c0):
```bash
crc32c/src/crc32c_arm64.cc:101:26: runtime error: load of misaligned address 0xffff84400406 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment
0xffff84400406: note: pointer points here
b9 c5 22 00 01 01 1a 6c 65 76 65 6c 64 62 2e 42 79 74 65 77 69 73 65 43 6f 6d 70 61 72 61 74 6f
^
...

💬 MarcoFalke commented on pull request "ci: use LLVM/clang-16 in ASAN job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1488552377)
You will probably need to do the same for fuzz asan?

💬 MarcoFalke commented on pull request "ci: use LLVM/clang-16 in ASAN job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1488554684)
Also, need to edit Cirrus CI to do the same

💬 hebasto commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1151910811)
Considering switching s/`stdio.h`/`cstdio`/, does it make sense to s/`FILE`/`std::FILE`/ as well? Maybe as a follow up?

👍 hebasto approved a pull request: "refactor: Extract util/fs from util/system"
(https://github.com/bitcoin/bitcoin/pull/27254)
ACK 00e9b97f37e0bdf4c647236838c10b68b7ad5be3

💬 hebasto commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1151936439)
nit:

```diff
--- a/src/qt/intro.cpp
+++ b/src/qt/intro.cpp
@@ -6,16 +6,16 @@
#include <config/bitcoin-config.h>
#endif

-#include <chainparams.h>
#include <qt/intro.h>
#include <qt/forms/ui_intro.h>
-#include <util/fs.h>

#include <qt/guiconstants.h>
#include <qt/guiutil.h>
#include <qt/optionsmodel.h>

+#include <chainparams.h>
#include <interfaces/node.h>
+#include <util/fs.h>
#include <util/fs_helpers.h>
#include <util/system.h>
#include <validation.h>

...

💬 meglio commented on issue "signrawtransactionwithwallet fails with signed non-wallet inputs and breaks the existing signatures":
(https://github.com/bitcoin/bitcoin/issues/26385#issuecomment-1488621015)
Am I correct to assume this is still a bug awaiting a fix?

💬 fanquake commented on pull request "ci: use LLVM/clang-16 in ASAN job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1488669908)
> You will probably need to do the same for fuzz asan?

Will add.

> Also, need to edit Cirrus CI to do the same

Can we even do this for native_asan given there is no 2304-lto image listed here: https://cloud.google.com/compute/docs/images/os-details#ubuntu_lts ?

💬 dergoegge commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#discussion_r1151979732)
```suggestion
inbounds_fanouted += pnode->IsInboundConn() && pnode->m_relays_txs && !m_txreconciliation->IsPeerRegistered(pnode->GetId());
```
Right? because other wise this includes erlay peers which we are not flooding to by default.

💬 dergoegge commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#discussion_r1151986544)
`m_connman.GetNodeCount(ConnectionDirection::In)` includes all inbound connections, even ones that do want transactions relayed to them. IIUC, you want this to be "number of all inbound peers that sent fRelay=true" and the second member of the pair should be "number of all non-erlay inbound peers that sent fRelay=true"

💬 MarcoFalke commented on pull request "ci: use LLVM/clang-16 in ASAN job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1488682494)
Yeah, I suspect that debian:bookworm will be added earlier by google to the compute images. You can ask Sylvestre to add clang-16 to bookworm, I guess?

💬 dergoegge commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1488686338)
Concept ACK