Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 l0rinc commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2048791750)
```suggestion
LogError("Failed to close file: %s", SysErrorString(errno));
```
💬 l0rinc commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2048785183)
Can you separate the `AutoFile` changes to a new commit, explaining these changes separately from the changes that are calling close explicitly?
💬 hebasto commented on pull request "depends: bump boost to 1.87.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#issuecomment-2812702334)
> > @hebasto Want to try putting that behind a variable and upstreaming it? It'd be nice if we didn't have to carry that patch.
>
> Sure thing!
>
> Done in [boostorg/test#445](https://github.com/boostorg/test/pull/445).

[Here](https://github.com/hebasto/bitcoin/commits/250417-boost-cmake/) is an updated branch based on the upstream changes.
💬 ismaelsadeeq commented on issue "Test Package Accept":
(https://github.com/bitcoin/bitcoin/issues/32160#issuecomment-2812704410)
Given that we want `testmempoolaccept` to relax the requirement that the transaction should be topologically connected due to the reasons discussed above, should we consider extending `submitpackage` to do the same since as you mentioned in practice, some transactions already in the mempool can cause the submitted package to become topological, the submitted package may not appear connected the outset because it was deduplicated (but it is with some in-mempool transaction).

> This allows us to
...
💬 l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2812715464)
> but I am not really reproducing any of the performance changes on my machine yet

I just removed the single-byte specializations, they were quite ugly, will have to add it back somehow later - will draft it until I do that, thanks for checking!
📝 l0rinc converted_to_draft a pull request: "[IBD] specialize block serialization"
(https://github.com/bitcoin/bitcoin/pull/31868)
This change is part of [[IBD] - Tracking PR for speeding up Initial Block Download](https://github.com/bitcoin/bitcoin/pull/32043)

### Summary

This PR contain a few different optimization I found by IBD profiling, and via the newly added block seralization benchmarks. It also takes advantage of the recently merged [`std::span` changes](https://github.com/bitcoin/bitcoin/pull/31519) enabling propagating static extents.

The commits merge similar (de)serialization methods, and separates th
...
🚀 fanquake merged a pull request: "fuzz: Avoid integer sanitizer warnings in policy_estimator target"
(https://github.com/bitcoin/bitcoin/pull/32154)
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2048861716)
Expanded the comment and commit description.
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2812748207)
Rebased after #32271.
💬 fanquake commented on pull request "ci: switch to LLVM 20 in tidy job":
(https://github.com/bitcoin/bitcoin/pull/32226#issuecomment-2812748458)
@l0rinc I've taken some of these, but I'm not planning on making further refactors here, just updating the infra. Happy to review a followup.
:lock: fanquake locked an issue: "Bitcoin Core Config Generator"
(https://github.com/bitcoin/bitcoin/issues/32106)
📝 TheCharlatan opened a pull request: "doc: Add deps install notes for multiprocess"
(https://github.com/bitcoin/bitcoin/pull/32293)
These just mirror the content in src/ipc/libmultiprocess/doc/install.md
💬 hebasto commented on pull request "depends: bump boost to 1.87.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#issuecomment-2812764618)
> > @hebasto Want to try putting that behind a variable and upstreaming it? It'd be nice if we didn't have to carry that patch.
>
> Sure thing!
>
> Done in [boostorg/test#445](https://github.com/boostorg/test/pull/445).

https://github.com/boostorg/test/pull/445 has just been merged.
💬 l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2812765790)
@TheCharlatan, I've added back the single-byte optimizations (simplified slightly after rebase), your feedback is appreciated.
👋 l0rinc's pull request is ready for review: "[IBD] specialize block serialization"
(https://github.com/bitcoin/bitcoin/pull/31868)
⚠️ maflcko opened an issue: "feefrac_mul_div: Integer-overflow in FeeFrac::Div"
(https://github.com/bitcoin/bitcoin/issues/32294)
https://issues.oss-fuzz.com/issues/411172125:

```
echo '//v////7/////f////////8=' | base64 --decode > /tmp/dat

UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=feefrac_mul_div ./bld-cmake/bin/fuzz /tmp/dat
```

```
src/util/feefrac.h:99:21: runtime error: signed integer overflow: 9223372036854775807 + 1 cannot be represented in type 'int64_t' (aka 'long')
💬 l0rinc commented on pull request "ci: switch to LLVM 20 in tidy job":
(https://github.com/bitcoin/bitcoin/pull/32226#issuecomment-2812782111)
ACK e1254c4bd99441ed954480a5883132786ac3c36a

<details>
<summary>Changes since last ACK</summary>

```patch
diff --git a/src/common/args.cpp b/src/common/args.cpp
index ab84d32722..1e3f6d1b88 100644
--- a/src/common/args.cpp
+++ b/src/common/args.cpp
@@ -189,7 +189,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
// internet" warning, and clicks the Open button, macOS passes
// a unique process serial number (PSN) as -psn_... com
...
💬 glozow commented on issue "Test Package Accept":
(https://github.com/bitcoin/bitcoin/issues/32160#issuecomment-2812786609)
> Given that we want testmempoolaccept to relax the requirement that the transaction should be topologically connected

There is already no requirement of connectedness now

> Given that we want testmempoolaccept to relax the requirement that the transaction should be topologically connected due to the reasons discussed above, should we consider extending submitpackage to do the same since as you mentioned in practice, some transactions already in the mempool can cause the submitted package to b
...
💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock, unhide wait methods in help":
(https://github.com/bitcoin/bitcoin/pull/30635#issuecomment-2812790873)
Ready for review now that #30635 landed.
👋 Sjors's pull request is ready for review: "rpc: add optional blockhash to waitfornewblock, unhide wait methods in help"
(https://github.com/bitcoin/bitcoin/pull/30635)