💬 fanquake commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-2708921843)
Run into this again recently. Did it ever get reported upstream?

💬 hebasto commented on pull request "cmake: Check for `makensis` tool before using it":
(https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2708949698)
> Now that `nsis` is required, the docs about installing `nsis` in `build-windows.md` should be updated.

Thanks for pointing this out!

Since the `deploy` target is optional, the NSIS tool shouldn't be a strict requirement for a successful configuration.

I've reworked this PR so that the error message "Error: NSIS not found" is only printed when attempting to build the `deploy` target.

📝 hebasto converted_to_draft a pull request: "cmake: Check for `makensis` tool before using it"
(https://github.com/bitcoin/bitcoin/pull/32019)
Fixes https://github.com/bitcoin/bitcoin/issues/32018.

👋 hebasto's pull request is ready for review: "cmake: Check for `makensis` tool before using it"
(https://github.com/bitcoin/bitcoin/pull/32019)

💬 l0rinc commented on pull request "RFC: Add `operator""_uint256` compile-time user-defined literal":
(https://github.com/bitcoin/bitcoin/pull/31991#discussion_r1986367249)
Removed the `consteval_ctor` change since godbolt and the MSVC docs have mixed signals about it

🚀 hebasto merged a pull request: "Docs: fix typos in documentation files"
(https://github.com/bitcoin/bitcoin/pull/32011)

💬 fanquake commented on pull request "cmake: Check for `makensis` tool before using it":
(https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2708956458)
> the NSIS tool shouldn't be a strict requirement for a successful configuration.

Then the same should be done for macOS and zip, otherwise it's still inconsistent.

💬 l0rinc commented on pull request "refactor: use original log string when no suspicious chars found":
(https://github.com/bitcoin/bitcoin/pull/31923#issuecomment-2708960491)
@maflcko, I've reorganized the PR as a refactor where the happy path is now easier to reason about (i.e. no change to the incoming string most of the time) - where the speed gain is just a side-effect of the simpler path taken (it's why the benchmarks are now `PriorityLevel::LOW`).

Please let me know if this fits better with the priorities.

💬 Chand-ra commented on pull request "refactor: simplify GetAncestor":
(https://github.com/bitcoin/bitcoin/pull/31778#issuecomment-2708960914)
Concept ACK [5983f1](https://github.com/bitcoin/bitcoin/pull/31778/commits/5983f166c94dd5ab172e38bf12a3330a3ed9004c)
`CBlockIndex::GetAncestor` _is_ easier to comprehend with this change, but I'm not sure if duplicating the two versions in `test/blockchain_tests.cpp` is the best way to test the change. A single test to verify `GetAncestor` agnostic of its implementation might be better?

💬 Chand-ra commented on issue "Bug: Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in `AddWalletDescriptor`":
(https://github.com/bitcoin/bitcoin/issues/31728#issuecomment-2708961954)
Hey @asklokesh, are you still working on this or can I chime in to help as well?

💬 furszy commented on issue "Bug: Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in `AddWalletDescriptor`":
(https://github.com/bitcoin/bitcoin/issues/31728#issuecomment-2708965049)
> Hey [@asklokesh](https://github.com/asklokesh), are you still working on this or can I chime in to help as well?

You can tackle it. asklokesh comment is from chatGPT and makes no sense in our code. We should delete it.

💬 hebasto commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2709025950)
> > the NSIS tool shouldn't be a strict requirement for a successful configuration.
>
> Then the same should be done for macOS and zip, otherwise it's still inconsistent.

Sure thing! Added.

The PR description has been updated.

💬 mabu44 commented on pull request "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds":
(https://github.com/bitcoin/bitcoin/pull/31979#issuecomment-2709111641)
Tested with same results as @eval-exec. After reaching the maximum "reconnect_timeout" value I started tor and it re-connected successfully, then I stopped tor again to check that reconnect_timeout restarts from 1 second.
ACK f708498293c27f63507cfa08c74909ba3d1aa675

🤔 l0rinc reviewed a pull request: "refactor: Use std::span over Span"
(https://github.com/bitcoin/bitcoin/pull/31519#pullrequestreview-2669535869)
Found a few more static extent opportunities, but I personally don't mind doing these in separate PRs either.

💬 l0rinc commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1986432102)
we could retain the static extents here as well:
```suggestion
s.write(std::as_bytes(std::span<uint8_t, 1>{&obj, 1}));
```

💬 l0rinc commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1986432892)
`last(Bytes)` seems to create a dynamic extent, we could keep the sizes with something like:
```C++
s.write(std::as_bytes(std::span{&raw, 1}).template last<Bytes>());
```
and similarly later:
```C++
s.read(std::as_writable_bytes(std::span{&raw, 1}).last<Bytes>());
```

which seems to eliminate the need for
```C++
template <typename Stream, BasicByte B> void Unserialize(Stream& s, std::span<B> span) { s.read(std::as_writable_bytes(span)); }
```

💬 l0rinc commented on pull request "optimization: speed up block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2709143828)
Drafting until https://github.com/bitcoin/bitcoin/pull/31519 is merged, as recommended in https://github.com/bitcoin/bitcoin/pull/31868#discussion_r1956480589

📝 l0rinc converted_to_draft a pull request: "optimization: speed up block serialization"
(https://github.com/bitcoin/bitcoin/pull/31868)
This PR contain a few different optimization I found by IBD profiling, and via the newly added block seralization benchmarks.

The commits merge similar (de)serialization methods, and separates them internally with `if constexpr` - similarly to how it has been [done here before](https://github.com/bitcoin/bitcoin/pull/28203). This enabled further `SizeComputer` optimizations as well.

Other than these, since single byte writes are used very often (used for every `(u)int8_t` or `std::byte` o
...

💬 l0rinc commented on pull request "optimization: speed up block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#discussion_r1986458092)
Rebased on top of https://github.com/bitcoin/bitcoin/pull/31519 and experimented with static extents - the speed is not the same as with bare `std::byte` parameters, but close enough and the code is more general.
Drafting until the span PR is merged - suggestions for further investigations are welcome.

📝 RiceChuan opened a pull request: "docs: remove repetitive words"
(https://github.com/bitcoin/bitcoin/pull/32022)
docs: remove repetitive words