Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 rebroad commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2380661215)
Rather than the crude 10 minute timeout - why not actually check if the block is being downloaded from the chosen peer - i.e. work out the throughput and make a decision based on this (compared to typical throughput from other peers)?
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779584565)
Weren't we already doing partial validation?
Doing a full one for these examples wouldn't be *that* difficult - do you suggest we do that instead?
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779656652)
Got it, thanks for clarifying!
💬 naiyoma commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#issuecomment-2380717731)
Concept ACK: additional test coverage to check that an address is marked as "terrible" after 3 or more unsuccessful connection attempts.
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779666783)
Alternatively, if that's not desired, what if we replaced the dynamic widths with explicit `PadLeft`/`PadRight` calls, e.g.
```C++
result += tfm::format("<-> type net v mping ping send recv txn blk hb %s%s%s ",
PadRight("addrp", m_max_addr_processed_length),
PadRight("addrl", m_max_addr_rate_limited_length),
PadRight("age", m_max_age_length));
```
and
```C++
result += tfm::format("%s %s%s\n",

...
📝 Jitphanu-051 opened a pull request: "Initial commit"
(https://github.com/bitcoin/bitcoin/pull/30995)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
hebasto closed a pull request: "Initial commit"
(https://github.com/bitcoin/bitcoin/pull/30995)
📝 hebasto locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30995)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779720533)
do we need to keep the `strprintf("%s%s"` on line 563?
🤔 glozow reviewed a pull request: "test: switch MiniWallet padding unit from weight to vsize"
(https://github.com/bitcoin/bitcoin/pull/30718#pullrequestreview-2335386388)
ACK 7d1c8d0aa3ebe30d7633caf9bc4765927e4d6e08, lmk if you want to update the second commit
💬 glozow commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1779726643)
a few more 1000s not changed:

L104 `assert_greater_than_or_equal(tx_v3_child_almost_heavy["tx"].get_vsize() + tx_v3_child_almost_heavy_rbf["tx"].get_vsize(), 1000)`

L194 `assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], 1000)`
💬 RandyMcMillan commented on pull request "doc/build-osx.md:brew relinking note":
(https://github.com/bitcoin/bitcoin/pull/30993#discussion_r1779729009)
good point - i will move it to the end of the "brew install" list and make a more general instruction.

-thanks
💬 torkelrogstad commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2380864234)
Concept ACK
📝 torkelrogstad opened a pull request: "doc: update signet documentation related to build directories"
(https://github.com/bitcoin/bitcoin/pull/30996)
While setting up my own signet I noticed that the binary paths in the documentation for this is out of date, after build artifacts moved to the `build` directory. This PR mimics what happened in #30741
💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2380875224)
> running different benchmarks won't necessarily give us a clearer picture.

It could get confusing if they're not pointing in the same direction (which seems to be the case here).

----

I reran the previous full IBD on the previous setup with HDD, but this time until 860000 with 30gb memory.

<details>
<summary>benchmark</summary>

```bash
hyperfine --runs 3 \
--export-json /mnt/ibd-30611-full.json \
--parameter-list COMMIT 33adc7521cc8bb24b941d959022b084002ba7c60,391c87640d78d98
...
💬 mzumsande commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2380879313)
> True. Lets dive a bit deeper - what are the reasons to be concerned about somebody inspecting the traffic to my node?

Some thoughts off the top my head:
- You don't want others (e.g. network admins, internet providers, etc.) to know that you are running a bitcoin node at all. Maybe it's not allowed in your network / country etc., maybe for other reasons such as personal security.
- Transaction privacy

In both of these cases, if you accept inbounds they could just connect to you as an a
...
💬 theStack commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1779753414)
Thanks, updated!
💬 theStack commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#issuecomment-2380892059)
Thanks for the reviews! Tackled https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1779726643 and also had to rebase on master (as otherwise the CI would complain, probably because the PR didn't have the switch to CMake included yet).
📝 hebasto opened a pull request: "build: Switch to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/30997)
The currently used Qt 5.15 is approaching [EOL](https://www.qt.io/blog/qt-5.15-extended-support-for-subscription-license-holders). The recent migration of the Bitcoin Core's build system to CMake makes it possible to switch to Qt 6.

This PR is not fully functional yet, but it raises some conceptual questions that need to be discussed:

1. Migrating to Qt 6 breaks compatibility with Debian 11 / Ubuntu 20.04 for the release binaries: https://github.com/bitcoin/bitcoin/blob/cdff967c2891a0a9ea7
...
💬 hodlinator commented on pull request "DO NOT MERGE: Windows bitcoind stall debugging":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2381014709)
Partial victory! - Latest run https://github.com/hodlinator/bitcoin/actions/runs/11087284207 actually uploaded a .DMP file (but it wasn't a real stall, just forced it to happen).

Still remains to see if it loads nicely in the debugger.

Other remaining issue is that I used a hardcoded absolute path for the upload-artifact step since I'm still struggling to find the correct glob-expression. Can't assume it will be *node0* that stalls, and unsure if the path will be the same when removing *--
...