🤔 tdb3 reviewed a pull request: "Always show 100% verification progress when done"
(https://github.com/bitcoin/bitcoin/pull/30293#pullrequestreview-2121441221)
Thanks for taking a look and presenting the idea.

With bitcoin providing an RPC interface (and the OS providing general process run state), sifting through the debug.log seems like a non-ideal method for status gathering. Are there some examples from out in the wild where this is relied upon?

A cool feature of the RPC interface is that it provides early feedback (e.g. that blocks are being verified). Here are some screenshots of what the RPC caller sees and what is occurring in the debug
...

💬 hMsats commented on pull request "Always show 100% verification progress when done":
(https://github.com/bitcoin/bitcoin/pull/30293#issuecomment-2171658767)
@tdb3

> There might be value in providing a definitive 100% message, but more for general accuracy to bitcoin itself rather than to machine observers of the debug.log.

That was the real reason for this pull request. My example was just a side note. Seeing the 100% is useful also for users looking at the `debug.log`.

💬 hMsats commented on pull request "Always show 100% verification progress when done":
(https://github.com/bitcoin/bitcoin/pull/30293#issuecomment-2171677111)
@pinheadmz Ah, maybe we need different strings here. Instead of "Verification progress" maybe "Validation progress" in order not to confuse the two processes.


Linux command: `ag "Verification progress"` gives (including the 100%):

```
bitcoin/test/functional/interface_bitcoin_cli.py
184: assert_equal(cli_get_info['Verification progress'], "%.4f%%" % (blockchain_info['verificationprogress'] * 100))

bitcoin/src/validation.cpp
4613: LogPrintf("Verification progress: 0%%\n");
...

💬 Prabhat1308 commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1641905080)
The number of parents per peer should mostly be in the range of (0-2/3) , This should include a significant numbers of peers to reconcile with . A constant number might not be ideal considering if the array returned is smaller than that constant number , we might be reconciling with non-existent peers .

💬 pinheadmz commented on pull request "Always show 100% verification progress when done":
(https://github.com/bitcoin/bitcoin/pull/30293#issuecomment-2171746348)
> wait for Bitcoin Core to finish

At this point in your script do you mean

- "wait for Bitcoin Core to be ready for RPC commands" or
- "wait for Bitcoin Core to sync to the most work chain it can find on the network so my UTXO state is most likely to be globally accurate"?

💬 hMsats commented on pull request "Always show 100% verification progress when done":
(https://github.com/bitcoin/bitcoin/pull/30293#issuecomment-2171750527)
@pinheadmz Irrespective of my (simple) shell script, in my opinion Bitcoin Core should always print "Verification progress=100%" or even better "Validation progress=100%" in `debug.log`.

💬 kevkevinpal commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#issuecomment-2171781426)
removed an assertion that didn't seem needed in [881724d](https://github.com/bitcoin/bitcoin/pull/30195/commits/881724d443d11f984a721ef1edd5777c24d1ed29)

The other two seem fine as they assert the file was replaced properly which `.replace` would not throw an error for

💬 achow101 commented on pull request "Always show 100% verification progress when done":
(https://github.com/bitcoin/bitcoin/pull/30293#issuecomment-2171781775)
If you want to know when bitcoind is ready to start receiving RPCs and generally finished its startup sequence, you should use `-startupnotify` and have it notify you, rather than parsing the debug.log.

ISTM the log line saying that no inconsistencies were found is a good enough indicator that verification was finished.

💬 kevkevinpal commented on pull request "Always show 100% verification progress when done":
(https://github.com/bitcoin/bitcoin/pull/30293#issuecomment-2171793485)
> That progress will never reach 100% by design

I agree with @pinheadmz

why not just look for this line instead for your script?
`Verification: No coin database inconsistencies`

💬 sdaftuar commented on issue "RFC: Assumeutxo and large forks and reorgs":
(https://github.com/bitcoin/bitcoin/issues/30288#issuecomment-2171794670)
> If I can summarize and clarify, neither of you think the current behavior of locking in snapshot block, and temporarily refusing to consider chains that don't include it is a good idea?

I'm not sure I'm following this point exactly: my recollection is that the current **observable** behavior in this scenario would be to crash, because even though the original/fully validated chainstate locks in ancestors of the snapshot block to be possible tips, we'd still try to reorg the snapshot chains
...

💬 kevkevinpal commented on pull request "test: write functional test results to csv":
(https://github.com/bitcoin/bitcoin/pull/30291#issuecomment-2171794835)
This line initially when I read it made me assume that 28 tests failed when in reality 28 passed and one failed
`ALL,Failed,28`

💬 kevkevinpal commented on pull request "test: write functional test results to csv":
(https://github.com/bitcoin/bitcoin/pull/30291#discussion_r1641941956)
Would be useful to have a log come from this assert
```suggestion
assert results_filepath.parent.exists(), "Results file path parent directory does not exist"
```
Before:
```
test/functional/test_runner.py --resultsfile ./fakedir/functional_test_results.csv feature_blocksdir
Temporary test directory at /tmp/test_runner_₿_🏃_20240616_141849
Traceback (most recent call last):
File "/home/kevkevin/DEVDIR/bitcoin/test/functional/test_runner.py", line 939, in <module>
main()

...

💬 hMsats commented on pull request "Always show 100% verification progress when done":
(https://github.com/bitcoin/bitcoin/pull/30293#issuecomment-2171803437)
@kevkevinpal (and @achow101) I do in my script but I still believe that the two lines:

```
2024-06-16T09:13:27Z Verification progress: 99%
2024-06-16T09:13:27Z Verification: No coin database inconsistencies in last 6 blocks (39935 transactions)
```
are ugly and
```
2024-06-16T09:13:27Z Verification progress: 99%
2024-06-16T09:13:27Z Verification progress: 100%
2024-06-16T09:13:27Z Verification: No coin database inconsistencies in last 6 blocks (39935 transactions)
```
is much bett
...

💬 hMsats commented on pull request "Always show 100% verification progress when done":
(https://github.com/bitcoin/bitcoin/pull/30293#issuecomment-2171810713)
Of course I can live without this (trivial) addition, so if you developers don't want to include it, it's fine with me

💬 hMsats commented on pull request "Always show 100% verification progress when done":
(https://github.com/bitcoin/bitcoin/pull/30293#issuecomment-2171818125)
Found it. It used to be like this (note the `[DONE]`):

```
2018-02-15 07:39:04 init message: Rewinding blocks...
2018-02-15 07:39:07 init message: Verifying blocks...
2018-02-15 07:39:07 Verifying last 144 blocks at level 3
2018-02-15 07:39:07 [0%]...[10%]...[20%]...[30%]...[40%]...[50%]...[60%]...[70%]...[80%]...[90%]...[DONE].
```

✅ hMsats closed a pull request: "Always show 100% verification progress when done"
(https://github.com/bitcoin/bitcoin/pull/30293)

💬 KristijanSajenko commented on pull request "Ephemeral Anchors, take 2":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2171868837)
Thx

💬 furszy commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1641999399)
The legacy spkm class will be deleted soon in #28710.
You might need to add a mechanism to mock the bdb reader class just so you can feed the migration process with a hand-crafted `LegacyDataSPKM`. This will let you avoid crafting the bdb database file manually when bdb is not compiled anymore.

👍 tdb3 approved a pull request: "test: Added test coverage to listsinceblock rpc"
(https://github.com/bitcoin/bitcoin/pull/30195#pullrequestreview-2121745429)
re ACK for 881724d443d11f984a721ef1edd5777c24d1ed29

re-ran test (passed) and temporarily moved up `blk_dat_moved.rename()` to induce failure with `assert_raises_rpc_error`.

💬 tdb3 commented on pull request "test: write functional test results to csv":
(https://github.com/bitcoin/bitcoin/pull/30291#discussion_r1642089051)
Thanks. This is an improvement. Included.