💬 instagibbs commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2127225225)
suggested changes were done, reviewed via `git range-diff master 7c3fb97 ef8de26`

💬 instagibbs commented on pull request "fuzz: More accurate coverage reports":
(https://github.com/bitcoin/bitcoin/pull/30156#issuecomment-2127236772)
concept ACK

💬 murchandamus commented on pull request "policy: restrict all TRUC (v3) transactions to 10kvB":
(https://github.com/bitcoin/bitcoin/pull/29873#discussion_r1611774155)
> As CPFP carve out does not explicitly require a second child to actually
exist, it has the effect of granting a free +10KvB descendant size limit
when a single child is enough to bust the descendant limit.

That sounds like an unintentional behavior. Should that be addressed with a bug fix, or can we just get rid of the carve-out altogether in a world where we have TRUC transactions?

🤔 murchandamus reviewed a pull request: "policy: restrict all TRUC (v3) transactions to 10kvB"
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2074103415)
crACK 154b2b2296edccb5ed24e829798dacb6195edc11

💬 glozow commented on pull request "contrib: Renew Windows code signing certificate":
(https://github.com/bitcoin/bitcoin/pull/30149#issuecomment-2127243747)
Backport for 26.x in #29899

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2127254156)
> Also, please split the interfaces changes into a different commit than the GUI changes.

did that. cleaner and atomic. thanks.

🤔 ismaelsadeeq reviewed a pull request: "refactor prep for package rbf"
(https://github.com/bitcoin/bitcoin/pull/30072#pullrequestreview-2074136766)
re-ACK 63cfa4ce88cac26abcacf3b243916e722ab17d75

👍 theStack approved a pull request: "refactor prep for package rbf"
(https://github.com/bitcoin/bitcoin/pull/30072#pullrequestreview-2074156905)
re-ACK 63cfa4ce88cac26abcacf3b243916e722ab17d75

💬 glozow commented on pull request "policy: restrict all TRUC (v3) transactions to 10kvB":
(https://github.com/bitcoin/bitcoin/pull/29873#discussion_r1611811645)
The first commit gets rid of it for TRUC, and it'll go away with cluster mempool.

I don't really think a fix in the interim is worth it; it's slightly involved to pull all the ancestors and check that their descendant count exceeds 1. I'd rather focus on getting rid of it tbh.

💬 glozow commented on pull request "policy: restrict all TRUC (v3) transactions to 10kvB":
(https://github.com/bitcoin/bitcoin/pull/29873#discussion_r1611813658)
Also, note that in a non-TRUC 1p1c scenario, the parent's increased descendant size limit is mooted by the ancestor size limit (of the child).

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611829012)
i don't think so, the pointer needs to advance by the size of the specific message, which is part of that message.

💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1611830313)
@josibake I looked around a bit and there might be something here. `GetHash()` is called before this point
- when we read the block from disk https://github.com/bitcoin/bitcoin/blob/e163d864d380956a4c0f89a4d80a76f5aefc9a08/src/node/blockstorage.cpp#L1092
- when `ActivateBestChain` is called https://github.com/bitcoin/bitcoin/blob/e163d864d380956a4c0f89a4d80a76f5aefc9a08/src/validation.cpp#L3354

There might be some gain in caching GetHash() but I think that has to be addressed on its own. I
...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611831862)
i'm not sure it makes sense to duplicate documentation that's already in the manual pages. Like if we're doing this here, why not for every system call we make. Do parameter names like `*newp=` `oldp=` even elucidate much?

Agree re using `nullptr` where possible.

💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1611833798)
The `TxStateBlockConflicted` requires both that's why I added the both of them

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611834614)
Yes, it's a minor optimization, we could always construct a CNetAddr if we wanted, but if we know we're not going to use it anyway might as well skip it.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611838998)
Well, i guess it wouldn't hurt checking (or adding an `Assume`), but i don't think the routing table is supposed to contain error entries.

🤔 glozow reviewed a pull request: "refactor prep for package rbf"
(https://github.com/bitcoin/bitcoin/pull/30072#pullrequestreview-2074201852)
ACK 63cfa4ce88

💬 glozow commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1611833441)
Is there a reason for `struct` (seems like a C thing?)
```suggestion
SubPackageState m_subpackage;
```

💬 glozow commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1611842178)
Seems this will need to be deleted immediately in the next PR?

💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1611846940)
> Any reason why `conflicting_block_hash` is passed by reference but `replacement_tx` isn't?

`CTransactionRef` is already a pointer https://github.com/bitcoin/bitcoin/blob/e163d864d380956a4c0f89a4d80a76f5aefc9a08/src/primitives/transaction.h#L423-L424