💬 maflcko commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1743855915)
Forgot the CBlockIndex `*` -> `&` ? :sweat_smile:

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2329149553)
@sdaftuar awesome data! Encouraging to see the timings move along with what @sipa was asserting :+1:

👍 maflcko approved a pull request: "rpc: dumptxoutset height parameter follow-ups (29553)"
(https://github.com/bitcoin/bitcoin/pull/30808#pullrequestreview-2280325494)
re-ACK 769dc84852e6f14876d595a7d628cc02fa074c75 🎃

<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: re-ACK 769dc84852e6f14876d5
...

💬 maflcko commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1743863145)
nit in the last commit: Shouldn't this be a LogWarning, so that the user knows that the log is related to an RPC error that happened? Also, it could include the name of the RPC that triggered the log.

🤔 l0rinc reviewed a pull request: "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage"
(https://github.com/bitcoin/bitcoin/pull/30618#pullrequestreview-2280307916)
> good point thanks, perhaps that would be good to clarify in a docstring?

Do you think it's important in this case? We're not using the `random_hex_string` as input here at all, only the output.
If you do, please give me a diff an I'll apply it.

💬 l0rinc commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1743859614)
Git blame without proper history is useless anyway, I always needed a bisect to see the real author, so I don't really see the advantage of optimizing for that.
But mirroring the header file's structure does simplify the code a bit.
Let me know if you feel strongly about it and I'll revert, but I would consider that a step back.

💬 l0rinc commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1743852827)
If we think this takes a lot of time, we can optimize it by removing cases that we think are unlikely to result in revealing failures.
We can also do it in separate steps, if we see that it's stable for months, remove the excess.
Maybe @maflcko can help us out here.

💬 l0rinc commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1743862579)
nice, if I retouch I'll apply it!

💬 fjahr commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1743870042)
Ah, yeah, I overlooked that. Fixed.

💬 fjahr commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1743875846)
I checked the developer notes on the definition again and I think you are right that a `LogWarning` is a better fit. I think it's not a "severe problem" necessarily but it does need to be addressed by the node admin if they want to run `dumptxoutset` successfully.

💬 maflcko commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-2329170695)
re-ACK a3108a7c5692d137b70b8442b4741936277e89be 🐸

<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: re-ACK a3108a7c5692d137b70b
...

🚀 glozow merged a pull request: "test: add check that too large txs aren't put into orphanage"
(https://github.com/bitcoin/bitcoin/pull/30784)

💬 fanquake commented on pull request "depends: build libevent with `-D_GNU_SOURCE`":
(https://github.com/bitcoin/bitcoin/pull/30743#issuecomment-2329212785)
> EDIT: Probably something with the zipping again?

Yea, looks like a permissions issue:
#### Comparing `bitcoin-556775408797-arm64-apple-darwin-unsigned.zip.fanquake` & `bitcoin-556775408797-arm64-apple-darwin-unsigned.zip.thecharlatan`

#### zipinfo {}

```diff
@@ -1,18 +1,18 @@
Zip file size: 16309329 bytes, number of entries: 46
drwxr-xr-x 3.0 unx 0 b- stor 24-Aug-28 16:35 Bitcoin-Qt.app/
drwxr-xr-x 3.0 unx 0 b- stor 24-Aug-28 16:35 Bitcoin-Qt.app/Contents/
--
...

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1743907527)
Yes, will fix if I retouch

💬 fjahr commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2329225316)
Concept ACK

I've read through https://github.com/bitcoin/bitcoin/pull/30385 and this seems like a good change.

💬 fjahr commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2329230808)
CI doesn't seem happy though. Kind of weird, the logs don't load for me.

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2329234757)
Follow-ups are here: https://github.com/bitcoin/bitcoin/pull/30808

💬 kevkevinpal commented on pull request "lint: Speed up and fix flake8 checks":
(https://github.com/bitcoin/bitcoin/pull/30723#issuecomment-2329240289)
ACK [fafdb7d](https://github.com/bitcoin/bitcoin/pull/30723/commits/fafdb7df34507eee735893aa871da6ae529e6372)

Removal of the unnecessary lint checks and the speed up of the checks are great updates!

💬 furszy commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2329262030)
> CI doesn't seem happy though. Kind of weird, the logs don't load for me.

Yeah, working on it. Almost there.

🤔 pablomartin4btc reviewed a pull request: "rpc: dumptxoutset height parameter follow-ups (29553)"
(https://github.com/bitcoin/bitcoin/pull/30808#pullrequestreview-2280450117)
cr ACK a3108a7c5692d137b70b8442b4741936277e89be

Thanks for addressing the 3 follow-ups. I'll test this soon.

Is it worth it to add a test to produce the situation where the reconsider didn't happen and the height stayed at the rollback/ invalidated height/ stale tip (test should fail in prod and not with this PR)?

<details>
<summary>Here some more details (rpc: <code>dumptxoutset</code>, <code>getchaintips</code> & <code>getcblockchaininfo</code>) about the situation described at the t
...