🤔 w0xlt reviewed a pull request: "scripted-diff: Remove unused leading newline in RPC docs"
(https://github.com/bitcoin/bitcoin/pull/32514#pullrequestreview-2847712472)
ACK https://github.com/bitcoin/bitcoin/pull/32514/commits/fa1f10a49e7c4f6377fbc7ae2f1520b38c86e5fa

💬 achow101 commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2093740236)
In 86e1111239cdb39dd32cfb5178653c608fa30515 "test: verify node skips loading legacy wallets during startup"

Is there a reason to use settings.json instead of passing in `-wallet=legacy_up...` as part of `extra_args` during the restart?

💬 TEU7ZoTHLa7sZchXmdqZn5pE75ikooVNfv commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#issuecomment-2887797539)
‏alvandisaeed43@gmail.com‏

💬 murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#issuecomment-2887797583)
reACK https://github.com/bitcoin/bitcoin/pull/27286/commits/29cf059e6e592f7f82b982493a043219cdfa5b40

via git range-diff efac285a0d709f48856c37b15bfa09af94c1d75b..51d4846f648e79e3e098f1f1bea7d5e823ebdea8 b15c386933eb7bc5ed8644dd37f8169d81ea5a22..29cf059e6e592f7f82b982493a043219cdfa5b40

Hooray for `Txid` class!

🤔 w0xlt reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2847729722)
ACK https://github.com/bitcoin/bitcoin/pull/27286/commits/29cf059e6e592f7f82b982493a043219cdfa5b40

💬 furszy commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2093771082)
> Is there a reason to use settings.json instead of passing in `-wallet=legacy_up...` as part of `extra_args` during the restart?

I just wanted to make the test case as realistic as possible (an existing legacy wallet with `load_on_startup=true`). But there shouldn't be any difference.

🤔 fjahr reviewed a pull request: "refactor: Improve assumeutxo state representation"
(https://github.com/bitcoin/bitcoin/pull/30214#pullrequestreview-2846790348)
Leaving some comments in case you want to address some of them with the rebase. I am mostly happy but there are a few things that I need to spend some more time on.

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2093147399)
Are we then commited to the 'historical' naming now with this and I can remove all the remaining ibd/background references in a follow-up? Because that naming inconsistency has always annoyed me and now we have three names with this. I think it's very confusing for newcomers as well.

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2093165330)
nit: Would be really great if this conditional could be broken up to be more readable. Or maybe move comments below inline. But feel free to leave this for a follow-up.

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2093781793)
I'm currently not sure why we need both `ChainValidity` and `ChainstateRole`, couldn't we have one thing that serves both purposes?

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2093178774)
(not directly related to this line but I have to put it somewhere) Looks like commit 83d19fd09818e11a03b9ebdbfbf76196f3808ae2 just contains some formatting fixes and has the same commit message as the commit before 55b7bfba6d12a95a6cccb3652c75dc9cee2f8751 , so they were probably meant to be squashed.

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2093782775)
This can be marked as resolved. Not sure why this comment landed in this spot here, pretty weird, but it was going in the same direction as @mzumsande 's comment here: https://github.com/bitcoin/bitcoin/pull/30214/files#r2040321756

💬 portlandhodl commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-2887935158)
> Any particular reason for marking this draft?

Yes, I was going to work on the tests for this PR to enable testing with multiple networks, currently the tests had to be refactored to work with mainnet based addresses because of the network awareness.

💬 maflcko commented on pull request "restrict std::cerr to errors; use std::cout for warnings and info":
(https://github.com/bitcoin/bitcoin/pull/32538#issuecomment-2888128803)
> > When something literally starts with Warning: ..., I fail to see how it can be misinterpreted.
>
> It is actually being misinterpreted by our own test framework. The framework does not look at the stderr content, it only checks whether there is something inside stderr or not during shutdown, failing when finds something there.

That's intentional. We want the tests to fail when there is any warning or error (from Bitcoin Core or any other tool/sanitizer/lib) that is not accounted for, s
...

📝 romanz opened a pull request: "rest: fetch spent transaction outputs by blockhash"
(https://github.com/bitcoin/bitcoin/pull/32540)
Today, it is possible to fetch a block's spent prevouts in order to build an external index by using the `/rest/block/HASH.json` endpoint. However, its performance is low due to JSON serialization overhead.

We can significantly optimize it by adding a new REST endpoint, using a binary response format:

```
$ ab -k -c 1 -n 100 http://localhost:8332/rest/block/00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054.json
Document Length: 1634536 bytes
Requests per second:
...

📝 romanz opened a pull request: "index: store per-block transaction locations for efficient lookups"
(https://github.com/bitcoin/bitcoin/pull/32541)
Currently, electrs and other indexers are used to maintain a map between an address/scripthash to the list of the relevant transactions.

However, in order to fetch those transactions from bitcoind, electrs relies on reading the whole block and post-filtering for a specific transaction [1]. Other indexers use a `txindex` to fetch a transaction using its txid [2,3,4].

The above approach has significant storage and CPU overhead, since the `txid` is a pseudo-random 32-byte value.

This PR is
...

💬 maflcko commented on pull request "Remove legacy `Parse(U)Int*`":
(https://github.com/bitcoin/bitcoin/pull/32520#discussion_r2094035676)
thx, added a new commit on top with you as co-author

💬 maflcko commented on pull request "Remove legacy `Parse(U)Int*`":
(https://github.com/bitcoin/bitcoin/pull/32520#discussion_r2094038039)
consensus rules aren't architecture or compiler-dependent, so I used uint32_t instead

💬 maflcko commented on pull request "Remove legacy `Parse(U)Int*`":
(https://github.com/bitcoin/bitcoin/pull/32520#discussion_r2094039002)
(same)

💬 maflcko commented on pull request "Remove legacy `Parse(U)Int*`":
(https://github.com/bitcoin/bitcoin/pull/32520#discussion_r2094039205)
thx, added some fuzz