💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3248203281)
@cryptomeow

> Great! Regarding the Greek translation, I've gone one round and did some fixes based on the review and it certainly is helpful and provides actionable issues.
>
> One false positive group of issues I noticed is the disagreement on translating terms.
>
> For example one example is `Peers`, it seems the LLM insists on suggesting `Ομότιμοι` which is more about comparing people, while `Κόμβοι` is more commonly accepted in our case. Not sure how to address this so these issues
...

💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3248219824)
@l0rinc

> @cryptomeow you can check out this PR having the translations in an XML format and feed the whole file to an LLM. http://aistudio.google.com let's you work with multiple complete translations. You could ask it to extract the key/value pairs you want and feed _that_ to the other LLM of yours choice.

This is an interesting idea.

However, I don’t think it’s reasonable to ask volunteer coordinators to review the entire PR.

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2318206490)
The example doesn't use `futures.wait`, but I guess `futures.wait` does not create copies of the futures, which would map differently as keys?

So I went ahead and implemented something like this.

💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3248226196)
> ... but fwiw I found Transifex's Glossary very interesting to help have long-running async consistency on term translations, but I don't see any option to export them in order to possibly provide it as a context to the LLM.

I’m not aware of such an option either.

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2318209717)
Yes, my understanding is that "sleep" and "wait" are words to describe a similar concept in this context. Leaving as-is for now.

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2318212456)
I prefer named fields with less floating symbols in the scope, so I'll leave as-is for now.

💬 fanquake commented on pull request "rpc: deprecateboolverbose Boolean verbosity is deprecated":
(https://github.com/bitcoin/bitcoin/pull/33288#issuecomment-3248315087)
There's no need to open a new PR here, you can make these changes in #33214.

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#issuecomment-3248324405)
Force pushed with a tiny refactor turning a list into a dict. Should be easy to re-review.

✅ maflcko closed a pull request: "rpc: deprecateboolverbose Boolean verbosity is deprecated"
(https://github.com/bitcoin/bitcoin/pull/33288)

🤔 kannapoix reviewed a pull request: "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet"
(https://github.com/bitcoin/bitcoin/pull/33268#pullrequestreview-3178686997)
ACK 69eeb11bc585e92a3b56811c3251f13dba05aa5f

I have run the all functional tests and reviewed the code.
I left a nit comment below.

💬 kannapoix commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2317552470)
This error message might be confusing.
A message like “Private keys are disabled for this wallet” would be more intuitive, similar to what sendtoaddress returns.
https://github.com/bitcoin/bitcoin/blob/46369583f3a99d2b403349790f8b26dbc28de132/src/wallet/rpc/spend.cpp#L177-L179

I’m not suggesting to remove the current error.
It should still be raised when the wallet has private keys enabled but fails to determine the transaction size.

💬 maflcko commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3248398527)
> WalletDescriptor

This is fixed (removed) in https://github.com/bitcoin/bitcoin/pull/33082.

🤔 hodlinator reviewed a pull request: "index: store per-block transaction locations for efficient lookups"
(https://github.com/bitcoin/bitcoin/pull/32541#pullrequestreview-3179176604)
Reviewed 94389c28e1068ffcc116614d16ac3047eb3068e3

Measured a ~11% speed increase by enabling `LocationsIndex`, running on NVMe. Are you seeing greater improvements on other setups?

#### `-locationsindex=0`

```
₿ hey -n 100000 http://localhost:8332/rest/txfromblock/0000000000000000000083a0cff38278aae196d6d923a7e8ee7e5a0e371226fe-42.bin

Summary:
Total: 2.3504 secs
Slowest: 0.0193 secs
Fastest: 0.0001 secs
Average: 0.0012 secs
Requests/sec: 42545.2261
```

#### `-loca
...

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2318157602)
Could cover both paths:
```diff
diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py
index 541dde3174..333b8c3f79 100755
--- a/test/functional/interface_rest.py
+++ b/test/functional/interface_rest.py
@@ -28,7 +28,7 @@ from test_framework.wallet import (
MiniWallet,
getnewdestination,
)
-from typing import Optional
+from typing import Optional, NamedTuple


INVALID_PARAM = "abc"
@@ -53,7 +53,7 @@ def filter_output_indices_by_value(vouts,
...

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2318047389)
> 94389c28e1068ffcc116614d16ac3047eb3068e3 seems to be passing (after a rebase over `master`) 🤔

Tried rebasing my suggestions-branch, and 528f79f010d1617163e4154753b85e04539af993 / #32835 (which I also reviewed :grimacing:) fixed the issue by waiting for indexes to sync up before restarting the node.

> would it be OK to squash 94389c28e1068ffcc116614d16ac3047eb3068e3 into bdd90ab25f5f3b05c03a8dfb7177399264bed377?

Fine by me! Could add me as `Co-authored-by: Hodlinator <172445034+hodlin
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3248422237)
`2bd4714fa9...8fbff5233f`: extend the functional test to send two transactions that have the same txid but different wtxid, made possible by https://github.com/bitcoin/bitcoin/pull/32385. Thank you, @stratospher!

The functional test has grown quite extensive. It is a further load on reviewers. That is in the last two commits. If you don't feel like reviewing it, you can omit those last two commits and review up to bb814cde2a `rpc: use private broadcast from sendrawtransaction RPC if -privateb
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3248461985)
`8fbff5233f...4327327dd0`: fix python linter `f-string without any placeholders`

💬 vasild commented on pull request "test: refactor out same-txid-diff-wtxid tx to reuse in other tests":
(https://github.com/bitcoin/bitcoin/pull/32385#issuecomment-3248470234)
This is [used](https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3248422237) now in https://github.com/bitcoin/bitcoin/pull/29415, thank you!

💬 fanquake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2318387281)
Wanted to note that the maintainer of this repo has said they aren't actively working on it: https://github.com/capnproto/pycapnp/issues/372#issuecomment-3247204007.

👍 hodlinator approved a pull request: "kernel: chainparams & headersync updates for 30.0"
(https://github.com/bitcoin/bitcoin/pull/33274#pullrequestreview-3179774960)
ACK 755152ac819a23acf2f9e70316134d74a04d589b