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

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1978269926)
Is there a reason why we forcefully unset `BLOCK_FAILED_VALID`? Since both flags store orthogonal information in separate bits, intuitively I'd expect we could have both co-exist? If this breaks other assumptions, perhaps good to add to the documentation here?

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1978437444)
Since this map is a superset of `candidate_blocks_by_work `, I think we could just have one, and instead check for `IsValid(BLOCK_VALID_TRANSACTIONS) && HaveNumChainTxs()` further down when we update `setBlockIndexCandidates`? I suspect the memory impllications should be negligible either way, but I think it would clean up the code a bit?

E.g. something like:

<details>
<summary>git diff on 4ba2e480ff</summary>

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index f9c900e
...

💬 asklokesh commented on issue "Bug: Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in `AddWalletDescriptor`":
(https://github.com/bitcoin/bitcoin/issues/31728#issuecomment-2696014125)
I've investigated this issue and have a fix for the uninitialized `subtype` variable in `LoadWalletFlags()`.

The bug occurs because `subtype` is being used for version compatibility checks but may remain uninitialized if the data stream doesn't contain subtype information.

**Fix approach:**
1. Initialize `subtype` with a safe default value (`DBWrapperSubType::UNKNOWN`) at declaration
2. Ensure all code paths properly set `subtype` with explicit handling for cases where:
- The stream contain
...

💬 achow101 commented on pull request "Add mainnet assumeutxo param at height 880,000":
(https://github.com/bitcoin/bitcoin/pull/31969#issuecomment-2696072544)
ACK 14f16748557faf57cf4b0f4c91c162592557434c

> if you agree we should host it, can you add it to bitcoincore.org and give me the link? I'll add it as a binary seed.

That will require more discussion with the group.

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2696652781)
Rebased to refresh the CI as all required actions are now enabled in this repo.