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

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