Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 ryanofsky commented on issue "test: `interface_ipc.py` (might?) start skipping if installed capnp version changes":
(https://github.com/bitcoin/bitcoin/issues/34016#issuecomment-3617445384)
Maybe this would get the failure to be reported properly:

```diff
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -19,7 +19,7 @@ from test_framework.wallet import MiniWallet
# Test may be skipped and not have capnp installed
try:
import capnp # type: ignore[import] # noqa: F401
-except ImportError:
+except ModuleNotFoundError:
pass

@asynccontextmanager
```
💬 hebasto commented on pull request "ci: Add IWYU job":
(https://github.com/bitcoin/bitcoin/pull/33810#issuecomment-3617452151)
@maflcko

Your suggestions have been taken. Thank you!
📝 stickies-v opened a pull request: "log: exempt all category-specific logs from ratelimiting"
(https://github.com/bitcoin/bitcoin/pull/34018)
Previously, running with `-debug=<category>` would exempt the debug and trace log messages in that category from rate limiting, but not the info/warning/error category-specific messages (which are rare). This is unintuitive and unnecessary.

When users run with `-debug`, we already assume they are power users and that they will have significantly higher log volumes, so there is no real benefit from suppressing info log messages in that category.

Fix this by applying ratelimiting exceptions
...
💬 stickies-v commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#issuecomment-3617473369)
> I probably won't get to picking that up anytime soon though, so if you want to open an alternative to this PR, feel free to do so!

I've opened #34018 as an alternative!
💬 fanquake commented on issue "test: `interface_ipc.py` (might?) start skipping if installed capnp version changes":
(https://github.com/bitcoin/bitcoin/issues/34016#issuecomment-3617499308)
@ryanofsky yes, that works.
👍 dergoegge approved a pull request: "fuzz: Add a test case for `ParseByteUnits()`"
(https://github.com/bitcoin/bitcoin/pull/34017#pullrequestreview-3545393178)
utACK 57b888ce0ebdeb34d866fd1511052fd740cc5ab8

Thank you for your interest in contributing to our fuzzing efforts! This looks fine to me.

`ParseByteUnits` is not publicly exposed, i.e. it doesn't handle untrusted inputs, and I would not consider adding fuzz tests for this type of function as a priority. As the in-repo fuzz tests are pretty saturated, it can be hard to spot valuable areas to improve (especially if you are new to the code base). A good path for making valuable contributions
...
👍 stickies-v approved a pull request: "scripted-diff: Use LogInfo over LogPrintf"
(https://github.com/bitcoin/bitcoin/pull/29641#pullrequestreview-3545434794)
ACK fa4395dffd432b999002dfd24eb6f8d7384fbcbe

nit: would be nice to update the scripted-diff to also remove trailing newlines if that's not a huge pita
⚠️ darosior opened an issue: "RFC: randomize over netgroups in outbound peer selection"
(https://github.com/bitcoin/bitcoin/issues/34019)
The current mechanism for choosing outbound peers picks one at randoms among known-reachable addresses, with the caveat that we do not connect twice to the same netgroup (by default /16's and, if an ASmap is configured, by AS's). A more robust mechanism for preventing an attacker to control all of a node's outbound connections would first randomize over netgroups and then pick a known-reachable address within that netgroup.

This alternative mechanism would make the probability for an attacker t
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3617582525)
`2d398050ee...810661dc07`: address suggestions (minor changes) and add release notes
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2593260778)
Added
💬 fanquake commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2593287377)
You can remove any `// for ` comments like this (#32562).
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3617672373)
`810661dc07...f391296b17`: do https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2593287377
💬 fanquake commented on issue "Split socket handling out of CConnman":
(https://github.com/bitcoin/bitcoin/issues/30694#issuecomment-3617709020)
Can you update the issue description to remove mentions of SV2 (given it doesn't need or uses this), and probably the libevent removal, given that seems to be taking a different approach now? I guess this could also be closed, given all relevant discussion is happening in the PR (#30988).
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3617721711)
The flames look impressive, my dfferential flames for all 900k blocks should also finish in a few days.

### Parallelism vs speedup on different platforms

<img width="1268" height="910" alt="image" src="https://github.com/user-attachments/assets/b9a7538d-0e28-46bf-b1cc-6861cc459bd8" />

<details>
<summary>reindex-chainstate | 700000 blocks | dbcache 450 | M4-Max.local | arm64 | Apple M4 Max | 16 cores | 64.0GiB RAM | SSD | macOS 26.1 25B78 | Apple clang version 17.0.0 (clang-1700.4.4.1)<
...
🤔 stickies-v reviewed a pull request: "Add util::Expected (std::expected)"
(https://github.com/bitcoin/bitcoin/pull/34006#pullrequestreview-3545590480)
Approach ACK fa76a6620012fb738639d8fd7ce17b185bfd376c
💬 stickies-v commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2593365020)
This should probably be `[[nodiscard]]`? Could perhaps more generally enforce that with `bugprone-unused-return-value` for `std::expected`?
💬 stickies-v commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2593341384)
`std::unexpected` exposes [`std::unexpected::error()`](https://en.cppreference.com/w/cpp/utility/expected/unexpected.html#error) instead of `.err`. Since this is a public member, it might get used and thus make drop-in replacements a bit more involved?
💬 fanquake commented on pull request "depends: Propagate native C compiler to `sqlite` package":
(https://github.com/bitcoin/bitcoin/pull/33995#issuecomment-3617807200)
Guix Build (x86_64):
```bash
5ea5588f1e2ee4e37f4b90313a8c32ec17474a39d1dff77d9d585ae9e106c761 guix-build-710031ebef83/output/aarch64-linux-gnu/SHA256SUMS.part
8d7b95ecb5950220f6d70c069d7fdf5add92f8135daee0d0acb9af753c9bab0c guix-build-710031ebef83/output/aarch64-linux-gnu/bitcoin-710031ebef83-aarch64-linux-gnu-debug.tar.gz
dd0f06c48c57e1243437298d02c219758ecd167c88d3a59be15a9051434a99cb guix-build-710031ebef83/output/aarch64-linux-gnu/bitcoin-710031ebef83-aarch64-linux-gnu.tar.gz
fe143b6
...
💬 fanquake commented on pull request "depends: Propagate native C compiler to `sqlite` package":
(https://github.com/bitcoin/bitcoin/pull/33995#issuecomment-3617808101)
ACK 710031ebef838d2f0a1effa19170bef7b130bbeb
🚀 fanquake merged a pull request: "depends: Propagate native C compiler to `sqlite` package"
(https://github.com/bitcoin/bitcoin/pull/33995)