💬 laanwj commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2646339414)
> I have a slight preference on BIP155 NetworkIDs over keeping-as-is or network name as string.

Agree. Strings are sensitive to typos, and require extra effort for matching when doing statistics. i think even the current enumeration is better than that. BIP155 is a good suggestion, but as you have to make up non-standard values, and handle NET_UNROUTABLE differently i'm not entirely sure.

The purpose of tracing is to gain insight into internal state so logging the actual values makes more
...

💬 eval-exec commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1948125076)
Thank you.

💬 sipa commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1948126145)
Even better, use this directly in the only place this function is called: `SeedHardwareSlow`, instead of the `g_rndr_supported` that is used there now.

💬 eval-exec commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1948126777)
Thank you, Updated.

🤔 sipa reviewed a pull request: "Limit retries in GetRNDRRS to avoid infinite loop"
(https://github.com/bitcoin/bitcoin/pull/31826#pullrequestreview-2604286273)
Code review ACK daa1fbe88171042ae39b257767137c8ba652e046. Please squash your commits.

💬 sipa commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1948128303)
Coding style nit: space after `}`.

💬 sipa commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2646354848)
> The BIP155 approach is a bit more involved with making GetBIP155Network() public, introducing a ConnectedThroughBIP155Network() helper, and passing 0 (or something else?) for NET_INTERNAL as there is no BIP155 equivalent.

Given that BIP155 network IDs are uint8_t's, I think it suffices to pick a value outside of that range for unreachable. For example, 0x100, or 0xffffffff for a `uint32_t, or -1 for a signed integer type.

🤔 sipa reviewed a pull request: "Limit retries in GetRNDRRS to avoid infinite loop"
(https://github.com/bitcoin/bitcoin/pull/31826#pullrequestreview-2604310733)
utACK 03beb9c0bf2cb4a4022a1f9f94a6093fb4021154

💬 fjahr commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2646395385)
> Note that GCOV_EXECUTABLE is hardcoded in the script currently, so setting it as environment variable won't have any effect.

Ah, I had indeed missed that. I tried this for llvm but it didn't change the outcome, seems to be the same issue. I didn't investigate further there and instead tried again with gcc-14, given this is at least a bit closer to your tooling. I get another error there with this. It appears the compile time macros are confusing gcov and it sees `secp256k1_scalar_split_lam
...

💬 Christewart commented on pull request "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`":
(https://github.com/bitcoin/bitcoin/pull/31823#issuecomment-2646450622)

> 1. Is there any test case that can be covered in the current implementation that needs this change to be implemented?

There is not currently. This short coming in the test framework was discovered while working on test cases for disallowing 64 byte transactions in the bitcoin blockchain. This python test case would not fail correctly if this change wasn't added to the test framework: https://github.com/ajtowns/bitcoin/pull/13/files#diff-d390e9e12bfa34a462fdff9f1894d7dc3edb45c26cf55df486d9
...

🤔 sins921 reviewed a pull request: "refactor: Print verbose serialize compiler error messages"
(https://github.com/bitcoin/bitcoin/pull/29056#pullrequestreview-2604454205)
Gracias

📝 glozow opened a pull request: "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs"
(https://github.com/bitcoin/bitcoin/pull/31829)
This PR is built on top of #31810 and is part of the orphan resolution project, see #27463.

We want to limit the CPU work and memory used by `TxOrphanage` to avoid denial of service attacks. On master, this is achieved by limiting the number of transactions in this data structure to 100, and the weight of each transaction to 400KWu (the largest standard tx) [0]. We always allow new orphans, but if the addition causes us to exceed 100, we evict one randomly. This is dead simple, but makes the
...

💬 hebasto commented on something "":
(https://github.com/bitcoin/bitcoin/commit/332655cb52c8f8ef64b29b09e38ef5d61235ed21#r152331640)
> Why not pass a version string to project command and use something like `PROJECT_VERSION_MAJOR` in places where `CLIENT_VERSION_MAJOR` is used at the moment?

This approach was indeed considered first.

Will the `project()` command handle the "29.0.0rc1" version string properly?

⚠️ brunoerg opened an issue: "Branch and Bound producing change"
(https://github.com/bitcoin/bitcoin/issues/31830)
My fuzz server crashed due to Branch and Bound producing change. I could reproduce the issue with the following test:

```cpp
BOOST_AUTO_TEST_CASE(bnb_change)
{
FastRandomContext fast_random_context{};
CoinSelectionParams coin_params{
/*rng_fast*/fast_random_context,
/*change_output_size=*/31,
/*change_spend_size=*/68,
/*min_change_target=*/50'000,
/*effective_feerate=*/CFeeRate(5000),
/*long_term_feerate=*/CFeeRate(10'000),
/*disca
...

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2646919080)
Rebased b375aeaeefb70023c3c2cd74e3841e382473b292 -> e7743aa5df6387964c23f0629debf8c3cbb8ed4c ([`pr/subtree.12`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.12) -> [`pr/subtree.13`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.13), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.12-rebase..pr/subtree.13)) adding https://github.com/chaincodelabs/libmultiprocess/pull/159 and marking capnp cmake variables as advanced as requested.

> Could the `CAPNP_EXECUT
...

✅ ryanofsky closed a pull request: "multiprocess: Lock CapnpProtocol::m_loop with mutex"
(https://github.com/bitcoin/bitcoin/pull/31815)

💬 ryanofsky commented on pull request "multiprocess: Lock CapnpProtocol::m_loop with mutex":
(https://github.com/bitcoin/bitcoin/pull/31815#issuecomment-2646931290)
I reproduced the bug locally (see https://github.com/chaincodelabs/libmultiprocess/issues/154#issuecomment-2646926155) and implemented a different fix in https://github.com/chaincodelabs/libmultiprocess/pull/159, which should make these changes unnecessary, because instead of trying to delay eventloop destruction in Bitcoin code until all locks are released, it just avoids reacquiring a lock unnecessary and pretents eventloop::loop() from returning until all locks are released so it won't be des
...

💬 rhysbeynon commented on pull request "Updated MacOS icon to more closely fit Apple's design standards":
(https://github.com/bitcoin-core/gui/pull/852#issuecomment-2646981748)

Some screenshots to show size difference in context of other apps in a populated dock.

📝 ShivaanjayNarula opened a pull request: "imp"
(https://github.com/bitcoin/bitcoin/pull/31831)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ hebasto closed a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31831)