💬 ryanofsky commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1566300943)
> GitHub isn't allowing me to reopen this

In order to reopen the PR, the branch needs to point at the same commit it was pointing to at the time the PR was closed.

Right now the branch https://github.com/LarryRuane/bitcoin/commits/2023-05-resource-pool-system-alloc points to d25b54346fed931830cf3f538b96c5c346165487, but the PR points to 576a270bbcbb21725770b07ba32af8e0c8965f44. If you temporarily force push 576a270bbcbb21725770b07ba32af8e0c8965f44 to the branch, I think you should be able
...

💬 theStack commented on issue "rpc_getblockfrompeer.py intermittent failure: assert_equal(pruneheight, 248); not(249 == 248)":
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566354933)
After digging deeper and analyzing the [failed CI log](https://api.cirrus-ci.com/v1/task/4776911524593664/logs/ci.log), I got a clearer picture what's going on here.
Comparing the "Leaving block file" debug messages from a successful and the failed test run gave the
essential hint:

```
$ grep "Leaving block" successful_run.log
Leaving block file 0: CBlockFileInfo(blocks=249, size=65319, heights=0...248, time=2011-02-02...2023-05-28)
Leaving block file 1: CBlockFileInfo(blocks=251, size=6
...

💬 Crypto2 commented on issue "listunspent, fundrawtransaction, getwalletinfo locks wallet for any other operation":
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-1566421689)
> `listunspent`, `fundrawtransaction`, and the `send*` RPCs all require figuring out the UTXOs that belong to the wallet. Currently, that means iterating through every single output of every transaction in the wallet, including received, sent, and unrelated, all while holding the wallet lock to ensure that atomicity of the operation.

I've suggested before just having a second map of only unspents, should speed it up massively at the expense of some memory. Could be default off and have to ena
...

💬 LarryRuane commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1566448078)
Thanks, Ryan!

📝 LarryRuane reopened a pull request: "util: generalize accounting of system-allocated memory in pool resource"
(https://github.com/bitcoin/bitcoin/pull/27748)
Follow-on to PR #25325, "Add pool based memory resource"

The `DynamicUsage()` function for the version of `unordered_map` that uses the `PoolAllocator` returns a close approximation of the amount of physical memory used by the map. (This is the map used for the dbcache.) It accounts for several of the allocator's internal data structures, such as the memory chunks and the freelist. It also includes the `unordered_map`'s bucket array (vector), which is a bit out of place because the rest of th
...

💬 mzumsande commented on issue "rpc_getblockfrompeer.py intermittent failure: assert_equal(pruneheight, 248); not(249 == 248)":
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566554075)
Nice analysis!

> Not sure how to best solve this issue. Is there a way to enforce linear blocks reception at least when only having a single peer?

I think the cause is that a failed DoS check
`if (pindex->nHeight <= m_chainman.ActiveChain().Height() + 2) ` ([link](https://github.com/bitcoin/bitcoin/blob/7d33ae755de2bff806fe600bdaebedbd7fa67aba/src/net_processing.cpp#LL4342C9-L4342C72))
will revert to HeadersProcessing if the chain of node2 hasn't caught up enough (the new block is too fa
...

🤔 MarcoFalke reviewed a pull request: "ci: Add test coverage job"
(https://github.com/bitcoin/bitcoin/pull/27547#pullrequestreview-1448964603)
I presume there is a setting to disable the comment and just have DrahtBot add a link in the form of `https://app.codecov.io/gh/{owner}/{repo}/pull/{number}` to the summary comment?

💬 MarcoFalke commented on pull request "ci: Add test coverage job":
(https://github.com/bitcoin/bitcoin/pull/27547#discussion_r1208978341)
Also, could move up the task to group it with the other native linux tasks, away from the macos and android task down here?

💬 MarcoFalke commented on pull request "ci: Add test coverage job":
(https://github.com/bitcoin/bitcoin/pull/27547#discussion_r1208977858)
any reason to use lunar, when an Ubuntu LTS or debian bookworm can be used?

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#discussion_r1208994069)
I don't think running macOS on Linux is in scope for the CI system.

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#discussion_r1208999859)
thx, mentioned in commit description

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#discussion_r1209001490)
Also, this was never supported, so seems unrelated to the changes here?

💬 MarcoFalke commented on pull request "refactor: Replace `std::optional<bilingual_str>` with `util::Result`":
(https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1566720140)
Yeah, that's why I suggested `{std::move()}`, not `std::move()`, see https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1561270092

💬 kallewoof commented on pull request "test: Throw error when -signetchallenge is non-hex":
(https://github.com/bitcoin/bitcoin/pull/27765#issuecomment-1566738266)
ACK fa6b11a

💬 hebasto commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#discussion_r1209045977)
You are right.

👍 hebasto approved a pull request: "ci: Add missing set -e to 01_base_install.sh"
(https://github.com/bitcoin/bitcoin/pull/27739#pullrequestreview-1449096752)
ACK fa12558d21aa03c22407a1458a0345d8a7ab6a4b

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1566747695)
CI failure seems unrelated.

👍 hebasto approved a pull request: "kernel: Remove util/system from kernel library, interface_ui from validation."
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1449106502)
re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e, only last two commits dropped since my [recent](https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1435394620) review.

💬 TheCharlatan commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1566766689)
Re https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564529747

> Thanks, the last commit should fix your bug.

Thank you for digging into this, seems like everything is working as it should now.
ACK [fa12558](https://github.com/bitcoin/bitcoin/pull/27739/commits/fa12558d21aa03c22407a1458a0345d8a7ab6a4b)

👍 real-or-random approved a pull request: "contrib/init: Better systemd integration"
(https://github.com/bitcoin/bitcoin/pull/25975#pullrequestreview-1449124089)
ACK 689a65d878638ae67b07f111dd77ea3c624e4f58 tested this service file with 25.0