Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 sdaftuar commented on pull request "Make non-standard tx acceptance a peer option.":
(https://github.com/bitcoin/bitcoin/pull/27772#issuecomment-1566232510)
Rather than open more PRs that are duplicative of other recently rejected PRs, it would be more helpful to directly address the feedback that was previously given.

Are you unaware that some of the DoS attacks that are prevented by our standardness checks include situations where a block might be mined that would take a very long time to validate, a cost that would be felt by the whole network? See for instance https://bitcointalk.org/?topic=140078%7CCVE-2013-2292]]. It is absurd that this i
...
👋 hebasto's pull request is ready for review: "ci: Use docker image cache for "Win64 native [vs2022]" task"
(https://github.com/bitcoin/bitcoin/pull/27771)
💬 hebasto commented on pull request "lint: stop ignoring LIEF imports":
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1566243044)
> > or an issue with the LIEF stubs. Am following up.
>
> This has now been resolved upstream in LIEF. See [lief-project/LIEF#909](https://github.com/lief-project/LIEF/issues/909) & [lief-project/LIEF@2e06bdb](https://github.com/lief-project/LIEF/commit/2e06bdb4af4586d1f5dcb9e9ccb027b55d457e24).

LIEF 0.13.1 has just been released with the fix.
💬 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.