💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1132837417)
good point about checking bounds. one way to do that is with simple checks like `assert(bucket <= ADDRMAN_TRIED_BUCKET_COUNT)` etc.

> Maybe change the arrays to std::array and use .at() here.

but I'd like to understand this option better. right now `vvTried` and `vvNew` are declared as C-style arrays. is your recommendation to change those `AddrManImpl` declarations so that we can utilize the bounds checking of the `.at()` function?

💬 adamjonas commented on issue "Consensus failure while upgrading bitcoind 0.8.3 > 0.22.0":
(https://github.com/bitcoin/bitcoin/issues/23913#issuecomment-1464375907)
There hasn't been forward progress on this for over a year. Appears to be a won't fix.

✅ adamjonas closed an issue: "Consensus failure while upgrading bitcoind 0.8.3 > 0.22.0"
(https://github.com/bitcoin/bitcoin/issues/23913)

💬 adamjonas commented on issue "Add CBOR RPC interface":
(https://github.com/bitcoin/bitcoin/issues/22866#issuecomment-1464379624)
There doesn't seem to be any forward progress on this feature request. Closing based on staleness.

✅ adamjonas closed an issue: "Add CBOR RPC interface"
(https://github.com/bitcoin/bitcoin/issues/22866)

💬 Xekyo commented on issue "Option to ignore small inputs when internal wallet is building TXes?":
(https://github.com/bitcoin/bitcoin/issues/20870#issuecomment-1464384675)
The Knapsack algorithm was kept as is, and still always permits uneconomical UTXOs. So, occasionally, the input set proposed by Knapsack could include an uneconomical UTXO in its inputs and still be the least wasteful input set overall. I would suggest that we remove Knapsack, and at the same time loosen the input selection for other algorithms, so we permit spending of uneconomical inputs when building transactions at `minRelayTxFeeRate`. That way we retain the Bitcoin Core wallet’s propensity
...

💬 adamjonas commented on issue "Coin selection algorithm proposal":
(https://github.com/bitcoin/bitcoin/issues/23164#issuecomment-1464458201)
This has gone stale and @brunoerg confirmed can close.

✅ adamjonas closed an issue: "Coin selection algorithm proposal"
(https://github.com/bitcoin/bitcoin/issues/23164)

💬 stratospher commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1132895083)
why do we mention outgoing? (isn't it incoming only)

👍 1440000bytes approved a pull request: "p2p: set `-dnsseed` and `-listen` false if `maxconnections=0`"
(https://github.com/bitcoin/bitcoin/pull/26899)
reACK https://github.com/bitcoin/bitcoin/pull/26899/commits/fabb95e7bf02f3d8e663a02dd845d42e09d330ec

💬 mzumsande commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1132925968)
I don't understand what this part of the test is testing - calling `SaveBlockToDisk` for a block at a specified but incorrect position is something that should never be possible in an actual, non-corrupted node unless I'm missing something.
But if it somehow happened anyway, even if the existing block isn't overwritten, I'd imagine that it would still cause some havoc, like changing block file statistics [here](https://github.com/bitcoin/bitcoin/blob/c7f1d95f52883d7087b74f45f5e4ce1100d51149/sr
...

💬 TheCharlatan commented on pull request "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks":
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1464563233)
On master I can successfully run `MAKEJOBS="-j12" FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh`. When I do this now (with all prior images removed) I get:

```
python3: can't open file '/home/drgrid/bitcoin/ci/scratch/iwyu/include-what-you-use/iwyu_tool.py': [Errno 2] No such file or directory
python3: can't open file '/home/drgrid/bitcoin/ci/scratch/iwyu/include-what-you-use/fix_includes.py': [Errno 2] No such file or directory
```

💬 mzumsande commented on pull request "blockstorage: add an assert to avoid running oom with `-fastprune`":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1132967989)
Nice idea! I think that would work - I could do a full signet sync with it.

I'm still a bit undecided on general grounds, i.e. if it's okay to add more logic to accommodate a test-only mode that doesn't make sense for anything else but easier testing of pruning in the functional and unit tests. One could also argue that if no one but a developer would ever have a reason to use it, it'd be ok to just tell them more clearly not to create large blocks alongside using this option and abort otherw
...

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1132968822)
oh interesting, I thought that the compiler optimizes away these sort of named-variables-used-shortly-after patterns, but maybe it's different for `std::string`? I took it to compiler explorer to see if I could observe the allocation- https://godbolt.org/z/GG78zv3KT. are the calls to `std::allocator<char>` what you are referring to?

⚠️ Lokoluis opened an issue: "Concept ACK"
(https://github.com/bitcoin/bitcoin/issues/27241)
Concept ACK

_Originally posted by @fanquake in https://github.com/bitcoin/bitcoin/issues/27233#issuecomment-1464352514_

✅ fanquake closed an issue: "Concept ACK"
(https://github.com/bitcoin/bitcoin/issues/27241)

:lock: fanquake locked an issue: "Concept ACK"
(https://github.com/bitcoin/bitcoin/issues/27241)

💬 fanquake commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#issuecomment-1464875015)
Tested 308ea03d95786db674e8cce3e78a56b498cf119a rebased on master (c7f1d95f52883d7087b74f45f5e4ce1100d51149) and I'm no-longer seeing the issue in #27229: `wallet_importdescriptors.py --descriptors | ✓ Passed | 652 s`

🚀 fanquake merged a pull request: "doc: update broken str util reference links on developer-notes"
(https://github.com/bitcoin/bitcoin/pull/27220)

✅ fanquake closed an issue: "Bug: ArgsManager::ReadSettingsFile can return false even when it does load settings"
(https://github.com/bitcoin/bitcoin/issues/22638)