💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2138571084)
`MaxPeerAnnouncements()` is affected by the number of peers, since it's the global limit / num peers.

However, I just realized this comment is also wrong because `m_peer_orphanage_info` size *can* change during the call. I think it's only possible when the per-peer reservation is below 400k, so we allow adding a tx, but immediately remove it because the peer is using "too much" space. Weird, but possible.

💬 PeterWrighten commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#issuecomment-2960288134)
> > I cannot run some CIs and tests on GitHub.
>
> Approved, they should be run automatically now.

Seems that I still could not run all ci tests without approval. Can you fix it?

💬 davidgumberg commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2138583239)
Looks good to me

💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2138610576)
Oh, duh, it's the per-peer announcement reservation, not the global announcement limit. :man_facepalming:.

But I don't think it matters really that this is a constant. We'd be ok with using `global_announcement_limit / total_peers` as per-peer reservation, but we are - as a resource optimization on top - using `global_announcement_limit / peers_with_at_least_one_orphan` instead. Not updating the constant here when the last orphan of a peer disappears means using something in between temporari
...

📝 achow101 opened a pull request: "wallet, rpc: Remove deprecated balances from getwalletinfo and getunconfirmedbalance"
(https://github.com/bitcoin/bitcoin/pull/32721)
`getwalletinfo` result fields `balance`, `immature_balance`, and `unconfirmed_balance`, and the `getunconfirmedbalance` RPC have all been deprecated since 0.19.0. It's been long enough that they should either be removed or undeprecated. The functionality provided by these RPCs is provided by `getbalances`.

💬 bigshiny90 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2960344612)
> > I don't think people understand that this is _by default_. You can still just set your own limits in your config. If you disagree, you can just change it on your nodes.
> > This was technically the correct thing to do, even if all the bots on X said otherwise. This whole thing should've been a big nothingburger. Most of you all commenting on the issue aren't understanding the effect of the changes at all. Mob behaviour towards devs should not be tolerated.
>
> It's on Core to have sane d
...

💬 glozow commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#issuecomment-2960356836)
> The developer notes could point to the incantation to run all the linters in a container.

Assuming I understand you correctly, that does exist here https://github.com/bitcoin/bitcoin/tree/master/test/lint#test-runner

🚀 glozow merged a pull request: "doc: Remove stale sections in dev notes"
(https://github.com/bitcoin/bitcoin/pull/32572)

🤔 instagibbs reviewed a pull request: "p2p: improve TxOrphanage denial of service bounds"
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-2910938872)
reviewed through 128ad62cd68038641ac7c3308ceb40c6c84d325e

CI failure seems unrelated

I'm going to think more about how evicting peers completely interacts with the behavior.

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2136251223)
498f1c019197a8e4105490cdc4a0605594ca97d5

if this line is hit, the `rit` iterator is never incremented; infinite loop?

alternative formulation to consider for the entire conditional starting at https://github.com/bitcoin/bitcoin/pull/31829/commits/498f1c019197a8e4105490cdc4a0605594ca97d5#diff-e6100361fa0e9e25478f808ca084e5f681d4dddbbee7b3bea0f9d5bcd29db3aaR596
```
for (auto rit = std::make_reverse_iterator(it_upper);
rit != std::make_reverse_iterator(it_lower); ++rit)
{
if
...

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2137941928)
22d6cdd4f9dd6e03ad88946c130dad98fc45d7ad

why signed int?

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2138115140)
a703a3086a6a3a6250fb97e799712443eaedf5d0

and helps determine the total memory limits based on number of entries

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2137961509)
9afbf15b99508982b1a73bc416246ffbbce22d89

was this logic change necessary for this commit?

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2138082482)
a703a3086a6a3a6250fb97e799712443eaedf5d0

exactly one *announcement* for this wtxid I presume

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2138563428)
nit(?): is it more of an AnnouncementMap?

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2138586549)
"copying one that exists" I assume means grabbing the CTransactionRef?

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2138044941)
a703a3086a6a3a6250fb97e799712443eaedf5d0

I think you can `const` all fields except `m_reconsider`?

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2138569907)
c21466b83d725ab38e8b2b6c5b3e01815b300745

it's not actually bytes; we we want to just refer to Usage() directly here

💬 glozow commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#issuecomment-2960426526)
Needs release note?

💬 pablomartin4btc commented on pull request "wallet, refactor: Remove Legacy wallet unused warnings and errors":
(https://github.com/bitcoin/bitcoin/pull/32481#issuecomment-2960434399)
_<ins>Updates</ins>_:
* Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2138353475) from @maflcko [and](https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2138470337) @achow101.
* Removed also wallet descriptor check from `CWallet::EncryptWallet()` and `CWallet::Create()`.