💬 maflcko commented on issue "Incorrect balance when dealing with coinbase UTXOs that will be mature on `h + 1`":
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2737798747)
I presume this was last touched in commit bf3a20a6e8cafdf723ef101af078df303ea06fec, which explains that there may be a race condition where the transaction is rejected, because the 100th block has not been relayed yet to the peer when the transaction is sent.

💬 maflcko commented on issue "Failure to run Fuzz tests when running with corpus":
(https://github.com/bitcoin/bitcoin/issues/32089#issuecomment-2737863589)
> libc++abi: terminating due to uncaught exception of type std::__1::ios_base::failure: DataStream::read(): end of data: unspecified iostream_category error

This looks like an upstream packaging bug or asan bug on your platform, given that it passes fine when asan is disabled on your platform.

💬 hebasto commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2737914378)
FWIW, at the end of configuration, Qt 6 prints:
```
If reconfiguration fails for some reason, try removing 'CMakeCache.txt' from the build directory
Alternatively, you can add the --fresh flag to your CMake flags.
```

🤔 maflcko reviewed a pull request: "contrib: Make deterministic-coverage error messages more readable"
(https://github.com/bitcoin/bitcoin/pull/32074#pullrequestreview-2700009626)
rebased with a move-only change, and replied to all feedback

💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004192240)
I found `Err(...)?` nicer as an early-return statement, but no strong opinion

💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004194037)
In Rust shadowing is a feature, but happy to change, no strong opinion.

💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004190137)
thx, done

💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004196829)
Nice. I think this is sufficient as an approach for a dev-only tool. Happy to push it here, or review your follow-up. Just let me know.

💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#discussion_r2004186423)
Yeah, the string ends up in `Err` eventually, so it should be type-safe enough for nothing to go wrong. At least I can't see any risk here.

👋 maflcko's pull request is ready for review: "contrib: Make deterministic-coverage error messages more readable"
(https://github.com/bitcoin/bitcoin/pull/32074)

👍 hodlinator approved a pull request: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2700044703)
re-ACK 25b56fd9b469f8e5d36f0132c3b79a5214e3372a

Changes since [previous ACK](https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2661348106):
- Added commit switching to Bash by default which lets us avoid annoying "&& `" in later commit which was required by PowerShell. Confirmed CI still seems to be executing unit/functional/fuzz tests.
- Added comment explaining why we can't use ctest.

💬 maflcko commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2737988575)
> If reconfiguration fails for some reason, try removing 'CMakeCache.txt' from the build directory
> Alternatively, you can add the --fresh flag to your CMake flags.

I don't think I've seen issues where re-configuration fails. So far the linked issues here were about linker errors.

🤔 LarryRuane reviewed a pull request: "improve MallocUsage() accuracy"
(https://github.com/bitcoin/bitcoin/pull/28531#pullrequestreview-2699829126)
@l0rinc - thank you for the excellent review and great suggestions. I especially like making `MallocUsage()` a `constexpr` function.

I'll force-push a commit taking all of them except moving the first (test-only) commit (I left a comment explaining my reasoning).

💬 LarryRuane commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2004163332)
Good point, I am going to back this change out until we see evidence for needing it. I think I "deduced" this based on macOS CI failing while the others were passing. But it's been too long.

Just now, after searching for a while with no luck, I asked X's Grok (I know, we shouldn't trust it too much), and it said LLVM's implementation of `std::unordered_map` that the node includes a next pointer but no previous pointer. You can indeed see that in the source code: https://github.com/llvm/llvm-
...

💬 LarryRuane commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2004100921)
I wrote this because the following line (`static_assert((step & (step-1)) == 0)`) doesn't actually assert that `step` has a single bit set (which is what I wanted to check); it also succeeds if `step` is zero. But using `std::has_single_bit()` is better, so we can remove `static_assert(step > 0)`, good catch.

💬 LarryRuane commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2004197157)
I think it's okay that it's initially dead code because it makes the test less fragile in general. In theory, a different project could cherry-pick just this test-only commit, and later make some other changes that would have caused this test to break (when it's really a false-positive failure), but because of taking this commit, that doesn't happen (the test continues to pass). Or, a similar way to think of it is, the test could have been written this way to begin with (and that would not have
...

💬 jamesob commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2738021374)
> I suggest a regtest-only deployment is defined so functional testing can be re-instated. Currently there is zero consensus coverage at the functional level from what I can tell: [4c57901](https://github.com/bitcoin/bitcoin/commit/4c5790105e7fb3a2cd207403af769fbbccd7296d)

Done in https://github.com/bitcoin/bitcoin/pull/31989/commits/5bb734b9d3628395c031fdf0eb0fbb249fe578e3.

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin-wallet -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/19460#issuecomment-2738025949)
Updated 240bc4798a3c4e991e153d6660509c78323fe937 -> ce32dc3958c9e3610bd7113551a8a43396d0019d ([`pr/ipc-connect.42`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-connect.42) -> [`pr/ipc-connect.43`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-connect.43), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-connect.42..pr/ipc-connect.43)) to fix CI failure in tool_wallet.py https://cirrus-ci.com/task/4513424845045760 where `bitcoin-wallet` throws an exception because the t
...

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2004255350)
I would like to avoid having a false sense of security by putting this before the fix, which usually indicates that the desired behavior is reproduced and is retained after this fix - which isn't the case here exactly. And the dead code part also bothers me for a similar reason.

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2004259676)
I forgot to include this in the full patch, my bad