💬 achow101 commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#issuecomment-2695738356)
Could also do testnet4, my seeder publishes a seeds.txt.gz at https://testnet4.achownodes.xyz/seeds.txt.gz

💬 darosior commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1978352110)
> Also not sure why we need to add `VARINT(0)`?

You mean in the comment? I guess it doesn't matter either way but it's clearer this way...

If like @Prabhat1308 you ask why it's necessary to have an empty varint in the serialization itself, this is simply because otherwise you would get a serialization error instead of a `MoneyRange` error.

To give more details:
- this test is checking the error returned there https://github.com/bitcoin/bitcoin/blob/79bbb381a1fd13ad456fde736b3c195a791d4
...

💬 darosior commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1978352350)
See https://github.com/bitcoin/bitcoin/pull/31907/commits/dedc2d9cecccb866230c07112d00288237d12eaa#r1978352110

💬 achow101 commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1978362605)
In 6597e87666daa7fabdf310e9ad39fc8a7c81ceea "makeseeds: fix incorrect regex"

Perhaps put the `|` in the front of every regex rather than the end, to make it easier to update in the future and not accidentally run into this problem again.

🤔 mzumsande reviewed a pull request: "qa: fix an off-by-one in utxo snapshot fuzz target and sanity check its snapshot data"
(https://github.com/bitcoin/bitcoin/pull/31910#pullrequestreview-2655550047)
Code Review ACK 63b534f97e591d4e107fd5148909852eb2965d27

💬 achow101 commented on pull request "delete release note fragments for v29":
(https://github.com/bitcoin/bitcoin/pull/31976#issuecomment-2695782322)
ACK ae92bd8e1b2c144697662ba0358c27f5c1892bb2

🚀 achow101 merged a pull request: "delete release note fragments for v29"
(https://github.com/bitcoin/bitcoin/pull/31976)

💬 pablomartin4btc commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1978380037)
Just for reference (_"hash_serialized_3 hash type cannot be queried for a specific block"_):
https://github.com/bitcoin/bitcoin/blob/afde95b4601e6ac44b9fa313747c28b656d1faf2/test/functional/feature_coinstatsindex.py#L296-L303

🤔 pablomartin4btc reviewed a pull request: "Add assumeutxo chainparams to release-process.md"
(https://github.com/bitcoin/bitcoin/pull/31940#pullrequestreview-2655571248)
reACK 9dcf4b6c4d183ab069459b9be49ef7acd7d3be4b

💬 achow101 commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1978381703)
In 13f2815f4caca68a3ab0d0b865f370956308cad6 "[kernel] update assumed blockchain and chainstate sizes for v29"

700 seems a little high, although not too big of a deal I suppose.

One one node, I have 618 GiB, and on 2 others its 626, so I'd expect 680 or 690 here.

💬 achow101 commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1978382840)
In 13f2815f4caca68a3ab0d0b865f370956308cad6 "[kernel] update assumed blockchain and chainstate sizes for v29"

This seems excessively high, I'd be surprised if testnet3's datadir size doubled in the past 6 months.

Across my 3 nodes, I see 111 GiB, 112 GiB, and 116 GiB, so I'd expect this to be 127.

💬 achow101 commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1978386605)
In 92a6f7b238cd8ff015e242d6a515452c401133a7 "[kernel] update assumevalid and minimumChainWork for v29"

This block appears to have been reorged out. Perhaps we should choose a block with actual work for testnet4.

💬 achow101 commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1978388187)
In 70e294c0930a412fb6945eb753d1a39ef52fb324 "[kernel] update chainTxData for v29"

```
$ bitcoin-cli -testnet4 getchaintxstats 4096 00000000798bd720800263b69bee01aae57ea376596415a04423da29f000c367
error code: -8
error message:
Block is not in main chain
```

💬 achow101 commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1978384335)
In 13f2815f4caca68a3ab0d0b865f370956308cad6 "[kernel] update assumed blockchain and chainstate sizes for v29"

I think this is a bit low.

Across my 3 nodes, I have 7.8 GiB, 6.2 GiB, and 9.2 GiB, so I'd expect this to be 11.

💬 achow101 commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1978403077)
Can we explicitly list out which things to take from the output?

Also, perhaps it would be better to use `dumptxoutset` which gives all of the same data, and it can automatically roll back and forward.

💬 sr-gi commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1978415813)
I think the exact number was 694, but I'm happy to double check it if needed

💬 achow101 commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#issuecomment-2695877123)
ACK 44041ae0eca9d2034b7c2bdef24724809cc35e90

✅ achow101 closed an issue: "Gracefully handle dropped UPnP support "
(https://github.com/bitcoin-core/gui/issues/843)

🚀 achow101 merged a pull request: "init: Handle dropped UPnP support more gracefully"
(https://github.com/bitcoin/bitcoin/pull/31916)

🤔 stickies-v reviewed a pull request: "validation: stricter internal handling of invalid blocks"
(https://github.com/bitcoin/bitcoin/pull/31405#pullrequestreview-2655396246)
Concept ACK for making `m_best_header` and `nStatus` more reliably correct, and the trade-offs seem reasonable. Even though the diff is small, I find reasoning about these changes fairly complex and require a lot of context, so I'll need to think through the implications more.

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1978292997)
It's not clear to me why we would only want to mark blocks with large-enough PoW as invalid descendants. If this is not a bug, I think behaviour is inconsistent with the commit message and should probably be document in the code too?