Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 sedited commented on pull request "doc: clarify libbitcoinkernel usage in libraries design":
(https://github.com/bitcoin/bitcoin/pull/34042#issuecomment-3637868117)
This has been discussed in https://github.com/bitcoin/bitcoin/pull/28690, which introduces a libbitcoin_kernel target. In the past the design doc was understood as a goal to work towards. I think we should finally decide whether to move forward with #28690, and if not, accept the changes you laid out here.
👋 fanquake's pull request is ready for review: "depends: Boost 1.90.0"
(https://github.com/bitcoin/bitcoin/pull/33428)
💬 fanquake commented on pull request "depends: Boost 1.90.0":
(https://github.com/bitcoin/bitcoin/pull/33428#issuecomment-3637893169)
Updated to `1.90.0`.
💬 billymcbip commented on pull request "test: Improve STRICTENC/DERSIG unit tests in script_tests.json":
(https://github.com/bitcoin/bitcoin/pull/33973#issuecomment-3637941000)
@darosior I added the STRICTENC tests back. Maintainers can we run CI?
💬 maflcko commented on pull request "util: Add some more Unexpected and Expected methods":
(https://github.com/bitcoin/bitcoin/pull/34032#discussion_r2607376375)
thx, done
💬 maflcko commented on pull request "util: Add some more Unexpected and Expected methods":
(https://github.com/bitcoin/bitcoin/pull/34032#discussion_r2607376833)
thx, done
💬 maflcko commented on pull request "test: Detect truncated download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/34040#discussion_r2607401275)
I am not touching those lines, so I'll leave this as-is for now
💬 maflcko commented on pull request "test: Detect truncated download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/34040#discussion_r2607405653)
will leave as-is for now
💬 maflcko commented on pull request "test: Detect truncated download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/34040#discussion_r2607407049)
`black` prefers this, so I'll leave this as-is for now
💬 stickies-v commented on pull request "kernel: Improve logging API":
(https://github.com/bitcoin/bitcoin/pull/33847#discussion_r2607414440)
I was actually wrong about my earlier example about chainman owning context: it is [passed](https://github.com/bitcoin/bitcoin/blob/56ce78d5f62c9eb9353d33eedd15495c1205465f/src/kernel/bitcoinkernel.h#L967) as a non-owning (`const` ptr) view, and lifetime then ensured via [`std::shared_ptr`](https://github.com/bitcoin/bitcoin/blob/56ce78d5f62c9eb9353d33eedd15495c1205465f/src/kernel/bitcoinkernel.cpp#L472). Whether we prevent destroying or automatically extend lifetimes is not really my concern he
...
🚀 fanquake merged a pull request: "test: Detect truncated download in get_previous_releases.py"
(https://github.com/bitcoin/bitcoin/pull/34040)
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2607449049)
4564e571f2ef22ac096c58033137154e1e4b4cf7
💬 cobratbq commented on pull request "doc: update interface, --stdin flag, `signtx` (#31005)":
(https://github.com/bitcoin/bitcoin/pull/33765#discussion_r2607459246)
Not all of the text is by my own judgement. However, you might leave out BIP-32 derivations for keys not under own management. Technically, those were never expected to be signed anyways.
💬 l0rinc commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3638047678)
ACK 8b417087aec4671a1ce58f2331d1688e665d9935

The implementation doesn't slow down the critical path anymore and is beautifully isolated from the rest of the raw block reading.
The tests contain the extremes (added since last ACK), docs were update, braces added.
💬 l0rinc commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2607375727)
It came back, as expected, it shows the benchmark getting slower because of the vector recreation (making it more realistic):
<img width="711" height="121" alt="image" src="https://github.com/user-attachments/assets/acb14880-0371-4a8a-910e-5e04bbf79aa3" />

---

I have checked that the refactor doesn't introduce any regression now 👍
💬 l0rinc commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2607453751)
nit: the above test makes res a `REQUIRE`, if you touch agan, consider unifying. Definitely not a blocker.
💬 l0rinc commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2607445185)
@romanz, can you please check if there's a performance difference in the RPC with and without a const here?
💬 mzumsande commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2607462801)
nothing for this PR, but since these things are quite common (recently #33121), I wonder if a mode `-test=cutoffexponentialtimers` or something similar would make sense which would cut off the long tail of all exponential timers (`rand_exp_duration`) at some percentile, so we don't need to deal with these probabilistic failures in functional tests.
💬 cobratbq commented on pull request "doc: update interface, --stdin flag, `signtx` (#31005)":
(https://github.com/bitcoin/bitcoin/pull/33765#discussion_r2607469350)
I took those from #33947. I'll tweak them a bit.
💬 l0rinc commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2607508978)
this is defintely too verbose, but some some explanation for the getters would be welcome