Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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 *--
...
💬 jarolrod commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2381026307)
concept ack 🌞
💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#issuecomment-2381032408)
crACK cb9be6729c4e27bf2d6d703c0d454a22cbcdb6e1

This simplifies the CCoinsCacheEntry interface, which was leaking the flags bitfield as an implementation detail. Now we don't have to worry about fuzzing that, and it will make removing the FRESH-but-not-DIRTY or FRESH-and-spent cases easier in https://github.com/bitcoin/bitcoin/pull/30673.

Also, the tests are much saner now and easier to parse by also removing the flags interface there.

There are some minor issues with the commits though.
...
👋 russeree's pull request is ready for review: "Enhanced error messages for invalid network prefix during address parsing."
(https://github.com/bitcoin/bitcoin/pull/27260)
💬 russeree commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-2381264024)
> 🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/30813837174


```
[07:54:50.521] + docker run --rm docker.io/amd64/ubuntu:24.04 bash -c 'env | grep --extended-regexp '\''^(HOME|PATH|USER)='\'''
[07:54:50.522] + tee --append /tmp/env-ci_worker_1726594866_672026932-ci_i686_multiprocess
[07:54:50.522] Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
[07:54:50.548] time="2024-09-29T07:54:50Z" level=warning msg="RunRoot is
...
⚠️ russeree opened an issue: "CI : Permission Denied - Multiple PRs"
(https://github.com/bitcoin/bitcoin/issues/30998)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Currently Multiple PRs seem to be failing CI due to permission issues.

https://github.com/bitcoin/bitcoin/pull/30997/checks?check_run_id=30815220500
https://github.com/bitcoin/bitcoin/pull/27260/checks?check_run_id=30814537553

All failures seen happen at approx 12 seconds.

![image](https://github.com/user-attachments/assets/3855d21b-33aa-47f5-88e8-516adfd2f745)


### Expected b
...
💬 russeree commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1779998302)
The choice to use chainparams to store the Base58 address prefixes has been utilized in the most recent pull. This solution simplifies the PR a noticeable amount. Long term it would be easier to modify default chain address constants than it would be to maintain the deterministic decoding mechanisms.
💬 russeree commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1779998510)
Thank you for this insight, the faulty logic has been removed as a part of the rebase/refactor in the most recent commit.
📝 hebasto opened a pull request: "qt6: Handle deprecated `QLocale::nativeCountryName`"
(https://github.com/bitcoin-core/gui/pull/838)
Split from https://github.com/bitcoin/bitcoin/pull/30997.

[`QLocale::nativeCountryName()`](https://doc.qt.io/qt-6/qlocale-obsolete.html#nativeCountryName) has been deprecated since Qt 6.6.

[`QLocale::nativeTerritoryName()`](https://doc.qt.io/qt-6/qlocale.html#nativeTerritoryName) was introduced in Qt 6.2.

This PR ensures compatibility across all supported Qt versions.

No behaviour change for the current codebase, which uses Qt 5.

---

This PR can be tested on macOS using the fir
...
📝 hebasto opened a pull request: "qt6, test: Handle deprecated code"
(https://github.com/bitcoin-core/gui/pull/839)
Split from https://github.com/bitcoin/bitcoin/pull/30997.

This PR ensures compatibility across all supported Qt versions.

---

This PR can be tested on macOS using the first commit from https://github.com/bitcoin/bitcoin/pull/30997 and Homebrew's `qt` package.
💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2381360747)
> Concept ACK
>
> Feels like _p2p_orphan_handling.py_/`test_same_txid_orphan()` might benefit from some augmented checking using this new `getorphantxs`. It could verify that same-txids-differing-wtxid orphans are handled in the expected way, with the bogus one being removed after the one with the valid witness gets adopted.

Great idea. Agreed. A branch was started (https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2327703573) to capture changes to `p2p_orphan_handling.py`. Update
...
💬 Khalilheyrani6367 commented on pull request "[refactor] Cleanup BlockAssembler mempool usage":
(https://github.com/bitcoin/bitcoin/pull/28843#issuecomment-2381363217)
60a0f2ac88c57fdf4482dade2ce409ef2da65998
🤔 glozow reviewed a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2323753194)
Thanks for the review @l0rinc. I've gone through them and addressed some of your comments, and marked some as out of scope. As you can see this PR is fairly large, so it is my preference not to add changes to the code that is being moved. Perhaps we can take some into a followup.

> I'll provide a higher-level review later, once I understand the problem better. I still need a few iterations to be able to zoom out and comment meaningfully, for now I only left nits about implementation specifics
...
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1780058381)
done
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1772385648)
- Are the other parameters not self-explanatory?
- What do you mean by weird?