💬 TheCharlatan commented on pull request "validation: set BLOCK_FAILED_CHILD correctly":
(https://github.com/bitcoin/bitcoin/pull/31835#discussion_r2018682489)
In commit 27b5abf946e737940bba5ea4b82385a100a35156:

The diagram in the commit message is a bit confusing. Are the numbers supposed to be block heights? Should the arrows be inverted then?

💬 TheCharlatan commented on pull request "validation: set BLOCK_FAILED_CHILD correctly":
(https://github.com/bitcoin/bitcoin/pull/31835#discussion_r2018755035)
Nit: Should this say "Check all ancestors of the invalidated block are validated up to..."?

⚠️ glozow opened an issue: "Test Package Accept"
(https://github.com/bitcoin/bitcoin/issues/32160)
### Please describe the feature you'd like to see added.

**Description of how the feature should work:**
The `testmempoolaccept` RPC should continue to take a list of hex transactions with minimal restrictions (no duplicates, no conflicting transactions, not too many). It should allow singleton transactions, transactions already in mempool, and any topology (multiple disconnected components should be ok). It should attempt to validate as much as possible and return each transaction's validation
...

💬 laanwj commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-2761607177)
Concept ACK

👍 ryanofsky approved a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2725831188)
Code review ACK fc88c2ec297dc93ba06008bd5ae10798e9f6aeac. Just dropped 0 timeout workaround and instant timeout test since the last review.

Ideally, I would want setting a 0 timeout to result in a comprehensible error message lke "Unable to connect to bitcoind after 0s" and not some harder to debug behavior, but it wouldn't be worth restructuring the code or adding a special case for.

💬 hebasto commented on pull request "depends: patch Qt rounding bugs":
(https://github.com/bitcoin/bitcoin/pull/32081#issuecomment-2761649016)
> I guess this won't be needed after #30997 (assuming that it makes it in for the next release)

I agree: https://github.com/bitcoin/bitcoin/blob/9ca76aa4210d15060fe35eb95da0862af57a8cc9/depends/patches/qt/normalize_round.patch#L3

💬 Lagrang3 commented on issue "v29.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/32052#issuecomment-2761649127)
I've upgraded my mainnet node to v29.0rc2 with no issues running the following services:
- electrs
- datum
- and core-lightning

🚀 ryanofsky merged a pull request: "wallet: remove redundant `Assert` call when block is disconnected"
(https://github.com/bitcoin/bitcoin/pull/32153)

💬 Lagrang3 commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2761677085)
> [@Lagrang3](https://github.com/Lagrang3) thanks for the testing. I assume like other Archer routers it was enabled by default?

Yes. Out of the box.

💬 dergoegge commented on pull request "cmake: Avoid fuzzer "multiple definition of `main'" errors":
(https://github.com/bitcoin/bitcoin/pull/31992#issuecomment-2761681405)
lgtm, afaiu this shouldn't have any downsides to existing infra

💬 laanwj commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2018881733)
Is it guaranteed that the reponse to `NLM_F_DUMP` is always multipart? Or do we need to check `nlmsg_flags` for `NLM_F_MULTI`, and if not, break after the first packet?

This is not clear to me from the documentation:
- https://man7.org/linux/man-pages/man7/netlink.7.html
- https://man7.org/linux/man-pages/man7/rtnetlink.7.html

👍 ryanofsky approved a pull request: "test: create assert_not_equal util"
(https://github.com/bitcoin/bitcoin/pull/29500#pullrequestreview-2725913318)
Code review ACK fc88c2ec297dc93ba06008bd5ae10798e9f6aeac. Just rebased and added f-string since last review.

💬 ryanofsky commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r2018882591)
re: https://github.com/bitcoin/bitcoin/pull/29500#discussion_r2018038275

I do like error_message parameter idea, fwiw. I could see these making tests more readable if they started being used for more important or more confusing checks. Tests are often written with some important checks directly related to the thing being tested, and other less important checks to see if other things are consistent. Having a place to indicate what it actually means when a check fails could be helpful for more
...

🤔 glozow reviewed a pull request: "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only"
(https://github.com/bitcoin/bitcoin/pull/26398#pullrequestreview-2725923391)
Thanks for rebasing! Needs release note.

💬 glozow commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#discussion_r2018888797)
This looks incorrect, it's still using <=64

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2761728012)
@hodlinator
> > A new style plugin causes minor visual glitches that will be fixed in follow-ups.
>
> Did not notice glitches, what are you referring to?

I've added a screenshot to the PR description.

💬 darosior commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#discussion_r2018916429)
This looks correct to me? It preserves existing behaviour, i don't think the wallet needs to be updated to support 61 62 and 63 bytes transactions in this PR?

🤔 glozow reviewed a pull request: "descriptors: Multipath/PR 22838 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/32134#pullrequestreview-2725971903)
utACK 7aa397a7d45ce0ae1b83cc6f3a65db0e41b21ee8, cc @achow101

💬 instagibbs commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#discussion_r2018920194)
I intended to maintain current behavior, which I think I've done?

💬 sipa commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2761742864)
utACK b6fe5a2196d53a3024dfe7f11b316a50acc3cb7

The code changes make sense, I think, given the fact that the block index failed flags are generally kept consistent already, apart from some edge cases around `invalidateblock`. I think the existing design stemmed from a desire not to have full block index iterations wherever possible, but if we've already bit the bullet on that, there is no need to for the complexity laziness brings here.