💬 mzumsande commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#issuecomment-3543689545)
[6d2c8ea](https://github.com/bitcoin/bitcoin/commit/6d2c8ea9dbd77c71051935b5ab59224487509559) to [6db2551](https://github.com/bitcoin/bitcoin/commit/6db2551dc27c4a9b989e8814054c93dd9d8f1b36):
Added hash of invalid block to the log.

📝 instagibbs opened a pull request: "policy: Remove individual transaction <minrelay restriction"
(https://github.com/bitcoin/bitcoin/pull/33892)
Prior to cluster mempool, a policy was in place that
disallowed non-TRUC transactions from being
TX_RECONSIDERABLE in a package setting if it was below
minrelay. This was meant to simplify reasoning about mempool
trimming requirements with non-trivial transaction
topologies in the mempool. This is no longer a concern
post-cluster mempool, so this is relaxed.

In effect, this makes 0-value parent transactions relayable
through the network without the TRUC restrictions and
thus the a
...

💬 hodlinator commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2535330614)
Good point regarding .clang-format, though that value from 13f36c020f0329b5e975282b45292fdf2a495e31 comes from clang-format defaults rather than a conscious decision on our part. (It will not remove newlines at EOF if they exist though, https://releases.llvm.org/16.0.0/tools/clang/docs/ClangFormatStyleOptions.html#insertnewlineateof).

GitHub displays a red warning symbol in diffs when one removes the empty newline at ends of files.

`git apply diff.patch` fails for...
```diff
diff --git a
...

💬 hodlinator commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2535372567)
Thanks! (I believe that's fixed in this PR - 093980af8e4e594716b9219931fd441266c1fcd4 "init, net: Implement usage of binary-embedded asmap data")

💬 hodlinator commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2535368296)
Ah, wasn't on my radar, cheers! Does simplify things a bit.

💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3543708460)
https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2432472005 still has invalidateblock in `test/functional/mempool_packages.py` ? If you don't have the time to carry the patch I can take over, just let me know.

🤔 hebasto reviewed a pull request: "refactor: Avoid -W*-whitespace in git archive"
(https://github.com/bitcoin/bitcoin/pull/33869#pullrequestreview-3474450076)
post-merge ACK.

🤔 glozow reviewed a pull request: "Cluster mempool"
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3455337470)
Code reviewed the "squashme"s (I think they should be squashed) and the end state for MemPoolAccept, miner, reorg, src/policy/* changes. I plan to put most of my comments on #33591. My suggestions here are to clean up the commits so the git history is easy to trace in the future.

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2519820632)
ccf7c5ac417 looks good and can be squashed

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2519700544)
nit if you retouch: commit message for cec26203732 should replace "descendantsize" with "descendant count"

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2519770412)
If you retouch: f02ff837008 can be squashed into the commit above it. I think the diff may have shrunk over time - the commit message mentions `addUnchecked` which no longer exists in the codebase.

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2519738828)
nit in 1eddff0b0ab if you retouch: `GetDescendantCount`, `GetAncestorCount` would be more similar to existing naming, and slightly less confusing since the count includes the tx itself.

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2535401285)
nit in c9efae8a00daf0ab1b9f94db231668aa823aba4e commit message: our docs call this carveout "Single-Conflict RBF Carve Out". It's distinct from CPFP carveout, so it could be confusing to mention that in the commit message.

🤔 hodlinator reviewed a pull request: "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation"
(https://github.com/bitcoin/bitcoin/pull/33878#pullrequestreview-3473921944)
Reviewed eaf5e12d2b52d66a75a68cf94d328d0ec9048986

Thanks for peeling off this PR as I suggested!

💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2535184260)
In c2274746d1b3d1a2712ef8bd567d25f5aa5cf5bb "refactor: Operate on bytes instead of bits in Asmap code": Line seems like a leftover from a merge?

💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2535169330)
In c2274746d1b3d1a2712ef8bd567d25f5aa5cf5bb "refactor: Operate on bytes instead of bits in Asmap code": It's unfortunate that you need to make the hex data a `vector` again when the later span commit will accept the `std::array`.

You could do:
```C++
NetGroupManager netgroup{std::vector(ASMAP_DATA.begin(), ASMAP_DATA.end())};
```
In this commit and avoid changing the type of `ASMAP_DATA`.

💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2535040100)
In c2274746d1b3d1a2712ef8bd567d25f5aa5cf5bb "refactor: Operate on bytes instead of bits in Asmap code": The `for` doing `ip_bits[byte_i] = std::byte(IPV4_IN_IPV6_PREFIX[byte_i]);` can now be removed since we have `copy_n` doing the same thing above.

💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2535279441)
In 0196f70a8c99b9c03c81e879cd9f05560fc20543: Could replace second sentence with "Returns true if the data is valid for 128 bits long inputs" or just remove it (no longer returns span).

💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2535412684)
Related: https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2522389508

Thanks for documenting the C++ implementation!

nit:
```suggestion
// in the input. If they all match, execution continues. If not, the default ASN is returned (or 0 if unset).
```

Could also correct the Python side:
```diff
--- a/contrib/asmap/asmap.py
+++ b/contrib/asmap/asmap.py
@@ -157,7 +157,7 @@ class _Instruction(Enum):
JUMP = 1
# A match instruction, encoded as [1,1,0] inspects 1 or more of the
...

💬 hodlinator commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2535226892)
nit in 4c8e4eee00f16bace634221277c694ee5dda23f8: Sorry, accidentally injected possessive apostrophe, "span's", commit message should be:
> The version hash changes due to spans being serialized without their size-prefix (unlike vectors).