💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1583019336)
I agree that adding this makes sense as the bcc error message isn't clear. If I retouch, I'll add it.

💬 willcl-ark commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1583020605)
Ah I see, that makes sense now then, thanks.

🤔 furszy reviewed a pull request: "Disable util::Result copying and assignment"
(https://github.com/bitcoin/bitcoin/pull/29906#pullrequestreview-2028393990)
ACK 9552619961049d7673f84c40917b0385be70b782

💬 instagibbs commented on pull request "test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py":
(https://github.com/bitcoin/bitcoin/pull/29986#discussion_r1583032012)
```suggestion
fee_to_beat = max(int(tx_v3_child_2["fee"] * COIN), int(tx_unrelated_replacee["fee"] * COIN))
```
would you consider this instead? seems the more direct thing we're trying to accomplish

👍 hebasto approved a pull request: "build: add `-Wundef`"
(https://github.com/bitcoin/bitcoin/pull/29876#pullrequestreview-2028426491)
ACK cc09c165eb0bbb4b41a9132734bee55969e9d9a8.

I suggest to consider updating a help string in https://github.com/bitcoin/bitcoin/blob/a46065e36cf868265c909dc5edf29dc17be53c1f/configure.ac#L1133 to "Define to 1 if O_CLOEXEC flag is available, and to 0 if not."

As a follow up, the https://github.com/bitcoin/bitcoin/pull/16344 might be reconsidered.

👍 fanquake approved a pull request: "build: Bump clang minimum supported version to 15"
(https://github.com/bitcoin/bitcoin/pull/29165#pullrequestreview-2028437917)
ACK fa1964c5b80ee28b0e06abdbd9a26e8e8c6f5acd - oss-fuzz LLVM will either be bumped globally tomorrow, or we'll land our own bump.

🚀 fanquake merged a pull request: "build: Bump clang minimum supported version to 15"
(https://github.com/bitcoin/bitcoin/pull/29165)

📝 willcl-ark opened a pull request: "gui: fix misleading signmessage error with segwit"
(https://github.com/bitcoin-core/gui/pull/819)
As described in https://github.com/bitcoin/bitcoin/issues/10542 (and numerous other places), message signing in Bitcoin Core does not support "signing with a segwit address" and likely will not in the foreseeable future, or at least until a new message-signing standard is agreed upon.

Therefore update the possibly misleading error message presented to the user in the GUI to detail more specifically the reason their message cannot be signed, in the case that a non P2PKH address is entered.


...

💬 0xB10C commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2082675302)
I was made aware of https://bencher.dev/learn/engineering/sqlite-performance-tuning/ recently and the [dashboard](https://bencher.dev/perf/0xb10c-s-bitcoin-core?key=true&reports_per_page=4&branches_per_page=8&testbeds_per_page=8&benchmarks_per_page=8&reports_page=1&branches_page=1&testbeds_page=1&benchmarks_page=1&report=915c1007-33dd-4d30-b9c1-531b60af8a06&branches=0ab6caf7-716b-4a6e-8101-f2b9c891a9a2&testbeds=3c0c4d9d-b40e-4d9a-a462-2379355b0785&benchmarks=993b5444-b391-43dd-a4c1-c157cb1d7209%
...

💬 sdaftuar commented on pull request "fuzz: don't allow adding duplicate transactions to the mempool":
(https://github.com/bitcoin/bitcoin/pull/29990#discussion_r1583059571)
> In this case, it is calling addUnchecked directly that's why it would be proper to check whether the transaction is in mempool before. Note that, in practice, this function is used in MemPoolAccept::Finalize which removes conflicting transactions from the mempool before adding.

Yes exactly. The correct interface for code that is doing no sanity checking would be `AcceptToMemoryPool()`. Since we're bypassing that in the fuzz tests, we should do something else to avoid putting the mempool in
...

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1583065711)
looked a bit closer, how is this different from the "# ...even if it's submitted with other transactions" case above in the test? To check package rbf the only difference is making sure it's evaluated as a single subpackage(by having the child feerate be larger)

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1583082284)
Ahh I see, it's the same code path.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1583085742)
oh right, neither of these cases will do a package rbf, as the package has in-mempool ancestors.

It would have to be a scenario where the ancestor limits are set to 1, then a package RBF of size 2 replaces that single tx?

💬 hebasto commented on pull request "build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29494#issuecomment-2082750970)
How is it supposed to automatically catch cases when code changes make `#include <config/bitcoin-config.h>` unneeded in a header or source file?

🚀 fanquake merged a pull request: "test: Add missing Assert(mock_time_in >= 0s) to SetMockTime"
(https://github.com/bitcoin/bitcoin/pull/29872)

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1583113813)
Hmm, I think it will always be the same code path that will be hit because the parent transaction (in the cluster size 2 replacement package) will be detected as conflict with the transaction that was carved out (because it has to go through `PreChecks`), and we don't allow package RBF carveout, so the subpackage evaluation will not be executed?

💬 sr-gi commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1583123904)
I think, design-wise, it's a bit counter-intuitive, but I do agree that not overloading `m_recent_rejects` is potentially better

👍 ismaelsadeeq approved a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2028579002)
Code Review ACK 7cede4caa0a6b3dd57397d96ee98239fb890ca32

My comments were addressed and I have reviewed all the commits and test this locally on regtest.

I've fuzz this PR for a while locally and no any crash.

💬 sr-gi commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-2082822497)
tACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f

💬 hebasto commented on pull request "doc: update release-process.md":
(https://github.com/bitcoin/bitcoin/pull/29645#issuecomment-2082833654)
> Thanks for the context @hebasto. I felt that it was appropriate to move from "before every release candidate" to ~"before every major or minor release"~(EDIT: correction, "before branch-off") because
>
> * I didn't observe that translations were being updated per release candidate.

It happens, but very seldom. We can really ignore them as it simplifies the release process significantly.

> * It made more sense to me that it'd be per release instead of per release candidate since,
...