💬 purpleKarrot commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#discussion_r2282215658)
This also sets the [policy version](https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html#policy-version). No matter what version of CMake you use, it will behave as if you used 3.22.

🤔 janb84 reviewed a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3128106813)
Concept ACK d3679017c64f336f94339373e563bd4967cd143c

PR moves functionality from python based script to cmake, which imho better aligns with the place from where the script is called (from a build) and it removes an external dependency.

- build ✅
- followed translation instruction that calls the script ✅

💬 janb84 commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#discussion_r2282214659)
Nit, Both define lines are a change in the `#ifdef` style (also from the previous generated file). Personally I do not care which style is followed. Just noting that this is a derivation.

```suggestion
#define UNUSED __attribute__((unused))
```

🤔 hebasto reviewed a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3128119729)
Concept ACK.

> Translate the `share/qt/extract_strings_qt.py` script to CMake. This removes the python dependency from the `translate` target.

I [support](https://github.com/bitcoin/bitcoin/pull/33156#issue-3303948221) this approach:
> An alternative approach would be to convert the `translate` build target into a CMake script.

🤔 hebasto reviewed a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3128124464)
CI linter complains:
```
[11:39:00.061] File "share/qt/translate.cmake" contains a shebang line, but has the file permission 644 instead of the expected executable permission 755. Do "chmod 755 share/qt/translate.cmake" (or remove the shebang line).
```

✅ hebasto closed a pull request: "cmake: Do not require Python to build GUI"
(https://github.com/bitcoin/bitcoin/pull/33156)

💬 hebasto commented on pull request "cmake: Do not require Python to build GUI":
(https://github.com/bitcoin/bitcoin/pull/33156#issuecomment-3196425727)
Closing in favour of https://github.com/bitcoin/bitcoin/pull/33209.

💬 purpleKarrot commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3196426884)
> CI linter complains:
>
> ```
> [11:39:00.061] File "share/qt/translate.cmake" contains a shebang line, but has the file permission 644 instead of the expected executable permission 755. Do "chmod 755 share/qt/translate.cmake" (or remove the shebang line).
> ```

What would you prefer:

a) Set executable bit, or
b) Remove shebang.

💬 macgyver13 commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3196441400)
@Eunovo Nice work on addressing the issues identified in previous tests. All of those are working as expected now.

I am including a patch that adds 2 more failing tests when adding silent-payments address in the transaction output:
self.test_sendrawtransaction()
self.test_mixed_output_types()

[test_sp_receive.patch](https://github.com/user-attachments/files/21834964/test_sp_receive.patch)

💬 hebasto commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3196449095)
> > CI linter complains:
> > ```
> > [11:39:00.061] File "share/qt/translate.cmake" contains a shebang line, but has the file permission 644 instead of the expected executable permission 755. Do "chmod 755 share/qt/translate.cmake" (or remove the shebang line).
> > ```
>
> What would you prefer:
>
> a) Set executable bit, or
> b) Remove shebang.

(b), for consistency with other CMake scripts in the repository,

📝 brunoerg opened a pull request: "fuzz: enhance wallet_fees by mocking mempool stuff"
(https://github.com/bitcoin/bitcoin/pull/33210)
Some functions in `wallet/fees.cpp` (fuzzed by the wallet_fees target) depends on some mempool stuff - e.g. relay current min fee, smart fee and max blocks estimation, relay dust fee and other ones. For better fuzzing of it, it would be great to have these values/interactions. That said, this PR enhances the `wallet_fees` target by:

- Setting mempool options - `min_relay_feerate`, `dust_relay_feerate` and `incremental_relay_feerate` - when creating the `CTxMemPool`.
- Creates a `ConsumeMemp
...

🤔 janb84 reviewed a pull request: "Release: Prepare "Translation string freeze" step"
(https://github.com/bitcoin/bitcoin/pull/33193#pullrequestreview-3128184088)
ACK d32fcd3df22671b633cae605ed06c6d0a353e2bf

PR updates translation strings, for 30.0 freeze.

Ran the commands against this PR; got 0 changes (reproduced the results) ✅
Ran the commands against Master; got the changes as provided in this PR ✅

🤔 hebasto reviewed a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3128236494)
It seems no need to track `in_msgstr`:
```diff
--- a/share/qt/translate.cmake
+++ b/share/qt/translate.cmake
@@ -34,23 +34,20 @@ function(extract_strings output)
set(messages)
set(msgid)
set(in_msgid OFF)
- set(in_msgstr OFF)

file(STRINGS "bitcoinstrings.text" text ENCODING "UTF-8")
foreach(line IN LISTS text)
if(line MATCHES "^msgid (.*)$")
- if(in_msgstr AND NOT msgid STREQUAL "\"\"")
+ set(in_msgid ON)
+ set(msgid "${CMAKE_MATCH_1}")
+
...

💬 alexanderwiederin commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2282357987)
I think we need to return a `nullptr` here instead of returning a `btck_BlockTreeEntry` wrapper with a null `m_block_index`. The current behavior causes segfaults when callers try to access the returned entry. What do you think?

💬 fanquake commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3196844620)
Guix Build:
```bash
410caa60435c79c8c862c95acac2e9d29d08ecce8540f8b999dff2c1bac595d6 guix-build-4fd05dadbf23/output/aarch64-linux-gnu/SHA256SUMS.part
177315e236e00d9e8d3d36e2b6fff0ce2d8cfcdd554f35f97547721617782822 guix-build-4fd05dadbf23/output/aarch64-linux-gnu/bitcoin-4fd05dadbf23-aarch64-linux-gnu-debug.tar.gz
a140b60a3e77d6bfcf2ca8a0808f5ca10ef1318bc5480b8401a82727076ac94d guix-build-4fd05dadbf23/output/aarch64-linux-gnu/bitcoin-4fd05dadbf23-aarch64-linux-gnu.tar.gz
ed981094dbe36e3e
...

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2282385073)
> I ran some benchmarks comparing `.5s` timeout to no timeout and this commit was always sligthly faster.

The reason why `.5` is picked here is to preserve the observable behavior when `print('.', end='', flush=True)` is hit: During the sleep/wait you'll only want a dot every half second, not in a busy-loop without delay. Using a busy-loop also sounds like a plausible explanation for your observed slow down, when decreasing it to `0`.

Other than that, I don't think the exact value here sho
...

💬 sipa commented on pull request "build: Enable ENABLE_IPC option by default":
(https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3196869655)
ACK 732c134bfc1208377f449e5956e01dd8b78d73d9. See also tests added in #33201.

I think #31802 should be seen as a successor to this PR, regardless of which release it goes into.

💬 maflcko commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2282400346)
Is the 256-byte value used at all? If not, seems easier to just hardcode a value (like 0 or 1 or 2), or use a deterministic random comtext?

Also, it may be good to document what the goal of this function is. Maybe with an `Assert(mempool.GetMinFee()==min_fee)` or similar?

💬 Eunovo commented on pull request "Silent Payments: sending":
(https://github.com/bitcoin/bitcoin/pull/28201#discussion_r2282414301)
`GetPrivKeyForSilentPayment` returns `{}` which is a default initialized object if the coin's keys are not available. `std::holds_alternative<CKey>` will pass for the retuen object and add the default intialized CKey to the `plain_keys` array.

💬 Eunovo commented on pull request "Silent Payments: sending":
(https://github.com/bitcoin/bitcoin/pull/28201#discussion_r2282367304)
https://github.com/bitcoin/bitcoin/pull/28201/commits/7c1093b08c0363a6d341967659f7b49e0f6d0266: We get a segfault later if the keys are not valid. We should check that keys are valid before adding them to `taproot_keys` or `plain_keys`