💬 ItIsOHM commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1628906716)
Hey there, I fixed the descriptions as suggested, and ran the tests and was able to build Bitcoin Core locally successfully.

💬 MarcoFalke commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1258239050)
Stray file?

💬 furszy commented on pull request "test: make assumeUTXO test capture the expected fatal error":
(https://github.com/bitcoin/bitcoin/pull/28050#issuecomment-1628936305)
> Also it may be good to name the commit that introduced the bug. Otherwise it will be harder for reviewers to see if there was a bug in that commit or if the change was intentional. Also, reviewers will be missing context.

Ok. It was introduced within the test 87a1108c8.
Still, this isn't as serious as you imagine. It is just an extra print. 100% sure that wasn't intended.

The test is exercising a specific fatal error (checking that is being triggered), and doing so it makes the node cal
...

💬 MarcoFalke commented on pull request "test: make assumeUTXO test capture the expected fatal error":
(https://github.com/bitcoin/bitcoin/pull/28050#issuecomment-1628940734)
Are you sure, because locally I don't see it in 25.0:

```
$ ./bitcoin-25.0/bin/test_bitcoin -t validation_chainstatemanager_tests/chainstatemanager_snapshot_completion_hash_mismatch
Running 1 test case...

*** No errors detected

💬 ItIsOHM commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1258250019)
Oh I'm sorry I must have forgotten to discard everything other than `mining.cpp`. Should I revert this commit, or something else?

🤔 MarcoFalke reviewed a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1521991201)
Looks like the tests fail

💬 MarcoFalke commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1258201606)
225e213110602b4fd1d345167f5f92d26557f6c1: Why ignore the return value? This makes the tests fail

💬 MarcoFalke commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1258195719)
18bf3abc2babbe066e5d4b5798209826316c8295: Why is this atomic, when only a single thread has access? Or is the other code multi thread safe?

💬 furszy commented on pull request "test: make assumeUTXO test capture the expected fatal error":
(https://github.com/bitcoin/bitcoin/pull/28050#issuecomment-1628956370)
> Are you sure, because locally I don't see it in 25.0:
>
> ```
> $ ./bitcoin-25.0/bin/test_bitcoin -t validation_chainstatemanager_tests/chainstatemanager_snapshot_completion_hash_mismatch
> Running 1 test case...
>
> *** No errors detected
> ```

Ok good, found the commit. 6eb33bd0c21b3e075fbab596351cacafdc947472 introduced it. It removed the `mock_shutdown` function from the test.

💬 furszy commented on pull request "test: make assumeUTXO test capture the expected fatal error":
(https://github.com/bitcoin/bitcoin/pull/28050#issuecomment-1628960452)
cc @TheCharlatan then. Thanks @MarcoFalke.

💬 MarcoFalke commented on pull request "test: make assumeUTXO test capture the expected fatal error":
(https://github.com/bitcoin/bitcoin/pull/28050#issuecomment-1628961804)
lgtm ACK 3e8bf2e10c26ef9c2f58307b523e4b674ac97a2c

💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1629006969)
@ryanofsky separate tests PR is ready for review: https://github.com/bitcoin/bitcoin/pull/27850

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1258287264)
At the end of the PR, two threads end up accessing the variable. The `BaseIndex::Init` call stays in the main thread, while the `BaseIndex ::StartBackgroundSync` call is moved to the 'loadblk' thread (now called 'initload')

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1258307033)
upps, fixed the intermediate commit. Thanks.

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1258307306)
done

🤔 furszy reviewed a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1522158509)
Updated per feedback, thanks Marco and TheCharlatan.

Changes:
* Fixed bug in one of the intermediate commits (per [comment](https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1258201606)).
* Inlined an if/else in one of the intermediate commits (per [comment](https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1257458241)).

💬 MarcoFalke commented on pull request "test: Check expected_stderr after stop":
(https://github.com/bitcoin/bitcoin/pull/28028#discussion_r1258317110)
thx, done

💬 brunoerg commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#issuecomment-1629071257)
Concept ACK

👍 ryanofsky approved a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1522224733)
Code review ACK ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550. Just rebase and suggested changes since last review (Start return check, and code simplification)

👍 TheCharlatan approved a pull request: "test: make assumeUTXO test capture the expected fatal error"
(https://github.com/bitcoin/bitcoin/pull/28050#pullrequestreview-1522225346)
ACK 3e8bf2e10c26ef9c2f58307b523e4b674ac97a2c

Thank you for fixing this.

💬 pinheadmz commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1629092956)
@jonatack thank you sir for the great review and suggested patches, all nits addressed