👋 fjahr's pull request is ready for review: "RFC: refactor: Split multithreaded case out of CheckInputScripts"
(https://github.com/bitcoin/bitcoin/pull/32575)

💬 fjahr commented on pull request "RFC: refactor: Split multithreaded case out of CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/32575#issuecomment-2895912036)
Cleaned up the comments so this should be ready for more detailed feedback, but expect a rebase once https://github.com/bitcoin/bitcoin/pull/32467 is merged.

💬 fjahr commented on pull request "RFC: refactor: Split multithreaded case out of CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/32575#discussion_r2098936599)
I have been contemplating kicking this out (here and in `CheckInputScripts`) or even turning this into an assert because the callers already handle this everywhere. On the other hand it conceptually makes sense to return early if we know there is nothing to check because we don't expect any inputs. So I am still undecided on this one.

💬 TheCharlatan commented on pull request "RFC: refactor: Split multithreaded case out of CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/32575#issuecomment-2895950866)
Concept ACK

💬 davidgumberg commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2895951950)
Review ACK https://github.com/bitcoin/bitcoin/commit/e8661aac752eb08fee318eb8f56e599578d78f9f.

Quickly tested with GUI built on master and GUI built on this branch. Confirmed the watch-only behavior removed here has nothing to do with no-private-keys descriptor wallets, this is dead code AFAICT.

💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-2896320121)
re: https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-2895479357

> I found that `bitcoin-node` will (still) segfault if i try to stop it while the client is running.

Thanks for testing this! I created a new issue in https://github.com/bitcoin-core/libmultiprocess/issues/176 to help debug. So far I couldn't reproduce the problem running a very basic client there, but I can try to extend it to do more of the operations your client is doing and see if I can make the problem happen. I
...

🤔 maflcko reviewed a pull request: "Fees: add Fee rate Forecaster Manager"
(https://github.com/bitcoin/bitcoin/pull/31664#pullrequestreview-2856553062)
(from the llm linter)

💬 maflcko commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2099527580)
unable a fee rate -> unable to find a fee rate [Incorrect grammar; "unable to find" or "unable to provide" would be clearer.]

💬 maflcko commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2099526827)
could remove the redundant comments, given that at least one of them is wrong?

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2896845391)
concept ack, but yeah, needs rebase

💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-2896961033)
utACK b23cefb4f3abe33c7bc933b60f1c0d15137c627c

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2099616827)
Sounds fine, forgot to push this?

💬 maflcko commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2896998818)
> I wonder if there is a platform without `posix_fallocate` (outside of macos and windows). If not, the fallback can probably be fully removed, along with the detection and it can just be assumed to be always present?

I am asking, because the fallback was just the normal code in the beginning (bba89aa82a80f0373dcb7288d96d5b0fcb453d73) and was not removed in 288fdc092aff9d7e0cea159196b2e96044a786c7.

However, even bioniC has `posix_fallocate` since API level 21 (https://android.googlesource
...

💬 maflcko commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897032758)
> And the other slimmer libcs may not have `ftruncate` (https://www.sourceware.org/newlib/libc.html), so they can't be used anyway.

Actually, it is used in https://github.com/bitcoin/bitcoin/pull/31425, so I wonder how that compiles and links. I guess it will start failing once more than a trivial dummy code is linked.

💬 vasild commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2099675521)
Did you try to start the timer before connecting? Like this:

```diff
# Get the underlying socket from HTTP connection so we can send something unusual
conn = http.client.HTTPConnection(urlNode2.hostname, urlNode2.port)
+ start = time.time()
conn.connect()
sock = conn.sock
sock.sendall(http_request.encode("utf-8"))
# Wait for response, but expect a timeout disconnection after 1 second
- start = time.time()
re
...

💬 vasild commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2099677203)
_continued the discussion in https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2081584805_

💬 maflcko commented on pull request "RFC: Riscv bare metal CI job":
(https://github.com/bitcoin/bitcoin/pull/31425#issuecomment-2897104226)
Coming from https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897032758, I noticed that https://www.sourceware.org/newlib/libc.html doesn't list `ftruncate`, so I guess the build will fail once more code is compiled/linked?

💬 fanquake commented on pull request "RFC: Riscv bare metal CI job":
(https://github.com/bitcoin/bitcoin/pull/31425#issuecomment-2897113177)
I think the docs might be incomplete / outdated? See https://github.com/bminor/newlib/blob/b39b510c1ce68757e79410585262ca2cd48da839/newlib/libc/include/sys/unistd.h#L287.

💬 maflcko commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-2897127443)
> > `createrawtransaction` doesn't do other policy checks either, such as OP_RETURN checks, so one could argue this is fine as well.
>
> `fundrawtransaction` does though, and that was my point.

I don't think `fundrawtransaction` checks for OP_RETURN. At least locally, the following passes for me:

```
createrawtransaction '[]' '[{"data":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
...

👍 vasild approved a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2856867196)
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e