👍 ryanofsky approved a pull request: "doc: Assert that assumed-valid blocks are not fully valid in CheckBlockIndex()"
(https://github.com/bitcoin/bitcoin/pull/29355#pullrequestreview-1893776410)
Code review ACK fa027e08f7be63c201f42d0e06160d2273b4a6dd. This change makes sense and is an improvement over the status quo. I think it could be considered it a "doc" change since it is just adding an assert, which is a form of documentation, and not changing actual code. Not sure if that is how "doc" has been used other places, though.

This PR is a little redundant with #29370, which drops the BLOCK_ASSUMED_VALID flag entirely, so there is no way it could conflict with BLOCK_VALID_SCRIPTS. B
...

💬 theuni commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1957262580)
> > > In the libsecp repo, the `--with-asm=no` option is still required to avoid MSan warnings.
> >
> >
> > Can you point to where this is an issue? It looks as though secp has proper `__msan_*` annotations in place these days.
>
> Sure. For example, [bitcoin-core/secp256k1#1169 (comment)](https://github.com/bitcoin-core/secp256k1/pull/1169#issuecomment-1370003449). It holds for the current libsecp master branch @ [bitcoin-core/secp256k1@0653a25](https://github.com/bitcoin-core/secp256k1
...

✅ maflcko closed a pull request: "doc: Assert that assumed-valid blocks are not fully valid in CheckBlockIndex()"
(https://github.com/bitcoin/bitcoin/pull/29355)

💬 maflcko commented on pull request "doc: Assert that assumed-valid blocks are not fully valid in CheckBlockIndex()":
(https://github.com/bitcoin/bitcoin/pull/29355#issuecomment-1957266392)
> This PR is a little redundant with https://github.com/bitcoin/bitcoin/pull/29370, which drops the BLOCK_ASSUMED_VALID flag entirely, so there is no way it could conflict with BLOCK_VALID_SCRIPTS. But it makes sense to check for conflicting flags as long as both flags exist.

Yeah, makes sense. I forgot to close this pull when the other was opened.

⚠️ achow101 pinned an issue: "Gathering Priorities of 28.0"
(https://github.com/bitcoin/bitcoin/issues/29439)
Please nominate projects that you are the champion of that could become priorities for the next ~6 months (until the 28.0 release). We (frequent contributors) will vote on which projects will be priorities next week in a separate issue.

There were concerns in the voting of previous priority projects that some projects nominated did not have champions who were willing to have their project be a priority project. To ensure that all nominated projects have champions who want their project to be
...

💬 achow101 commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1497971188)
Fixed

💬 achow101 commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-1957402165)
Changed all of the remaining tx.nVersion that I could find.

💬 glozow commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1497979346)
I've repurposed this test to check that sibling eviction is not allowed through multi-testmempoolaccept or submitpackage

💬 glozow commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1497979453)
Added checks that all the others have `nullptr`. Also added unit tests for whether sibling is returned when {1p1c, 1p2c, 3gen} is in mempool.

💬 glozow commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1497979615)
added

💬 theuni commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1957432296)
Any other sanitizer false-positives while we're at it? :)

🤔 mzumsande reviewed a pull request: "Choose earliest-activatable as tie breaker between equal-work chains"
(https://github.com/bitcoin/bitcoin/pull/29284#pullrequestreview-1894004543)
> The attacker mines blocks B and C in secret, broadcasts the header of B, and the full block data of C. Other miners cannot build on top of this (except empty blocks). As soon as the rest of the network catches up (by mining B' and C'), the attacker broadcasts B. With the current logic, the attacker's A-B-C chain will become the active chain.

I don't think that would happen. Since #6010 `nSequenceId` is only set if the block and all of its predecessors have transactions, see [here](https://g
...

💬 ariard commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1498025796)
can be constified same than `OUTBOUND_FANOUT_DESTINATIONS` and `INBOUND_FANOUT_DESTINATIONS_FRACTION` as explanation for this value is tight to them

💬 sdaftuar commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1498032703)
Do we care about the case where the other child is not itself v3 and is not signaling for opt-in-rbf? As this can only happen as the result of a reorg, I don't think we care, but maybe it's worth mentioning somewhere that we'd process the replacement anyway.

💬 achow101 commented on pull request "test: Fix SegwitV0SignatureMsg nLockTime signedness":
(https://github.com/bitcoin/bitcoin/pull/29400#issuecomment-1957562604)
ACK fab15723b0518acbb1015e64df47dcac0187e92f

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-1957566802)
> At the risk of having asked this previously: Why not postpone the [1c7d8be](https://github.com/bitcoin/bitcoin/commit/1c7d8bea4f0b25f9adb89e402a130fa114220494) commit to a later pull? This would also make review easier, because the code change comes with the changes to actually use it in real code, outside of just unit tests.

Sure, I'm happy to split up this PR.

I don't think this idea came up before (or I can't remember if it did). From my perspective commit 1c7d8bea4f0b25f9adb89e402a13
...

🚀 achow101 merged a pull request: "test: Fix SegwitV0SignatureMsg nLockTime signedness"
(https://github.com/bitcoin/bitcoin/pull/29400)

💬 glozow commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1498086476)
I agree that we shouldn't care about it signaling v3 or BIP125. Will add comment.

💬 instagibbs commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1498102589)
```suggestion
// the descendant count is done separately in SingleV3Checks for v3 transactions.
```

🤔 cbergqvist reviewed a pull request: "test: test_bitcoin: allow -testdatadir=<datadir>"
(https://github.com/bitcoin/bitcoin/pull/26564#pullrequestreview-1893059876)
Generally great debugging UX improvement in my eyes!