💬 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

💬 hodlinator commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2318451553)
`MINCHAINWORK_HEADERS` matches assume valid block and `HEADER_COMMITMENT_PERIOD` + `REDOWNLOAD_BUFFER_SIZE` are close to numbers predicted for this release in #32579.

💬 hodlinator commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2318446931)
My *~/.bitcoin/* dir was 736G after removing other chains. Seldom online with that node, so will not have many stale blocks, but it's in a similar ballpark. Same for chainstate which was 11G.

Increases in other chains seem reasonable.

💬 hodlinator commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2318449630)
Mainnet matches output from:
```
₿ bitcoin-cli getchaintxstats 4096 00000000000000000000611fd22f2df7c8fbd0688745c3a6c3bb5109cc2a12cb
```
Others seem acceptable.

💬 hodlinator commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2318454867)
Mainnet assume UTXO block confirmed:
```
₿ bitcoin-cli getblock 0000000000000000000108970acb9522ffd516eae17acddcb1bd16469194a821
{
"hash": "0000000000000000000108970acb9522ffd516eae17acddcb1bd16469194a821",
"confirmations": 2908,
"height": 910000,
```
Downloaded torrent as kindly provided by Sjors.

<details><summary>Loaded assume utxo snapshot, saw some odd behavior with validating 2 times and only removing the second chainstate directory after restarting. But snapshot worked.</
...

💬 hodlinator commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2318448632)
Mainnet assume valid block checks out:
```
₿ bitcoin-cli getblock 00000000000000000000611fd22f2df7c8fbd0688745c3a6c3bb5109cc2a12cb
{
"hash": "00000000000000000000611fd22f2df7c8fbd0688745c3a6c3bb5109cc2a12cb",
"confirmations": 181,
"height": 912683,
...
"chainwork": "0000000000000000000000000000000000000000dee8e2a309ad8a9820433c68",
```

Other chains all increased minimum chainwork as expected.

💬 hodlinator commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2318362184)
Unlike the mainnet one, this one will not download for me. :\