💬 hebasto commented on issue "build: ccache doesn't hit across build dirs":
(https://github.com/bitcoin/bitcoin/issues/31994#issuecomment-2708848882)
Amending our docs, as demonstrated in https://github.com/davidgumberg/bitcoin/commit/1c6ae1043d59d01cbf52f353e50a63b0afa883c4, could be an alternative to https://github.com/bitcoin/bitcoin/pull/30861.

💬 maflcko commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-2708856306)
Thanks for the survey and summary, as well as the additional input on this topic!

I don't have a strong opinion on this, and anything is probably fine, as long as it "works".

Just some random thoughts:

* The GHA runners that are included for public repositories are likely too weak to run all of the CI tasks in the maximum timeout (apart from the cache being too small as well, as mentioned above). So the goal that more of the CI tasks can run on forks trivially may not be achievable.
* The 32-
...

💬 dongwook-chan commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1986324475)
@rkrux
Utilized existing `raw_multisig_transaction_legacy_tests` but with version 3 raw transaction. This seems to cover all the domain you specified. Could you pls take a look?
https://github.com/bitcoin/bitcoin/blob/1121cb7a91e0dfa4f247273bf89e2e1b47c82628/test/functional/rpc_rawtransaction.py#L96

💬 dongwook-chan commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1986325419)
Added variable to denote min, max value of the version.
https://github.com/bitcoin/bitcoin/blob/1121cb7a91e0dfa4f247273bf89e2e1b47c82628/src/rpc/rawtransaction_util.cpp#L159-L160

💬 dongwook-chan commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1986325898)
@glozow
Are you implying that we should use the variable instead of constant values in the test code?

💬 dongwook-chan commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1986326221)
@luke-jr @adamandrews1
So should I remove the line? If I should, could you advise which code should be updated to avoid failure of the aforementioned validation?

💬 maflcko commented on pull request "qa: Enable feature_init.py on Windows":
(https://github.com/bitcoin/bitcoin/pull/32021#issuecomment-2708870591)
lgtm ACK 59c4930394cafc939eb396224b3d60d01ba0ce37

💬 maflcko commented on pull request "Docs: fix typos in documentation files":
(https://github.com/bitcoin/bitcoin/pull/32011#issuecomment-2708875507)
lgtm ACK 5601bab4f8b01fdef7a54c9e397d513217ab1c1f

💬 maflcko commented on pull request "random: Initialize variables in hardware RNG functions":
(https://github.com/bitcoin/bitcoin/pull/31912#issuecomment-2708878173)
The pull request title and commit message looks off. Also there seems to be an ongoing discussion in https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2687917758

📝 glozow converted_to_draft a pull request: "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs"
(https://github.com/bitcoin/bitcoin/pull/31829)
This PR is part of the orphan resolution project, see #27463.

We want to limit the CPU work and memory used by `TxOrphanage` to avoid denial of service attacks. On master, this is achieved by limiting the number of transactions in this data structure to 100, and the weight of each transaction to 400KWu (the largest standard tx) [0]. We always allow new orphans, but if the addition causes us to exceed 100, we evict one randomly. This is dead simple, but has problems:
- It makes the orphanage
...

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

💬 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?