💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1194103566)
Make sense, going to address it.

💬 glozow commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548213087)
> By the way, about the diamond example in https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1542695316

> This isn't a "child with parents" package

Sorry, that was a bad diagram (of the unit test I had added). The child is also spending the grandparent. I've updated the comment now.

💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1194105588)
noted. not convinced how desirable the additional code complexity is. would like to hear how other people think about code complexity too before deciding what to do next.

💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1548215577)
Rebased. Looking for feedback on [this comment](https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173966799).

💬 joostjager commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548224928)
It is still not completely clear to me what the risk is with merging this PR if #27611 is less restrictive. It would be great if it makes the next release, but also it is another six months delay for this important step.

💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1194120554)
I think it was accidental, sorry. I will revert it.

💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1548238840)
Force pushed addressing:
- https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191183121
- https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191191757
- https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191197012
- https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191201370

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1194127839)
perfect

💬 instagibbs commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548247127)
If we're not shooting for a backport for 25.x, I don't think the time difference is meaningful? This PR is adding stuff that will directly be removed and is sort of a distraction for reviewers themselves.

💬 ryanofsky commented on pull request "init: verify blocks data existence only once for all the indexers":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1548248095)
re: https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1544648748

> We could also move the entire `hasDataFromTipDown` from the interface to somewhere else in this way. So there is no `CBlockIndex` dependency in the interface neither here nor in the #24230 intermediate commits.

Yes I don't think it makes sense to expose that method as part of the `Chain` interface, and even if it did make sense, it wouldn't make sense to put the implementation in the `ChainImpl` class, because the
...

💬 Sjors commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1194135762)
Note to others: we may send outbound peers 7 * 2.5 * 60 * 2 ~= 2100 transaction ids per 2 minutes. Depending on random variation it can be more. `UNCONDITIONAL_RELAY_DELAY = 2min;` is the period of time that we have to remember what we sent to a specific peer. For privacy reason we don't want to reveal these transactions to _other_ (spy) peers yet. We do this tracking with a bloom filter `m_recently_announced_invs` that has a 1 in a million chance of messing up. I forgot if it's false positive (
...

💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1194151828)
Thank you for the time you took for these detailed comments. Fleshing this out to a more proper kernel interface sounds good to me. How about moving `ValidationNotifications` to the `KernelContext` instead? Or does that conflict with the vision you have for getting rid of `uiInterface` global?

💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1194168060)
> Thank you for the time you took for these detailed comments. Fleshing this out to a more proper kernel interface sounds good to me. How about moving `ValidationNotifications` to the `KernelContext` instead? Or does that conflict with the vision you have for getting rid of `uiInterface` global?

Hmm, It doesn't make sense to me that an object which is supposed to receive notifications from the kernel would be owned by the kernel. Outside code that wants to receive notifications should be able
...

💬 joostjager commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548293535)
Yes, backport was the idea 😎 😃

🤔 instagibbs reviewed a pull request: "validate package transactions with their in-package ancestor sets"
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1427036930)
looking good

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1194144678)
even more sensible than my suggestion :+1: no issues running so far

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1194150250)
ASCII :gem: needs connection between grandparent and child

💬 instagibbs commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548332581)
fwiw I'm ok getting this in for backport. This is most cruft at the RPC layer that can be reverted on the next PR.

💬 codexnakamoto commented on pull request "build: Drop support for g++-8":
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1548338939)
Concept ACK - this is logical and seems uncontentious.

👍 pinheadmz approved a pull request: "init: Error if ignored bitcoin.conf file is found"
(https://github.com/bitcoin/bitcoin/pull/27302#pullrequestreview-1426880326)
ACK 0319de5cbedd1a8f8766cfec61151c58b3fb27ef

Code review and run bitcoind and -qt with various conf file configurations to see errors and warnings. A few style nits below, can be ignored (especially the release note since that will get reviewed again upon actual release)

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 0319de5cbedd1a8f8766cfec61151c58b3fb27ef
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJ
...