Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 mossein commented on issue "combinerawtransaction confusing with distinct transactions":
(https://github.com/bitcoin/bitcoin/issues/25980#issuecomment-3621298976)
Closing in favor of #31298 which has been in review since November 2024 and addresses the same issue. I wasn't aware of the existing PRs when I opened this. I'll contribute by reviewing #31298 instead.

Apologies for the duplicate!
mossein closed a pull request: "rpc: Reject distinct transactions in combinerawtransaction"
(https://github.com/bitcoin/bitcoin/pull/34024)
💬 mossein commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-3621302427)
Tested ACK a26e9fd5b5908853d37b81915f5e12a5ad822f86

Built on macOS, ran
rpc_createmultisig.py
- passes. The approach of stripping scriptSigs/scriptWitnesses and comparing txids is clean.

Agree with the minor nits from other reviewers (style changes, error message verbosity).
🤔 furszy reviewed a pull request: "index: Deduplicate HashKey / HeightKey handling"
(https://github.com/bitcoin/bitcoin/pull/32997#pullrequestreview-3548499753)
utACK 5646e6c0d3581f12419913b88745f51c7a3161b9
📝 ajtowns opened a pull request: "net: Waste less time in socket handling"
(https://github.com/bitcoin/bitcoin/pull/34025)
Cuts out some wasted time in net socket handling. First, only calculates the current time once every 50ms, rather than once for each peer, which given we only care about second-level precision seems more than adequate. Second, caches the value of the `-capturemessages` setting in `CConnman` rather than re-evaluating it every time we invoke `PushMessaage`.
💬 ajtowns commented on pull request "net: Waste less time in socket handling":
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3621332052)
Not sure if the flame graph is usable, but:

![perf](https://github.com/user-attachments/assets/e3d6595b-d48c-4582-8bb2-6a05eacd6ca8)

GetBoolArg takes up 0.31% of total time, as part of PushMessage that takes up 1.75% of total time, in b-msghand.

GetTime takes up 0.82% of total time, as part of InactivityCheck that takes up 1.78% of total time, in b-net.

Converting from std::chrono::microseconds to NodeClock::time_point is a lot more intrusive (impacting at least net_processing and no
...
💬 sedited commented on pull request "net: Waste less time in socket handling":
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3621358666)
Concept ACK
💬 ajtowns commented on pull request "test: p2p: check that peer's announced starting height is remembered":
(https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3621363780)
Seems fine to test it if we have code to export it over RPC.

I don't think I've ever seen it be useful for debugging anything, and just knowing the height without knowing if it's the same header you have for that height or what the chainwork is doesn't seem very useful. So dropping `peer->m_starting_height` entirely (and only reporting it in the `receive version message` debug log entry) might be better?
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2595703803)
This is tested now by having zero worker threads.
💬 yuvicc commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#issuecomment-3621481762)
Concept ACK
💬 Ataraxia009 commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#issuecomment-3621582615)
Personally I prefer this approach over https://github.com/bitcoin/bitcoin/pull/34018 @0xB10C @stickies-v

The only problem I see with this approach is the code duplication. Which I think you solved right? Using:


```
const std::string details = std::format(
"transport={} version={} blocks={} peer={}{}{}",
TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
pfrom.nVersion.load(),
int{peer->m_starting_height},
pfrom.GetId(),
(fLogIPs ? std::fo
...
💬 Ataraxia009 commented on pull request "log: exempt all category-specific logs from ratelimiting when running with debug":
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3621587297)
Concept NAck

Users that are having issues that are not power users would also use the `-debug` option, to help submit logs.
if `-debug` is used with this implementation, it would stop rate-limiting of all category logs.
💬 Ataraxia009 commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2596019236)
If you are going to do this, then add a comment here saying `ProcessNewBlockHeaders` assumes that the caller has checked for min pow or something along those lines, for clarity
💬 laanwj commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3621690386)
FWIW: My bisect is not done yet (2 steps to go, it will likely finish today), but in the current commit range https://github.com/gcc-mirror/gcc/commit/1d82fc2e6824bf83159389729c31a942f7b91b04 `optimize std::vector::push_back` is the most suspicious as it affects vector appending. It's a change to `libstdc++-v3`, not the compiler, so it doesn't directly point at a root cause.
📝 Chand-ra opened a pull request: "fuzz: Add tests for `CCoinControl` methods"
(https://github.com/bitcoin/bitcoin/pull/34026)
The `ccoincontrol` fuzzer misses tests for `GetSequence()` and `GetScripts()`. Add them.
📝 ryslan25500-cloud opened a pull request: "Create PROPOSAL-V34-Tetrad-Research.md"
(https://github.com/bitcoin/bitcoin/pull/34027)
Add PROPOSAL-V34-Tetrad-Research.md

Detailed technical proposal for a federated research mining pool with deterministic 27-symbol tetrad embedding in Bitcoin blocks.

This is the V34 version of the Tetrad Research Proposal — a fully open, scientific, and strictly non-esoteric project aimed at:

• Creating the first long-term deterministic pattern dataset embedded directly into Bitcoin blocks • Studying potential emergent structures in perfectly uniform pseudorandom sequences • Demonstrat
...
💬 Sjors commented on issue "rpc: getrawtransaction lookup by witness txid":
(https://github.com/bitcoin/bitcoin/issues/34013#issuecomment-3621878889)
> for a while

I would not expect this to land before v31 in spring 2026, so by that time @Fi3 might have finished implementing IPC support and could just use the more performant #34020.