💬 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.

💬 tdb3 commented on pull request "test: write functional test results to csv":
(https://github.com/bitcoin/bitcoin/pull/30291#issuecomment-2172107885)
> Concept ACK
>
> 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`
>
> Also I think the durration(seconds) here is equal to number of tests for all which seems to be inaccurate too
>
> I would expect
>
> ```
> test,status,duration(seconds)
> feature_blocksdir.py,Passed,1
> feature_config_args.py,Passed,28
> wallet_startup.py,Passed,2
> mempool_accept.py,Failed,1
> ALL,Failed,32
> ```

Thank you fo
...

💬 stratospher commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1642205744)
agree! done.

💬 stratospher commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1642208728)
oh looks like 0 garbage bytes being returned can also mess up the functions where we're tampering the garbage bytes (`WRONG_GARBAGE`) and `SEND_NO_AAD` where AAD is the garbage bytes sent. so I've change `generate_keypair_and_garbage()` in `EncryptedP2PState` to return 1 bytes garbage minimum instead.

💬 stratospher commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1642204643)
`PEP 8: E303 too many blank lines (2)` was showing up in my code editor. so maybe we can keep it since we're touching the function here.

💬 stratospher commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1642210992)
done.

💬 Mazzika1 commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1642252383)
Okay