💬 ryanofsky commented on issue "Update nChainTx to 64bit type":
(https://github.com/bitcoin/bitcoin/issues/29258#issuecomment-1934434005)
> Not sure if it so easy. On 32-bit platforms, `nTx` currently starts at byte 60, so making it 16-bit will still eat 32-bit with padding.

I didn't test on a 32-bit platform, but aren't bitfields a good solution to this? The following seems to work for me, increasing `nChainTx` range without increasing `CBlockIndex` size on x86_64 (144 bytes):


<details><summary>diff</summary>
<p>

```diff
--- a/src/chain.h
+++ b/src/chain.h
@@ -178,7 +178,7 @@ public:
//! Note: this value is f
...

🚀 hebasto merged a pull request: "release: Update translations for v27.0 soft translation string freeze"
(https://github.com/bitcoin/bitcoin/pull/29397)

💬 hebasto commented on issue "Release schedule for 27.0":
(https://github.com/bitcoin/bitcoin/issues/29028#issuecomment-1934449827)
> ## 2024-02-01 🚧
>
> * Open Transifex translations for `v27.0`
>
> * Soft translation string freeze (no large or non-critical string changes until release)
>
> * Finalize and close translations for `v25.0`

Done.

See the announcement on the Transifex: https://app.transifex.com/bitcoin/communication/d:a5d3f4a9-8278-4435-8a19-dacfa304cc07/

💬 moonsettler commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1934450852)
wish to change my feedback to Approach NACK, it might be more appropriate to express how i see this whole thing.

💬 maflcko commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1934480710)
> Our docs (developer-notes.md and fuzzing.md) mention the use of `--disable-asm` to avoid address sanitizer failures. I've not used `--disable-asm` in any of my fuzzing and have not observed any ASan failures but they might depend on the specific sha256 optimizations used? Could also just be a relic of the past...

This is macOS specific? It may be good if someone tested or bisected this.

🤔 instagibbs reviewed a pull request: "v3 transaction policy for anti-pinning"
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1870391990)
reviewed through 8d2c91ea40be1c33b6a64168203c51a19afcd7f8

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483077399)
Sub-packages of size 1 still need the check.
```suggestion
/** Must be called for every transaction, even if not v3. Not strictly necessary for transactions accepted through AcceptMultipleTransactions().
```

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483187885)
nit for reading clarity:
```suggestion
ParentInfo(const Txid& txid, const Wtxid& wtxid, decltype(CTransaction::nVersion) version, bool has_mempool_descendant) :
```

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483187378)
nit for reading clarity:
```suggestion
bool m_has_mempool_descendant;
```

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483215614)
already checked here https://github.com/bitcoin/bitcoin/pull/28948/files#diff-a19c07329799a164e1e3bd89e7cd07f1a8e5e97b88f2b24c9bef17df04479c74R193

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483245289)
I think this is more correct in the case of a reorg? We should accept if it double-spends all siblings.

```suggestion
std::all_of(children.cbegin(), children.cend(),
```

A case that exercises this stuff in `test_v3_reorg` would be nice too. Try submitting another child when there's more than one child already due to reorg.

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483195880)
May be easier to read if we immediately return std::nullopt if `!has_parent`? Could even push the check above https://github.com/bitcoin/bitcoin/pull/28948/files#diff-a19c07329799a164e1e3bd89e7cd07f1a8e5e97b88f2b24c9bef17df04479c74R69

simplest case made clear

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483222917)
Might want to be extra clear there could be multiple children already due to a reorg.

💬 maflcko commented on pull request "bitcoin-config.h includes cleanup":
(https://github.com/bitcoin/bitcoin/pull/29404#discussion_r1483189853)
unrelated question: Is there any reason for this check? I understand that it is possible to leave the symbol unset and then set any symbols over the command line, but is anyone doing this, or is it realistic that anyone will ever do it?

If not, it could make sense to unconditionally include the config header?

Otherwise, I could see a bug silently sneak in when there is a typo in this line, such as `#if defined(_HAVE_CONFIG_H)` (or any other intentional or accidental typo)

🤔 maflcko reviewed a pull request: "bitcoin-config.h includes cleanup"
(https://github.com/bitcoin/bitcoin/pull/29404#pullrequestreview-1870581585)
> Include them manually with the exception of some files in crypto:

unrelated question: Is there a reason they need to use the flag at all? The files are never compiled and linked when the flag isn't set, so making them appear "empty" when the flag isn't set is not needed?

💬 maflcko commented on issue "Update nChainTx to 64bit type":
(https://github.com/bitcoin/bitcoin/issues/29258#issuecomment-1934519682)
> bitfields

Jup, that didn't work for me. Or maybe I did something wrong. See https://godbolt.org/z/z6EoWKbjY

```
60:0-15 | int16_t nTx
64:0-47 | int64_t nChainTx
```

(nTx eats byte 60-64, even though it is only a two byte bitfield)

💬 dergoegge commented on issue "p2p: lingering entries in `mapBlockSource`":
(https://github.com/bitcoin/bitcoin/issues/29410#issuecomment-1934529375)
I think removing entries from `mapBlockSource` once the corresponding peers disconnect would make the most sense because we want to be able to punish the source in case the block turns out to be invalid. If we expire or otherwise prematurely remove from the map we loose that ability.

Something like the following in `FinalizeNode`:
```c++
for (auto it = mapBlockSource.begin(); it != mapBlockSource.end();) {
auto& [hash, entry] = *it;
if (entry.first == node.GetId()) {

...

🤔 ryanofsky reviewed a pull request: "assumeutxo: Get rid of faked nTx and nChainTx values"
(https://github.com/bitcoin/bitcoin/pull/29370#pullrequestreview-1870720275)
Updated 2f4e7e8dc82adb60a339211cdc15408b5a6919ce -> c9ca4ca4d970f9d42d0254775c04d114bd2152c3 ([`pr/nofake.6`](https://github.com/ryanofsky/bitcoin/commits/pr/nofake.6) -> [`pr/nofake.7`](https://github.com/ryanofsky/bitcoin/commits/pr/nofake.7), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/nofake.6..pr/nofake.7)) added suggested unset nChainTx check in GuessVerificationProgress

💬 ryanofsky commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1483275071)
re: https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1479169504

> unrelated to this pull: Maybe an `Assume(nchaintx)` can be added to `GuessVerificationProgress`?

I went ahead and added this in a separate commit

💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483298313)
I don't think so, otherwise this is needlessly making things worse than it needs to be.

Imagine transactions C1 and C2 are both v3, both in the mempool, with confirmed v3 parent P. C2 can be arbitrarily big, since it entered the mempool with no unconfirmed parents.

P is reorged out, and now P, C1, and C2 are all in the mempool.

Suppose D is an RBF of C1 that would pass our RBF rules. However, C2 is a big transaction, and being forced to conflict with C2 (eg due to sibling eviction or
...