📝 maflcko opened a pull request: "28.1rc3 backports"
(https://github.com/bitcoin/bitcoin/pull/31594)
Backports:

- #31502
- #31563

💬 maflcko commented on pull request "[28.x] Finalize 28.1":
(https://github.com/bitcoin/bitcoin/pull/31582#issuecomment-2567775660)
> There are still things tagged for backport for 28.x that haven't been backported.

Fixed in https://github.com/bitcoin/bitcoin/pull/31594

💬 maflcko commented on pull request "depends: Fix `CXXFLAGS` on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31502#issuecomment-2567776487)
28.x backport in https://github.com/bitcoin/bitcoin/pull/31594

💬 maflcko commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900881285)
No strong opinion from my side. I'll leave as-is for now, but other reviewers are encouraged to chime in with up/down votes.

🤔 tdb3 reviewed a pull request: "qa: Use `sys.executable` when invoking other Python scripts"
(https://github.com/bitcoin/bitcoin/pull/31541#pullrequestreview-2527577705)
Concept ACK

Ran both tests with `test_runner` on NetBSD 10.1 and Ubuntu 24.04. Both passed. Encountered a failure with `tool_wallet` on NetBSD that I want to check out before full ACK.

💬 maflcko commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#discussion_r1900884815)
not sure about hardcoding. It would be better to have a symbol for this. This would also allow to remove `mktemp` and just use the build dir.

💬 l0rinc commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900895399)
Personally, I find the `ByteType auto` construct slightly more confusing than the more verbose template.

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r1900884712)
The chainman reset may trigger validation interface notifications, so I think it makes sense to sync with the queue afterwards? Unlike the shutdown procedure, we keep the validation interface around beyond the restarts there.

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r1900883476)
I could not come up with a satisfactory alternative yet, suggestions welcome!

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r1900883589)
Yeah, that makes more sense.

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r1900905175)
This seems fine as is? I'd prefer not to document these deeper behaviours, that the caller can expect to "just work".

💬 stickies-v commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r1900927965)
Sorry, I meant to to remove the flushing reference since that's now automatically handled by clearing chainman, so I agree with your comment.

> Simulate a restart of the node by clearing the existing ChainstateManager, and unloading the block index.

💬 ismaelsadeeq commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#discussion_r1900961662)
Done, thanks

💬 ismaelsadeeq commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#discussion_r1900961919)
good idea, pushed.
Thanks

💬 ismaelsadeeq commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#discussion_r1900964573)
I think this is benching just `blockToJSON` .
But it's a good idea to write a bench for `ReadRawBlockFromDisk` as well.

I will leave that as an idea for a follow-up PR.

💬 theStack commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#discussion_r1900970312)
Yeah, this is currently only a minimum-diff fix, happy to refine further. I assume with "symbol" you mean just another shell script variable, e.g. BUILD_DIR? (Could even go further and allow to specify the build-dir as an optional script parameter that defaults to "build".)

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2567933827)
@Sjors

> I still think we should use -blockmaxweight (default 4000000)

Agreed this reflects the current behaviour of this PR

> and then have a separate -maxcoinbaseweight (default 8000). This keeps the same behavior for miners with default settings.

> Miners who manually configured -blockmaxweight before will, after this update, need to manually set -maxcoinbaseweight=4000 to preserve the same behaviour. But if they don't do this, it's not dangerous.

Why do we want to preserve t
...

👍 hodlinator approved a pull request: "depends: Update capnproto to 1.1.0"
(https://github.com/bitcoin/bitcoin/pull/31552#pullrequestreview-2527760917)
cr-ACK b0b8d96d93ea4971e7941ddca03f4393deaac293

Verified hash:
```
₿ curl https://capnproto.org/capnproto-c++-1.1.0.tar.gz -o summed_file
₿ sha256sum summed_file
07167580e563f5e821e3b2af1c238c16ec7181612650c5901330fa9a0da50939 summed_file
```
Cloned repo from GH and ran:
```
₿ git diff v1.0.2 v1.1.0
+
₿ git range-diff v2 v1.0.2 v1.1.0
```
Fairly minor changes. Adding some extra metadata for LSPs, disabling Android CI, some platform specific bugfixes and features, zlib dependency
...

💬 Prabhat1308 commented on issue "Fuzz: Runtime errors when running fuzz tests on MacOs":
(https://github.com/bitcoin/bitcoin/issues/31591#issuecomment-2567952080)
> Does it work with the `libfuzzer-nosan` preset?

works with the following warnings and without any errors.
```
WARNING: Failed to find function "__sanitizer_acquire_crash_state". Reason dlsym(RTLD_DEFAULT, __sanitizer_acquire_crash_state): symbol not found.
WARNING: Failed to find function "__sanitizer_print_stack_trace". Reason dlsym(RTLD_DEFAULT, __sanitizer_print_stack_trace): symbol not found.
WARNING: Failed to find function "__sanitizer_set_death_callback". Reason dlsym(RTLD_DEFAUL
...

👍 ryanofsky approved a pull request: "test: have miner_tests use Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31581#pullrequestreview-2527647194)
Code review ACK e4e76f27d73646d887ed8b660c67da4b4b785d8e. Pretty straightforward test updates that add some more coverage. I left some comments below but none are very important.