💬 vasild commented on pull request "build: enable libc++ hardening in debug builds":
(https://github.com/bitcoin/bitcoin/pull/31424#issuecomment-2725133536)
@maflcko:

```diff
--- i/src/test/random_tests.cpp
+++ w/src/test/random_tests.cpp
@@ -17,12 +17,18 @@ BOOST_FIXTURE_TEST_SUITE(random_tests, BasicTestingSetup)

BOOST_AUTO_TEST_CASE(osrandom_tests)
{
BOOST_CHECK(Random_SanityCheck());
}

+BOOST_AUTO_TEST_CASE(t)
+{
+ std::vector<int> v{1, 2, 3};
+ (void)v[10];
+}
+
BOOST_AUTO_TEST_CASE(fastrandom_tests_deterministic)
{
// Check that deterministic FastRandomContexts are deterministic
SeedRandomFor
...

💬 Sjors commented on issue "v29.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/32052#issuecomment-2725161513)
This was fixed in #32064. So I guess we can't do the guix-sign step until the next release candidate?

💬 Sjors commented on pull request "build: Remove manpages when making MacOS app":
(https://github.com/bitcoin/bitcoin/pull/32064#issuecomment-2725164412)
Having the man files in the `.tar.gz` release, where `bitcoind` and `bitcoin-cli` live, should be enough.

💬 l0rinc commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#issuecomment-2725167625)
> since the BufferedReadOnlyFile class implemented there duplicated functionality of AutoFile, didn't support writing, and didn't work with any type of stream

There was a writer as well, but it resulted in some [weird CI failures](https://github.com/bitcoin/bitcoin/pull/31539#issuecomment-2612262087) and given this alternative I just closed the PR.

> disappointed if you initially had a general solution that made it easy to enable buffering any place and then were told that instead of using
...

💬 davidgumberg commented on pull request "test: Add support for mainnet addresses in address_to_scriptpubkey":
(https://github.com/bitcoin/bitcoin/pull/32060#discussion_r1995892980)
What you're saying doesn't make any sense

💬 davidgumberg commented on pull request "test: Add support for mainnet addresses in address_to_scriptpubkey":
(https://github.com/bitcoin/bitcoin/pull/32060#discussion_r1995894067)
You could also apply the suggestion yourself

💬 yancyribbens commented on pull request "test: replace hardcoded fee with node relay fee based calculation":
(https://github.com/bitcoin/bitcoin/pull/32058#issuecomment-2725238772)
Concept ACK. The changes appear to address the TODO correctly.

💬 instagibbs commented on pull request "test: Rename send_message to send_without_ping":
(https://github.com/bitcoin/bitcoin/pull/31859#issuecomment-2725240640)
ACK https://github.com/bitcoin/bitcoin/pull/31859/commits/fa9cf38ab666b50b0d8e82cb17f9e5a8a613547d

Some one-time churn that should help future-proof testing code a bit.

💬 yancyribbens commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#discussion_r1995938540)
```suggestion
// (with N being the number of peers from which we're downloading validated blocks), disconnect due to timeout
```

💬 ryanofsky commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#issuecomment-2725260200)
> I can get behind that, can you point me to other usages where we could use this?

I don't know, but if I were looking I'd just grep for places where AutoFile class is used. I don't have any real concerns here and I'm fine with the current approach. To the extent I do have concerns they are about readability and maintainability of code not performance, so I like an approach that makes it trivial to turn buffering on and off, which is I why thought BufferedReader / BufferedWriter stream adapte
...

💬 yancyribbens commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#issuecomment-2725268217)
Concept ACK. I'm curious if there should be any doc updates accompanying this change.

💬 janb84 commented on pull request "contrib: Update coverage.cpp to drop linux restriction":
(https://github.com/bitcoin/bitcoin/pull/32059#discussion_r1995978301)
### Main problem:

> "macOS’s linker (ld64) doesn’t silently resolve unreferenced weak symbols to NULL when they’re called—it demands a definition".

Combined with calling `__gcov_reset` and `__llvm_profile_reset_counters` directly, forcing the linker to find a definition, will result in that the linker will break on trying to link the following code:
```C
extern "C" void __llvm_profile_reset_counters(void) __attribute__((weak));
extern "C" void __gcov_reset(void) __attribute__((weak)
...

💬 janb84 commented on pull request "contrib: Update coverage.cpp to drop linux restriction":
(https://github.com/bitcoin/bitcoin/pull/32059#issuecomment-2725326675)
In my write-up of the explanation to hodlinator I discovered one edge case that needed fixing, sorry for the new commit.

Reason:
Weak fallbacks (__attribute__((weak))) were added instead of strong ones to ensure the profiling runtime’s implementations are used when profiling is enabled, **_avoiding linker conflicts or silent overrides_**. This guarantees correct counter resets while keeping safe, empty fallbacks when profiling is off, improving portability and reliability across Clang and G
...

💬 VolodymyrBg commented on pull request "test: Add support for mainnet addresses in address_to_scriptpubkey":
(https://github.com/bitcoin/bitcoin/pull/32060#issuecomment-2725346210)
@maflcko Done

💬 VolodymyrBg commented on pull request "test: Add support for mainnet addresses in address_to_scriptpubkey":
(https://github.com/bitcoin/bitcoin/pull/32060#issuecomment-2725350636)
@davidgumberg I applied myself. Thank you

💬 zzzi2p commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-2725369358)
Thanks for CC'ing me. I also asked eyedeekay and orignal from i2pd to take a look.

10 standby sessions is a lot, except maybe at startup. If your I2P connections limit is 10, then you're doubling your network resource usage. 2 or 3 is probably enough, depending on how fast your connections churn and how long is long enough for a unused session to be up to give you the anonymity you're looking for.

A couple of alternatives:

```c
int max = MAX_UNUSED_I2P_SESSIONS_SIZE;
if (uptime() > 15
...

💬 yancyribbens commented on pull request "wallet: refactor: various master key encryption cleanups":
(https://github.com/bitcoin/bitcoin/pull/31398#discussion_r1996031367)
```suggestion
static constexpr unsigned int DEFAULT_DERIVE_ITERATIONS = 25'000;
```

💬 yancyribbens commented on pull request "wallet: refactor: various master key encryption cleanups":
(https://github.com/bitcoin/bitcoin/pull/31398#discussion_r1996031577)
```suggestion
// 25,000 rounds is just under 0.1 seconds on a 1.86 GHz Pentium M
```

💬 yancyribbens commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2725437236)
rust-bitcoin maintains a file of [constants](https://github.com/tcharding/rust-bitcoin/blob/0ca9fcfd0ea81a7bb0d781bdc07a136cea9d0796/bitcoin/src/policy.rs) which are meant to mirror values in core. We've discussed trying to find an automated solution to keep these consts synchronized since right now, these are manually maintained. This is a point of annoyance since these values are constantly becoming stale (pun intended). Would it be possible to use these C headers to automatically build a r
...

💬 yancyribbens commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2725441239)
Furthermore, besides keeping values stale, an automated solution which would generate all available consts would be ideal.