💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2131471717)
Ping @TheCharlatan @maflcko :)

💬 i-am-yuvi commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#issuecomment-2948079431)
Concept ACK

Great observation and thanks for adding. I was thinking if we could add this timeout test in [`p2p_initial_headers_sync.py`](https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_initial_headers_sync.py) instead of creating another one.

💬 i-am-yuvi commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#issuecomment-2948151373)
ACK fac00d4ed361e5e8c8989b2bb5a4a22dd54e2c72

Thanks for the cleanup. The minor suggestion regarding linter instructions is noted and can be addressed in a future update.

💬 Nicholasdieringer-creator commented on issue "avoid using invalidateblock to directly test reorg behavior":
(https://github.com/bitcoin/bitcoin/issues/32531#issuecomment-2948204392)
Hello so I'm new at coding and find AI fascinating. I'm trying to help build open source and platforms to contribute to Ai and bring Innovation to health mental health and Discovery and technology. I'd love to contribute. I'm new at this and I'm learning. It's exciting and I definitely have some amazing ideas, a powerful vision and incredible insight in learning all about this and being a part of the ecosystem. As I said I'm new so I would love the opportunity to help anyway I can, and earn the
...

💬 maflcko commented on pull request "ci: Rewrite test-each-commit as py script":
(https://github.com/bitcoin/bitcoin/pull/32680#issuecomment-2948223714)
Hmm, yeah the boilerplate overhead is a bit unfortunate. I switched to python, which still has downsides such as https://github.com/bitcoin/bitcoin/pull/29068#issuecomment-1852746032, but it seems manageable here. Also, updated the pull description.

I think the danger of Bash, that it looks harmless and superficially simple and correct, but in many such "harmless" constructs can silently drop a failing return status is reason enough to prefer a slightly more verbose language. Maybe rust is to
...

💬 Muniru0 commented on issue "doc: references to unshipped documentation":
(https://github.com/bitcoin/bitcoin/issues/32565#issuecomment-2948296702)

> > [@fanquake](https://github.com/fanquake) why not just updating the the docs to point to the online versions. e.g:
> > **Replace:** `see doc/cjdns.md`
> > With: `see https://github.com/bitcoin/bitcoin/blob/master/doc/cjdns.md`
> > This make's sure that:
> >
> > 1. **Accessibility**: Users can easily access the documentation regardless of how they obtained the binaries.
> > 2. **Up-to-Date Information**: Linking to the online repository ensures that users are directed to the most current ver
...

💬 Muniru0 commented on issue "doc: references to unshipped documentation":
(https://github.com/bitcoin/bitcoin/issues/32565#issuecomment-2948298581)
@Zeegaths check I hope you check the comment from @brunoerg before proceeding.

🤔 vasild reviewed a pull request: "Replace libevent with our own HTTP and socket-handling implementation"
(https://github.com/bitcoin/bitcoin/pull/32061#pullrequestreview-2903739719)
Approach ACK 7d301184016a3f59c2e363dff631263cdbe21da0

💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2131479604)
Because the context will always be set, ie will never be `nullptr`, then it is better to use a reference here instead of a pointer: `NodeContext& m_context;`

💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2131632124)
`std::map` would keep the entries sorted by the key in alphabetical order and lookup time is `O(log(number of elements in the map))` whereas `std::unordered_map` does not maintain an order and has a lookup time of `O(1)`. We do not need any order here, right? This probably is irrelevant since we are only going to put a bunch of entries in this map, but anyway, if there is no reason to use `std::map` I guess we should default to `std::unordered_map`.

(from commit e95c6f5b65 `http: Implement HT
...

💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2131642866)
This returns a reference (or a "pointer") to the outside world of an element from the private `m_map` member. That map is not immutable. What happens if the strings it contains get moved around if the map is internally resized when adding new entries? Also there is the `HTTPHeaders::Remove()` method which deletes entries from the map, rendering any returned pointers by `Find()` as dangling.

54a36093f7 `http: using string_view to avoid unnecessary copy in Headers` -- if there is no any measura
...

💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2131476990)
Might use `std::chrono::milliseconds` for the type of the argument instead of `int64_t`:

```cpp
HTTPRPCTimer(NodeContext* context, std::function<void()>& func, std::chrono::milliseconds after)
{
context->scheduler->scheduleFromNow(func, after);
```
(in commit 2950597694 `use CScheduler for HTTPRPCTimer`)

💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2131490238)
Can simplify the code a few lines below:

```diff
- std::string_view strUserPass64 = TrimStringView(std::string_view{strAuth}.substr(6));
+ std::string_view strUserPass64 = TrimStringView(strAuth.substr(6));
```
(in commit 54a36093f7 `http: using string_view to avoid unnecessary copy in Headers`)

💬 Sjors commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2131708208)
But `LockCoin` isn't loading the from the database, which is what makes this call site confusing.

⚠️ Sjors opened an issue: "depends: OpenBSD needs gcc (instruction) for libevent"
(https://github.com/bitcoin/bitcoin/issues/32691)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

The current depends instructions for OpenBSD do not mention `gcc`, which is required to build libevent.

A simple `pkg_add gcc` doesn't cut it either, because that only install `egcc`: https://unix.stackexchange.com/questions/554752/no-gcc-executable-after-pkg-add-gcc-on-openbsd

### Expected behaviour

Explain what the user needs to do to build depends (or change depends).

### Steps to r
...

🤔 hebasto reviewed a pull request: "depends: fix multiprocess build on OpenBSD (apply capnp patch, correct SHA256SUM command)"
(https://github.com/bitcoin/bitcoin/pull/32690#pullrequestreview-2904142213)
Concept ACK 8713e8060d504f561fed705b4aa5af7b96c36e75.

https://github.com/hebasto/bitcoin-core-nightly/actions/runs/15485455235/job/43599018182 looks good.

💬 hebasto commented on issue "depends: OpenBSD needs gcc (instruction) for libevent":
(https://github.com/bitcoin/bitcoin/issues/32691#issuecomment-2948467284)
I'm not sure. The following seems [working](https://github.com/hebasto/bitcoin-core-nightly/blob/2973f9d010f6581a7663d5cea4246ee7f6dc4070/.github/workflows/openbsd.yml#L104):
```yml
prepare: pkg_add bash curl gmake gtar-- cmake git ccache python py3-zmq
```

💬 rkrux commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2131744550)
The way I understood it is that `LoadLockedCoin` is loading the data into memory and the value (at least parts of it) passed to it in the first argument comes from the database.

💬 rkrux commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2131751750)
Unrelated nit but mentioning because I noticed it now, the `value` is not used in this load func, only `key` is used.

```diff
- [] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) {
+ [] (CWallet* pwallet, DataStream& key, DataStream&, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) {
```

🤔 hebasto reviewed a pull request: "depends: fix multiprocess build on OpenBSD (apply capnp patch, correct SHA256SUM command)"
(https://github.com/bitcoin/bitcoin/pull/32690#pullrequestreview-2904181437)
Why 8713e8060d504f561fed705b4aa5af7b96c36e75 is necessary for libmultiprocess, but not for other packages?