💬 vostrnad commented on pull request "Add a `-permitbarepubkey` option":
(https://github.com/bitcoin/bitcoin/pull/29309#issuecomment-1912628778)
@shaavan The section "Problems of P2PK" seems completely wrong to me:

1. P2TR also directly exposes a public key in its output, and all other output types expose it at spend time.
2. P2PK outputs actually have a smaller footprint than P2PKH over their lifecycle because there's no pubkey hash overhead.
3. Exposing public keys has nothing to do with linkage of transactions, and again, all output types do it.

There are problems with P2PK, just not these ones. Did you perhaps use ChatGPT to
...

💬 TheBlueMatt commented on issue "Cluster mempool, CPFP carveout, and V3 transaction policy":
(https://github.com/bitcoin/bitcoin/issues/29319#issuecomment-1912632386)
> I think that pattern-matching / imbuing v3 is doable (that sounds like a concept ack for v3?). Assuming sibling eviction in v3, so far, it seems like it applies nicely to existing commitment transactions without any modifications. On the LN side, the requirement is no other unconfirmed inputs in the child, implying no batching.

Yes, though as I mention above I do think we may need to figure out a few additional specific details around v3 (eg how do we handle 0conf ln channels? I need to mak
...

💬 achow101 commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1468118212)
Done, removed the comment.

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

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

💬 achow101 commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1468119307)
I think it makes sense to leave them in to make sure that everything still matches previous versions.

💬 achow101 commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-1912659433)
I've added a commit that renames `nVersion` to just `version`. Note that it is not a scripted diff since a bunch of different variables share the same name.

🚀 hebasto merged a pull request: "Avoid non-self-contained Windows header"
(https://github.com/bitcoin-core/gui/pull/789)

💬 sr-gi commented on pull request "Nuke adjusted time from validation (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1912675545)
> This PR now only removes adjusted time from validation code while keeping all the code for the warning in place as is. A follow-up can change the warning (as it has many problems) but this PR will focus on the removal of adjusted time from consensus.
>
> Re-implementing the same warning seemed unnecessary (even if it was less code) and prone to bugs.

Does this mean that the adjusted time is now being collected and used exclusively to raise the warning?

💬 stickies-v commented on pull request "doc: update `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1468138472)
I don't understand why we'd delete it entirely either. It belongs together with the next sentence. I suppose it can be deduced from it too, but the current approach doesn't seem like an improvement to me. I still think my suggested phrasing works best from what I've seen so far. I don't think it's ambiguous, it just doesn't specify which RPC is called, which is irrelevant to the rest of the documentation and as such we shouldn't make it look like it is.

📝 brunoerg opened a pull request: "addrman: delete addresses that don't belong to the supported networks"
(https://github.com/bitcoin/bitcoin/pull/29330)
This PR adds a new flag `-cleanupaddrman`. When initializing a node with this flag, it will delete addresses from addrman (both new and tried tables) that do not belong to the supported networks (e.g. `-onlynet`). Deleting addresses that do not belong to the supported networks can avoid a lot of unnecessary addrman `Select` calls (especially when turning from clearnet + Tor/I2P/CJDNS to Tor/I2P/CJDNS). This flag is not enabled by default.

💬 stickies-v commented on pull request "Nuke adjusted time from validation (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1912691682)
> Does this mean that the adjusted time is now being collected and used exclusively to raise the warning?

This was the case before the force-push, too. In this current approach, the diff is much smaller so review can focus more on whether we're okay removing network-adjusted time, instead of on the adjusted time calculation refactoring.

💬 brunoerg commented on pull request "addrman: delete addresses that don't belong to the supported networks":
(https://github.com/bitcoin/bitcoin/pull/29330#issuecomment-1912698266)
cc: @mzumsande

📝 russeree opened a pull request: "redeclare nChainTx to use uint64_t"
(https://github.com/bitcoin/bitcoin/pull/29331)
Following up on https://github.com/bitcoin/bitcoin/issues/29258 the type of ```nChainTx``` has now been changed to a ```uint64_t``` allowing for the counting of TXs well into the future. The note about changing the type in 2024 was also removed. This passes all extended tests.

🤔 stickies-v reviewed a pull request: "test: Treat msg_version.relay as unsigned, Remove `struct` packing in messages.py"
(https://github.com/bitcoin/bitcoin/pull/29067#pullrequestreview-1846592629)
Concept ACK

💬 stickies-v commented on pull request "test: Treat msg_version.relay as unsigned, Remove `struct` packing in messages.py":
(https://github.com/bitcoin/bitcoin/pull/29067#discussion_r1468173825)
Aren't these supposed to be `signed=True`? Given that we're serializing a `0` I suppose it doesn't really matter there, but would still keep that consistent with the actual type.

👍 TheCharlatan approved a pull request: "depends: patch libool out of libnatpmp/miniupnpc"
(https://github.com/bitcoin/bitcoin/pull/29298#pullrequestreview-1846642861)
ACK 5b9d5bf866506b22270598aa2dc1269bc02e38e2

Guix build (x86):

```
8b32a57c3b091605070673f6338768f6ee3d5731a553ae96b426827afaf8b8f1 guix-build-5b9d5bf86650/output/aarch64-linux-gnu/SHA256SUMS.part
02a85cac626984fb8d0ea14b449af6ea916c9a3f9f9e3d68137505f5cddded60 guix-build-5b9d5bf86650/output/aarch64-linux-gnu/bitcoin-5b9d5bf86650-aarch64-linux-gnu-debug.tar.gz
f504943f0cea5bfa512a8677c8d0d1968eabcfc5a17d3fc7b810e098d7303019 guix-build-5b9d5bf86650/output/aarch64-linux-gnu/bitcoin-5b9
...

📝 russeree converted_to_draft a pull request: "redeclare nChainTx to use uint64_t"
(https://github.com/bitcoin/bitcoin/pull/29331)
Following up on https://github.com/bitcoin/bitcoin/issues/29258 the type of ```nChainTx``` has now been changed to a ```uint64_t``` allowing for the counting of TXs well into the future. The note about changing the type in 2024 was also removed. This passes all extended tests.

💬 achow101 commented on pull request "test: ensure output is large enough to pay for its fees":
(https://github.com/bitcoin/bitcoin/pull/29283#issuecomment-1912825257)
ACK 3bfc5bd36e38ca07306a41a3f56ea102ffe02b67

🚀 achow101 merged a pull request: "test: ensure output is large enough to pay for its fees"
(https://github.com/bitcoin/bitcoin/pull/29283)