💬 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

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

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

👍 rkrux approved a pull request: "test: Added test coverage to listsinceblock rpc"
(https://github.com/bitcoin/bitcoin/pull/30195#pullrequestreview-2121134118)
tACK [881724](https://github.com/bitcoin/bitcoin/pull/30195/commits/881724d443d11f984a721ef1edd5777c24d1ed29)

Make successful, so are all the functional tests. Asked a question for my clarity.

💬 rkrux commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1641643772)
Super Nit: `node1_last_blockhash`

💬 rkrux commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1642285756)
I've been trying to understand how does calling `listsinceblock nodes1_last_blockhash` on `node0` is causing the last condition to be true here: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpc/transactions.cpp#L663

The comment above this line talks about requesting a reorg'ed block, but isn't the last blockhash on node0 part of the main chain from POV of node0?

👍 rkrux approved a pull request: "test: write functional test results to csv"
(https://github.com/bitcoin/bitcoin/pull/30291#pullrequestreview-2122088460)
tACK [ad06e68](https://github.com/bitcoin/bitcoin/pull/30291/commits/ad06e68399da71c615db0dbf5304d0cd46bc1f40)

Make is successful so are all the functional tests. Passed the `resultsfile` param and generated the results csv few times.

Thanks @tdb3 for this, it's a good addition to the testing suite.
I can imagine using this feature and also sorting the tests in descending order based on time taken. I mentioned couple points but none are blockers.

💬 rkrux commented on pull request "test: write functional test results to csv":
(https://github.com/bitcoin/bitcoin/pull/30291#discussion_r1642322154)
The write function below `write_results` intentionally creates a csv file, should we add a check here for the `csv` file extension?