💬 kevkevinpal commented on pull request "doc: archive release notes for v30.0":
(https://github.com/bitcoin/bitcoin/pull/33601#discussion_r2424692750)
Haha just `kevkevinpal` works

💬 shyrwall commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-3395190327)
Bit late to add this but just saw it because of v30 so just wanted to comment. There are millions upon millions of, mostly chinese, routers out there that only supports UPnP.

For example, basically all of Thailands ISPs use routers from Fiberhome and Huawei. Mostly for PON-deployments. None of these supports anything other than UPnP. That's a single example of tens of millions of connections.

💬 kamol1988 commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-3395202500)
Yed

📝 l0rinc opened a pull request: "[IBD] coins: reduce lookups in dbcache layer propagation"
(https://github.com/bitcoin/bitcoin/pull/33602)
Previously, when the parent coins cache had no entry and the child did, `BatchWrite` performed a find followed by `try_emplace`, which resulted in multiple `SipHash` computations and bucket traversals on the common insert path.

This change uses a single leading `try_emplace` and branches on the returned `inserted` flag. In the `FRESH && SPENT` case (only exercised by tests), we erase the just-inserted placeholder (which is constant time with no rehash anyway). Semantics are unchanged for all
...

🤔 andrewtoth reviewed a pull request: "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`"
(https://github.com/bitcoin/bitcoin/pull/32313#pullrequestreview-3329398843)
ACK 24d861da7894add47747eff69dd3fc71fbcdd7d0

⚠️ prusnak opened an issue: "Missing shell completions for the `bitcoin` command"
(https://github.com/bitcoin/bitcoin/issues/33603)
### Please describe the feature you'd like to see added.

v30 introduced a new `bitcoin` command but there are no shell completions for this command in contrib/completions/{bash,fish}

it would be great if we had them :)

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'd like

_No response_

### Describe any alternatives you've considered

_No response_

### Please leave any additional context

_No response_

💬 fanquake commented on issue "Missing shell completions for the `bitcoin` command":
(https://github.com/bitcoin/bitcoin/issues/33603#issuecomment-3395305027)
cc @willcl-ark

📝 stringintech opened a pull request: "p2p: Allow block downloads from peers without snapshot block after assumeutxo validation"
(https://github.com/bitcoin/bitcoin/pull/33604)
Currently, after assumeutxo background validation finishes, the node continues to skip peers that don't have the snapshot block in their best chain until restart. This unnecessarily excludes peers from block downloads even though the background sync has completed and undo data is available.

The restriction persists because `GetSnapshotBaseBlock()` continues to return the snapshot base block even after validation completes. While `m_ibd_chainstate->m_disabled` is set to true when validation fi
...

💬 TheCharlatan commented on pull request "p2p: Allow block downloads from peers without snapshot block after assumeutxo validation":
(https://github.com/bitcoin/bitcoin/pull/33604#issuecomment-3395334349)
Nice, Concept ACK.

💬 fanquake commented on pull request "p2p: Allow block downloads from peers without snapshot block after assumeutxo validation":
(https://github.com/bitcoin/bitcoin/pull/33604#issuecomment-3395336034)
cc @mzumsande

🤔 l0rinc requested changes to a pull request: "index: initial sync speedup, parallelize process"
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-2965376357)
I started a detailed review of this PR and I'm enthusiastic about the concept - it addresses a real problem and the potential performance gains are significant. \
Strong Concept ACK!

However, I've encountered some concerns that make me hesitant to continue reviewing the current implementation in depth:
* The txindex parallelization appears to be broken (showing 3-13% slowdowns rather than speedups), and this went unnoticed for years
* The complexity of the changes makes thorough review very cha
...

💬 l0rinc commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2424838828)
based on my benchmarks on various platforms it seems this isn't speeding up txindex - was this measured by anyone?

💬 l0rinc commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2423291668)
nit: since it's strongly typed we don't need to include the type name in the variable anymore

💬 l0rinc commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2424805627)
I think it might be clearer if this commit came later in the series, after the groundwork is established. In my experience, it's often easier to review when the problem definition is presented before the solution.

As it stands, this commit introduces code whose purpose isn't immediately clear without reading ahead to future commits.

I'd like to echo the concern other reviewers raised about separating the thread pool from the script parallelization work. Since script parallelization is alr
...

💬 l0rinc commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2424874997)
3087bd1ef1e9f85bf6a0b7c24be650f77a6c030a

I'm finding this commit difficult to review in depth due to its complexity. Like before, I'm seeing the solution (out-of-order processing with task tracking and opportunistic post-processing) before experiencing the problem it solves.

Could we break this down into focused, trivial commits, where low-risk changes are separated from high-risk ones and where the commits tell a story, where the pain is experienced before we propose a solution.

💬 l0rinc commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2424865671)
It seems to me we can loosen the args and avoid string concat:
```suggestion
template <typename T>
void WaitFor(std::span<const std::future<T>> futures, std::string_view context)
{
for (size_t i = 0; i < futures.size(); ++i) {
if (futures[i].wait_for(TIMEOUT_SECS) != std::future_status::ready) {
throw std::runtime_error(strprintf("Timeout waiting for: %s, task index %d", context, i));
}
}
}
```

💬 stringintech commented on pull request "p2p: Allow block downloads from peers without snapshot block after assumeutxo validation":
(https://github.com/bitcoin/bitcoin/pull/33604#issuecomment-3395390753)
This change allows the snapshot chainstate to reorg to chains that don't include the snapshot block after validation completes (not sure yet if this is the only mechanism that would allow such reorgs), so we must not assume anywhere in the codebase that `SnapshotBase()` is always an ancestor of the active tip. I'll take a closer look to see if any such assumptions exist, but I thought this was worth noting here.

👍 TheCharlatan approved a pull request: "refactor: inline constant return values from `dbwrapper` write methods"
(https://github.com/bitcoin/bitcoin/pull/33042#pullrequestreview-3329597424)
ACK 743abbcbde9e5a2db489bca461c98df461eff7d0

⚠️ gmart7t2 opened an issue: "Remove remaining mention of datacarriersize being deprecated"
(https://github.com/bitcoin/bitcoin/issues/33605)
#33453 undeprecated the datacarrier settings, but https://github.com/bitcoin/bitcoin/blob/v30.0/src/qt/bitcoinstrings.cpp#L165 still says:

QT_TRANSLATE_NOOP("bitcoin-core", "Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in a future version."),

Best to remove that too?

💬 l0rinc commented on issue "Remove remaining mention of datacarriersize being deprecated":
(https://github.com/bitcoin/bitcoin/issues/33605#issuecomment-3395500448)
this will be removed automatically