💬 hMsats commented on issue "bitcoind 29.0 much slower than 28.0 on my system: cause found":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2956519223)
@l0rinc Thanks also. Well, I observe that the '.ldb' files in `.bitcoin/blocks/index` and `.bitcoin/chainstate` were increased quite fast but not much is happening in `.bitcoin/indexes/txindex` where only a very low percentage of files have changed and it seems that Fulrum and CLN keep complaining ...

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2136241875)
ah yep, you can mark as resolved

🤔 BrandonOdiwuor reviewed a pull request: "test: Turn util/test_runner into functional test"
(https://github.com/bitcoin/bitcoin/pull/32697#pullrequestreview-2910947669)
Concept ACK

🤔 sipa reviewed a pull request: "Embed default ASMap as binary dump header file"
(https://github.com/bitcoin/bitcoin/pull/28792#pullrequestreview-2911137518)
ACK 61714f390e74f49d0f37d7f96cc0645c8215728a

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2136371355)
In commit "init, net: Implement usage of binary-embedded asmap data"

Nit: use `std::move(asmap)` here to avoid a copy (of the entire loaded asmap blob).

💬 achow101 commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#issuecomment-2956842297)
ACK f16c8c67bf137e64fbeea1242431baaa915a5c53

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2136416174)
Thanks - will open a new PR :)

🚀 achow101 merged a pull request: "tests: Expand HTTP coverage to assert libevent behavior"
(https://github.com/bitcoin/bitcoin/pull/32408)

💬 maflcko commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2136422941)
a "naked" address isn't valid json. This is just the same category of failure that was just discussed above

💬 polespinasa commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2136434815)
It's the same value as in other tests that are working:

See tests in lines 37, 48, 95, etc.
The value is declared in line 36 and it's never changed. It only fails in that test case.

💬 maflcko commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2136449422)
Yes, but the others are not using `node.cli`. This is exactly https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2956338245

`"aa"` is not equal to `aa`.

💬 polespinasa commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2136496826)
Oh you're right, I didn't know about `node.cli` difference.
Thanks for the help!

💬 achow101 commented on pull request "rpc, doc: update `listdescriptors` RCP help":
(https://github.com/bitcoin/bitcoin/pull/32708#issuecomment-2957016478)
ACK b44514b876333a94ae242da8b1e4cee439c2d37e

🤔 achow101 reviewed a pull request: "wallet: Allow read-only database access for info and dump commands"
(https://github.com/bitcoin/bitcoin/pull/32685#pullrequestreview-2911360807)
We require that all commits compile and pass the tests by themselves, but it looks like the commits in this PR do not. Please revise your commits so that they pass all of the tests individually.

💬 achow101 commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#discussion_r2136513672)
In f6f1f4199b4a3c2d7ef7c1bcfa122a9d8c5eeb64 "wallet: Allow read-only database access for info and dump commands"

I don't think we need to log for these, especially since we don't log that they happen in the first place.

💬 achow101 commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#discussion_r2136506889)
In f6f1f4199b4a3c2d7ef7c1bcfa122a9d8c5eeb64 "wallet: Allow read-only database access for info and dump commands"

The `if`s can be combined:

```suggestion
if (!m_read_only && sqlite3_db_readonly(m_db, "main") != 0) {
```

💬 achow101 commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#discussion_r2136527582)
In f6f1f4199b4a3c2d7ef7c1bcfa122a9d8c5eeb64 "wallet: Allow read-only database access for info and dump commands"

These don't actually write anything to the database, they are runtime options. Removing the guard around these does not cause `info` or `dump` to fail.

💬 achow101 commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#discussion_r2136540796)
In 6189a20491f71e3a6f499d2a510b38bb914e00d9 "test: Add tests for readonly database access"

This test and the cursor test below are not testing anything. They use the `MockableDatabase`, which in the current implementation, doesn't even enforce the read-only status of a mock database. To actually verify behavior of a read-only database, you need to be attempting to write something to it and that write should fail. But also, testing that on the `MockableDatabase` directly is not meaningfully us
...

💬 achow101 commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#discussion_r2136510729)
In f6f1f4199b4a3c2d7ef7c1bcfa122a9d8c5eeb64 "wallet: Allow read-only database access for info and dump commands"

If the table doesn't exist and we are in read-only mode, we should throw an exception because nothing else is going to work here. We expect the "main" table to exist for any read operations.

💬 achow101 commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#discussion_r2136546758)
In 6189a20491f71e3a6f499d2a510b38bb914e00d9 "test: Add tests for readonly database access"

I don't think that this test needs to be it's own test case. I think all of the tests added in this file can be combined into a single `walletdb_readonly_database` test