💬 tdb3 commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1630519343)
Verified that `generatetoaddress` (and response) specify jsonrpc 2.0 successfully. Also verified the same for `-generate`'s requests to `getnewaddress` and `generatetoaddress` (successful).

💬 tdb3 commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1630505568)
Verified that `bitcoin-cli -addrinfo`'s request to `getnodeaddresses` and the resultant response now specify jsonrpc 2.0 successfully.


💬 tdb3 commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1630534653)
This would also throw if 204 was received. This seems ok because none of our tests should send a "notification" (i.e. without `id` param), so 204 should not be received.

⚠️ jaemoney135 opened an issue: "Smart contract scam"
(https://github.com/bitcoin/bitcoin/issues/30240)
Can someone help me check if my address is black listed ?

✅ fanquake closed an issue: "Smart contract scam"
(https://github.com/bitcoin/bitcoin/issues/30240)

💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2154154694)
Thanks for the review and ACKs, I will address the left over nits here shortly, I think re-ACKing should be easy enough.

💬 maflcko commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-2154271957)
ACK 659663af32f02c570e334e8f375fd5f5737258d7 🚋

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 659663af32f02c570e334e8f37
...

💬 maflcko commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#issuecomment-2154279892)
`git ls-files` is also used in `./test/lint/lint-python.py`, for reference.

⚠️ maflcko opened an issue: "fuzz: timeout/oom in package_rbf"
(https://github.com/bitcoin/bitcoin/issues/30241)
I stumbled upon a fuzz seed that oom'ed in CI for `package_rbf`. Since the seed appears to be too big to paste, I provide the output of `$ base64 95a3b1b42d43ac7190f24ff116b2cd6515e7a566 > /tmp/base64-95a3b1b4-oom.txt`:
[base64-95a3b1b4-oom.txt](https://github.com/user-attachments/files/15685350/base64-95a3b1b4-oom.txt)

If the prior format is unsatisfactory, the problematic seed `95a3b1b42d43ac7190f24ff116b2cd6515e7a566` is also in this commit to qa-assets: https://github.com/b
...

💬 maflcko commented on issue "fuzz: timeout/oom in package_rbf":
(https://github.com/bitcoin/bitcoin/issues/30241#issuecomment-2154289292)
It spends a long time hashing the large transaction, which has a large witness. The runtime is NUM_ITERS (aka 10'000) times n (size of the transaction), because the hashing is done in the loop.

💬 AngusP commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#issuecomment-2154290717)
> Not sure if that is worth addressing since it's so rare but perhaps simply adding a comment that the test might fail every 2^48 runs is good?

I think it's worth avoiding, I don't really like the notion of flaky tests maybe failing for a random reason, even if it is very improbable. I'd guess short-ID collisions would also affect some of the other existing tests in here so there's additional benefit in them not being flaky too.

👍 dergoegge approved a pull request: "fuzz: add I2P harness"
(https://github.com/bitcoin/bitcoin/pull/30230#pullrequestreview-2103980713)
tACK 193c748e44f8647a056121fc9cbb9c2efbcbfc49

Thanks for picking this up!

I fuzzed the `i2p` harrness for a few hundred CPU hours and even without the dictionary it reaches almost everything. The dictionary will still help though, so adding it to qa-assets later would be good.

My only note: I think it would be worthwhile to point out in the PR description what changes you have made to the original harness and why they were necessary.

💬 maflcko commented on issue "test: Intermittent issue in p2p_leak_tx.py in test_notfound_on_replaced_tx":
(https://github.com/bitcoin/bitcoin/issues/29621#issuecomment-2154400935)
https://drahtbot.space/temp_scratch/p2p_leak_tx_115_b.tar.zstd

```
test 2024-06-07T02:34:22.778000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/ci_con
...

📝 hebasto opened a pull request: "ci: Native Windows CI job cleanup"
(https://github.com/bitcoin/bitcoin/pull/30242)
This PR:

1. Removes no longer needed workaround for GHA Windows images.

GHA Windows images previously had multiple VC Build Tools installed, which required specifying the `VCPKG_PLATFORM_TOOLSET_VERSION` explicitly to avoid linker errors. This issue has been resolved as per
https://github.com/actions/runner-images/issues/9701.

2. Switches all references to temporary files to relative ones for consistency and readability.

💬 AngusP commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1630897082)
re. this comment https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1630237777 and needing to use the full path again, I think it is that `rename(...)` returns a new `Path` instance, which is then the 'correct' one to use going forwards for further changes you want to happen on-disk (alas Python has no 'use of moved value' concept like Rust et al. that would've complained at you trying to do this).

I think the way to think of this is that a `Path` is just a *path*, not a file-handle, s
...

💬 AngusP commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1630910069)
I've added https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1630897082 with a suggestion

💬 AngusP commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1630912317)
(Could even `blk_dat_moved.rename(blk_dat.name)` to avoid repeating the literal `"blk00000.dat"` but meh)

📝 hebasto converted_to_draft a pull request: "ci: Native Windows CI job cleanup"
(https://github.com/bitcoin/bitcoin/pull/30242)
This PR:

1. Removes no longer needed workaround for GHA Windows images.

GHA Windows images previously had multiple VC Build Tools installed, which required specifying the `VCPKG_PLATFORM_TOOLSET_VERSION` explicitly to avoid linker errors. This issue has been resolved as per
https://github.com/actions/runner-images/issues/9701.

2. Switches all references to temporary files to relative ones for consistency and readability.

👋 hebasto's pull request is ready for review: "ci: Native Windows CI job cleanup"
(https://github.com/bitcoin/bitcoin/pull/30242)

💬 hebasto commented on pull request "ci: Native Windows CI job cleanup":
(https://github.com/bitcoin/bitcoin/pull/30242#issuecomment-2154501642)
Friendly ping @m3dwards @sipsorcery ;)