💬 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

💬 furszy commented on issue "migratewallet crashes (wallet/scriptpubkeyman.cpp:1915: std::optional<MigrationData> wallet::LegacyScriptPubKeyMan::MigrateToDescriptor(): Assertion `IsMine(desc_spk) != ISMINE_NO' failed.)":
(https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1629096788)
Could you provide the `getaddressinfo mqRzyzrW7DPGT1KZN2mLuyGPVUugpfEVPW` output please.

And also, do you have steps to reproduce it? how did you import that address?

💬 MarcoFalke commented on issue "migratewallet crashes (wallet/scriptpubkeyman.cpp:1915: std::optional<MigrationData> wallet::LegacyScriptPubKeyMan::MigrateToDescriptor(): Assertion `IsMine(desc_spk) != ISMINE_NO' failed.)":
(https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1629109069)
No idea where it is from. Apparently it is from a "test_2" :sweat_smile:

```
{
"address": "mqRzyzrW7DPGT1KZN2mLuyGPVUugpfEVPW",
"scriptPubKey": "76a9146cc16c24e249bab4384d4a759fe53ab23227de1988ac",
"ismine": false,
"solvable": false,
"iswatchonly": true,
"isscript": false,
"iswitness": false,
"ischange": false,
"timestamp": 0,
"labels": [
"test_2"
]
}